DIFM: Manage Purchases: Show extra page price breakdown using price tiers #66559

ddc22 · 2022-08-15T07:41:50Z

Proposed Changes

Adds a proper description and cost breakdown for DIFM purchase receipt view.

Testing Instructions

Apply D85709-code
Apply D86680-code
Sandbox Public API
Use the below calypso live link go to /start/do-it-for-me
Go through the flow and select more than 5 pages in the page picker step
Complete purchase
Go to purchases http://calypso.localhost:3000/purchases/subscriptions/<site-slug>
Select the DIFM purchase and make sure you see below

Pre-merge Checklist

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

Fixes 937-gh-Automattic/martech

matticbot · 2022-08-15T07:42:04Z

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

App Entrypoints (~86 bytes added 📈 [gzipped])

name                 parsed_size           gzip_size
entry-stepper             +263 B  (+0.0%)      +86 B  (+0.0%)
entry-gutenboarding        +12 B  (+0.0%)      +14 B  (+0.0%)

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

Sections (~2041 bytes added 📈 [gzipped])

name                             parsed_size           gzip_size
site-purchases                       +1822 B  (+0.1%)     +628 B  (+0.2%)
purchases                            +1822 B  (+0.1%)     +628 B  (+0.1%)
jetpack-connect                       +935 B  (+0.1%)     +346 B  (+0.1%)
plans                                 +852 B  (+0.1%)     +302 B  (+0.1%)
jetpack-cloud-pricing                 +852 B  (+0.2%)     +302 B  (+0.2%)
email                                 +852 B  (+0.1%)     +330 B  (+0.2%)
domains                               +852 B  (+0.1%)     +353 B  (+0.1%)
checkout                              +852 B  (+0.1%)     +305 B  (+0.1%)
themes                                +263 B  (+0.0%)      +84 B  (+0.0%)
theme                                 +263 B  (+0.1%)      +78 B  (+0.1%)
settings                              +263 B  (+0.0%)      +76 B  (+0.0%)
plugins                               +263 B  (+0.0%)      +77 B  (+0.0%)
jetpack-cloud-plugin-management       +263 B  (+0.0%)      +78 B  (+0.0%)
home                                  +263 B  (+0.0%)      +77 B  (+0.0%)
backup                                +263 B  (+0.0%)      +76 B  (+0.0%)
add-ons                               +263 B  (+0.1%)      +76 B  (+0.1%)
jetpack-cloud-settings                +251 B  (+0.1%)      +69 B  (+0.1%)
help                                  +251 B  (+0.0%)      +69 B  (+0.0%)
account-close                         +251 B  (+0.1%)      +73 B  (+0.1%)
signup                                 +83 B  (+0.0%)      +44 B  (+0.1%)
accept-invite                          +83 B  (+0.0%)      +44 B  (+0.0%)
woocommerce                            +12 B  (+0.0%)       +6 B  (+0.0%)
stats                                  +12 B  (+0.0%)       +6 B  (+0.0%)
settings-writing                       +12 B  (+0.0%)       +6 B  (+0.0%)
settings-security                      +12 B  (+0.0%)       +6 B  (+0.0%)
settings-performance                   +12 B  (+0.0%)       +6 B  (+0.0%)
scan                                   +12 B  (+0.0%)       +6 B  (+0.0%)
purchase-product                       +12 B  (+0.0%)       +6 B  (+0.0%)
posts-custom                           +12 B  (+0.0%)       +7 B  (+0.0%)
posts                                  +12 B  (+0.0%)       +7 B  (+0.0%)
people                                 +12 B  (+0.0%)       +6 B  (+0.0%)
migrate                                +12 B  (+0.0%)       +6 B  (+0.0%)
media                                  +12 B  (+0.0%)       +8 B  (+0.0%)
marketplace                            +12 B  (+0.0%)       +5 B  (+0.0%)
marketing                              +12 B  (+0.0%)       +6 B  (+0.0%)
jetpack-search                         +12 B  (+0.0%)       +6 B  (+0.0%)
jetpack-cloud-partner-portal           +12 B  (+0.0%)       +6 B  (+0.0%)
jetpack-cloud-agency-dashboard         +12 B  (+0.0%)       +6 B  (+0.0%)
hosting                                +12 B  (+0.0%)       +6 B  (+0.0%)
earn                                   +12 B  (+0.0%)       +6 B  (+0.0%)
activity                               +12 B  (+0.0%)       +6 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 (~699 bytes added 📈 [gzipped])

