Plans 2023: Migrate plan-properties computation and other refactors #79733

chriskmnds · 2023-07-21T11:15:16Z

Fixes #79003, #79000
Partly addresses #79004, #79005 -- which can be deprioritized until package extraction

Proposed Changes

extracts the plan properties computation into respective hooks for putting together GridPlan concept.
- GridPlan is a more complete data structure with most data needed by the respective grid components i.e. components are now fed only the GridPlans they need to render (e.g. ComparisonGrid gets the full set of plans that features are built out from) and these include all Pricing and Features metadata.
starts to surface more the splitting up of individual components - you will notice two instances of grid context provider, which is what the eventual split will be (with FeaturesGrid, ComparisonGrid, SpotlightPlan be enclosed within their own grid context)

A few more:

More work/refactors on the hooks used for putting together GridPlan can help down the line (use-grid-plan, use-plan-features-). Nothing final, and the calls made in plan-features-main can potentially be reduced once everything public-facing for the plans package.
Decouples the grid components from client/lib/plans/features-list (some parts are intermediate until feature definitions migrate to calypso-products). So partly addresses Plans Next: Migrate the Features list/metadata to @automattic/calypso-products #79004 to get us closer to a package
Decouples the grid from local calypso state for pricing computation (also intermediate until site and plans data stores are built out more, although this looks like it will be a long way out endeavor). So also partly addresses Plans 2023: Refactor grid plans pricing to derive from Plans data-store #79005 to get us closer to a package

Testing Instructions

Everything plans related:

@southp has written out a careful testing plan down below. For a more general sanity check:

Test /start and tailored flows (e.g. /start/hosting, /setup/newsletter, /setup/link-in-bio)
Test /plans/[ paid, free]
Test /plans with WooExpress (grabbed from /setup/wooexpress) and Newsletter sites
Confirm plan features render correctly in both features table and comparison grid (compare with production that all the checkmarks are correct)
Confirm pricing, discounts, and notices render correctly (try /plans/[paid] with a paid site to show the pro-rated credits notice)

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)?

github-actions · 2023-07-21T11:15:29Z

Calypso Live (direct link)

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

Jetpack Cloud live (direct link)

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

matticbot · 2023-07-21T11:26:15Z

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

Sections (~5763 bytes added 📈 [gzipped])

name                  parsed_size           gzip_size
update-design-flow        +5723 B  (+0.7%)    +1464 B  (+0.7%)
plugins                   +5723 B  (+0.3%)    +1501 B  (+0.3%)
plans                     +5723 B  (+0.5%)    +1400 B  (+0.5%)
link-in-bio-tld-flow      +5723 B  (+0.4%)    +1414 B  (+0.4%)

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 (~341 bytes added 📈 [gzipped])

name                                                 parsed_size           gzip_size
async-load-signup-steps-plans-theme-preselected          +5723 B  (+2.5%)    +1420 B  (+2.1%)
async-load-signup-steps-plans                            +5723 B  (+2.5%)    +1420 B  (+2.2%)
async-load-calypso-my-sites-plan-features-2023-grid      -3942 B  (-2.0%)    -1079 B  (-1.7%)

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.

southp · 2023-07-26T08:09:41Z

ddc22 · 2023-07-26T17:11:57Z

Reviewer Story - aka what I understood

This is a set of personal notes that other reviewer can find useful and possibly also thinks out loud so that any misconceptions can be clarified by the PR author as well.

Examining the New Hooks Introduced

`useGridPlans` - data-store/use-grid-plans.tsx

This is the root of the change where the GridPlan data structure is built. The useGridPlan is currently wired up in the client at plans-features-main/index.tsx. It acts as a central consolidating hook to several other hooks (and simple Functions?) . Some hooks that branch out from here include.

usePlanTypesWithIntent - this is not a hook but a simple function that generates an array based on variables
usePlansFromTypes - Same here this is a simple function which talks to other functions but this not related to the current change
usePlanUpgradeabilityCheck - Selects from client state
useHighlightLabels - Unrelated to this PR
usePricedAPIPlans - The simple API returned plain plan price object
usePricingMetaForGridPlans - Even more plan pricing related derived data based on PricedAPIPlan

