First off, this module rocks and thank you for all your hard work into it. BUT....

This module seems to assume all hook_user() calls will always be done by users (by submitting the form) and therefore have $edit information, however, say i want to batch process my users via code, i $account = user_load($uid) them, manipulate the $account object then user_save($account).

This will mean that there is not the $edit data avilable which this module is expecting (as there was no form submitted), which means checks like the one on line 89 of mailchimp.module

                // unsubscribe a subscribed user who unchecked the box for this newsletter
                else if ($is_subscribed && !@$edit['chimpmail_list_'. $list['id']]) {
                  $ret = _mailchimp_unsubscribe_user($list, $account->mail, TRUE, $q);
                }

will ALWAYS fail. Not good.

So, when you try to batch process say 500 users, they will all get unsubscribed from your mailing lists.

Sorry for just pointing out a problem without also offering a patch, but I was running batch updates on users on our test system (which has the same DB as the production, so mailchimp was still "working") so now have to do damage limitation, as this bug has unsubscribed 70 or so users before i caught it :S

CommentFileSizeAuthor
#3 mailchimp_edit.patch934 bytestomchuk
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

levelos’s picture

Looks like a problem. I didn't know there could be a case where $edit would empty. I committed a simple fix to check for set $edit variable to prevent accidental unsubscribes just to plug the whole.

A real fix would obviously need to test both the $account and $edit vars for changes before triggering the MC update. I'll get to it soon, or patches welcome.

tomchuk’s picture

Here's a patch that is working for me. Tested with the following code:

include_once './includes/bootstrap.inc';
drupal_bootstrap(DRUPAL_BOOTSTRAP_FULL);
$account = user_load(array('uid' => 1));
$account->created = time();
user_save($account);

Without the patch, the user would be unsubscribed their subscribed mailing lists. With the patch, this doesn't happen. Subscribe/unsubscribe actions through the user_edit form still work fine.

(Here's the patch, upload wasn't working for me)

--- mailchimp.module.old	2009-03-27 11:14:46.550598113 -0400
+++ mailchimp.module	2009-03-27 11:30:17.071597892 -0400
@@ -73,8 +73,10 @@
               $ret = true;
 
               if (
-                  variable_get( 'mailchimp_user_edit', true ) && $op == "update" && $category == 'account' ||
-                  variable_get( 'mailchimp_user_register', true ) && $op == "insert"
+                  ( variable_get( 'mailchimp_user_edit', true ) && $op == "update" && $category == 'account' ||
+                    variable_get( 'mailchimp_user_register', true ) && $op == "insert" 
+                  ) && 
+                  !empty( $edit )
                 ) {
                 // subscribe the user if they are not previously subscribed and want to be
                 if ( !$is_subscribed && @$edit['chimpmail_list_'. $list['id']] ) {

tomchuk’s picture

FileSize
934 bytes

Let me try uploading again...

torgosPizza’s picture

I am seeing this same issue but the patch didn't help.

If I am a user who has already subscribed, but then I unsubscribe using the form, that works.

If I then go back into my User Edit form and try to re-subscribe, I get the Subscribe function getting fired, followed immediately by the Unsubscribe function, What happens is that I remain unsubscribed from the list.

I'm looking into this now, hopefully I can fix and if so, will submit another patch.

torgosPizza’s picture

For some more info, here's a drupal_set_message I added to the hook_user('update') $op, to print the $is_subscribed var, at the point when I try to subscribe again:

# Is-sub:
# You have subscribed to The Mailing List.
# Is-sub: 1
# Is-sub: 1
# You have unsubscribed from The Mailing List.
# Is-sub:
# The changes have been saved.

EDIT: Feel free to ignore my Issue. It was all PEBKAC :)

levelos’s picture

Status: Active » Fixed
levelos’s picture

Status: Fixed » Closed (fixed)

Included in 6.x-1.1