name                                                            parsed_size           gzip_size
async-load-calypso-my-sites-checkout-modal                           +852 B  (+0.1%)     +302 B  (+0.1%)
async-load-calypso-blocks-editor-checkout-modal                      +852 B  (+0.1%)     +302 B  (+0.1%)
async-load-signup-steps-plans-atomic-store                           +263 B  (+0.1%)      +79 B  (+0.1%)
async-load-signup-steps-plans                                        +263 B  (+0.1%)      +78 B  (+0.1%)
async-load-design-blocks                                             +263 B  (+0.0%)     +108 B  (+0.0%)
async-load-signup-steps-theme-selection                              +251 B  (+0.2%)      +73 B  (+0.2%)
async-load-calypso-blocks-inline-help-popover                        +251 B  (+0.0%)      +71 B  (+0.0%)
async-load-automattic-help-center                                    +251 B  (+0.0%)      +71 B  (+0.0%)
async-load-signup-steps-woocommerce-install-step-business-info        +12 B  (+0.0%)       +6 B  (+0.0%)
async-load-signup-steps-domains                                       +12 B  (+0.0%)       +6 B  (+0.0%)
async-load-signup-steps-add-ons                                       +12 B  (+0.0%)       +6 B  (+0.0%)
async-load-masterbar-cart-masterbar-cart-wrapper                      +12 B  (+0.0%)       +6 B  (+0.0%)
async-load-design-playground                                          +12 B  (+0.0%)       +6 B  (+0.0%)
async-load-design                                                     +12 B  (+0.0%)       +6 B  (+0.0%)
async-load-calypso-reader-sidebar                                     +12 B  (+0.0%)       +6 B  (+0.0%)
async-load-calypso-post-editor-editor-media-modal                     +12 B  (+0.0%)       +6 B  (+0.0%)
async-load-calypso-my-sites-current-site-notice                       +12 B  (+0.0%)       +6 B  (+0.0%)
async-load-calypso-components-web-preview-component                   +12 B  (+0.0%)       +6 B  (+0.0%)
async-load-calypso-blocks-jitm-templates-sidebar-banner               +12 B  (+0.0%)       +6 B  (+0.0%)
async-load-calypso-blocks-jitm-templates-notice                       +12 B  (+0.0%)       +6 B  (+0.0%)
async-load-calypso-blocks-jitm-templates-default                      +12 B  (+0.0%)       +6 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.

github-actions · 2022-08-15T07:42:05Z

Calypso Live (direct link)

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

Jetpack Cloud live (direct link)

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

client/me/purchases/manage-purchase/index.jsx

client/me/purchases/manage-purchase/purchase-meta.jsx

packages/wpcom-checkout/src/can-item-be-removed-from-cart.ts

matticbot · 2022-08-29T12:40:16Z

This PR modifies the release build for editing-toolkit

To test your changes on WordPress.com, run install-plugin.sh editing-toolkit add/extra-page-reciept-display on your sandbox.

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

aneeshd16

Works well with new purchases. Great job on the backend changes! 👍

I have left some code comments and some feedback below:

This change should also handle past purchases. The price for past purchases is shown incorrectly. Maybe we can show the new text only if tier data is available on the purchase object?

The purchase description does seem a bit confusing on this page (Manage Purchases). The user has already "hired" a professional. Maybe this could be something like "Website built by a WordPress.com professional. This build contains 9 pages". What do you think, @mikeicode?

