-
Notifications
You must be signed in to change notification settings - Fork 0
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
MaxwellGarceau
wants to merge
11
commits into
develop
Choose a base branch
from
bug/double-opt-in-triggered-despite-being-disabled-in-settings
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
fd66b47
Ignore vscode settings
MaxwellGarceau e38f0b0
Update comments to clarify functionality
MaxwellGarceau e0b5cc5
Early return subscribed users
MaxwellGarceau 2865c28
Make double opt-in the only factor in a confirmation email
MaxwellGarceau f7e0992
Add comments
MaxwellGarceau 3fea7b1
Fix bug causing existing users to sign up regardless of subscribe exi…
MaxwellGarceau bb3b577
Restrict any previous subscriber from signing up while update existin…
MaxwellGarceau f30dc64
Add location where Mailchimp sign up form should display
MaxwellGarceau 01c00b9
Clarified logic for double opt-in
MaxwellGarceau 6181802
Simplify todo note
MaxwellGarceau 8e1e3f4
Fix lint errors
MaxwellGarceau File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,3 +14,6 @@ tests/cypress/reports | |
tests/cypress/downloads | ||
|
||
mailchimp.zip | ||
|
||
# IDE | ||
.vscode |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 missedunsubscribed
,pending
, and the other possible Mailchimp statuses.unsubscribed
andpending
users as well? If we don't updateunsubscribed
users then it will be functionally impossible for any user to resubscribe once unsubscribed. This directly relates to #93.There was a problem hiding this comment.
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 subscribeThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I 100% agree with you.
subscribed
, the sign up request will update the existing contact instead of creating a duplicate.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 thepending
contact will be created immediately without needing to verify the original email.Yes! Current functionality.
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