We have the mailchimp module set up with a user field hooked up to a MailChimp list, including interest groups. We have the module set to operate in batch mode which means that updates aren't applied straight away, but queued.
The config page explains this, with the note:
Note: May cause confusion if caches are cleared, as requested changes will appear to have failed until cron is run.
However, the option causes confusion irrespective of cache clearing. See steps below.
1. Set up a list, with interest groups
2. Map the list to a user field
3. Set the mailchimp module to batch mode
4. Visit your user edit page (user/xx/edit) (Assuming you're not subscribed to the list)
5. Check the subscribe box, and hit "Save"
6. The page reloads with the message, "Your changes have been saved"
Actual behaviour: The Subscribe checkbox is un-ticked
Expected behaviour: The subscribe checkbox is ticked
You can resolve the issue by running cron, which will push the change to MailChimp, after which loading the user edit page will correctly show the Subscribe option as selected. However, that's not really a feasible solution to what is a hugely confusing problem, pretty much rendering "batch" mode useless for any site that has the fields available to users to view / edit.
The root of the issue is that the local cache (cache_mailchimp) is only populated when data is fetched from MailChimp. This cache should also be updated / populated when operations are queued. This would make the behaviour consistent with the description of batch mode, in that misleading results would still be shown if caches were cleared without running cron, but would be much more reliable in the normal case.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 2503597mailchimp_batch_processing_message.patch | 1.25 KB | BabaYaga64 |
| #1 | 2503597-update-local-cache.patch | 3.63 KB | leewillis77 |
Comments
Comment #1
leewillis77 commentedI've been looking at this to see how feasible it would be to fix this. There's a patch for discussion attached. This patch (against 7.x-3.3) works by trying to update the locally cached member info when updates are queued.
Notes:
1. This attempts to always do the right thing, however cache_mailchimp contains the raw response from get_memberinfo. This naturally is a richer object than we can generate within the module itself. So there are some potential issues around "updating" the cache when we don't already have cached data. In this case (Which should only happen if there is no local cache for a user, and the user's information can't be retrieved from MailChimp) we create a dummy object. It's possible that this is missing some information that other code expects to be there - although I haven't experienced that in testing.
2. If the module is trying to queue an update, and there is no local cache data for a user, the code *will* hit the MailChimp API when "queuing" updates to read the current memberinfo. In practice I would think this is unlikely, however it's worth noting. If we want to force this codepath not to hit the API, then we could replace the call to mailchimp_get_memberinfo() with a cache_get(). This increases the likelihood of the issues raised in (1) happening, since a new object will be created when there is no local cached data for the user (We effectively remove the fallback to grabbing the info from MailChimp).
3. This has undergone some simple testing on a dev environment only at this time. Our setup is a single list, with a single set of interest groups. Some testing on more complex setups might be useful.
I'm not sure how I feel about this patch as-is. On one hand it seems like it's the only real solution to this, on the other it feels like it might be frail - other opinions definitely welcome.
Comment #2
BabaYaga64 commentedI was able to recreate this issue, and I added a warning message to indicate to users that the Subscribe box does not appear as checked right away, but is checked after the queued process is run (after Cron is run).
Comment #4
leewillis77 commentedHi;
Your patch doesn't really fix the issue, it just adds a note to the page. That's not good enough for the client we're working with at the moment hence my original patch.
That said, if your approach was something that could go in quickly then it's definitely a band aid in the short term - although bearing in mind the message is going to get shown to end users I think it needs to be a bit more user friendly than the message in your proposed patch which is pretty "techy (IMO).
I'd really love it if you could review my original patch and/or give it a run out and offer any feedback?
Comment #5
BabaYaga64 commentedThank you for your feedback on the patch. I reworded the warning message to make is more user-friendly:
Regarding your patch, it's a more complex and better solution, but the concerns you raised about it being potentially frail, concern me also. I'll try applying it and testing it, but it might take a person with a deeper understanding of the architecture than I to make it work successfully.
Comment #6
jami commentedAdded a very simple notification that the settings will update soon when a user edits their subscription with batch processing enabled: http://drupalcode.org/project/mailchimp.git/commit/02c5e7c
Syncing with MailChimp each time a user changes their subscription would defeat the purpose of enabling batch mode.
Comment #7
leewillis77 commentedHi;
I'd be grateful if you could review the patch I supplied in #1, and the notes. My proposed changes don't defeat the purpose of enabling batch mode, and don't cause the code to sync with MailChimp every time a user changes their subscription, so I think it's still relevant to discuss?
Comment #8
gcbHey Lee, as the guy who wrote the original "config note" that you mention, I am going to take credit for causing this confusing bug. I've asked the team here to have another look at this patch to save me any further shame: we're testing it now and will try to roll it in to the code assuming that goes well. It's a pretty strange corner case but I'm convinced that the checkbox is better as an indicator of the user's expressed preference than as an indicator of the current reality of their subscription status.
Thanks for the patch!
Comment #9
BabaYaga64 commentedI have tested and applied your patch. The subscribe checkbox is now staying checked after users are added to lists, and staying unchecked when removing them from lists. Thank you.
Comment #11
greg boggsI've committed the patch in #1. It will be in the next 3.5 release.
Comment #12
leewillis77 commentedThat's great - thanks!