client/me/purchases/manage-purchase/index.jsx

client/me/purchases/manage-purchase/purchase-meta.jsx

client/lib/purchases/assembler.ts

client/lib/purchases/types.ts

client/me/purchases/manage-purchase/index.jsx

client/me/purchases/manage-purchase/purchase-meta.jsx

client/lib/purchases/types.ts

client/lib/signup/step-actions/index.js

client/me/purchases/manage-purchase/index.jsx

…page views

aneeshd16

Tests well! I left some code feedback below.

aneeshd16 · 2022-09-01T11:30:26Z

client/lib/purchases/types.ts

+	maximumPriceDisplay?: string | null | undefined;
+}
+
+export interface RawPurchasdPriceTierEntry extends PriceTierEntry {


Typo :) Should be RawPurchasePriceTierEntry.

aneeshd16 · 2022-09-01T11:41:07Z

client/me/purchases/manage-purchase/index.jsx

@@ -988,6 +1017,7 @@ function PurchasesQueryComponent( { isSiteLevel, selectedSiteId } ) {
 export default connect(
 	( state, props ) => {
 		const purchase = getByPurchaseId( state, props.purchaseId );
+		const difmTieredPurchaseDetails = getDIFMTieredPurchaseDetails( state, props.purchaseId );


I like the refactor to extract this into a function! Can you please explain why this function needs to fetch the purchase object from the state again? The purchase object is already available near both usages of the function. The new function is not using state for anything other than to fetch the purchase object, so it seems redundant to pass in the state when the purchase object is already available. Can we directly pass this object to the new function? This function signature could look like getDIFMTieredPurchaseDetails( purchase ) => <return value>.

This new function could live in client/lib/purchases/index.ts, where almost all functions have a similar signature. ( fn( purchase ) => return value). Admittedly, this file is getting quite large, but since the functions seem to be pure, it should be easy to refactor in the future.

Actually I was modelling it as a REDUX selector so it can be used anywhere that state slice needs to be calculated. I actually don't understand why it needs to go in a separate lib directory when this is clearly a calculation of existing state 🤔.

The calculation is on the purchase object, which could come from different places - it could be from the global redux state (like here), from some other store accessed by @wordpress/data (example usage), or directly from the results of an API call (since a lot of implementations are moving to react-query). The state is not used for anything other than to fetch the purchase object. Converting this to a lib function also makes writing automated tests easier.

Also, before calling this function, we are relying on the fact that the code has checked if this is a DIFM purchase. Otherwise, the output of this function does not make sense for non-DIFM purchases.

The calculation is on the purchase object, which could come from different places - it could be from the global redux state (like here), from some other store accessed by @wordpress/data (example usage), or directly from the results of an API call. The state is not used for anything other than to fetch the purchase object.

Actually the intention of this piece of code was to extract the purchase details from the REDUX state slice which holds the information cached from /purchases endpoint. Nothing else. What your explaining sounds familiar to the util method anti pattern similar to util class but for functions. Or am I wrong? This function does one thing. Transforms and calculates DIFM related price tier details if available in the identified purchase of the purchases slice of the supplied redux tree and will return null if not.

Also, before calling this function, we are relying on the fact that the code has checked if this is a DIFM purchase. Otherwise, the output of this function does not make sense for non-DIFM purchases.

Yes the code anticipates that it will be call on any variation of the universal state slice. And it will handle all those variations as appropriate :). We can possibly check the purchase Id to see if it's actually DIFM an reject the result but I don't think that is necessary. If someone wants to reuse this logic for something identical they can refactor the name when they find this function. But for others this encapsulates the price tier's extraction logic for a DIFM purchase so long as you provide the proper purchase ID of a difm purchase you will get the proper details back.

I added some changes to improve the clarity of the selector 👍
7a245a2

Transforms and calculates DIFM related price tier details if available in the identified purchase of the purchases slice of the supplied redux tree and will return null if not.

I understand that this function is currently designed to pick the purchase object from the state slice. However, IMO, this function is better suited to work on the purchase object itself. My reasoning:

The function currently does not use any other part of the state to calculate the result. It only picks the finds the purchase object again, which we already have access to when we call this function.

This function is executed for all types of purchases since it is being called in the connect function here, which seems redundant to me.

The function is tightly coupled to the redux state, and assumes that the purchase object will always be in the redux state. However, it may not be the case as:

The recommended approach for data is using react-query. For future changes that involve fetching purchase data from the backend, this function will have to be refactored to work on the purchase object.

Not all data is stored in the global redux state. In the stepper framework for example, the data is stored in @wordpress/data stores. (link)

According to the article you linked to, I don't this is an anti-pattern as the lib/purchases directory has a clearly defined boundary (purchase object functions) and it is not a part of general app-wide util module.

However, let's keep this PR moving forward 🙂 If we do decide to keep the function as it is, I have added one more bit of feedback for the change in client/me/purchases/manage-purchase/purchase-meta.jsx.

Oh I wasn't aware of the move to react-query TIL! . I had no idea we were evolving the data layer to something different!

In which case trying to maintain a set of clean set of selectors is a non starter anyway (which was my main motivation)!
I will move this to the lib folder as you recommended! :)

aneeshd16 · 2022-09-01T11:43:32Z

client/me/purchases/manage-purchase/purchase-meta.jsx

+									oneTimeFee,
+								},
+								components: {
+									period: <span className="manage-purchase__time-period" />,


This class has no styles in the code base, so maybe we can remove the span element here?

aneeshd16 · 2022-09-02T06:07:11Z

client/me/purchases/manage-purchase/purchase-meta.jsx

@@ -224,11 +226,48 @@ function PurchaseMetaOwner( { owner } ) {

 function PurchaseMetaPrice( { purchase } ) {
 	const translate = useTranslate();
+	const overallState = useSelector( ( state ) => state );


We might as well use useSelector( state => getDIFMTieredPurchaseDetails( state, purchase.id ) here. Otherwise, this component will be re-rendered if any part of the global state changes.

aneeshd16

LGTM! Great work!

a8ci18n · 2022-09-02T11:18:13Z

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

Thank you @jdc91 for including a screenshot in the description! This is really helpful for our translators.

emilyaudela · 2022-09-02T12:44:58Z

Hi all! Any chance w can add a plural to the string

A professionally built %(numberOfIncludedPages)d page website in 4 business days or less

In some languages, a separate translation is needed if you have more than one page.

Also, I think there should be hyphen between the number and the word "page", such as "A professionally built 2-page website."

There is also an extra space before the colon in this string:

Service : %(oneTimeFee)s (one-time)

Thanks!

ddc22 · 2022-09-05T05:59:06Z

Hey @emilyaudela thanks for this feedback.

Technically there might not be a scenario where a single page is provided as the default offering but theoretically it is a possibility. So I am going to include a version with the word "single". Just in case.
A professionally built single page website in 4 business days or less

The extra space before the colon is also added here which I will fix.

wp-calypso/client/me/purchases/manage-purchase/purchase-meta.jsx

Lines 253 to 254 in ed1f87d

    
           '%(extraPageCount)d extra page : %(costOfExtraPages)s (one-time)', 
        
           '%(extraPageCount)d extra pages : %(costOfExtraPages)s (one-time)',

Is there a reason or best practice for not adding a space before a colon ( English is not my first language 😄 ). I just added it because it looked more pleasing to the eye 😁

emilyaudela · 2022-09-05T08:08:32Z

Thanks for looking into this!

Technically there might not be a scenario where a single page is provided as the default offering but theoretically it is a possibility. So I am going to include a version with the word "single". Just in case.
A professionally built single page website in 4 business days or less

Awesome! Thanks! 👍

Is there a reason or best practice for not adding a space before a colon.

In English, there aren't usually spaces before punctuation marks. 😄

…iers (#66559) * Added custom text and price breakdown * Reciept display tweaks * Review fixes * Fix price formatting * Price tier changes * Fixed issues related to untiered price legacy viewing, and non extra page views * Review fixes * Review fixes * Unit test fixes * review fixes * Bug fixes * Review fixes * Add clarity to DIFM price tier selector * Refactor difm purchase details function * Minor bug fix

a8ci18n · 2022-09-08T11:39:20Z

Translation for this Pull Request has now been finished.

ddc22 added the DIFM Express Built By Express Onboarding Pipeline related tasks label Aug 15, 2022

ddc22 self-assigned this Aug 15, 2022

aneeshd16 self-requested a review August 23, 2022 11:13

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

ddc22 marked this pull request as ready for review August 23, 2022 11:24

ddc22 requested a review from a team as a code owner August 23, 2022 11:24

sirbrillig reviewed Aug 23, 2022

View reviewed changes

client/me/purchases/manage-purchase/index.jsx Outdated Show resolved Hide resolved

client/me/purchases/manage-purchase/purchase-meta.jsx Outdated Show resolved Hide resolved

packages/wpcom-checkout/src/can-item-be-removed-from-cart.ts Outdated Show resolved Hide resolved

ddc22 force-pushed the add/extra-page-reciept-display branch 4 times, most recently from 5ad7fd0 to 235857d Compare August 24, 2022 09:39

aneeshd16 mentioned this pull request Aug 24, 2022

Checkout: Show sublabel for DIFM extra pages #66872

Merged

5 tasks

ddc22 force-pushed the add/extra-page-reciept-display branch 2 times, most recently from 9865ae0 to 7d79b8f Compare August 26, 2022 12:07

aneeshd16 requested changes Aug 31, 2022

View reviewed changes

ddc22 force-pushed the add/extra-page-reciept-display branch from f7aa023 to 6b63120 Compare August 31, 2022 15:50

ddc22 requested a review from aneeshd16 August 31, 2022 15:52

aneeshd16 reviewed Sep 1, 2022

View reviewed changes

client/lib/purchases/types.ts Outdated Show resolved Hide resolved

client/lib/signup/step-actions/index.js Outdated Show resolved Hide resolved

client/me/purchases/manage-purchase/index.jsx Outdated Show resolved Hide resolved

ddc22 force-pushed the add/extra-page-reciept-display branch 2 times, most recently from d14f192 to fcfc5b6 Compare September 1, 2022 09:21

ddc22 added 7 commits September 1, 2022 13:59

Added custom text and price breakdown

d8ca3ef

Reciept display tweaks

133036a

Review fixes

d728eea

Fix price formatting

d3481f8

Price tier changes

eb42528

Fixed issues related to untiered price legacy viewing, and non extra …

aee9f2b

…page views

Review fixes

a4362d0

ddc22 added 2 commits September 1, 2022 13:59

Review fixes

aaa9ce5

Unit test fixes

95bb51c

ddc22 force-pushed the add/extra-page-reciept-display branch from fcfc5b6 to cfb28e9 Compare September 1, 2022 09:59

review fixes

19080a2

ddc22 force-pushed the add/extra-page-reciept-display branch from cfb28e9 to 19080a2 Compare September 1, 2022 10:49

Bug fixes

93d3b00

ddc22 requested a review from aneeshd16 September 1, 2022 11:22

aneeshd16 reviewed Sep 1, 2022

View reviewed changes

ddc22 added 2 commits September 1, 2022 16:18

Review fixes

f3ee09f

Add clarity to DIFM price tier selector

7a245a2

ddc22 requested a review from aneeshd16 September 1, 2022 14:43

aneeshd16 reviewed Sep 2, 2022

View reviewed changes

ddc22 added 2 commits September 2, 2022 11:56

Refactor difm purchase details function

eed89dd

Minor bug fix

5782aaf

ddc22 requested a review from aneeshd16 September 2, 2022 08:54

ddc22 changed the title ~~Add/extra page receipt display~~ DIFM: Manage Purchases: Show extra page price breakdown using price tiers Sep 2, 2022

aneeshd16 approved these changes Sep 2, 2022

View reviewed changes

ddc22 merged commit ed1f87d into trunk Sep 2, 2022

ddc22 deleted the add/extra-page-reciept-display branch September 2, 2022 09:04

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 Sep 2, 2022

ddc22 mentioned this pull request Sep 5, 2022

Purchases: Fix translation bugs for DIFM #67385

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DIFM: Manage Purchases: Show extra page price breakdown using price tiers #66559

DIFM: Manage Purchases: Show extra page price breakdown using price tiers #66559

ddc22 commented Aug 15, 2022 •

edited

Loading

matticbot commented Aug 15, 2022 •

edited

Loading

github-actions bot commented Aug 15, 2022 •

edited by matticbot

Loading

matticbot commented Aug 29, 2022

aneeshd16 left a comment

aneeshd16 left a comment

aneeshd16 Sep 1, 2022

aneeshd16 Sep 1, 2022

ddc22 Sep 1, 2022 •

edited

Loading

aneeshd16 Sep 1, 2022 •

edited

Loading

ddc22 Sep 1, 2022 •

edited

Loading

ddc22 Sep 1, 2022

aneeshd16 Sep 2, 2022

ddc22 Sep 2, 2022

aneeshd16 Sep 1, 2022

aneeshd16 Sep 2, 2022

aneeshd16 left a comment

a8ci18n commented Sep 2, 2022

emilyaudela commented Sep 2, 2022 •

edited

Loading

ddc22 commented Sep 5, 2022 •

edited

Loading

emilyaudela commented Sep 5, 2022

a8ci18n commented Sep 8, 2022

DIFM: Manage Purchases: Show extra page price breakdown using price tiers #66559

DIFM: Manage Purchases: Show extra page price breakdown using price tiers #66559

Conversation

ddc22 commented Aug 15, 2022 • edited Loading

Proposed Changes

Testing Instructions

Pre-merge Checklist

matticbot commented Aug 15, 2022 • edited Loading

github-actions bot commented Aug 15, 2022 • edited by matticbot Loading

matticbot commented Aug 29, 2022

aneeshd16 left a comment

Choose a reason for hiding this comment

aneeshd16 left a comment

Choose a reason for hiding this comment

aneeshd16 Sep 1, 2022

Choose a reason for hiding this comment

aneeshd16 Sep 1, 2022

Choose a reason for hiding this comment

ddc22 Sep 1, 2022 • edited Loading

Choose a reason for hiding this comment

aneeshd16 Sep 1, 2022 • edited Loading

Choose a reason for hiding this comment

ddc22 Sep 1, 2022 • edited Loading

Choose a reason for hiding this comment

ddc22 Sep 1, 2022

Choose a reason for hiding this comment

aneeshd16 Sep 2, 2022

Choose a reason for hiding this comment

ddc22 Sep 2, 2022

Choose a reason for hiding this comment

aneeshd16 Sep 1, 2022

Choose a reason for hiding this comment

aneeshd16 Sep 2, 2022

Choose a reason for hiding this comment

aneeshd16 left a comment

Choose a reason for hiding this comment

a8ci18n commented Sep 2, 2022

emilyaudela commented Sep 2, 2022 • edited Loading

ddc22 commented Sep 5, 2022 • edited Loading

emilyaudela commented Sep 5, 2022

a8ci18n commented Sep 8, 2022

ddc22 commented Aug 15, 2022 •

edited

Loading

matticbot commented Aug 15, 2022 •

edited

Loading

github-actions bot commented Aug 15, 2022 •

edited by matticbot

Loading

ddc22 Sep 1, 2022 •

edited

Loading

aneeshd16 Sep 1, 2022 •

edited

Loading

ddc22 Sep 1, 2022 •

edited

Loading

emilyaudela commented Sep 2, 2022 •

edited

Loading

ddc22 commented Sep 5, 2022 •

edited

Loading