Add social_notifications_subscribe option in WP Admin by htdat · Pull Request #14239 · Automattic/jetpack

htdat · 2019-12-15T14:15:51Z

Fixes #709

When /wp-admin/options-discussion.php is fully loaded, the option looks like this:

Changes proposed in this Pull Request:

Basically, I followed the same approach for another similar option Someone likes one of my posts in "Likes" module https://github.com/Automattic/jetpack/search?q=social_notifications_like&type=Code

Handle social_notifications_subscribe option when "Subscriptions" module is activated/deactivated.
Add Someone follows my blog in WP Admin -> Settings -> Discussions.
Use jQuery to align up this option in the Email me whenever section.
Validate its value to only on/off.
Add this option to \Jetpack_Core_Json_Api_Endpoints class.

I do not add tests as social_notifications_subscribe is covered already. Can a dev help to double check this?

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

This is more an enhancement. See p9o2xV-3Q-p2 and Send email to site owner when the site gets a new email subscriber #709 (comment)

Testing instructions:

Disable Subscriptions module => the option should not display in "Discussions" settings
Enable Subscriptions => the checkbox of this option should not be selected by default.
Enable this option, try to subscribe with a new email => the Jetpack connection owner gets an email notification about the new email.

Proposed changelog entry for your changes:

Add Email me whenever: Someone follows my blog option to wp-admin settings.

…cussion.php

jetpackbot · 2019-12-15T14:17:50Z

	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 2386e8d

zinigor

This change works well for me, but I have several comments. I don't really like how we're copying the same code that's used in the Likes module, but I guess that's the quickest way to fix the issue that we're aiming at.
Thanks for working on this!

modules/subscriptions.php

… translators

htdat · 2019-12-26T16:06:55Z

@zinigor Thanks for your thorough review. I've added a new commit to address your suggestions.

I don't really like how we're copying the same code that's used in the Likes module, but I guess that's the quickest way to fix the issue that we're aiming at.

Well, me too, I do not like this approach much. But since we have moved new options to Calypso or Jetpack setting pages, it's not efficient to refactor related code.

zinigor

Thanks for fixing everything! I have added the changes that linter required, please don't skip checks when you commit.

Also added some new entries to the testing file.

* Changelog: 8.1 additions * Changelog: add #13858 * Changelog: add #13963 * Changelog: add #14174 * Changelog: add #14178 * Changelog: add #14175 * Changelog: add #14192 * Changelog: add #14196 * Changelog: add #14182 * Changelog: add #14218 * Changelog: add #14214 * Changelog: add #13757 * Changelog: add #14190 * Changelog: add #14131 * Changelog: add #14101 * Changelog: add #14203 * Changelog: add #14211 * Changelog: add #14224 * Changelog: add #14230 * Changelog: add #14241 * Changelog: add #14249 * Changelog: add #14264 * Changelog: add #14263 * Changelog: add #14256 * Changelog: add #10189 * Changelog: add #14240 * Changelog: add #14239 Also added some new entries to the testing file. Co-authored-by: Igor Zinovyev <zinigor@gmail.com>

Add the social_notifications_subscribe option to wp-admin/options-dis…

ae4d6b8

…cussion.php

htdat requested a review from a team December 15, 2019 14:15

matticbot added the [Status] Needs Tracks Review Added/removed/modified a tracks event. label Dec 15, 2019

htdat added [Status] Needs Review This PR is ready for review. Enhancement Changes to an existing feature — removing, adding, or changing parts of it and removed [Status] Needs Tracks Review Added/removed/modified a tracks event. labels Dec 15, 2019

htdat mentioned this pull request Dec 15, 2019

Correctly validate on/off values for the boolean type in REST API #14240

Merged

jeherve assigned htdat Dec 16, 2019

jeherve added this to the 8.1 milestone Dec 16, 2019

jeherve added the [Feature] Subscriptions All subscription-related things such as paid and unpaid, user management, and newsletter settings. label Dec 16, 2019

zinigor suggested changes Dec 25, 2019

View reviewed changes

modules/subscriptions.php Outdated Show resolved Hide resolved

modules/subscriptions.php Outdated Show resolved Hide resolved

modules/subscriptions.php Outdated Show resolved Hide resolved

modules/subscriptions.php Outdated Show resolved Hide resolved

zinigor added [Status] In Progress [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Dec 25, 2019

[not verified] Add coding style improvements and clarify a string for…

5dc2253

… translators

htdat added [Status] Needs Review This PR is ready for review. and removed [Status] In Progress [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Dec 26, 2019

zinigor added 2 commits December 27, 2019 14:42

Added changes that were skipped in the previous commit.

9040d52

Placed a comment iside a PHP block.

2386e8d

zinigor approved these changes Dec 27, 2019

View reviewed changes

zinigor added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Dec 27, 2019

zinigor merged commit fd0d69e into master Dec 30, 2019

zinigor deleted the fix/709 branch December 30, 2019 17:16

matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Dec 30, 2019

zinigor added a commit that referenced this pull request Dec 30, 2019

Changelog: add #14239

a9e741f

Also added some new entries to the testing file.

zinigor removed the [Status] Needs Changelog label Dec 30, 2019

htdat mentioned this pull request Jan 15, 2020

Subscriptions: Fix "deactivating module unsets Follow email option" #14325

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add social_notifications_subscribe option in WP Admin#14239

Add social_notifications_subscribe option in WP Admin#14239
zinigor merged 4 commits intomasterfrom
fix/709

htdat commented Dec 15, 2019 •

edited

Loading

Uh oh!

jetpackbot commented Dec 15, 2019 •

edited

Loading

Uh oh!

zinigor left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

htdat commented Dec 26, 2019

Uh oh!

zinigor left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

htdat commented Dec 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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:

Uh oh!

jetpackbot commented Dec 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zinigor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

htdat commented Dec 26, 2019

Uh oh!

zinigor left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

htdat commented Dec 15, 2019 •

edited

Loading

jetpackbot commented Dec 15, 2019 •

edited

Loading