Plans: Add parameter to show only specific plans for the Jetpack app plan selection flow #77029

staskus · 2023-05-17T14:44:36Z

Related to #76968

Details

pcdRpT-2Bo-p2
Scope: pc8eDl-Vv-p2
Discussion: p1684158877461119-slack-CFFF01Q4V

Create a WP.com Plans selection for the Jetpack app limiting it to only showing WP.com Personal and Premium plans.

Proposed Changes

Check jetpackAppPlans parameter to set planTypes props of PlansFeaturesMain component to limit plan selection to Personal and Premium Plans:

/plans/{siteURL}?jetpackAppPlans=true
/plans/yearly/{siteURL}?domainAndPlanPackage=true&jetpackAppPlans=true

Follow a similar pattern to #66022 which added domainAndPlanPackage parameter

Testing Instructions

Normal plan selection flow

Select simple/atomic site
Visit /plans/{siteURL}?jetpackAppPlans=true page
Confirm that only personal and premium plans are visible
Switch between annual and monthly plans
Confirm that only personal and premium plans are visible
Confirm that after selecting the plan, the checkout view is opened

Domain and plan package plan selection flow

Select simple/atomic site
Visit /plans/yearly/{siteURL}?domainAndPlanPackage=true&jetpackAppPlans=true page
Confirm that only personal and premium plans are visible
Confirm that header with steps and back button is not visible
Confirm that after selecting the plan, the checkout view is opened

Regression normal plan selection flow

Select simple/atomic site
Visit /plans page
Confirm that all the available plans are offered

Regression domain and plan package plan selection flow

Select simple/atomic site
Visit /plans/yearly/{siteURL}?domainAndPlanPackage=true page
Confirm that all the available plans are offered
Confirm that header with steps and back button appears on top

Videos

Before. Plans for domain and plan package

Plans.Purchasing.Before.mov

After. Plans for domain and plan package and `jetpackAppPlans=true`

plan.and.domain.purchasing.flow.new.mov

After. Plans with `jetpackAppPlans=true`

plan.only.purchase.new.mov

Pre-merge Checklist

Has the general commit checklist been followed? (PCYsg-hS-p2)
Have you written new tests for your changes?
Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
Have you checked for TypeScript, React or other console errors?
Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-ajp-p2)?

…s set Support configuring wp.com/plans/yearly/{siteURL} flow with jetpackAppPlans parameter, that limits the number of plans shown to support Jetpack app-specific flows

…arameter is set Support the Jetpack app plan purchasing flows by hiding a navigation header that displays the position inside domain and plan upsell flow and allows to come back

github-actions · 2023-05-17T14:44:49Z

Calypso Live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-68429

Jetpack Cloud live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-68429&env=jetpack

staskus · 2023-05-17T14:46:37Z

@southp, we would be thankful if you could take a look at this PR when you're available. Since we're not contributing to Calypso often, let us know if anything is missing. Thank you!

CC: @ravishanker, @irfano, @guarani

matticbot · 2023-05-17T14:55:11Z

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Sections (~118 bytes added 📈 [gzipped])

name                  parsed_size           gzip_size
plans                      +286 B  (+0.0%)      +81 B  (+0.0%)
update-design-flow          +87 B  (+0.0%)      +21 B  (+0.0%)
plugins                     +87 B  (+0.0%)      +16 B  (+0.0%)
link-in-bio-tld-flow        +87 B  (+0.0%)      +21 B  (+0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~21 bytes added 📈 [gzipped])

