We have a pretty standard setup: A list created and imported to Drupal which creates a signup block. We've allowed anonymous users to subscribe.

The HUGE problem is this: If a user comes to this block and is already subscribed, there is no visual indication, after submission, that they're already subscribed. So with the way this module works, they enter an email address already on the list and they're then REMOVED from the list without so much as a warning! This means they're trying to signup for a list (which they're already on) and they get the opposite result (being removed).

This is bad, really bad, UX.

Unless I'm missing some way to stop this, I'd call this a huge UX bug that needs to be addressed.

I'd propose either:
a) if they're already subscribed, dont do anything...just bypass submitting anything to mailchimp so they STAY subscribed (with a drupal messsage that says "you're already subscribed to this list").
or
b) if they're subscribed, the submit handler redirects to a unsubscribe confirmation screen which says something like "youre already subscribed to this list, would you like to unsubscribe? " with 2 buttons: unsubscribe | cancel

CommentFileSizeAuthor
#17 mailchimp-subscribe.png7.78 KBjoelpittet
#16 mailchimp-existing-anonymous-emails-unsubscribed-when-they-try-to-subscribe-2118771-12.patch2.56 KBjoelpittet
#12 mailchimp-existing-anonymous-emails-unsubscribed-when-they-try-to-subscribe-2118771-12.patch2.89 KBbzitzow
#9 interdiff-2118771-7-9.txt5.43 KBidebr
#9 mailchimp-allow-unsubscribes-2118771-9.patch5.2 KBidebr
#8 interdiff-2118771-8.txt5.28 KBidebr
#8 mailchimp-allow-unsubscribes-2118771-8.patch5.15 KBidebr
#7 unsubscribes-2118771-5.patch2.24 KBJarlskov
#4 unsubscribes-2118771-4.patch4.26 KBprdctvtxt
#3 unsubscribes-2118771-3.patch5.59 KBprdctvtxt
#2 mailchimp-anon_user_unsubscribe_fix-2118771-2.patch1.1 KBTyler the Creator

Comments

Rob_Feature’s picture

Rob_Feature commented

This is the offending piece of code from line 622 of mailchimp_lists.module

   // Determine what function is appropriate.
    // Unsubscribe a subscribed user who unchecked an Optional list:
    if ($is_subscribed && !$selected && !$list->settings['required']) {
      $function = 'remove';
    }

I think this slice of code intends to target authenticated users updating their subscription settings in a form....but it actually fires (also) for anonymous users who are just filling out the block form (they have no box to check on an optional list). This probably needs an additional check to make sure the user !$is_anonymous or something. Not sure what the intent is here, completely, so not totally sure what to suggest as a fix.

Tyler the Creator’s picture

Tyler the Creator commented
Status: Active » Needs review
StatusFileSize
new mailchimp-anon_user_unsubscribe_fix-2118771-2.patch1.1 KB

Created a patch relating to the first (a) proposition. I think that eventually there should be an unsubscribe option, but this is a quick fix in the meantime. Let me know if anything needs to be changed/fixed.

edit: I also realized through testing that the users get cached once they subscribe to a list. So if you apply this patch, and subscribe to a list and unsubscribe through Mailchimp, the Drupal system still thinks that the email is subscribed until the cache is cleared.

prdctvtxt’s picture

prdctvtxt commented
StatusFileSize
new unsubscribes-2118771-3.patch5.59 KB

Attaching another patch - a different approach for this issue.
This adds a checkbox on the mailchimp admin settings form to enable/disable unsubscribes.
Displays a message if the user is already subscribed.
More work should be done to improve the message handling and the logic around if the user is subscribed already and what should be presented to the user given the legalities of opt-in terms etc..

I had to come up with a solution quickly for a client so adding here if anyone else finds it useful, great :)

prdctvtxt’s picture

prdctvtxt commented
StatusFileSize
new unsubscribes-2118771-4.patch4.26 KB

Updated patch removed some error logging. This one is cleaner.

prdctvtxt’s picture

prdctvtxt commented
Issue summary: View changes

add clarifiying details

dave bruns’s picture

dave bruns commented

Just adding a link to a related issue: https://drupal.org/node/2086069

That issue can probably be marked as a dup at some point.

darksnow’s picture

darksnow
Location Scotland
commented

Thanks Dave, best to keep everything together.

I completely agree with the OP regarding the UX issue. I can't really see how we could efficiently check that the anonymous user is already subscribed in order to display something like "You're already subscribed" but additional checks are needed in the subscription code.

To my mind, if a user clicks on "Subscribe" they intend to subscribe so removing an existing subscription is not at all the expected behaviour. So, check for an existing subscription, then do northing since the user is already subscribed. Maybe a change of confirmation message would be enough in that case.

I should also add that I've had no time to look into this myself to bit thanks to those that have bothered to supply a patch, I really should have ;)

Jarlskov’s picture

Jarlskov commented
Issue summary: View changes
StatusFileSize
new unsubscribes-2118771-5.patch2.24 KB

Removed .info changes so the patch applies cleanly.

idebr’s picture

idebr commented
StatusFileSize
new mailchimp-allow-unsubscribes-2118771-8.patch5.15 KB
new interdiff-2118771-8.txt5.28 KB

The current behavior is certainly unexpected. Thanks above posters for your work on this patch!. I have rerolled the patch against the latest git version and made these changes:
- fixed white-spacing issues (mostly trailing spaces)
- The feedback presented to the user now includes the list's 'form label' when available or the list's label otherwise

idebr’s picture

idebr commented
StatusFileSize
new mailchimp-allow-unsubscribes-2118771-9.patch5.2 KB
new interdiff-2118771-7-9.txt5.43 KB

Upon further testing, I found a flaw in the logic in the previous patches. Since registered users can see which lists they are currently subscribed to, they should be unsubscribed when they deselect a list in the registration form. The option to allow unsubscribes should therefor only apply for anonymous users.

Attached patch includes the change above. I have updated the admin setting accordingly.

greatmatter’s picture

greatmatter commented

Subscribe...

dmsmidt’s picture

dmsmidt
he/him
Primary language Dutch
Location Amsterdam 🇳🇱🇪🇺
commented

Interesting.
The patch works, thanks for the work.

Thinking aloud:
Wouldn't it be better to use form validation and throw an error message instead of a status message after submit?
I was checking for 'executed' in the $form_status and with the error it returned 'true' while I was expecting 'false'.

bzitzow’s picture

bzitzow commented
Assigned: Unassigned » bzitzow
StatusFileSize
new mailchimp-existing-anonymous-emails-unsubscribed-when-they-try-to-subscribe-2118771-12.patch2.89 KB

Rerolled patch -

  • Improved "visual indication" as listed in original issue description
  • Added fallback behavior for the error message scenario where the form label is empty

Improved "visual indication" as listed in original issue description
The message displayed is now of status type "error", to indicate that the expected behavior of signing up to the list did not happen.

Added fallback behavior for the error message scenario where the form label is empty
If (and only if) the form label is empty, then the error message displayed will read:

"You are already subscribed to the mailing list."

Note that I used two drupal_set_message() calls because the "%label" token in the first was placing emphasis tags around the "%label" value, which resulted in the message reading:

"You are already subscribed to the mailing list."

(I don't know if this is theme related or what, but since "the mailing list" should not be emphasized in any scenario, I hard coded it into the string.)

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Reviewed & tested by the community

The latest patches are not correct, because they change mailchimp_lists_execute_change() is an API function and should just do what it says without altering stuff suddenly.

The patch from #2 seems the right approach to me and fixes the problem for me. Marking patch in #2 as RTBC.

Anonymous’s picture

Anonymous (not verified) commented

subscribing

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

@klausi I think you're probably right, but #12 is what I'm looking for.

@bzitzow thanks!

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
StatusFileSize
new mailchimp-existing-anonymous-emails-unsubscribed-when-they-try-to-subscribe-2118771-12.patch2.56 KB

Here's the same patch from #12 but with --relative flag on the diff to keep it relative to the module.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
StatusFileSize
new mailchimp-subscribe.png7.78 KB

I do like that #2 is using a warning though. Maybe #12/ #16 could use warning's instead of errors too? It's not really error worthy I think.

I'm a bit torn between #2 and #12 because I also have this subscription block input field (not checkboxes) showing for logged in users and anonymous and in my specific case I'd like it to react the same for both. (I think I had to do a form_alter to get that working so it's not a feature of this module).

Trying to get this module to work similar to the wordpress version where this is constant block layout:

Also using this patch: #2051273: Signup fields missing?

chiebert’s picture

chiebert commented

I'm a bit confused - which patch is RTBC here? From @klausi's comment in #13, it would seem to refer to #2 - but @joelpittet has subsequently submitted #16, which is based on @bzitzow's from #12, which is a reroll of...? What? This seems to be a significant issue - I'm about to start building a feature that matches the OP's use case, but don't want to zig when the community later decides to zag...

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Assigned: bzitzow » Unassigned
Status: Reviewed & tested by the community » Needs review

Sorry for confusing. #2 seems to be just stopping anonymous users from unsubscribing when they subscribe again.

#16 makes this feature configurable and also deals with the same configuration within rules.

Going to move it back to RTBC to get some more people testing the later patches, but I'm still using #16;)

CountPacMan’s picture

CountPacMan commented
Related issues: +#2510640: Existing subscriber data overwritten when creating a new entity with a subscription field, +#1035906: Existing subscriber data overwritten when using a signup form, +#2086069: Freeform block unsubscribes user if already in list

added related issues

CountPacMan’s picture

CountPacMan commented
Version: 7.x-2.x-dev » 7.x-3.x-dev
Status: Needs review » Reviewed & tested by the community

The latest patch here https://www.drupal.org/node/2510640 seems to fix this issue. (https://www.drupal.org/files/issues/mailchimp-prevent-overwrite-on-entit...)

Anything beyond this seems like it would be a feature request.

greg boggs’s picture

greg boggs
he/him
Primary language English
Location Portland Oregon
commented
Status: Reviewed & tested by the community » Fixed

Please reopen this issue if the related patch posted above does not indeed fix the issue.

Status: Fixed » Closed (fixed)

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