From fd66b4757c42d4109223d34db3c5bc9b5c396665 Mon Sep 17 00:00:00 2001 From: MaxwellGarceau Date: Wed, 8 Jan 2025 18:53:37 -0500 Subject: [PATCH 01/11] Ignore vscode settings --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 62369c7..10f5851 100644 --- a/.gitignore +++ b/.gitignore @@ -14,3 +14,6 @@ tests/cypress/reports tests/cypress/downloads mailchimp.zip + +# IDE +.vscode \ No newline at end of file From e38f0b08d2d2af6d9e62fa1fe9824649b4e7a009 Mon Sep 17 00:00:00 2001 From: MaxwellGarceau Date: Wed, 8 Jan 2025 18:53:54 -0500 Subject: [PATCH 02/11] Update comments to clarify functionality --- mailchimp.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/mailchimp.php b/mailchimp.php index f9870e5..70c7b45 100644 --- a/mailchimp.php +++ b/mailchimp.php @@ -944,8 +944,8 @@ function mailchimp_sf_signup_submit() { * @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 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 ) { @@ -953,6 +953,7 @@ function mailchimp_sf_subscribe_body( $merge, $igs, $email_type, $email, $status $body->email_address = $email; $body->email_type = $email_type; $body->merge_fields = $merge; + if ( ! empty( $igs ) ) { $body->interests = $igs; } @@ -970,10 +971,10 @@ function mailchimp_sf_subscribe_body( $merge, $igs, $email_type, $email, $status } /** - * 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 an error occurred. */ function mailchimp_sf_check_status( $endpoint ) { $endpoint .= '?fields=status'; From e0b5cc5976de83f1af19c1281a1e53a32615658d Mon Sep 17 00:00:00 2001 From: MaxwellGarceau Date: Wed, 8 Jan 2025 18:55:04 -0500 Subject: [PATCH 03/11] Early return subscribed users --- mailchimp.php | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/mailchimp.php b/mailchimp.php index 70c7b45..20b238e 100644 --- a/mailchimp.php +++ b/mailchimp.php @@ -958,15 +958,19 @@ function mailchimp_sf_subscribe_body( $merge, $igs, $email_type, $email, $status $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; } + + // single opt-in that covers new subscribers + if ( false === ! $status && $double_optin ) { + $body->status = 'subscribed'; + } else { + // anyone else + $body->status = 'pending'; + } + return $body; } From 2865c281ccb92205753d039e15b64f4ca24362a0 Mon Sep 17 00:00:00 2001 From: MaxwellGarceau Date: Wed, 8 Jan 2025 19:18:36 -0500 Subject: [PATCH 04/11] Make double opt-in the only factor in a confirmation email Remove the user status from double opt-in since it's not clear that logic affects why a user should receive a confirmation email or not --- mailchimp.php | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/mailchimp.php b/mailchimp.php index 20b238e..0b6d66a 100644 --- a/mailchimp.php +++ b/mailchimp.php @@ -963,13 +963,8 @@ function mailchimp_sf_subscribe_body( $merge, $igs, $email_type, $email, $status return $body; } - // single opt-in that covers new subscribers - if ( false === ! $status && $double_optin ) { - $body->status = 'subscribed'; - } else { - // anyone else - $body->status = 'pending'; - } + // Subscribe the user immediately unless double opt-in is enabled + $body->status = $double_optin ? 'pending' : 'subscribed'; return $body; } @@ -978,7 +973,7 @@ function mailchimp_sf_subscribe_body( $merge, $igs, $email_type, $email, $status * Check the status of a subscriber in the list. * * @param string $endpoint API endpoint to check the status. - * @return string|false The subscription status ('subscribed', 'unsubscribed', 'pending', etc.) or false if an error occurred. + * @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'; From f7e09927a39b26fbb0c5c8629af201f68952c701 Mon Sep 17 00:00:00 2001 From: MaxwellGarceau Date: Wed, 8 Jan 2025 19:32:49 -0500 Subject: [PATCH 05/11] Add comments --- mailchimp.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mailchimp.php b/mailchimp.php index 0b6d66a..722c5c1 100644 --- a/mailchimp.php +++ b/mailchimp.php @@ -905,6 +905,7 @@ function mailchimp_sf_signup_submit() { $status = mailchimp_sf_check_status( $url ); // If update existing is turned off and the subscriber exists, error out. + // TODO: Shouldn't this catch any user with any status if "Update Existing Subscriber's?" is disabled? if ( get_option( 'mc_update_existing' ) === false && 'subscribed' === $status ) { $msg = esc_html__( 'This email address is already subscribed to the list.', 'mailchimp' ); $error = new WP_Error( 'mailchimp-update-existing', $msg ); @@ -964,6 +965,7 @@ function mailchimp_sf_subscribe_body( $merge, $igs, $email_type, $email, $status } // Subscribe the user immediately unless double opt-in is enabled + // No need to check user $status because we're handling that before this point $body->status = $double_optin ? 'pending' : 'subscribed'; return $body; From 3fea7b1c1aaca698406788c2618a995504daa351 Mon Sep 17 00:00:00 2001 From: MaxwellGarceau Date: Wed, 8 Jan 2025 19:34:01 -0500 Subject: [PATCH 06/11] Fix bug causing existing users to sign up regardless of subscribe existing user check get_option( 'mc_update_existing' ) returns '1' or '', but never false --- mailchimp.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mailchimp.php b/mailchimp.php index 722c5c1..ca10ebd 100644 --- a/mailchimp.php +++ b/mailchimp.php @@ -906,7 +906,7 @@ function mailchimp_sf_signup_submit() { // If update existing is turned off and the subscriber exists, error out. // TODO: Shouldn't this catch any user with any status if "Update Existing Subscriber's?" is disabled? - if ( get_option( 'mc_update_existing' ) === false && 'subscribed' === $status ) { + if ( ! get_option( 'mc_update_existing' ) && 'subscribed' === $status ) { $msg = esc_html__( 'This email address is already subscribed to the list.', 'mailchimp' ); $error = new WP_Error( 'mailchimp-update-existing', $msg ); mailchimp_sf_global_msg( '' . $msg . '' ); From bb3b5771c8dd159be5d58387bde4d27611c4f31a Mon Sep 17 00:00:00 2001 From: MaxwellGarceau Date: Wed, 8 Jan 2025 19:38:58 -0500 Subject: [PATCH 07/11] Restrict any previous subscriber from signing up while update existing subscribers is off --- mailchimp.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/mailchimp.php b/mailchimp.php index ca10ebd..0c42f94 100644 --- a/mailchimp.php +++ b/mailchimp.php @@ -904,10 +904,10 @@ 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. - // TODO: Shouldn't this catch any user with any status if "Update Existing Subscriber's?" is disabled? - if ( ! get_option( 'mc_update_existing' ) && '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' ); $error = new WP_Error( 'mailchimp-update-existing', $msg ); mailchimp_sf_global_msg( '' . $msg . '' ); return false; From f30dc6427e5512b9c2f3079e4c2b927b9b3d6211 Mon Sep 17 00:00:00 2001 From: MaxwellGarceau Date: Wed, 8 Jan 2025 20:36:16 -0500 Subject: [PATCH 08/11] Add location where Mailchimp sign up form should display --- mailchimp.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/mailchimp.php b/mailchimp.php index 0c42f94..341db44 100644 --- a/mailchimp.php +++ b/mailchimp.php @@ -913,6 +913,13 @@ function mailchimp_sf_signup_submit() { return false; } + // A former subscriber is trying to resubscribe + // If "Update Existing Subscriber?" is enabled and the user is unsubscribed + // then show them the Mailchimp hosted signup form + if ( get_option( 'mc_update_existing' ) && 'unsubscribed' === $status ) { + // TODO: Make 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' ); From 01c00b97dec0a01741f3d6d5b3ed7f980873f349 Mon Sep 17 00:00:00 2001 From: MaxwellGarceau Date: Wed, 8 Jan 2025 22:01:01 -0500 Subject: [PATCH 09/11] Clarified logic for double opt-in --- mailchimp.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/mailchimp.php b/mailchimp.php index 341db44..9240cd7 100644 --- a/mailchimp.php +++ b/mailchimp.php @@ -971,8 +971,9 @@ function mailchimp_sf_subscribe_body( $merge, $igs, $email_type, $email, $status return $body; } - // Subscribe the user immediately unless double opt-in is enabled - // No need to check user $status because we're handling that before this point + // 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'; return $body; From 61818024fbf2a187abaced94d293a0b01a4878d4 Mon Sep 17 00:00:00 2001 From: MaxwellGarceau Date: Wed, 8 Jan 2025 22:08:04 -0500 Subject: [PATCH 10/11] Simplify todo note --- mailchimp.php | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/mailchimp.php b/mailchimp.php index 9240cd7..8192dd8 100644 --- a/mailchimp.php +++ b/mailchimp.php @@ -913,12 +913,8 @@ function mailchimp_sf_signup_submit() { return false; } - // A former subscriber is trying to resubscribe - // If "Update Existing Subscriber?" is enabled and the user is unsubscribed - // then show them the Mailchimp hosted signup form - if ( get_option( 'mc_update_existing' ) && 'unsubscribed' === $status ) { - // TODO: Make API request to fetch Mailchimp hosted sign up form and display to user - } + // 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' ); From 8e1e3f481f1a75d27192a7def17eb158620675e8 Mon Sep 17 00:00:00 2001 From: MaxwellGarceau Date: Wed, 8 Jan 2025 22:10:53 -0500 Subject: [PATCH 11/11] Fix lint errors --- mailchimp.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/mailchimp.php b/mailchimp.php index 8192dd8..aa35bd7 100644 --- a/mailchimp.php +++ b/mailchimp.php @@ -944,12 +944,12 @@ 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 [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. + * @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 ) {