Problem/Motivation
on $op == 'insert', 'delete', or 'after_update' in hook_user (line 71) The lists to act agains are generated from _mailchimp_get_required_lists. However, not every site has required lists. For those instances, no lists will be returned and the user will not get updated.
It seems like a more appropriate call would be to _mailchimp_get_available_lists, such as occurs in op == 'register' (line 22).
Proposed resolution
Change line 72 from:
foreach ((array)_mailchimp_get_required_lists() as $list) {
to
foreach ((array)_mailchimp_get_available_lists($account) as $list) {
so that users will get updated, regardless of whether lists are required or not.
This issue is related to but not the same as
http://drupal.org/node/708910
http://drupal.org/node/859334
Comment | File | Size | Author |
---|---|---|---|
#5 | mailchimp-sync-optional-lists-1226958.patch | 1.33 KB | thebuckst0p |
#4 | mailchimp.module.patch | 559 bytes | kanani |
#2 | mailchimp.module.patch | 947 bytes | Netrics |
Comments
Comment #1
Netrics CreditAttribution: Netrics commentedAlso change the 'after_update' on line 71 to 'update' as they use reference to the $edit parameter to check the old edited e-mail address.
But now this allow changing the e-mail address, but doesn't resolve the conflict with this issue: http://drupal.org/node/1097718#comment-4759712NVM I tried creating new profile fields in drupal, then creating them as MERGE tags in MailChimp admin, and linked them via the module admin. I then tested this with many accounts changing the profile fields and e-mail addresses for all accounts. They all synced correctly.Comment #2
Netrics CreditAttribution: Netrics commentedI've submitted a patch that will make these changes to the module. Whoever has CVS/Git access please review it and summit it through.
Comment #3
kanani CreditAttribution: kanani commentedThanks for the patch.
But I think so that we don't conflate the two we need to keep the two issues your addressing separated.
One for the update/after_update issue, and a second for the required vs available lists. I was actually hoping to get some feedback from levelos as to desired behavior since there has been some back and forth regarding update vs after_update.
Comment #4
kanani CreditAttribution: kanani commentedComment #5
thebuckst0p CreditAttribution: thebuckst0p commentedPorting the patch to 7.x-2.0. However, this is not a good approach for this version -- if it filters by optional lists, then it excludes required lists! There needs to be some hierarchy, so optional equals optional+required but required only equals required... or something like that. Thoughts?
Comment #6
levelos CreditAttribution: levelos commentedThis does not apply to 7.x-2.x.
Comment #7
levelos CreditAttribution: levelos commentedThe hook_user questions is being discussed over at #1097718. At this, time, we are not going to re-architect the 6.x version of module to process optional lists in hook_user. It's not as simple as changing _mailchimp_get_required_lists() to _mailchimp_get_available_lists(). For one thing, optional lists are not processed via cron, so if the cron flag is set, none of the update code will fire. The intent of the design to mirror what happens with required lists during cron, but do it synchronously.
I've changed the title and category of the ticket to reflect the source issue. Happy to leave it open for now.
Comment #8
j0rd CreditAttribution: j0rd commentedI've got a solution for this over at
#859334: Update/sync optional lists as well as required lists with cron for D6 version.
I don't believe my version deals with hook_user though, but it does do cron.
Comment #9
torgosPizzaWe just ran into this, and I assure you it is a bug.
The expected behavior is the User's email address gets updated at MailChimp if they change it on the Drupal side. The actual behavior is that nothing gets updated except for in Drupal. Even the description for the Cron checkbox is: "Sync Required Lists During Cron. If this is set, users will be subscribed to the required list during cron runs. Otherwise subscription will take place when a user is added/edited." All of these added together makes it seem as though you must have your lists set to "Required" to make sure email addresses are synced with MailChimp.
This is a bug, or at least a major design flaw.
For now it seems I will have to enable the Cron updates, but this is not ideal.
Comment #10
torgosPizzaFYI, I just tested the patch in #4 and it seems to work exactly as described (and as one might expect):
1. With Cron sync disabled, the update happens and the MailChimp API sync is fired.
2. With Cron sync enabled, the user gets added to the queue.
Our lists are not Required, however, and so I'm not sure if this feature requires more testing - but IMO, it shouldn't matter what the list type is. If a user is changing their email address, it should update wherever it is subscribed. Otherwise what's the point of having an update hook?
I'd say this is RTBC - or if the maintainer believes more attention needs to be in place due to the Required Lists/Cron interaction, I'd be happy to work on a more in-depth patch.
Comment #14
nrackleff CreditAttribution: nrackleff commented“And now our watch [for support of the 6.x version of the MailChimp module] has ended…” With the end of Drupal 6 support, I’m sad to say we too must turn the page.
Fret not! The 7.x-4.x and 8.x versions come highly recommended. Both are using Mailchimp’s new API 3.0 and are being actively maintained. “What is dead may never die, but rises again, harder and stronger!”