Plans (Storage): Separate plan features from add-ons, holistically. Additional refactors

[chriskmnds]

Related to #

Proposed Changes

The main change here revolves around separating the storage associated with a plan as a feature (which equates to the default storage the plan comes with), and the upgrade/add-on that may be available for purchase.

We previously intermingled these concepts at the data layer, defining the StorageOption property (now removed) on the plan definitions that included both (storage feature and storage add-ons).

The other changes involve a bit of cleanup:

• Renaming some files (to also make the distinction clearer e.g. we had a "add-on dropdown" whilst it was a dropdown of both "plan storage feature" and "available add-ons")
• Some CSS class renames to go along with the above
• Deleting redundant code
• Migrates some of the logic to AddOns data-store (useAvailableStorageAddOns)

In follow-up PRs, we will separate these concepts even further, and rework the add-ons' logic to provide an indexed structure via add-on slugs (hence stop referencing storage add-on slugs in their featureSlugs properties).

Why are these changes being made?

Main motivation:

The storage add-ons are really a platform/UI concern, not a plan property. The only related property that a plan can/should have is (1) the default storage that it comes with and (2) any upper limit on the amount of storage it could expand on. The UI can then serve products for that difference in 50GB, or 100GB, or 99GB, or any other increment or variable amount it desires. In principle/theory, this would be the more flexible approach, while keeping the data layer closer to definition - and further from client features.

Testing Instructions

• Ensure storage add-ons work as previously - /start/plans
• Ensure current plan in /plans/[ site ] is upgradeable with storage once (upgrade , then revisit /plans - it should show the upgraded volume along with additional cost)
• Ensure storage is automatically added to the cart when in query param e.g. /start/business/?storage=50gb-storage-add-on

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

[github-actions]

Calypso Live (direct link)

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

Jetpack Cloud live (direct link)

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

Automattic for Agencies live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-112202&env=a8c-for-agencies

[matticbot]

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

• editing-toolkit
• happy-blocks
• notifications
• odyssey-stats
• wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug update/plans-grid-storage-separate-features-from-addons on your sandbox.

[matticbot]

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

App Entrypoints (~75 bytes removed 📉 [gzipped])

name           parsed_size           gzip_size
entry-stepper       -587 B  (-0.0%)      -75 B  (-0.0%)
entry-main          -587 B  (-0.0%)      -75 B  (-0.0%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~71 bytes removed 📉 [gzipped])

name                     parsed_size           gzip_size
update-design-flow            -444 B  (-0.0%)      -56 B  (-0.0%)
plugins                       -444 B  (-0.0%)      -56 B  (-0.0%)
plans                         -444 B  (-0.0%)      -56 B  (-0.0%)
link-in-bio-tld-flow          -444 B  (-0.0%)      -56 B  (-0.0%)
jetpack-app                   -444 B  (-0.1%)      -56 B  (-0.0%)
import-flow                    -86 B  (-0.0%)      -15 B  (-0.0%)
import-hosted-site-flow        -83 B  (-0.0%)      -14 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 (~78 bytes removed 📉 [gzipped])

name                                             parsed_size           gzip_size
async-load-signup-steps-plans-theme-preselected       -444 B  (-0.1%)      -56 B  (-0.0%)
async-load-signup-steps-plans                         -444 B  (-0.1%)      -56 B  (-0.0%)
async-load-signup-steps-new-or-existing-site           -31 B  (-0.0%)      -22 B  (-0.1%)

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.

[chriskmnds]

@southp @jeyip I think this goes back to early discussions about the storage dropdown. It's definitely been touched on in #79041, but I can't find a clear reference. As of these changes, and a couple of follow-ups, the distinction between storage as plan feature and storage as add-on/upgrade should be fairly discrete. This is essentially the outcome from this layer of refactors (quoting from description):

The storage add-ons are really a platform/UI concern, not a plan property. The only related property that a plan can/should have is (1) the default storage that it comes with and (2) any upper limit on the amount of storage it could expand on. The UI can then serve products for that difference in 50GB, or 100GB, or 99GB, or any other increment or variable amount it desires. In principle/theory, this would be the more flexible approach, while keeping the data layer closer to definition - and further from client features.

[jeyip]

Hey @chriskmnds! Pretty busy today but will make sure to take a look at this tomorrow morning 👍

