Subscriptions: inform a clear error for pending confirmations

[htdat]

Fixes #276

Heads-up: To test this PR, it requires to have a sandbox.

Changes proposed in this Pull Request:

As explained in this comment #276 (comment), what I am trying to do in this PR:

1, Apply patch D37557-code to introduce a new status=confirming. With this change, Jetpack is able to handle 2 different cases:

• confirming: an email tries to subscribe a site for the first time.
• pending: this email tries to subcribe the site again while the confirmation link (sent during the first try) has not been clicked.

2, Jetpack plugin code sends respective messages for two statuses in the front-end. A notice here is that I am changing the handling of pending status. You can see my comment above, it was not correctly handled.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

Fix a bug in a current feature.

Testing instructions:

Before trying two test cases below, make sure you've done two actions:

1. Apply D37557-code in your sandbox.
2. Configure JETPACK__SANDBOX_DOMAIN in your wp-config.php

Case 01 - Compatibility for the previous JP versions

1. DO NOT apply this Jetpack PR in your testing site.
2. Use a test email to subscribe your testing site
3. See a successful message Success! An email was just sent to confirm your subscription. Please find the email now and click 'Confirm Follow' to start subscribing.
4. Try to use this email and subscribe again.
5. Continue to see the successful message above.

Case 02 - Fix the bug in new releases

1. APPLY this Jetpack PR in your testing site.
2. In jetpack.php, change constant JETPACK__VERSION to 8.2. https://github.com/Automattic/jetpack/blob/master/jetpack.php#L18
3. Go to JPDB of your site > Full Sync section > choose options, constants, functions, updates > Run Schedule Sync. Make sure the full-sync is finished.
4. Refresh JPDB, the version of JP should be 8.2 now.
5. Use a test email to subscribe your testing site.
6. See a successful message Success! An email was just sent to confirm your subscription. Please find the email now and click 'Confirm Follow' to start subscribing.
7. Try to use this email and subscribe again.
8. See an error message You subscribed this site before but you have not clicked the confirmation link yet. Please check your mailbox. Otherwise, you can manage your preferences at subscribe.wordpress.com.

Extra steps to restore your site version:

• In jetpack.php, revert the previous value of JETPACK__VERSION.
• Follow steps 3 and 4 but verify that your site version is back to the previous value.

Proposed changelog entry for your changes:

• Subscriptions: inform a clear error for pending confirmations

[matticbot]

Caution: This PR has changes that must be merged to WordPress.com
Hello htdat! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D37554-code before merging this PR. Thank you!

[htdat]

Update: I have a patch on WP.com already but still need to create a PR (Diff) there, and update its description.

Update: Added WP.com patch already. Ready to get a review now!

[jetpackbot]

	Warnings
⚠️	pre-commit hook was skipped for one or more commits

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against 152aeb0

[matticbot]

htdat, Your synced wpcom patch D37554-code has been updated.

[htdat]

Current PHPCS issue:

FILE: modules/subscriptions.php
-----------------------------------------------------------------------------------------------
FOUND 6 ERRORS AND 0 WARNINGS AFFECTING 6 LINES
-----------------------------------------------------------------------------------------------
 591 | ERROR | Line indented incorrectly; expected 4 tabs, found 3
 591 | ERROR | There must be no space before the colon in a CASE statement
 594 | ERROR | Line indented incorrectly; expected 4 tabs, found 3
 594 | ERROR | There must be no space before the colon in a CASE statement
 595 | ERROR | Line indented incorrectly; expected at least 5 tabs, found 4
 596 | ERROR | Terminating statement must be indented to the same level as the CASE body

They're all around here

jetpack/modules/subscriptions.php

Lines 581 to 600 in bc3f632

	switch ( $response[0]['status'] ) {
	case 'error' :
	$r[] = new Jetpack_Error( 'not_subscribed' );
	continue 2;
	case 'disabled' :
	$r[] = new Jetpack_Error( 'disabled' );
	continue 2;
	case 'active' :
	$r[] = new Jetpack_Error( 'active' );
	continue 2;
	case 'confirming' :
	$r[] = true;
	continue 2;
	case 'pending' :
	$r[] = new Jetpack_Error( 'pending' );
	continue 2;
	default :
	$r[] = new Jetpack_Error( 'unknown_status', (string) $response[0]['status'] );
	continue 2;
	}

I do not fix this PHPCS error so reviewers can easily to see the logic. After this is tested OK, I willl fix this PHPCS error.

[jeherve]

This looks good to me, I only have one minor remark.

I am not a native speaker though, so I am not sure I am the best person to approve the new phrase. "Otherwise.." seems to make the phrase a bit long to me, and I would phrase it a bit differently, but I'll let a native weigh in on that.

[jeherve]

I do not fix this PHPCS error so reviewers can easily to see the logic. After this is tested OK, I willl fix this PHPCS error.

Don't hesitate to address those now, and then add single comments to your own PR if you think it can make it easier for folks to review. Folks can also view the changes commit per commit if they need to break things down.

[matticbot]

htdat, Your synced wpcom patch D37554-code has been updated.

[htdat]

Don't hesitate to address those now,

Gotcha! My last commit fixed the PHPCS errors.

and then add single comments to your own PR if you think it can make it easier for folks to review. Folks can also view the changes commit per commit if they need to break things down.

Make sense! I will do this next time.

[jeherve]

This works well for me now. I'll let @kraftbj take a look as well, for functionality and for the wording. 👍

[matticbot]

htdat, Your synced wpcom patch D37554-code has been updated.

[kraftbj]

I tweaked the language (we use inbox elsewhere), but otherwise, let's do it.

[jeherve]

r202136-wpcom and r202137-wpcom