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

Subs Widget: better error for when emails has blocked subscriptions #2599

Merged
merged 4 commits into from
Oct 4, 2015

Conversation

dereksmart
Copy link
Member

Fixes #2007

To test:
append this to a url that has the subscription widget ?subscribe=opted_out

@dereksmart dereksmart added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Feature] Subscriptions All subscription-related things such as paid and unpaid, user management, and newsletter settings. labels Aug 23, 2015
@dereksmart dereksmart added this to the 3.8 milestone Aug 23, 2015
@dereksmart dereksmart added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] In Progress labels Aug 23, 2015
@kraftbj
Copy link
Contributor

kraftbj commented Aug 23, 2015

Not at a computer to adjust the PR, but we could make the language better. "This e-mail address has opted out of subscriptions e-mails. You can adjust this preference at https://subscribe.wordpress.com." or something like that.

@dereksmart
Copy link
Member Author

sure, but the option could also be set in their "wordpress.com/me/notifications" page under "Block all email updates from blogs you’re following on WordPress.com".

@kraftbj
Copy link
Contributor

kraftbj commented Aug 23, 2015

I'm not sure if those are the same or different settings, tbh. The subscribe. site works for any e-mail adr; not wp.com user dependent.

@dereksmart
Copy link
Member Author

me neither. time to test :)

@dereksmart
Copy link
Member Author

Confirmed it's the same option in both wordpress.com/me/notifications and subscribe.wordpress.com/?option=settings

@dereksmart dereksmart force-pushed the enhance/subscription-widget-error-messages branch from d15cd9b to b53c88b Compare September 30, 2015 03:16
@dereksmart dereksmart force-pushed the enhance/subscription-widget-error-messages branch from b53c88b to ebe840d Compare September 30, 2015 20:01
@@ -734,6 +737,12 @@ function widget( $args, $instance ) {
case 'invalid_email' : ?>
<p class="error"><?php esc_html_e( 'The email you entered was invalid. Please check and try again.', 'jetpack' ); ?></p>
<?php break;
case 'opted_out' : ?>
<p class="error"><?php printf( _x( "The email address has opted out of subscription emails. %1s You can manage your preferences at %2s", '%1s is a line break, %2s is a website', 'jetpack' ),
Copy link
Member

Choose a reason for hiding this comment

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

Let's embed the HTML into translation and only insert links. Also please use a translator comment here instead of context to provide explanations for translation placeholders.

@zinigor zinigor added [Status] In Progress [Status] Requires String Changes and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Oct 2, 2015
@zinigor
Copy link
Member

zinigor commented Oct 2, 2015

Looking good except for the i18n issue. This has my 👍 after fixing

@dereksmart dereksmart added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] In Progress labels Oct 4, 2015
dereksmart added a commit that referenced this pull request Oct 4, 2015
…error-messages

Subs Widget: better error for when emails has blocked subscriptions
@dereksmart dereksmart merged commit 64fe70e into master Oct 4, 2015
@dereksmart dereksmart removed the [Status] Ready to Merge Go ahead, you can push that green button! label Oct 4, 2015
@dereksmart dereksmart deleted the enhance/subscription-widget-error-messages branch October 4, 2015 15:47
@finlanderid
Copy link

I am getting the case 'opted_out' error (The email address has opted out of subscription emails.) when there is no reason for me to be getting that error. The email address is following the particular blog and is not opting out of receiving email notifications, either from the blog or from wordpress.com globally.

I actually should be getting the case 'already' error (You have already subscribed to this site.), though. I previously signed up on the blog for subscription, and that sign-up process went fine.

Has anyone else reported this? I am certain that I should not be getting first error above, and that I should be getting the second error instead.

@jeherve
Copy link
Member

jeherve commented Nov 11, 2015

@finlanderid Could you contact us via this contact form and let us know your email address, so we can look into this with you?

Thanks!

@csonnek
Copy link
Member

csonnek commented Nov 13, 2015

@jeherve: 2416945-t

Checked and showing:

  1. Not blocking emails from WordPress.com
  2. Subscribed to site on 2015-11-11 09:58:34 per our system.

@finlanderid
Copy link

@jeherve https://github.com/jeherve: 2416945-t
@csonnek https://github.com/csonnek

exactly, that's why it's odd that I get the:

 switch/case 'opted_out' error ("The email address has opted out of 

subscription emails.")

instead of the:

 switch/case 'already' error ("You have already subscribed to this 

site.")

when I attempt to subscribe with an email address that is already
subscribed.

On 11/13/2015 3:28 PM, Carolyn Sonnek wrote:

@jeherve https://github.com/jeherve: 2416945-t

Checked and showing:

  1. Not blocking emails from WordPress.com
  2. Subscribed to site on |2015-11-11 09:58:34| per our system.


Reply to this email directly or view it on GitHub
#2599 (comment).

@jeherve
Copy link
Member

jeherve commented Nov 15, 2015

@finlanderid I replied to your email a few minutes ago.

Thanks for the extra details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Subscriptions All subscription-related things such as paid and unpaid, user management, and newsletter settings. [Status] Requires String Changes [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants