Use a site-aware pricing endpoint instead of a generic one #92088

merkushin · 2024-06-25T15:27:37Z

Related to #91787

Proposed Changes

Use a site-aware pricing enpoint instead of a generic one

Why are these changes being made?

The generic pricing endpoint doesn't take into account the current plan for the website, and doesn't provide information about introductory offers.
The site-aware endpoint solves both problems.

Testing Instructions

Go to http://calypso.localhost:3000/start
- Choose a domain and a free plan
- Check "Import existing content or website"
- Enter the site address to migrate
- Choose "Migrate site"
On the Upgrade step ("The plan you need"):
- Make sure there is a request to https://public-api.wordpress.com/rest/v1.3/sites/{site-id}/plans?http_envelope=1
- Make sure there are no requests to https://public-api.wordpress.com/rest/v1.5/plans?http_envelope=1
- Switch the period from "Pay annually" to "Pay monthly".
  - Make sure you see correct prices (USD 25 and 40).
  - Make sure the period switch doesn't produce new requests to https://public-api.wordpress.com/rest/v1.3/sites/{site-id}/plans?http_envelope=1
- Go to https://wordpress.com/me/account and switch your language to another one (I use Spanish).
  - Make sure everything looks good (and translated).
- Change currency for your account (you can do it in SA).
  - Note: switching currencies <...> will switch the currency for all of your current subscriptions, so it’s not a bad idea to do this on a test account.
  - Check the Upgrade step:
    - Make sure you see prices in proper currency (check both annual and monthly payments).

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 · 2024-06-25T15:27:51Z

Calypso Live (direct link)

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

Jetpack Cloud live (direct link)

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

Automattic for Agencies live (direct link)

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

matticbot · 2024-06-25T15:30:53Z

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

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

notifications
wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug update/use-site-aware-prices-endpoint on your sandbox.

matticbot · 2024-06-25T15:36:25Z

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

Sections (~80 bytes added 📈 [gzipped])

name                     parsed_size           gzip_size
import-hosted-site-flow        +91 B  (+0.0%)      +70 B  (+0.0%)
import-flow                    +91 B  (+0.0%)      +70 B  (+0.0%)
update-design-flow             +18 B  (+0.0%)      +10 B  (+0.0%)
plugins                        +18 B  (+0.0%)      +10 B  (+0.0%)
plans                          +18 B  (+0.0%)      +10 B  (+0.0%)
link-in-bio-tld-flow           +18 B  (+0.0%)      +10 B  (+0.0%)
jetpack-app                    +18 B  (+0.0%)      +10 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 (~10 bytes added 📈 [gzipped])

name                                             parsed_size           gzip_size
async-load-signup-steps-plans-theme-preselected        +18 B  (+0.0%)      +10 B  (+0.0%)
async-load-signup-steps-plans                          +18 B  (+0.0%)      +10 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.

merkushin · 2024-06-25T18:46:05Z

client/blocks/importer/wordpress/upgrade-plan/upgrade-plan-details.tsx

-
-	const currencyCode = useSelector( getCurrentUserCurrencyCode );
-	const rawPrice = useSelector( ( state ) => getPlanRawPrice( state, planId as number, true ) );
+	const sitePlans = useSitePlans( { siteId: site.ID } );


@daledupreez Thank you for your suggestions in #91787 (comment)

A bit earlier, I found useSitePlans, which returns plans for the site. Do you think this option is fine?

I'm going to check your suggestions more thoroughly though, to better understand the differences.

As a side note, in this PR, I only replace the call to the generic plans' endpoint with a site-aware. The price display will be addressed here: #91166

Updates:

I thought the downside of my current approach is that it is not cached on the frontend anyhow. Though I didn't find repetitive requests on the state change (for example, on the period switch).

Another possible downside is that it doesn't become a part of the app state. It looks like this assumption was also wrong, because using both of them (useSitePlans and <QuerySitePlans/>) simultaneously didn't result in two requests.

I leave it with my initial solution (useSitePlans) then as it looks better because of its size and clarity. (The latter one is pretty subjective though :))

client/blocks/importer/wordpress/upgrade-plan/upgrade-plan-details.tsx

daledupreez · 2024-06-26T09:06:39Z

@merkushin, we had some further discussion around this in this internal thread: pdxWSz-1yg-p2#comment-475
The TL;DR is that we need a clearer way to show both the upfront price and the renewal price, and we don't have that fully worked out from a design/UX perspective. I ended up adding a dedicated notice rather than a reduced price as an interim fix in #92094, which should allow us to tackle the UX we need for this.

So I think we can put this PR on hold until we have a clearer approach for the design/UX we want here.

Imran92

Just to document what I looked into further-

I tried using the QuerySitePlans as Dale suggested. It worked nicely and was pretty simple as well, and doesn't have issues like the app state not being updated.

Also, there are some subtle issues that may occur otherwise. For example, you can see the two following videos -

With wrapper : Only sends a single call

Screen.Recording.2024-06-26.at.4.34.42.PM.mov

Without wrapper - when on this branch : Sends multiple calls

Screen.Recording.2024-06-26.at.4.41.27.PM.mov

Also, another nice thing is a little while ago this PR by Dale got merged to trunk which already calls the aforementioned wrapper when site id is available. So we probably won't have to even call the wrapper explicitly

merkushin · 2024-06-26T16:19:13Z

@daledupreez

So I think we can put this PR on hold until we have a clearer approach for the design/UX we want here.

I don't think we need to put it on hold. The changes here won't have a negative effect on our current UI/UX, but will enable further changes towards the introductory offer.

Taking into account we don't have a final design, I think it will be better to iterate on it as soon as we figure out the decision for it.

merkushin · 2024-06-26T22:52:55Z

@Imran92 Thanks for your tests.
I updated the PR. Could you take a look one more time?

Imran92 · 2024-06-27T06:13:34Z

client/blocks/importer/wordpress/upgrade-plan/index.tsx

@@ -180,7 +180,7 @@ export const UpgradePlan: React.FunctionComponent< Props > = ( props: Props ) =>
 				/>
 			) }

-			<UpgradePlanDetails>{ renderCTAs() }</UpgradePlanDetails>
+			<UpgradePlanDetails siteId={ site.ID }>{ renderCTAs() }</UpgradePlanDetails>


I'm testing the changes, but I just have a concern about this line. If the look at the code above (line 175-181), you'll see that we are assuming that the value of the variable site can be falsy and also, there's probably a possibility of site.ID being falsy as well.

Should we have checks/provisions for that scenario too? Have we checked if this component is being rendered or can be rendered in a page/scenario where these two variables can be absent? If that's not the case, we're okay. But if that is the case, should we have some code in place to handle it?

Good catch, thanks! I'll check it now and update the PR.

That's interesting, in the same file we can see a similar usage without that check:

wp-calypso/client/blocks/importer/wordpress/upgrade-plan/index.tsx

Line 51 in b16d43e

const { data: migrationTrialEligibility } = useCheckEligibilityMigrationTrialPlan( site.ID );

I suppose it is related to the fact that the type (SiteDetails) doesn't allow null|undefined.

Though, taking into account that the type check happens only at the compile step, I think it is better to add a check here to feel safe.

Updated here: b171b59

I checked the code a bit, site is always presented here. So I'm going to revert these changes.

daledupreez

🤦 I forgot about needing to call the site-specific API for the introductory offer data.

But I still have a valid concern/bug that we need to address.

daledupreez · 2024-06-27T09:20:50Z

client/blocks/importer/wordpress/upgrade-plan/upgrade-plan-details.tsx

+	const rawPrice = planDetails
+		? calculateMonthlyPriceForPlan( selectedPlan, planDetails?.rawPrice )
+		: undefined;


We have a bug that stems from this logic as a result of it happening alongside my PR to show the upgrade credits, which is part of why I suggested we pause. But we need the introductory offer details from this API, so I think it's being explicit about what we should, and should not do.

The core issue is that because we're showing the banner due to my work in #92094, we need to show the original/standard price in the main block here. It's probably easiest to explain using the screenshot below:

This is a site where I have a $40 upgrade credit. I bought a plan a few months ago, but I have a credit towards a plan upgrade. Due to the banner, we should show the standard price of $25/month. However, your calculation is including the available discount, so it's showing $21.67/month (calculated from $260/12). I think we can work around that to simply use the existing getSitePlanRawPrice() selector from calypso/state/plans/selectors as this selector intentionally excludes any discount from the plan upgrade credit. (i.e. it does exactly what we want!)

Suggested change

const rawPrice = planDetails

? calculateMonthlyPriceForPlan( selectedPlan, planDetails?.rawPrice )

: undefined;

const rawPrice = useSelector( ( state ) => getSitePlanRawPrice( state, siteId, selectedPlan ) );

I also double-checked to see what the response looks like for a site that's eligible for the introductory discount, and it looks like the discount and introductory offer details don't overlap, and the selector I mentioned above doesn't look at the introductory offer details.

Thanks @daledupreez, I update the PR.
Here are the related changes: 22a1cc1

The generic pricing endpoint doesn't take into account the current plan for the website, and doesn't provide information about introductory offers. The site-aware endpoint solves both problems.

useSitePlans resulted in multiple requests on the focus change and didn't populate the app state. QuerySitePlans fixes both issues.

At the moment, we have a bug with displaying the discount. As a workaound we show a notice about the credits the user have. And when we display the price we want to show the original price.

daledupreez · 2024-06-27T18:52:14Z

client/my-sites/plans-features-main/components/plan-notice-credit-update.tsx

+	if ( ! planUpgradeCreditsApplicable ) {
+		return null;
+	}


I don't think we should add this early return for two reasons:

I think we want to render the QuerySitePlans content below so we get updated data -- on the first render, I am not sure what state we can/should expect to be present, so I'd err on (re)querying on render.

We are already wrapping the display of the inner Notice component in a check on planUpgradeCreditsApplicable.

So I don't know that we should keep this check. While it may work now because the wrapping component is also including QuerySitePlans, that's not an assumption we can continue to make in general.

Suggested change

if ( ! planUpgradeCreditsApplicable ) {

return null;

}

Removed it here and improved the condition: 303dbb9

A bit more context on what we're trying to fix here:
When we switch to the monthly payment, planUpgradeCreditsApplicable is 0. React prints 0.

The straightforward solution is to use !! in front of the variable to turn it into bool.
Though I preferred a longer check and extracted it into a constant.

Excellent catch!

daledupreez

I've retested and things are looking good! 🚢

The generic pricing endpoint doesn't take into account the current plan for the website, and doesn't provide information about introductory offers. The site-aware endpoint solves both problems.

chriskmnds · 2024-06-28T11:03:31Z

client/blocks/importer/wordpress/upgrade-plan/upgrade-plan-details.tsx

 import { useSelectedPlanUpgradeMutation } from 'calypso/data/import-flow/use-selected-plan-upgrade';
 import { useSelector } from 'calypso/state';
-import { getCurrentUserCurrencyCode } from 'calypso/state/currency-code/selectors';
-import { getPlanRawPrice } from 'calypso/state/plans/selectors';
+import { getSitePlan, getSitePlanRawPrice } from 'calypso/state/sites/plans/selectors';


It would be great if we started using the usePricingMetaForGridPlans hook for pricing (or the respective usePlans / useSitePlans queries) from the @automattic/data-stores package (/packages/data-stores/plans) - and stop using the Redux state.

We have work in progress for stripping out entirely the Calypso/Redux state for plans (and site plans) - see #86638

You should be able to reach the original or discounted price from usePricingMetaForGridPlans (I don't think there's any case that's not covered by that hook right now).

Any chance someone can follow up on this? :-)

@daledupreez

Here's an example PR with similar refactor #89396 (passing in the siteId will generate prices equivalent to getSitePlanRawPrice

@chriskmnds we can shift that across, yes. However, how should we handle the conflict that's effectively been flagged in #92210? The issue there is that we're not showing discounted prices because we're not showing renewal prices, and are relying on a separate notice for upgrade credits.

Also, this serves as preparatory work for us to take advantage of site-specific introductory offers on the Creator plan. Will those hooks surface the offers as we'd expect via the site plans API?

It doesn't look like we can tell why a discount was applied, which may be an issue -- we're not even getting the textual message back from the server via usePricingMetaForGridPlans() AFAICT.

The issue there is that we're not showing discounted prices because we're not showing renewal prices, and are relying on a separate notice for upgrade credits.

I am not sure. The hook does return "discountedPrice" and "originalPrice", which should give you a choice? See the PlanPricing type for details on what these are (TLDR the discounted price considering a siteId will be a prorated price, and without a siteId it will be a currency discount). and like I mention below, you can reach for the more "raw" values from the usePlans/useSitePlans hooks.

Also, this serves as preparatory work for us to take advantage of site-specific introductory offers on the Creator plan. Will those hooks surface the offers as we'd expect via the site plans API?

Yes, they should. The same hook was used for the WOO intro offers and the WPCOM intro offers project later (if you recall). We map these to an introOffer property from the main query hook (use-site-plans) and also resurface it from the usePricingMetaForGridPlans hook (so go with this first). The usePlans/useSitePlans hooks are closer to the API returns (just camelCased), while usePricingMetaForGridPlans is more for convenience.

See the PlanIntroductoryOffer type:

export interface PlanIntroductoryOffer { formattedPrice: string; rawPrice: number; intervalUnit: string; intervalCount: number; isOfferComplete: boolean; }

It is probably better to delay the switch for one week. Do you think it is acceptable?

Totally fine. I still have other/previous cases to refactor. My concern in this work (that also involves intro offers apparently) is that we build out more functionality over the Redux state that is already being treated as "deprecated". We are actively migrating things to the data-store hooks. We've already removed most plan pricing selectors and getPlanDiscountedRawPrice and getPlanRawPrice are next. Once these are migrated, then we will target the Query components QueryPlans and QuerySitePlans for removal.

We also don't get many chances to target refactors like these. It's kinda by accident that now is the time that I can continue work on it, and I'm not sure for how long before another priority arises. We are generally treating refactors as secondary work.

but it had a side effect — repeated requests, described here: #92088 (review)

Was this a problem? To understand the issue, this wasn't loopy right (meaning, firing endlessly)? The React-query hooks can fire multiple requests in the background but that shouldn't be an issue to the frontend. Once the data settles (retrieved) then nothing will change unless the API data changes.

p.s. all of the Signup flows that query plans data (including for introductory offers) are using the data-store hooks. So I'm not sure I see any blocker from using it from another Calypso component if that's what this work is.

The only case where we've had issues with the hooks was in ETK and things outside of Calypso - something unknown (yet) was not compiling the data properly there, which needs investigation. I don't think this is related though, but pls correct me if I'm wrong. Was the data retrieved and stabilized in your case (irrespective of multiple calls in the background)?

@merkushin, me (or someone else from Serenity) can focus on getting this refactor done, ideally in parallel with your work to get the offers shown to users. That was my thinking all along, @chriskmnds -- we don't want to block this work, but we also don't want to let it linger in a bad state for very long.

It doesn't look like we can tell why a discount was applied, which may be an issue -- we're not even getting the textual message back from the server via usePricingMetaForGridPlans() AFAICT.

Hi, we have updated useSitePlans, so it returns an array of cost_overrides, which contains the reason for a discount. We also updated the usePricingMetaForGridPlans hook to include discounts from plan proration based on a parameter. I think you should now be able to use the usePricingMetaForGridPlans hook for your use case. Please see #92311 for more details!

Hi @aneeshd16!
Thank you! We're migrating to usePricingMetaForGridPlans here: #92201

merkushin requested a review from a team June 25, 2024 15:27

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

merkushin linked an issue Jun 25, 2024 that may be closed by this pull request

Migration Flow Improvements: Use a different endpoint to get plans on the Upgrade step #91787

Closed

merkushin commented Jun 25, 2024

View reviewed changes

merkushin marked this pull request as ready for review June 25, 2024 22:47

merkushin requested a review from a team as a code owner June 25, 2024 22:47

Imran92 reviewed Jun 26, 2024

View reviewed changes

client/blocks/importer/wordpress/upgrade-plan/upgrade-plan-details.tsx Outdated Show resolved Hide resolved

This comment was marked as duplicate.

Sign in to view

Imran92 reviewed Jun 26, 2024

View reviewed changes

donnapep changed the title ~~Use a site-aware pricing enpoint instead of a generic one~~ Use a site-aware pricing endpoint instead of a generic one Jun 26, 2024

merkushin force-pushed the update/use-site-aware-prices-endpoint branch 2 times, most recently from a8c0af7 to b16d43e Compare June 26, 2024 22:50

Imran92 reviewed Jun 27, 2024

View reviewed changes

daledupreez requested changes Jun 27, 2024

View reviewed changes

merkushin added 5 commits June 27, 2024 12:47

Use a site-aware pricing endpoint instead of a generic one

682aa58

The generic pricing endpoint doesn't take into account the current plan for the website, and doesn't provide information about introductory offers. The site-aware endpoint solves both problems.

Update import of useSitePlans

3e40adb

Use QuerySitePlans for loading site plans

e788652

useSitePlans resulted in multiple requests on the focus change and didn't populate the app state. QuerySitePlans fixes both issues.

Make sure site exists before using its property

b171b59

Use getSitePlanRawPrice to display price without discounts

22a1cc1

At the moment, we have a bug with displaying the discount. As a workaound we show a notice about the credits the user have. And when we display the price we want to show the original price.

merkushin force-pushed the update/use-site-aware-prices-endpoint branch from e91ff44 to 22a1cc1 Compare June 27, 2024 18:48

daledupreez reviewed Jun 27, 2024

View reviewed changes

Replace the early return with an improved condition

303dbb9

daledupreez approved these changes Jun 27, 2024

View reviewed changes

merkushin merged commit d6af4db into trunk Jun 27, 2024
11 checks passed

merkushin deleted the update/use-site-aware-prices-endpoint branch June 27, 2024 22:28

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 Jun 27, 2024

daledupreez mentioned this pull request Jun 28, 2024

Migration signup flow: Incorrect INR/MXN/PHP plan price and refund period #92210

Open

chriskmnds reviewed Jun 28, 2024

View reviewed changes

merkushin mentioned this pull request Jun 28, 2024

Migration Flow Improvements: Replace Redux state with hooks for getting site-aware pricing and introductory offer on the Upgrade plan step #92221

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use a site-aware pricing endpoint instead of a generic one #92088

Use a site-aware pricing endpoint instead of a generic one #92088

merkushin commented Jun 25, 2024 •

edited

Loading

github-actions bot commented Jun 25, 2024 •

edited by matticbot

Loading

matticbot commented Jun 25, 2024 •

edited

Loading

matticbot commented Jun 25, 2024 •

edited

Loading

merkushin Jun 25, 2024 •

edited

Loading

daledupreez commented Jun 26, 2024

This comment was marked as duplicate.

Imran92 left a comment •

edited

Loading

merkushin commented Jun 26, 2024

merkushin commented Jun 26, 2024

Imran92 Jun 27, 2024

merkushin Jun 27, 2024

merkushin Jun 27, 2024

merkushin Jun 27, 2024 •

edited

Loading

merkushin Jun 27, 2024

daledupreez left a comment

daledupreez Jun 27, 2024

merkushin Jun 27, 2024 •

edited

Loading

daledupreez Jun 27, 2024

merkushin Jun 27, 2024 •

edited

Loading

daledupreez Jun 27, 2024

daledupreez left a comment

chriskmnds Jun 28, 2024

chriskmnds Jun 28, 2024

daledupreez Jun 28, 2024

daledupreez Jun 28, 2024

chriskmnds Jun 28, 2024 •

edited

Loading

chriskmnds Jun 30, 2024 •

edited

Loading

chriskmnds Jun 30, 2024

daledupreez Jul 1, 2024

aneeshd16 Jul 8, 2024 •

edited

Loading

merkushin Jul 8, 2024

Use a site-aware pricing endpoint instead of a generic one #92088

Use a site-aware pricing endpoint instead of a generic one #92088

Conversation

merkushin commented Jun 25, 2024 • edited Loading

Proposed Changes

Why are these changes being made?

Testing Instructions

Pre-merge Checklist

github-actions bot commented Jun 25, 2024 • edited by matticbot Loading

matticbot commented Jun 25, 2024 • edited Loading

matticbot commented Jun 25, 2024 • edited Loading

merkushin Jun 25, 2024 • edited Loading

Choose a reason for hiding this comment

daledupreez commented Jun 26, 2024

This comment was marked as duplicate.

Imran92 left a comment • edited Loading

Choose a reason for hiding this comment

merkushin commented Jun 26, 2024

merkushin commented Jun 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

merkushin Jun 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daledupreez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

merkushin Jun 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

merkushin Jun 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daledupreez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chriskmnds Jun 28, 2024 • edited Loading

Choose a reason for hiding this comment

chriskmnds Jun 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aneeshd16 Jul 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

merkushin commented Jun 25, 2024 •

edited

Loading

github-actions bot commented Jun 25, 2024 •

edited by matticbot

Loading

matticbot commented Jun 25, 2024 •

edited

Loading

matticbot commented Jun 25, 2024 •

edited

Loading

merkushin Jun 25, 2024 •

edited

Loading

Imran92 left a comment •

edited

Loading

merkushin Jun 27, 2024 •

edited

Loading

merkushin Jun 27, 2024 •

edited

Loading

merkushin Jun 27, 2024 •

edited

Loading

chriskmnds Jun 28, 2024 •

edited

Loading

chriskmnds Jun 30, 2024 •

edited

Loading

aneeshd16 Jul 8, 2024 •

edited

Loading