I have just installed this module and updated it from a previous version (although never really used it).

I have come to this module after trying emf, and although I like the idea of the other one being more 'framework-like', it was lacking of a few things and this one looked like a more feature-rich module and more actively maintained as I'll likely be submitting patches if needed.

So far I noticed that the update 6201 was adding users to the queue (which seems to be a new feature).

As I am starting off fresh I wasn't planning on importing my registered users into my mailchimp list(s) as of yet, so I wonder if this should be something that should be done on the install stage of the module.

Even more, the updates should match the freshly installed module, so if this is a required feature, I believe that bit of code should be added to the install hook as well, but I rather believe it should be better placed in some kind of maintenance task under the admin section of mailchimp.

Might report a bit more on this later.

Comments

levelos’s picture

levelos CreditAttribution: levelos commented

Take a look at http://drupal.org/node/682716 and let me know if you have any improvements/changes to suggest. They'd be more than welcome.

hanoii’s picture

hanoii
Primary language Spanish
Location 🇦🇷UTC-3
CreditAttribution: hanoii commented

I just read through the issue you pointed me. There's not much to add to that right now. I haven't got that deep into the cron code/method to suggest an alternative as I don't have much experience yet on both the module code and the mailchimp API, and I am not sure how deep I am going to go, but as soon as I find something not working, I'd probably fix it and submit a patch.

As for the scope of this issue, I still believe that the update should be removed. It will only affect upgraded users and not brand new installed module. I also think that what's currently done by that update should be placed under a maintenance task accessed by the admin section of the module.

I will point users on that issue to this one to see if either of them can contribute or, if I can, I will supply a patch myself, but also to hear what they think.

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
CreditAttribution: xurizaemon commented

hanoii,

The reason for adding all valid users to the queue is fairly simple, and outlined in comment #7 on that issue. As loubabe says, any improvements you can suggest are welcome. However,

You raise a good point, in that sites which install the mailchimp module fresh after #682716: Improve cron handling (don't update all users) will not need to queue the entire user DB (though this probably isn't such a problem for sites which have a smaller userbase).

One solution might be to do something like the following -

  1. In hook_install, set a variable "mailchimp_dont_queue_all_users".
  2. In hook_upgrade_6201, check for mailchimp_dont_queue_all_users before queueing the entire user DB.
  3. In hook_upgrade_6201, delete mailchimp_dont_queue_all_users.

Any check which identifies if the site is upgrading or installing at update #6201 should do the trick; there may be a better way to do it. Does the system table have the module added when the upgrade hook is run? I suspect so ... but maybe you could check for some existing mailchimp_* variables there instead?

I'm unsure if it's acceptable to alter an upgrade hook after release, but as Mailchimp is at 6.x-2.0-rc5 currently, this might be OK.

Provided it doesn't mess with existing sites, I don't see any issue myself (note that I'm not the maintainer, so it's my decision anyway).

Hope that helps!

hanoii’s picture

hanoii
Primary language Spanish
Location 🇦🇷UTC-3
CreditAttribution: hanoii commented
Title: update 6201 might not be needed there? » Auto-update of the userbase (remove update 6201?)

Thanks for taking time to discuss this.

I kind of get what you mean, the update was trying to emulate previous behavior.

I am thinking on the following:

What happens if a site with a fairly amount of users wants to send subscriptions to all of the users?

I don't think that anything like update 6201 should be on the install, because that should be a site administer decision (I don't like that to happen right now), but it might be good to have some kind of maintenance link to do that if needed (update all userbase) or something like that.

As for the update itself, I'd leave it like that, but, if this piece of code I am suggesting is added to the module, we can safely make the update 6201 do nothing on newer releases and we can show a message to the user upgrading the module letting him know of the new cron handling and that if need, it can run that maintenance task.

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
CreditAttribution: xurizaemon commented

If the users aren't subscribed to lists on the site, they shouldn't be updated to Mailchimp. The cron task will run through all users, but only update users who should be subscribed to a list (either by dint of their profile settings OR because you have a compulsory list). If 0 users match these criteria, the cron task should not perform any updates to Mailchimp.

Your comments have made another penny drop, though - #840996: Handle user updates for lists being set as compulsory.

levelos’s picture

levelos CreditAttribution: levelos commented
Status: Active » Postponed (maintainer needs more info)

Not seeing an actual issue here and marking as postponed pending any additional information.

nrackleff’s picture

nrackleff CreditAttribution: nrackleff commented
Component: Code » General
Issue summary: View changes
Status: Postponed (maintainer needs more info) » Closed (won't fix)

“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!”