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

Protect: Dismissible Network Activation Warning #13266

Merged
merged 2 commits into from
Aug 23, 2019

Conversation

mdawaffe
Copy link
Member

Changes proposed in this Pull Request:

The warning about Jetpack Protect's network activation warning is not dismissible.

It is also not pretty :)

Both have been problems since #3841.

  • Use markup compatible with Core's notices.
  • Hook some JS into Core's notice dismissal to handle changing saved state.

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

No: bug fix.

Testing instructions:

  1. Create a multisite.
  2. Do not network activate the Jetpack plugin.
  3. Activate the Jetpack plugin on a single site.
  4. Connect Jetpack on that site.
  5. Go to that site's wp-admin/ dashboard.

Prior to this patch: See an ugly message and note that it cannot be dismissed.

After this patch: See a prettier, dismissible warning. To test repeated dismissals, you can do:
wp site option delete jetpack_dismissed_protect_multisite_banner
to reset the dismissal to the default state.

UI Changes

Before:
Screen Shot 2019-08-20 at 12 22 59 AM

After:
Screen Shot 2019-08-20 at 12 22 37 AM

Copy Changes

I think it's the case that we're not using "Jetpack Protect" for this feature anymore. Is that true?

Before:

Protect cannot keep your site secure.

Thanks for activating Protect! To start protecting your site, please network activate Jetpack on your Multisite installation and activate Protect on your primary site. Due to the way logins are handled on WordPress Multisite, Jetpack must be network-enabled in order for Protect to work properly. Learn More

View Network Admin

After:

Jetpack Brute Force Attack Prevention cannot keep your site secure

Thanks for activating Jetpack's brute force attack prevention feature! To start protecting your whole WordPress Multisite Network, please network activate the Jetpack plugin. Due to the way logins are handled on WordPress Multisite Networks, Jetpack must be network activated in order for the brute force attack prevention feature to work properly.

[View Network Admin] [Learn More]

Proposed changelog entry for your changes:

  • Fix a Jetpack Brute Force Attack Prevention notice on WordPress Multisites.

@mdawaffe mdawaffe added [Feature] Protect Also known as Brute Force Attack Protection [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Status] Needs Design Review Design has been added. Needs a review! [Status] Needs Copy Review Copy has been added. Marketing will be notified for a copy review. labels Aug 20, 2019
@mdawaffe mdawaffe added this to the 7.7 milestone Aug 20, 2019
@mdawaffe mdawaffe requested a review from a team August 20, 2019 07:29
@mdawaffe mdawaffe self-assigned this Aug 20, 2019
@jetpackbot
Copy link

jetpackbot commented Aug 20, 2019

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: September 3, 2019.
Scheduled code freeze: August 27, 2019

Generated by 🚫 dangerJS against d10cc53

lezama
lezama previously approved these changes Aug 20, 2019
modules/protect.php Outdated Show resolved Hide resolved
modules/protect.php Outdated Show resolved Hide resolved
@jeherve 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 Aug 20, 2019
The warning about Jetpack Protect's network activation warning is not dismissible.

It is also not pretty :)

Both have been problems since #3841.

* Use markup compatible with Core's notices.
* Hook some JS into Core's notice dismissal to handle changing saved state.
@jeherve jeherve force-pushed the fix/protect-multisite-activation-warning branch from 487dcc0 to b523421 Compare August 20, 2019 11:14
Co-Authored-By: Miguel Lezama <[email protected]>
@jeherve jeherve 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 Aug 20, 2019
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

This works well for me. 👍

@jeherve jeherve removed the [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. label Aug 20, 2019
Copy link
Member

@dereksmart dereksmart left a comment

Choose a reason for hiding this comment

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

Very nice improvement

@jeherve jeherve added the [Status] Ready to Merge Go ahead, you can push that green button! label Aug 23, 2019
@jeherve jeherve merged commit d40347a into master Aug 23, 2019
@jeherve jeherve deleted the fix/protect-multisite-activation-warning branch August 23, 2019 10:51
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Aug 23, 2019
jeherve added a commit that referenced this pull request Aug 23, 2019
jeherve added a commit that referenced this pull request Aug 27, 2019
* 7.7 changelog: initial set of changes

* Changelog: add #13154

* Changelog: add #13134

* Changelog: add #12699 and many others

* Changelog: add #13127

* Changelog: add #13167

* Changelog: add #13225

* Changelog: add #13179

* Changelog: add #13173

* Changelog: add #13232

* Changelog: add #13137

* Changelog: add #13172

* Changelog: add #13182

* Changelog: add #13200

* Changelog: add #13244

* Changelog: add #13267

* Changelog: add #13204

* changelog: add #13205

* Changelog: add #13183

* Changelog: add #13278

* Changelog: add #13162

* Changelog: add #13268

* Changelog: add #13286

* Changelog: add #13273

* Changelog: add #12474

* Changelog: add #13085

* Changelog: add #13266

* Changelog: add #13306

* Changelog: add #13311

* Changelog: add #13302

* Changelog: add #13196

* Changelog: add #12733

* Changelog: add #13261

* Changelog: add #13322

* Changelog: add #13333

* Changelog: add #13335
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Protect Also known as Brute Force Attack Protection [Status] Needs Copy Review Copy has been added. Marketing will be notified for a copy review. [Status] Needs Design Review Design has been added. Needs a review!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants