Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug/double opt in triggered despite being disabled in settings #115

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,6 @@ tests/cypress/reports
tests/cypress/downloads

mailchimp.zip

# IDE
.vscode
46 changes: 26 additions & 20 deletions mailchimp.php
Original file line number Diff line number Diff line change
Expand Up @@ -904,14 +904,18 @@ function mailchimp_sf_signup_submit() {
$url = 'lists/' . $list_id . '/members/' . md5( strtolower( $email ) );
$status = mailchimp_sf_check_status( $url );

// If update existing is turned off and the subscriber exists, error out.
if ( get_option( 'mc_update_existing' ) === false && 'subscribed' === $status ) {
$msg = esc_html__( 'This email address is already subscribed to the list.', 'mailchimp' );
// If update existing is turned off and the subscriber is not new, error out.
$is_new_subscriber = false === $status;
if ( ! get_option( 'mc_update_existing' ) && ! $is_new_subscriber ) {
$msg = esc_html__( 'This email address has already been subscribed to this list.', 'mailchimp' );
Comment on lines +907 to +910
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

‼️ ‼️ I changed the functionality slightly here according to my interpretation of the use case.

If "Update Existing Subscriber?" is disabled and the email does not belong to a new subscriber then don't let them sign up.

The previously functionality only checked for emails with a subscribed status. This missed unsubscribed, pending, and the other possible Mailchimp statuses.

‼️ Question: Do we want to update unsubscribed and pending users as well? If we don't update unsubscribed users then it will be functionally impossible for any user to resubscribe once unsubscribed. This directly relates to #93.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do think we'll want to allow other statuses to work here besides just subscribed. As a user, I think I'd consider it a bug if someone that has unsubscribed can't resubscribe if that setting is on. The way I think this should work is if someone is subscribed or is in a pending state (I would assume that means they subscribed but haven't verified that yet), we shouldn't allow them to resubscribe if this option is enabled. But if they have never subscribed or if they have unsubscribed, they should be allowed to subscribe

Copy link
Collaborator Author

@MaxwellGarceau MaxwellGarceau Jan 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way I think this should work is if someone is subscribed or is in a pending state, we shouldn't allow them to resubscribe if this option is enabled mc_update_existing

I 100% agree with you.

  • In the case of subscribed, the sign up request will update the existing contact instead of creating a duplicate.
  • In the case of pending (subscribed, but haven't verified), their information will also be updated. However, the user must still confirm the email before the contact is created. The exception to this is if single opt-in has been enabled then the pending contact will be created immediately without needing to verify the original email.

[If mc_update_existing is enabled or disabled] But if they have never subscribed they should be allowed to subscribe

Yes! Current functionality.

[If mc_update_existing is disabled] If they have unsubscribed they should be allowed to subscribe

I agree as well. The current Mailchimp functionality is that contacts can not be resubscribed through the API. However, there's a solution in #117 to still address this scenario.

I added a comment directly after "Update Existing Subscriber?" check to make an API request for the native Mailchimp sign up form if the contact has a status of unsubscribed and "Update Existing Subscriber?" is enabled.

wordpress/mailchimp.php

Lines 916 to 917 in 8e1e3f4

// TODO: If get_option( 'mc_update_existing' ) && 'unsubscribed' === $status then
// make an API request to fetch Mailchimp hosted sign up form and display to user

$error = new WP_Error( 'mailchimp-update-existing', $msg );
mailchimp_sf_global_msg( '<strong class="mc_error_msg">' . $msg . '</strong>' );
return false;
}

// TODO: If get_option( 'mc_update_existing' ) && 'unsubscribed' === $status then
// make an API request to fetch Mailchimp hosted sign up form and display to user

$body = mailchimp_sf_subscribe_body( $merge, $igs, $email_type, $email, $status, get_option( 'mc_double_optin' ) );
$retval = $api->post( $url, $body, 'PUT' );

Expand Down Expand Up @@ -940,40 +944,42 @@ function mailchimp_sf_signup_submit() {
* Cleans up merge fields and interests to make them
* API 3.0-friendly.
*
* @param [type] $merge Merge fields
* @param [type] $igs Interest groups
* @param string $email_type Email type
* @param string $email Email
* @param string $status Status
* @param bool $double_optin Whether this is double optin
* @param [type] $merge Merge fields
* @param [type] $igs Interest groups
* @param string $email_type Email type
* @param string $email Email
* @param string|false $status Status The subscription status ('subscribed', 'unsubscribed', 'pending', etc.) or false if an error occurred.
* @param string $double_optin Whether double opt-in is enabled. "1" for enabled and "" for disabled.
* @return stdClass
*/
function mailchimp_sf_subscribe_body( $merge, $igs, $email_type, $email, $status, $double_optin ) {
$body = new stdClass();
$body->email_address = $email;
$body->email_type = $email_type;
$body->merge_fields = $merge;

if ( ! empty( $igs ) ) {
$body->interests = $igs;
}

if ( 'subscribed' !== $status ) {
// single opt-in that covers new subscribers
if ( false === ! $status && $double_optin ) {
$body->status = 'subscribed';
} else {
// anyone else
$body->status = 'pending';
}
// Early return for already subscribed users
if ( 'subscribed' === $status ) {
return $body;
}

// Subscribe the email immediately unless double opt-in is enabled
// "unsubscribed" and "subscribed" existing emails have been excluded at this stage
// "pending" emails should follow double opt-in rules
$body->status = $double_optin ? 'pending' : 'subscribed';

MaxwellGarceau marked this conversation as resolved.
Show resolved Hide resolved
return $body;
}

/**
* Check status.
* Check the status of a subscriber in the list.
*
* @param string $endpoint Endpoint.
* @return string
* @param string $endpoint API endpoint to check the status.
* @return string|false The subscription status ('subscribed', 'unsubscribed', 'pending', etc.) or false if the API returned 404 or an error occurred.
*/
function mailchimp_sf_check_status( $endpoint ) {
$endpoint .= '?fields=status';
Expand Down
Loading