Skip to content

Conversation

@rcanepa
Copy link
Contributor

@rcanepa rcanepa commented Aug 5, 2020

Changes proposed in this Pull Request:

  • Configure constants for the new plans that are part of the Offer Reset project: Jetpack Security Daily, Jetpack Security Real-Time, and Jetpack Complete.
  • Enable these plans in the My Plan page.
  • Add a SVG icon for each of them.

Jetpack product discussion

  • Ticket: 1169247016322522-as-1187323761633593

Does this pull request change what data or activity we track or use?

  • No. It doesn't.

Testing instructions:

Notes:

  • Since we haven't configure yet these plans in our backend, it isn't possible to test this unless you have access to the code (*). It might be possible to see the new products by modifying the state of the app (I haven't tried it yet).
  • Please, don't forget to review the copy associated with each plan (the icon too).

(*) If you want to edit the code to test each plan, you can modify the /jetpack/_inc/client/my-plan/index.jsx and add to the start of the MyPlan component the following:

export function MyPlan( props ) {
    // Set the slug of the plan you want to check
	if ( props.sitePlan ) {
		props.sitePlan.product_slug = 'jetpack_security_daily';
	}
    // the rest of the code goes here...

Instructions:

  • Run this PR (locally or with the Beta Tester plugin).
  • Buy one of the new plans.
  • Visit the My Plan page.
  • Check that you see your new plan in the My Plan section.
Jetpack Security Daily

Jetpack Security Daily

Jetpack Security Real-Time

Jetpack Security Real-Time

Jetpack Complete

Jetpack Complete

Proposed changelog entry for your changes:

  • Configure Jetpack Security Daily, Jetpack Security Real-Time, and Jetpack Complete as part of the Offer Reset project.

@rcanepa rcanepa added [Status] Needs Review This PR is ready for review. Plans labels Aug 5, 2020
@rcanepa rcanepa requested a review from a team August 5, 2020 21:18
@rcanepa rcanepa added the Admin Page React-powered dashboard under the Jetpack menu label Aug 5, 2020
@jetpackbot
Copy link
Collaborator

jetpackbot commented Aug 5, 2020

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.

E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-16739

Scheduled Jetpack release: September 1, 2020.
Scheduled code freeze: August 25, 2020

Generated by 🚫 dangerJS against 0aeac0a

@monsieur-z
Copy link

I wasn't able to test the PR locally, but code and copy look good. We've released the icons in the components package in Calypso (see this component). I'm not familiar with the Jetpack codebase but I'm wondering if they could easily be reused here.

@rcanepa
Copy link
Contributor Author

rcanepa commented Aug 6, 2020

I wasn't able to test the PR locally, but code and copy look good. We've released the icons in the components package in Calypso (see this component). I'm not familiar with the Jetpack codebase but I'm wondering if they could easily be reused here.

Good point. I'll investigate that.

robertf4
robertf4 previously approved these changes Aug 6, 2020
Copy link
Contributor

@robertf4 robertf4 left a comment

Choose a reason for hiding this comment

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

LGTM!

@robertf4 robertf4 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 Aug 6, 2020
jeherve
jeherve previously approved these changes Aug 10, 2020
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 looks good to me.

Since we haven't configure yet these plans in our backend, it isn't possible to test this unless you have access to the code (*). It might be possible to see the new products by modifying the state of the app (I haven't tried it yet).

Perhaps for a follow-up PR, but this would be a good opportunity to add those new plans to our debug tools here:

https://github.com/Automattic/jetpack/blob/master/_inc/client/components/dev-card/index.jsx

For reference, those debug tools can be accessed here:
image

@rcanepa
Copy link
Contributor Author

rcanepa commented Aug 10, 2020

This looks good to me.

Since we haven't configure yet these plans in our backend, it isn't possible to test this unless you have access to the code (*). It might be possible to see the new products by modifying the state of the app (I haven't tried it yet).

Perhaps for a follow-up PR, but this would be a good opportunity to add those new plans to our debug tools here:

https://github.com/Automattic/jetpack/blob/master/_inc/client/components/dev-card/index.jsx

For reference, those debug tools can be accessed here:
image

TIL. I wasn't aware of these debug tools. I'm adding the new plans right now.

@rcanepa rcanepa dismissed stale reviews from jeherve and robertf4 via 0aeac0a August 10, 2020 16:10
@rcanepa
Copy link
Contributor Author

rcanepa commented Aug 10, 2020

👋 @jeherve, I added the new Offer Reset plans to the dev tools.

Kapture 2020-08-10 at 13 08 18

@jeherve jeherve added [Status] Needs Review This PR is ready for review. and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Aug 11, 2020
@jeherve jeherve added this to the 8.9 milestone Aug 11, 2020
@jeherve jeherve 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 Aug 11, 2020
@rcanepa rcanepa merged commit 33c58d3 into master Aug 12, 2020
@rcanepa rcanepa deleted the add/offer-reset-new-plans branch August 12, 2020 16:36
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Aug 12, 2020
davidlonjon added a commit that referenced this pull request Aug 14, 2020
* master: (41 commits)
  use blog token to make the request (#16635)
  External Media: Add account disconnect button (#16759)
  CI: Try collect js coverage (#16786)
  Sync: Fix nonce action string in theme edit sync (#16702)
  Connect-in-place: hide new heading during connection process (#16703)
  Update dependency eslint-plugin-jsdoc to v30.2.1 (#16765)
  Theme Tools: Resolve PHP 7.4 array offset notice. (#16795)
  New shell command for easier access to the database. (#16761)
  My Plan: Add Offer Reset project new plans (Jetpack Security, Jetpack Complete) (#16739)
  Increase the `editor.MediaUpload` hook priority (#16669)
  External Media: Remove `speak` announcement when inserting media.
  Extensions: make `render_callback` optional when checking block registration against plan (#16746)
  Conditional check for wrapper before giving focus to new page (#16817)
  Docker: Add package testing shortcut (#16810)
  Settings: Recognize valid Akismet keys from wp-config and restrict input (#16542)
  Social Previews: Add Modal (#16704)
  Update dependency preact to v10.4.7 (#16768)
  Improve a11y of amp-social-share (#16737)
  Instant Search: Tweak expanded result path styling (#16762)
  Docker: Add phpmyadmin to the docker-composer.yml (#16806)
  ...
pereirinha pushed a commit that referenced this pull request Sep 10, 2020
…curity, Jetpack Complete) (#16739)

* Add Offer Reset new plan constants

* Enable new plans in the My Plan page

* Add new plans SVG icons

* Add Offer Reset plans to dev tools
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Admin Page React-powered dashboard under the Jetpack menu Plans

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants