Subscriptions: Add a clear message when an email having many pending confirmations tries to subscribe a site. #14275

htdat · 2019-12-22T13:18:44Z

Fixes #2278
Similar PRs: #9093 and #2599

Changes proposed in this Pull Request:

For A12s, see 23696-pb for the reason I am implementing flooded_email status.

Before - unclear message:

After (with this fix) - clearer error message:

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

It improves the current Subscription feature.

Testing instructions:

Use your test email, try to subscribe at least 5 WP.com and Jetpack sites without clicking confirmation links. Some suggestions: jetpack.com, en.blog.wordpress.com (es, ja, fr blog sites too).
Go to your testing Jetpack site
Try to subscribe with this email
Get redirected to a link like this http://site.com/?subscribe=many_pending_subs#blog_subscription-3, AND see the message in the "after" screenshot above.

Proposed changelog entry for your changes:

Subscriptions: Add a clear message when an email having many pending confirmations tries to subscribe a site.

…ving many pending subscriptions

matticbot · 2019-12-22T13:18:50Z

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 D37046-code before merging this PR. Thank you!

jetpackbot · 2019-12-22T13:20:28Z

	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 4df8fe4

htdat · 2019-12-22T13:26:15Z

pre-commit hook was skipped for one or more commits

This is intentional as I am following the style of the current code.

kraftbj · 2019-12-25T15:53:37Z

Thanks! For the code styling, the intent of the changed line test is for updated code to get current styling to reduce the need (and size of) the more massive "PHPCS everything!" PRs. Merging those back to wp.com isn't the most fun thing!

htdat · 2019-12-26T08:07:36Z

@kraftbj Gotcha, thanks for the explanation! So should I fix all style coding issues in this PR or I keep it as-is now?

modules/subscriptions/views.php

jeherve · 2020-01-07T08:48:00Z

modules/subscriptions/views.php

@@ -150,6 +150,12 @@ static function render_widget_status_messages( $instance ) {
 							__( 'Manage your email preferences.', 'jetpack' )
 						); ?></p>
 					<?php break;
+				case 'many_pending_subs' : ?>
+					<p class="error"><?php printf( __( 'You already have several pending email subscriptions. <br /> Approve or delete a few subscriptions at <a href="%1$s" title="%2$s" target="_blank">subscribe.wordpress.com</a> before continuing.', 'jetpack' ),


I don't know if several is the right word here, since this can happen with just a few pending subs. Should we remove that word?

several means more than two but not many. (I checked on the Oxford dictionary :D) Basically, I think its meaning is similar to "a few"?

Originally, I used "a few" too and then I followed the same message in WP.com side
https://github.com/Automattic/jetpack/blob/8.0/modules/subscriptions/views.php#L177

I'll let a native chime in on this :)

cc @kraftbj

jeherve · 2020-01-07T08:49:48Z

So should I fix all style coding issues in this PR or I keep it as-is now?

I would suggest adhering to coding standards in the lines you changed. In a follow-up PR, you could then update the code around it to follow the same pattern.

matticbot · 2020-01-07T14:38:26Z

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

htdat · 2020-01-07T14:51:58Z

I would suggest adhering to coding standards in the lines you changed. In a follow-up PR, you could then update the code around it to follow the same pattern.

Thanks, Jeremy. I've updated code per your feedback and follow up coding standards.
When this PR is ready to merge, I will create another PR to fix the code around it.

modules/subscriptions/views.php

matticbot · 2020-01-11T09:07:07Z

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

htdat · 2020-01-11T09:09:23Z

I've added a new commit to address the recent feedback. Thank you, @jeherve.
Hope it's OK now :D

jeherve

This tests well for me now.

kraftbj · 2020-01-14T16:40:32Z

modules/subscriptions/views.php

+							wp_kses(
+								/* translators: 1: Link to Subscription Management page https://subscribe.wordpress.com/, 2: Description of this link */
+								__( 'You already have several pending email subscriptions. <br /> Approve or delete a few subscriptions at <a href="%1$s" title="%2$s" target="_blank" rel="noopener noreferrer">subscribe.wordpress.com</a> before continuing.', 'jetpack' ),
+								self::$allowed_html_tags_for_message


This is a nice way to keep things tidy and makes it easily reusable in the future. Cool.

kraftbj · 2020-01-14T16:49:43Z

r201611-wpcom

#14275 is merged

* [not verified] Rebase from the master branch and resolve conflict after #14275 is merged * Fix the space issue (again) * Add rel attribute Co-Authored-By: Jeremy Herve <jeremy@jeremy.hu> * Fix PHPCS error for switch statement * Update wording. Co-authored-by: Jeremy Herve <jeremy@tagada.hu> Co-authored-by: Brandon Kraft <public@brandonkraft.com>

* [not verified] Remove empty readme section * Initial changelog for 8.2 * Changelog: add #14220 * Changelog: add #14252 * Changelog: add #14291 * Changelog: add #14309 * Changelog: add #14304 * Changelog: add general connection log. * Changelog: add #14275 * Changelog: add #14313 * Changelog: add #14213 * Changelog: add #14357 * Add sync testing instructions * Add 8.1.1 changelog back See eeaafab and 61757eb * Changelog: add #14371 * Changelog: add #14386 * Changelog: add #14471 * Changelog: add #14325 * Changelog: add #14194 * Changelog: add #14340 * Changelog: add #14418 * Changelog: add #14417 * Changelog: add #14075 * Changelog: add #14467 * Changelog: add #14307 * Changelog: add #14326

htdat added 2 commits December 22, 2019 19:48

[not verified] Give a clear error message when email addresses are ha…

9729f04

…ving many pending subscriptions

[not verified] Update the message to visitors

b354227

htdat requested a review from a team December 22, 2019 13:18

matticbot added the Touches WP.com Files label Dec 22, 2019

jeherve added this to the 8.2 milestone Jan 7, 2020

jeherve assigned htdat Jan 7, 2020

jeherve requested changes Jan 7, 2020

View reviewed changes

jeherve added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Jan 7, 2020

htdat added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Jan 7, 2020

Improve per feedback and follow up coding standards

b368167

htdat force-pushed the fix/many-pending-sub-confirmations branch from 01fff0d to b368167 Compare January 7, 2020 14:49

jeherve requested changes Jan 8, 2020

View reviewed changes

modules/subscriptions/views.php Outdated Show resolved Hide resolved

modules/subscriptions/views.php Outdated Show resolved Hide resolved

jeherve added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Jan 8, 2020

Add more changes from the recent feedback

4df8fe4

htdat removed the [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! label Jan 11, 2020

htdat added the [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. label Jan 11, 2020

jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Jan 13, 2020

jeherve approved these changes Jan 13, 2020

View reviewed changes

kraftbj reviewed Jan 14, 2020

View reviewed changes

kraftbj merged commit dd76244 into master Jan 14, 2020

kraftbj deleted the fix/many-pending-sub-confirmations branch January 14, 2020 16:40

matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Jan 14, 2020

htdat added a commit that referenced this pull request Jan 16, 2020

[not verified] Rebase from the master branch and resolve conflict after

c790bd4

#14275 is merged

jeherve added [Status] Has Changelog and removed [Status] Needs Changelog labels Jan 17, 2020

jeherve added a commit that referenced this pull request Jan 17, 2020

Changelog: add #14275

6063914

htdat mentioned this pull request Aug 16, 2020

Subscriptions: Improve Messaging when an email has too many Pending subscriptions #16790

Closed

bindlegirl mentioned this pull request Apr 13, 2022

Subscriptions: Subscribing to comments silently fails for users with pending subscriptions block #23922

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Subscriptions: Add a clear message when an email having many pending confirmations tries to subscribe a site. #14275

Subscriptions: Add a clear message when an email having many pending confirmations tries to subscribe a site. #14275

htdat commented Dec 22, 2019 •

edited

Loading

matticbot commented Dec 22, 2019

jetpackbot commented Dec 22, 2019 •

edited

Loading

htdat commented Dec 22, 2019

kraftbj commented Dec 25, 2019

htdat commented Dec 26, 2019

jeherve Jan 7, 2020

htdat Jan 7, 2020 •

edited

Loading

jeherve Jan 8, 2020

jeherve commented Jan 7, 2020

matticbot commented Jan 7, 2020

htdat commented Jan 7, 2020

matticbot commented Jan 11, 2020

htdat commented Jan 11, 2020 •

edited

Loading

jeherve left a comment

kraftbj Jan 14, 2020

kraftbj commented Jan 14, 2020

Subscriptions: Add a clear message when an email having many pending confirmations tries to subscribe a site. #14275

Subscriptions: Add a clear message when an email having many pending confirmations tries to subscribe a site. #14275

Conversation

htdat commented Dec 22, 2019 • edited Loading

Changes proposed in this Pull Request:

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

Testing instructions:

Proposed changelog entry for your changes:

matticbot commented Dec 22, 2019

jetpackbot commented Dec 22, 2019 • edited Loading

htdat commented Dec 22, 2019

kraftbj commented Dec 25, 2019

htdat commented Dec 26, 2019

jeherve Jan 7, 2020

Choose a reason for hiding this comment

htdat Jan 7, 2020 • edited Loading

Choose a reason for hiding this comment

jeherve Jan 8, 2020

Choose a reason for hiding this comment

jeherve commented Jan 7, 2020

matticbot commented Jan 7, 2020

htdat commented Jan 7, 2020

matticbot commented Jan 11, 2020

htdat commented Jan 11, 2020 • edited Loading

jeherve left a comment

Choose a reason for hiding this comment

kraftbj Jan 14, 2020

Choose a reason for hiding this comment

kraftbj commented Jan 14, 2020

htdat commented Dec 22, 2019 •

edited

Loading

jetpackbot commented Dec 22, 2019 •

edited

Loading

htdat Jan 7, 2020 •

edited

Loading

htdat commented Jan 11, 2020 •

edited

Loading