[chriskmnds]

Hey @chriskmnds! Pretty busy today but will make sure to take a look at this tomorrow morning 👍

Thanks @jeyip .

If I can get a bit of help with testing, then that would be really helpful. It's obviously quite prone to conflicts with 20+ file changes. I'm building on top of this more refactors 😬

[jeyip]

Hey Christos! Thanks for the patience and apologies for the delay. I'll be testing this after dinner tonight 🙂

[jeyip]

Testing

Requirements

Testing with Calypso live.

For each of the flows listed below:

• Ensure storage add-ons work as previously - /start/plans
  • Storage add-ons only display for yearly plans
  • Storage add-on selections are synchronized between the comparison and features grid
  • Storage add-on upgraded is automatically added to cart when upgrade CTA is clicked
  • Storage add-ons are only displayed for creator and entrepreneur plans
• Ensure current plan in /plans/[ site ] is upgradeable with storage once (upgrade , then revisit /plans - it should show the upgraded volume along with additional cost)
• Ensure storage is automatically added to the cart when in query param e.g. /start/business/?storage=50gb-storage-add-on

Browsers

• Chrome

Notes

• Issue is now fixed After upgrading a plan with storage add-ons through /plans and revisiting /plans, the behaviour of the dropdown seems odd. Below is a GIF where I've purchased a 50GB storage add-on upgrade. When I revisit /plans, it defaults to the 150GB add-on option, and when I select the 100GB option, the 150GB add-on option disappears.

[jeyip]

Started code reviewing but running out of steam tonight. Will continue reviewing first thing tomorrow.

[chriskmnds]

Thank you @jeyip ! I really appreciate your insightful testing, always 😊

After upgrading a plan with storage add-ons through /plans and revisiting /plans, the behaviour of the dropdown seems odd. Below is a GIF where I've purchased a 50GB storage add-on upgrade. When I revisit /plans, it defaults to the 150GB add-on option, and when I select the 100GB option, the 150GB add-on option disappears.

Thanks. It was missing a check on "available storage add-ons", so it was rendering the dropdown irrespective of purchased/not-purchased (and getting messed up too). It should be fixed in the last updates. Also extracted a useAvailableStorageAddons hook into the AddOns data-store now and imported it from there. (will refactor next the add-ons section to also use this same hook)

Edit: well actually, moving the hook and plan eligibility to AddOns needs additional refactor. The eligible plans are only relevant to plans grid. need to extract/move that back.

[jeyip]

Thanks. It was missing a check on "available storage add-ons", so it was rendering the dropdown irrespective of purchased/not-purchased (and getting messed up too). It should be fixed in the last updates.

Confirmed that the bug is fixed :D

Edit: well actually, moving the hook and plan eligibility to AddOns needs additional refactor. The eligible plans are only relevant to plans grid. need to extract/move that back.

I'll keep an eye out for this refactor and retest when completed. I was really excited when I saw the logic moved into there 😆 It feels way more intuitive, but I understand why we need to move it back.

[jeyip]

Non-blocking / no response needed:

Thanks for slowly disentangling the storage add-on from the concept of features 🎉

[jeyip]

There are quite a large volume of changes 😆

I did my best to go through the files and understand the intention behind each of your updates. I didn't see anything concerning. In fact, more than anything, the changes bring lots of clarity to the structure and organization of storage add-ons as they relate to the plans grid.

• Thoughtful renaming of the dropdown 👍
• Happy to see the removal of tangled storage add-on references and features from within the plans-list, the storage-feature component, scss file, etc.

Things feel way more straightforward, and I'm excited to see this in production. Besides updating the location of the use-available-storage-add-ons hook, not sure if you have anything else planned for this PR, but the code looks great to me.

[chriskmnds]

I was really excited when I saw the logic moved into there 😆 It feels way more intuitive, but I understand why we need to move it back.

Thanks @jeyip ! We can actually keep the useAvailableStorageAddOns hook under the add-ons data-store. Just the ELIGIBLE_PLANS_FOR_STORAGE_UPGRADE variable is grid-specific right now, so moved that out (which is a candidate for refactoring next). I will try and incorporate the hook in the main add-ons selector at some point. It might end up being renamed or refactored later - but for now, it's a generic function that returns "available storage add-ons for purchase".

(keeping you happy 😃 )