Hooks Injected from client

/data-store/use-priced-api-plans.ts - usePricedAPIPlans - This seems to currently get data from the calypso static data structures and uses the getPlan method where the api returned data sliced from the redux state. It lives in client due to intense coupling so it's passed as a prop for now.

wp-calypso/packages/data-stores/src/plans/types.ts

Lines 57 to 61 in 95bb3be

    
           export interface PricedAPIPlan { 
        
           	product_id: number; 
        
           	product_name: string; 
        
           	path_slug?: PlanPath; 
        
           	product_slug: StorePlanSlug;

/data-store/use-pricing-meta-for-grid-plans.ts Similar to above mainly exacts pricing data from usePricedAPIPlans and api returned plan price information. It lives in client due to intense coupling so it's passed as a prop for now. This utilises some existing redux selectors to structure and build the data type shown below.

wp-calypso/client/my-sites/plan-features-2023-grid/hooks/npm-ready/data-store/use-grid-plans.tsx

Lines 44 to 48 in b70413d

    
           export interface PricingMetaForGridPlan { 
        
           	billingPeriod?: PricedAPIPlan[ 'bill_period' ] | null; 
        
           	currencyCode?: PricedAPIPlan[ 'currency_code' ] | null; 
        
           	originalPrice: { 
        
           		monthly: number | null;

plans-features-main/hooks/use-plan-upgradeability-check.ts This is another hook that is tightly coupled to client/state which resolves if the current site plan is upgradeable to any plan. Runs the following selector on a given array of slugs.

wp-calypso/client/my-sites/plans-features-main/hooks/use-plan-upgradeability-check.ts

Lines 20 to 22 in abc79dc

    
           	[ planSlug ]: 
        
           		!! selectedSiteId && isPlanAvailableForPurchase( state, selectedSiteId, planSlug ), 
        
           };

Hooks ready to be moved into separate package

data-store/use-grid-plans.tsx/usePlanTypesWithIntent. - This hooks is an internal utility helper function to useGridPlan that resolves an array of related plan slugs given a specific intent and other relevant logic.

wp-calypso/client/my-sites/plan-features-2023-grid/hooks/npm-ready/data-store/use-grid-plans.tsx

Lines 186 to 194 in b70413d

    
           case 'plans-default-wpcom': 
        
           	planTypes = [ 
        
           		TYPE_FREE, 
        
           		...( isBloggerAvailable ? [ TYPE_BLOGGER ] : [] ), 
        
           		TYPE_PERSONAL, 
        
           		TYPE_PREMIUM, 
        
           		TYPE_BUSINESS, 
        
           		TYPE_ECOMMERCE, 
        
           		...( isEnterpriseAvailable ? [ TYPE_ENTERPRISE_GRID_WPCOM ] : [] ),

Hooks used outside useGridPlan

`useIsLargeCurrency` - use-is-large-currency.ts

This hook was previously introduced and is just refactored to use the GridPlan data structure and moved. It is wired closer near the grid.

Other Related Notes

usePlansFromTypes Takes a set of planTypes ( Which are essentially plan slugs but for the frontend ) and a term. and provides the actual mapped planSlugs which are the actual plan unique identifiers in the back end.

jeyip

Summary for my own understanding:

Plans Properties becomes Grid Plans

This PR focuses on plansProperties, a data structure that we used to build inclient/my-sites/plan-features-2023-grid/index.tsx
plansProperties was a collection of data about each plan ( ecommerce, business, premium, etc. ). The data was used to help the features grid and comparison grid render properly in onboarding, /plans, etc. ( Ex. what storage options were available for each plan, what kind of billing was available for each plan, etc. )
This PR moves the construction of plansProperties into a hook, use-grid-plans.tsx
This PR also renames plansProperties into gridPlans

Grid Plans are made accessible through React Context

gridPlans are retrieved from the hook and made available to components through the PlansGridContext Provider defined in grid-context.tsx https://github.com/Automattic/wp-calypso/pull/79733/files#diff-b4abc92980ca26396e8c6842ad40c6228bed83151e150efafe902162abb94f71R27-R33
-PlansGridContext wraps the Feature Grid and the Comparison Grid. All components that used to need planProperties now have access to gridPlans through the usePlansGridContext hook

The large surface area of file changes and lines of code changed is due to the widespread usage of plansProperties.

jeyip · 2023-07-26T21:59:01Z

Testing

Requirements

Spot checked with production:

line items in the feature grid
storage labels in the feature grid
header prices in feature + comparison grid
highlighted plan columns
monthly terms
annual terms

Also tested:

plan selection and checkout
plan switcher in /plans
plan upgrades from /plans
pro-rated credits

Browsers

Chrome

Issues

@chriskmnds

The /start/hosting flow in this branch displays all plans instead of just Business and Commerce plans. I double checked, and Calypso trunk also only displays Business and Commerce plans

ddc22

Initial Round of review comments.

client/my-sites/plan-features-2023-grid/hooks/npm-ready/data-store/use-grid-plans.tsx

chriskmnds · 2023-08-01T17:14:30Z

The only major thing I've found is that the upgrade credit notification is gone.

this should be fixed. good TS lesson to not do ... as PlanSlug[] 😬

@southp probably not worth a re-review yet. a few conflicts to address (and a few more coming in still from @jeyip's work)

chriskmnds · 2023-08-01T17:24:30Z

The /start/hosting flow in this branch displays all plans instead of just Business and Commerce plans. I double checked, and Calypso trunk also only displays Business and Commerce plans

Thanks @jeyip . Good catch. This should be fixed. missed to update the set for that intent in a rebase

^ probably worth adding test coverage for more intents (I think we only cover newsletter and wpcom-default)

chriskmnds · 2023-08-07T13:41:09Z

@jeyip care to give this another round of testing pls? everything should be addressed and rebased

jeyip · 2023-08-07T20:36:03Z

Yes! I’d be happy to 🎉

Taking a look before eod. Let’s get this over the finish line

jeyip · 2023-08-07T23:06:09Z

Testing

Requirements

Spot checked with production:

line items in the feature grid
storage labels in the feature grid
header prices in feature + comparison grid
highlighted plan columns
monthly terms
annual terms

Also tested:

plan selection and checkout
plan switcher in /plans
plan upgrades from /plans
pro-rated credits
The issue I brought up about /start/hosting has been resolved Plans 2023: Migrate plan-properties computation and other refactors #79733 (comment)
As far as I can tell, the issue brought up by @southp has been resolved Plans 2023: Migrate plan-properties computation and other refactors #79733 (comment)

Browsers

Chrome

Issues

The transaction fees for payments appears to be incorrect for free plans in /start

The start/p2 plans page now has a broken thank-you page that we redirect to after plan purchase. This problem is not caused by the current PR and there is no action to take in this pull request. I've asked about who to report the problem to here p1691450964830269/1691450530.670719-slack-C03N25JPCE4

jeyip · 2023-08-08T00:14:13Z

After we resolve the issue in #79733 (comment) with transaction fees for payments ( whether we decide it be through a follow-up or a fix in this PR ) , the testing side of this looks good. Consider it a +1 from me when completed

matticbot · 2023-08-08T10:48:58Z

This PR modifies the release build for happy-blocks

To test your changes on WordPress.com, run install-plugin.sh happy-blocks update/plans-grid-2023-plan-properties on your sandbox.

To deploy your changes after merging, see the documentation: PCYsg-r7r-p2

matticbot · 2023-08-08T10:56:05Z

This PR modifies the release build for editing-toolkit

To test your changes on WordPress.com, run install-plugin.sh editing-toolkit update/plans-grid-2023-plan-properties on your sandbox.

To deploy your changes after merging, see the documentation: PCYsg-mMA-p2

chriskmnds · 2023-08-08T11:34:22Z

The transaction fees for payments appears to be incorrect for free plans in /start

Thanks @jeyip . I did notice this from early on but forgot about it. It should be addressed now. Thanks for being thorough :-)

Also, when I tested the onboarding-pm flow and some others, there was one time this warning popping up:

I notice this too @southp, but didn't got to the bottom of it. As far as I checked, those slugs are part of the plans list, so it seemed a little weird 😕 . I will create a follow-up issue and leave it up for grabs/later, as it's not actually blocking anything.

jeyip · 2023-08-08T17:46:03Z

Thanks @jeyip . I did notice this from early on but forgot about it. It should be addressed now.

Can confirm it has been addressed 👍

jeyip

🚢 🚀

chriskmnds · 2023-08-09T05:58:45Z

Thnaks @jeyip for the trusted reviews.

This is obviously not the final shape. Right now we are mostly removing dependencies and separating concerns. We will iterate and improve things more. GridPlan in particular can be optimized with prospect of being easier to mock next.

a8ci18n · 2023-08-09T07:18:52Z

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/8692603

Hi @chriskmnds, could you please edit the description of this PR and add a screenshot for our translators? Ideally it'd include all of the following strings:

per month, %(discountedPriceFullTermText)s for the first year, Excl. Taxes
per month, %(originalPriceFullTermText)s billed annually, Excl. Taxes
per month, %(originalPriceFullTermText)s billed every two years, Excl. TaxesText
per month, %(originalPriceFullTermText)s billed every three years, Excl. TaxesText

Thank you in advance!

a8ci18n · 2023-08-09T09:34:06Z

Translation for this Pull Request has now been finished.

chriskmnds self-assigned this Jul 21, 2023

This was referenced Jul 21, 2023

Plans Next: Migrate the Features list/metadata to @automattic/calypso-products #79004

Closed

Plans 2023: Refactor grid plans pricing to derive from Plans data-store #79005

Closed

chriskmnds changed the title ~~Plans 2023: Extract plan-properties computation and other refactors~~ Plans 2023: Migrate plan-properties computation and other refactors Jul 21, 2023

chriskmnds force-pushed the update/plans-grid-2023-plan-properties branch from 5918886 to b70413d Compare July 25, 2023 12:54

chriskmnds marked this pull request as ready for review July 25, 2023 12:54

chriskmnds requested review from a team as code owners July 25, 2023 12:54

chriskmnds requested a review from aneeshd16 July 25, 2023 12:54

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

jeyip reviewed Jul 26, 2023

View reviewed changes

ddc22 reviewed Jul 26, 2023

View reviewed changes

chriskmnds mentioned this pull request Jul 28, 2023

Plans 2023: Implement storage add on dropdown #79041

Merged

7 tasks

chriskmnds force-pushed the update/plans-grid-2023-plan-properties branch from ceb42b0 to c3d4af8 Compare August 4, 2023 13:24

chriskmnds mentioned this pull request Aug 4, 2023

Pricing grid: Implement tooltips on mobile #80082

Merged

chriskmnds added 4 commits August 5, 2023 22:44

and we ended up in a stash

9199f8a

cleanup. more rebasing

5808323

rewire spotlight. use gridPlan

68cf13a

on rebase contd

6d488b8

chriskmnds force-pushed the update/plans-grid-2023-plan-properties branch from 60cda02 to 6d488b8 Compare August 5, 2023 19:54

chriskmnds added 3 commits August 5, 2023 22:58

cleanup. price strings for sticky bar

ef1dff4

cleanup after rebase

2d5f93d

remove withoutProRatedCreditsForPricing

f169bf6

chriskmnds added 4 commits August 7, 2023 13:48

update tests

4c1019a

update tests

af2194e

update tests

716692b

type checks

bc3fc4f

mj-collab mentioned this pull request Aug 7, 2023

Activity log: #80291

Closed

chriskmnds added 2 commits August 8, 2023 10:01

cleanup. gridplan

f2844a5

fix transaction fees for free plan

7b37916

jeyip approved these changes Aug 8, 2023

View reviewed changes

chriskmnds merged commit 7f3a272 into trunk Aug 9, 2023

chriskmnds deleted the update/plans-grid-2023-plan-properties branch August 9, 2023 05:53

github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Aug 9, 2023

aneeshd16 mentioned this pull request Aug 9, 2023

Plans Billing Timeframe: Revert translated strings #80363

Merged

7 tasks

chriskmnds mentioned this pull request Sep 12, 2023

Plans 2023 (NPM essential): Refactor useIsLargeCarrency to avoid Calypso state calls #79000

Closed

southp mentioned this pull request Oct 3, 2023

Plans 2023: Split and export the individual grids (FeaturesGrid, ComparisonGrid) - take 2 #82249

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Plans 2023: Migrate plan-properties computation and other refactors #79733

Plans 2023: Migrate plan-properties computation and other refactors #79733

chriskmnds commented Jul 21, 2023 •

edited

Loading

github-actions bot commented Jul 21, 2023 •

edited by matticbot

Loading

matticbot commented Jul 21, 2023 •

edited

Loading

southp commented Jul 26, 2023 •

edited

Loading

ddc22 commented Jul 26, 2023 •

edited

Loading

jeyip left a comment •

edited

Loading

jeyip commented Jul 26, 2023 •

edited

Loading

ddc22 left a comment

chriskmnds commented Aug 1, 2023

chriskmnds commented Aug 1, 2023 •

edited

Loading

chriskmnds commented Aug 7, 2023

jeyip commented Aug 7, 2023 •

edited

Loading

jeyip commented Aug 7, 2023 •

edited

Loading

jeyip commented Aug 8, 2023 •

edited

Loading

matticbot commented Aug 8, 2023

matticbot commented Aug 8, 2023

chriskmnds commented Aug 8, 2023

jeyip commented Aug 8, 2023

jeyip left a comment

chriskmnds commented Aug 9, 2023

a8ci18n commented Aug 9, 2023

a8ci18n commented Aug 9, 2023

Plans 2023: Migrate plan-properties computation and other refactors #79733

Plans 2023: Migrate plan-properties computation and other refactors #79733

Conversation

chriskmnds commented Jul 21, 2023 • edited Loading

Proposed Changes

Testing Instructions

Pre-merge Checklist

github-actions bot commented Jul 21, 2023 • edited by matticbot Loading

matticbot commented Jul 21, 2023 • edited Loading

southp commented Jul 26, 2023 • edited Loading

ddc22 commented Jul 26, 2023 • edited Loading

Reviewer Story - aka what I understood

Examining the New Hooks Introduced

useGridPlans - data-store/use-grid-plans.tsx

Hooks Injected from client

Hooks ready to be moved into separate package

Hooks used outside useGridPlan

useIsLargeCurrency - use-is-large-currency.ts

Other Related Notes

jeyip left a comment • edited Loading

Choose a reason for hiding this comment

Summary for my own understanding:

Plans Properties becomes Grid Plans

Grid Plans are made accessible through React Context

jeyip commented Jul 26, 2023 • edited Loading

Testing

Requirements

Browsers

Issues

ddc22 left a comment

Choose a reason for hiding this comment

chriskmnds commented Aug 1, 2023

chriskmnds commented Aug 1, 2023 • edited Loading

chriskmnds commented Aug 7, 2023

jeyip commented Aug 7, 2023 • edited Loading

jeyip commented Aug 7, 2023 • edited Loading

Testing

Requirements

Browsers

Issues

jeyip commented Aug 8, 2023 • edited Loading

matticbot commented Aug 8, 2023

matticbot commented Aug 8, 2023

chriskmnds commented Aug 8, 2023

jeyip commented Aug 8, 2023

jeyip left a comment

Choose a reason for hiding this comment

chriskmnds commented Aug 9, 2023

a8ci18n commented Aug 9, 2023

a8ci18n commented Aug 9, 2023

chriskmnds commented Jul 21, 2023 •

edited

Loading

github-actions bot commented Jul 21, 2023 •

edited by matticbot

Loading

matticbot commented Jul 21, 2023 •

edited

Loading

southp commented Jul 26, 2023 •

edited

Loading

ddc22 commented Jul 26, 2023 •

edited

Loading

`useGridPlans` - data-store/use-grid-plans.tsx

`useIsLargeCurrency` - use-is-large-currency.ts

jeyip left a comment •

edited

Loading

jeyip commented Jul 26, 2023 •

edited

Loading

chriskmnds commented Aug 1, 2023 •

edited

Loading

jeyip commented Aug 7, 2023 •

edited

Loading

jeyip commented Aug 7, 2023 •

edited

Loading

jeyip commented Aug 8, 2023 •

edited

Loading