Most of this module's "stuff" is happening during the insert and update operations of hook_user(). However, there are lots of cases where users get updated WITHOUT the full user form being submitted. For example, when user accounts are created automatically using user_save() during an Übercart purchase, or when accounts are updated using bulk operations on the admin/user/user page. In these cases, I'm seeing the users being unsubscribed from all lists.

I'm thinking it would be better and more flexible to instead use hook_form_alter() to add the MailChimp fields to the user registration/edit pages. And then create new validate/submit functions to handle submissions. This would be a lot less likely to fire at the wrong time, and would also make it easier to re-use these validate/submit functions in other places (like the Übercart checkout form, as we're doing on store.lullabot.com).

Thoughts?

CommentFileSizeAuthor
#3 mailchimp_unsubscribe1.patch1.72 KBsmoothify
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

levelos’s picture

levelos CreditAttribution: levelos commented

That's a great point Jeff, and I think explains some of the other issues seen with the module (E.g., #413968: User gets unsubscribed from Required list when editing user profile). So, in short, I agree, and that sounds like a good approach. I just recently started a new branch using the updated MailChimp API class (not yet committed), and can put this on the list. That said, I'm pretty slammed with the day job right now, so it may take me a bit. Patches are definitely welcome in the mean time. For that matter, I'd be happy to have a co-maintainer for the module if anyone on your team (or not) is interested. I'd be psyched to work with Lullabot on it.

BTW - I'm know Matt and we met briefly when you were last in Portland (and SXSW).

smoothify’s picture

smoothify CreditAttribution: smoothify commented

I've been running into this issue on a drupal 5 installation of mailchimp.

Today i've been experimenting with how the module works with hook_user.

User Registration
I agree with jeff that a new submit handler would be a good idea for the register form. It would mean that the subscription is fired after the user object is created in the db.

Account Editing

For the the account edit, i think it should still use hook_user because the module should always update subscriptions at mailchimp each time the user is updated (for email or merged variable changes);

The *really important* thing is that the module should detect the presence of the subscription form elements before unsubscribing which I believe is the cause of most of these mystery unsubscribes. It could be done quite simply by making sure the relevant form element is actually present as well as empty.

isset($edit['chimpmail_list_'. $list['id']]);

I will post up a patch for drupal 6 shortly.

Finally, I think it may be better to use the 'after_update' op rather than 'update' as this way the user object is final and saved to the database. The only thing that wouldn't work this way is the email address change, but i'm sure there is a way around that.

smoothify’s picture

smoothify CreditAttribution: smoothify commented
FileSize
mailchimp_unsubscribe1.patch1.72 KB

Here is my first go at this, it seems to fix the issue I was having where it unsubscribes when you assign roles from the admin/user screen.

However, I do also feel that all the code in hook_user needs to be reviewed to make sure its working in the right way, as I don't think it addresses all the cases where mailchimp should be updated through the api.

BenK’s picture

BenK CreditAttribution: BenK commented

Subscribing...

levelos’s picture

levelos CreditAttribution: levelos commented
Status: Active » Needs review

Thanks for the patch smoothify, however I started down the path of removing much of the processing from hook_user as Jeff suggested and now have an active dev branch with the new approach, http://drupal.org/node/544380. The basics seem to be functioning, although it's still a work in progress. Please take it for a spin and let me know if there are any issues and/or submit patches against the new branch.

levelos’s picture

levelos CreditAttribution: levelos commented
Version: 6.x-1.1 » 6.x-2.x-dev

switching issue to 6.x-2.x branch.

levelos’s picture

levelos CreditAttribution: levelos commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

rickmanelius’s picture

rickmanelius CreditAttribution: rickmanelius commented

The only problem I'm facing now is that say, when an order is made in ubercart and a new user is created, there is no sign up to the newsletter despite the fact that I have my list set to opt-in.

I know that cron is only run for "required" lists... and since the functionality is not in hook_user anymore, it would be awesome if there was another function we could call to perhaps use hook_user on insert... because for me, that's the biggy. I need to be able to get every user subscribed on insert/new, but then let them change/leave if they want to.

BTW... I'm not trying to throw tasks at anyone. So if this is a "won't fix", i'll just implement myself... after i catch up on the learning curve :)