name                                             parsed_size           gzip_size
async-load-signup-steps-plans-theme-preselected        +87 B  (+0.0%)      +21 B  (+0.0%)
async-load-signup-steps-plans-pm                       +87 B  (+0.0%)      +21 B  (+0.0%)
async-load-signup-steps-plans                          +87 B  (+0.0%)      +21 B  (+0.0%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

guarani

👋 This looks good @staskus!
There might be a convention that values of true and false for "boolean" query params are equivalent, but I haven't found one. However, jetpackAppPlans follows the same convention set by domainAndPlanPackage, so this seems OK.

I tested locally using the quick start testing steps and it worked as per the test plan. I can't see any potential regressions so I think this is good from my side.

Let me know if I should provide an approval or leave that for another reviewer.

…both true and false values Parse using JSON.parse so parameter=true would be correctly parsed as a true Boolean and parameter=false would be parsed as a false Boolean

…the-jp-app

staskus · 2023-05-18T11:22:56Z

There might be a convention that values of true and false for "boolean" query params are equivalent, but I haven't found one. However, jetpackAppPlans follows the same convention set by domainAndPlanPackage, so this seems OK.

@guarani, you spotted it well. I don't think that's correct, even if it was unharmful for domainAndPlanPackage. I made a fix to parse values to Booleans.

southp · 2023-05-26T09:26:15Z

@staskus Sorry for the late response. I've tested it and it works well in general. However, I'm running out of time to look into the code deeper and to go through potential regression parts. I'll ping the team for a more timely review 🙇🏼

guarani · 2023-05-26T14:29:16Z

Thanks for the review, @southp! I appreciate it ❤️

jeyip · 2023-05-26T18:18:40Z

jeyip · 2023-05-26T19:16:31Z

client/my-sites/plans/controller.jsx

@@ -38,7 +38,8 @@ export function plans( context, next ) {
 					? context.query.addDomainFlow === 'true'
 					: undefined
 			}
-			domainAndPlanPackage={ context.query.domainAndPlanPackage }
+			domainAndPlanPackage={ JSON.parse( context.query.domainAndPlanPackage ?? false ) }


Non-blocking / nitpick:

Two notes here:

If we want to keep JSON.parse, I think this could be more readable as JSON.parse( Boolean( context.query.domainAndPlanPackage ) )

It appears you're using JSON.parse to handle string types that express boolean values E.G. "true" instead of true. I would actually prefer context.query.domainAndPlanPackage === "true" because it more clearly represents our intentions of coercing some value. I'm unsure about any conventions either

Thanks! That's is a good idea to explicitly check for the "true" case rather than doing parsing and conversion that obscures the readability of the code 👍

jeyip

I haven't working in plans for too long, but from what I can tell, this tests well, and I only have smaller non-blocking concerns. 🚢

jeyip · 2023-05-26T19:43:41Z

client/my-sites/plans/main.jsx

@@ -259,6 +261,7 @@ class Plans extends Component {
 		}

 		const hideFreePlan = ! is2023PricingGridVisible || this.props.isDomainAndPlanPackageFlow;
+		const planTypes = this.props.jetpackAppPlans ? [ TYPE_PERSONAL, TYPE_PREMIUM ] : null;


Non-blocking / nitpick:

Spitballing here, but wondering if a small inline comment might be useful?

// The jetpack mobile app only wants to display two plans -- personal and premium

Yes, I think the more explicit we are in such cases the better so the comment would only increase the clarity by expressing intentions 👍

…the Jetpack app

…the-jp-app

staskus · 2023-05-30T09:21:54Z

@jeyip thank you for the review, I really appreciate that!

Is it enough to merge or are any additional actions required for the changes to be deployed?

guarani · 2023-05-31T16:11:55Z

From the Contributing docs, this needs a [Status] Ready to Merge label from a reviewer for it to be ready to merge.

jeyip · 2023-05-31T17:21:49Z

Is it enough to merge or are any additional actions required for the changes to be deployed?

You should be good to go!

From the Contributing docs, this needs a [Status] Ready to Merge label from a reviewer for it to be ready to merge.

@guarani

I’m not sure if there’s something particular to the part of the codebase updated in this PR, but I haven’t used the “Ready to Merge” label in any Calypso dev workflows before. Apologies if it was blocking progress on the PR, but, for me, everything seems fine to merge 🚢

guarani · 2023-05-31T18:03:50Z

I’m not sure if there’s something particular to the part of the codebase updated in this PR

I might have mis-read the docs or come across outdated ones, because I'm new to the Calypso codebase here.

No worries and thanks for the help on this PR, @jeyip! ❤️

I'm going to go ahead and merge now. Nice work on this, @staskus!

staskus added 3 commits May 17, 2023 17:42

Show only personal and premium plans when jetpackAppPlans parameter i…

886d47c

…s set Support configuring wp.com/plans/yearly/{siteURL} flow with jetpackAppPlans parameter, that limits the number of plans shown to support Jetpack app-specific flows

Hide domain and plan package header from Plans when jetpackAppPlans p…

0ffb9a8

…arameter is set Support the Jetpack app plan purchasing flows by hiding a navigation header that displays the position inside domain and plan upsell flow and allows to come back

Remember jetpackAppPlans parameter when toggling between Plan intervals

a070609

staskus added [Feature] Plans & Upgrades All of the plans on WordPress.com and flow for upgrading plans. [Type] Task Mobile Application WordPress.com App for mobile labels May 17, 2023

staskus requested review from southp and guarani May 17, 2023 14:44

matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label May 17, 2023

guarani reviewed May 17, 2023

View reviewed changes

ravishanker mentioned this pull request May 18, 2023

Plans on JP: Free domain selection and plan selection wordpress-mobile/WordPress-Android#18461

Merged

3 tasks

staskus added 2 commits May 18, 2023 14:20

Parse domainAndPlanPackage and jetpackAppPlans parameters to support …

a7e3f3c

…both true and false values Parse using JSON.parse so parameter=true would be correctly parsed as a true Boolean and parameter=false would be parsed as a false Boolean

Merge branch 'trunk' into update/76968-different-plans-selection-for-…

a3ea576

…the-jp-app

Handle parameters being null before parsing to Boolean

5d02efe

This was referenced May 19, 2023

Plans on JP: Add plan step to free-to-paid purchase flow wordpress-mobile/WordPress-iOS#20716

Merged

Plans on JP: Create Plans selection and integrate in the flow wordpress-mobile/WordPress-iOS#20688

Closed

staskus mentioned this pull request May 23, 2023

Plans on JP: Use a single modal for the entire flow wordpress-mobile/WordPress-iOS#20731

Merged

10 tasks

ravishanker mentioned this pull request May 24, 2023

Plans on JP: Checkout Success wordpress-mobile/WordPress-Android#18497

Merged

12 tasks

southp requested a review from a team May 24, 2023 05:51

jeyip reviewed May 26, 2023

View reviewed changes

jeyip approved these changes May 26, 2023

View reviewed changes

jeyip reviewed May 26, 2023

View reviewed changes

Use explicit value comparison for query parameters

23db848

staskus added 3 commits May 30, 2023 10:09

Add a comment explaining filtering of premium and personal plans for …

9671427

…the Jetpack app

Define domainAndPlanPackage prop as boolean

00ecf1d

Merge branch 'trunk' into update/76968-different-plans-selection-for-…

7818de5

…the-jp-app

jeyip added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels May 31, 2023

guarani merged commit 6a8c1e7 into trunk May 31, 2023

guarani deleted the update/76968-different-plans-selection-for-the-jp-app branch May 31, 2023 18:05

github-actions bot removed the [Status] Ready to Merge label May 31, 2023

guarani mentioned this pull request Jun 5, 2023

Plans on JP: Update UI test to reach purchase result screen wordpress-mobile/WordPress-iOS#20805

Closed

4 tasks

southp mentioned this pull request Jun 29, 2023

Plans 2023: (foundation) Sync plans grid views across signup/tailored flows and admin #77667

Merged

25 tasks

This was referenced Sep 20, 2023

Expand plan selection with business and ecommerce plans on Jetpack app #81965

Merged

Plans in Site Creation: Investigate plans web view integration into site creation flow before site is created wordpress-mobile/WordPress-iOS#21598

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Plans: Add parameter to show only specific plans for the Jetpack app plan selection flow #77029

Plans: Add parameter to show only specific plans for the Jetpack app plan selection flow #77029

staskus commented May 17, 2023

github-actions bot commented May 17, 2023 •

edited by matticbot

Loading

staskus commented May 17, 2023

matticbot commented May 17, 2023 •

edited

Loading

guarani left a comment

staskus commented May 18, 2023

southp commented May 26, 2023

guarani commented May 26, 2023

jeyip commented May 26, 2023 •

edited

Loading

jeyip May 26, 2023 •

edited

Loading

staskus May 30, 2023

jeyip left a comment

jeyip May 26, 2023 •

edited

Loading

staskus May 30, 2023

staskus commented May 30, 2023

guarani commented May 31, 2023

jeyip commented May 31, 2023

guarani commented May 31, 2023

Plans: Add parameter to show only specific plans for the Jetpack app plan selection flow #77029

Plans: Add parameter to show only specific plans for the Jetpack app plan selection flow #77029

Conversation

staskus commented May 17, 2023

Details

Proposed Changes

Testing Instructions

Normal plan selection flow

Domain and plan package plan selection flow

Regression normal plan selection flow

Regression domain and plan package plan selection flow

Videos

Before. Plans for domain and plan package

After. Plans for domain and plan package and jetpackAppPlans=true

After. Plans with jetpackAppPlans=true

Pre-merge Checklist

github-actions bot commented May 17, 2023 • edited by matticbot Loading

staskus commented May 17, 2023

matticbot commented May 17, 2023 • edited Loading

guarani left a comment

Choose a reason for hiding this comment

staskus commented May 18, 2023

southp commented May 26, 2023

guarani commented May 26, 2023

jeyip commented May 26, 2023 • edited Loading

Testing

Requirements

Browsers

jeyip May 26, 2023 • edited Loading

Choose a reason for hiding this comment

staskus May 30, 2023

Choose a reason for hiding this comment

jeyip left a comment

Choose a reason for hiding this comment

jeyip May 26, 2023 • edited Loading

Choose a reason for hiding this comment

staskus May 30, 2023

Choose a reason for hiding this comment

staskus commented May 30, 2023

guarani commented May 31, 2023

jeyip commented May 31, 2023

guarani commented May 31, 2023

After. Plans for domain and plan package and `jetpackAppPlans=true`

After. Plans with `jetpackAppPlans=true`

github-actions bot commented May 17, 2023 •

edited by matticbot

Loading

matticbot commented May 17, 2023 •

edited

Loading

jeyip commented May 26, 2023 •

edited

Loading

jeyip May 26, 2023 •

edited

Loading

jeyip May 26, 2023 •

edited

Loading