Billing: Remove unused code not behind new payment methods flag #48494

nbloomf · 2020-12-18T16:02:34Z

Changes proposed in this Pull Request

Once we're confident we won't need to turn off the purchases/new-payment-methods flag, we can clean up this code that's only active when the flag is not set.

Testing instructions

This PR should only be removing code that is inactive due to the purchases/new-payment-methods flag being permanently set in all deployment environments.

We should also make sure the old urls redirect correctly.

/me/purchases/:site/:purchaseId/payment/add to /me/purchases/:site/:purchaseId/payment-method/add
/me/purchases/add-credit-card to /me/purchases/add-payment-method
/me/purchases/:site/:purchaseId/payment/change/:cardId to /me/purchases/:site/:purchaseId/payment-method/change/:cardId

Legacy URLs (not sure if these are used anywhere; it might be worth figuring that out so these can be removed):

/purchases/:siteName/:purchaseId/payment/add to /purchases/subscriptions/:siteName/:purchaseId/payment-method/add
/purchases/:siteName/:purchaseId/payment/edit/:cardId to /purchases/subscriptions/:siteName/:purchaseId/payment-method/change/:cardId
/purchases/subscriptions/:site/:purchaseId/payment/add to /purchases/subscriptions/:site/:purchaseId/payment-method/add
/purchases/subscriptions/:site/:purchaseId/payment/edit/:cardId to /purchases/subscriptions/:site/:purchaseId/payment-method/change/:cardId
/purchases/add-credit-card/:site to /purchases/add-payment-method/:site

matticbot · 2020-12-18T16:02:36Z

Test live: https://calypso.live/?branch=try/remove-unused-code-not-behind-new-payment-methods-flag

matticbot · 2020-12-18T16:14:16Z

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

Sections (~443 bytes removed 📉 [gzipped])

name               parsed_size           gzip_size
purchases              -1231 B  (-0.1%)     -179 B  (-0.0%)
site-purchases         -1152 B  (-0.1%)     -180 B  (-0.1%)
email                   -531 B  (-0.1%)     -100 B  (-0.1%)
plans                   -315 B  (-0.0%)      -49 B  (-0.0%)
incoming-redirect       -315 B  (-4.0%)      -65 B  (-2.1%)
domains                 -285 B  (-0.0%)      -66 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 (~49 bytes removed 📉 [gzipped])

name                                        parsed_size           gzip_size
async-load-signup-steps-plans-atomic-store       -315 B  (-0.2%)      -49 B  (-0.1%)
async-load-signup-steps-plans                    -315 B  (-0.1%)      -49 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.

sirbrillig

Everything appears to work correctly. I have some questions about the code but overall I think this is great!

sirbrillig · 2021-01-14T00:30:09Z

client/me/purchases/index.js


 	// redirect legacy urls
 	router( '/me/billing', () => page.redirect( paths.billingHistory ) );
 	router( '/me/billing/:receiptId', ( { params: { receiptId } } ) =>
 		page.redirect( paths.billingHistoryReceipt( receiptId ) )
 	);
+	router( paths.addCardDetails( ':site', ':purchaseId' ), ( { params: { site, purchaseId } } ) =>


🤔 Since it appears these routes were effectively removed by the flag until now, maybe they're not necessary and/or are covered by something else? Let's check.

It's also possible that they are necessary, but were disabled accidentally as a result of our flag and we need to make sure that the legacy routes remain as you've done here; I just wanted to be sure that was the case.

client/me/purchases/manage-purchase/change-payment-method/index.jsx

sirbrillig · 2021-01-14T00:35:24Z

client/me/purchases/paths.js

@@ -46,6 +46,7 @@ export function confirmCancelDomain( siteName, purchaseId ) {
 	return managePurchase( siteName, purchaseId ) + '/confirm-cancel-domain';
 }

+// legacy path


Are these still necessary? Can we replace their remaining uses with the new paths?

I need to make sure I understand the suggestion. If we remove these completely, any old links to (for instance) /payment/add will break, right?

sirbrillig · 2021-01-14T00:38:10Z

client/my-sites/purchases/index.js

+				`/purchases/subscriptions/${ siteName }/${ purchaseId }/payment-method/change/${ cardId }`
+			)
+	);
+	router(


Similar to my above comment: I'm surprised that we had legacy routes here that have been disabled since the flag was introduced... are we sure they aren't covered elsewhere?

sirbrillig · 2021-02-10T23:26:16Z

I removed some of these in #49897 as a side-effect of the clean-up there.

nbloomf · 2021-04-21T18:58:03Z

Rebased again

…chaseChangePaymentMethod component

sirbrillig

This looks fine to me. If there's any more clean-up or further discussion to be had, we can do it in another PR.

nbloomf added the [Feature Group] WPCOM Store & Purchases All things billing on WordPress.com. This includes the backend store, plans, and billing management. label Dec 18, 2020

nbloomf requested a review from a team December 18, 2020 16:02

nbloomf self-assigned this Dec 18, 2020

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

sirbrillig removed the request for review from a team December 18, 2020 16:22

sirbrillig removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Dec 18, 2020

nbloomf force-pushed the try/remove-unused-code-not-behind-new-payment-methods-flag branch from b398036 to 2ac998b Compare January 13, 2021 21:00

nbloomf requested a review from a team January 13, 2021 21:10

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

nbloomf marked this pull request as ready for review January 13, 2021 21:22

sirbrillig reviewed Jan 14, 2021

View reviewed changes

nbloomf force-pushed the try/remove-unused-code-not-behind-new-payment-methods-flag branch from 2ac998b to 172d6b6 Compare February 15, 2021 21:18

sirbrillig removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Mar 10, 2021

nbloomf added 12 commits April 21, 2021 13:39

Label text in notices/expiring-credit-card

bc6af41

Unused code in change-payment-method

ca06f5b

Label text in payment-method-list

4e8277a

Label text in manage-purchase/notices

80ade7f

Label text in manage-purchase/index

3ac476a

Links in add-new-payment-method

e969417

purchase/utils

33d654c

Routes in purchases/index

cd18464

purchases/paths

ffe15bc

my-sites routes

e208d54

Redirect me/ routes

77d9278

Remove unused addCardDetails controller

92d9eb0

nbloomf force-pushed the try/remove-unused-code-not-behind-new-payment-methods-flag branch from 36e6565 to 92d9eb0 Compare April 21, 2021 18:57

nbloomf added 3 commits April 21, 2021 14:04

Replace titles.editCardDetails with titles.changePaymentMethod in Pur…

8b6cce8

…chaseChangePaymentMethod component

Remove unused editCardDetails controller

6079d53

Remove unused titles.editCardDetails property in purchases/titles

50fc80f

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

sirbrillig approved these changes Apr 21, 2021

View reviewed changes

sirbrillig 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 Apr 21, 2021

nbloomf merged commit 3aa1cf4 into trunk Apr 22, 2021

nbloomf deleted the try/remove-unused-code-not-behind-new-payment-methods-flag branch April 22, 2021 14:51

nbloomf added the [Estimate] 2 label Apr 22, 2021

github-actions bot removed the [Status] Ready to Merge label Apr 22, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Billing: Remove unused code not behind new payment methods flag #48494

Billing: Remove unused code not behind new payment methods flag #48494

nbloomf commented Dec 18, 2020 •

edited

Loading

matticbot commented Dec 18, 2020

matticbot commented Dec 18, 2020 •

edited

Loading

sirbrillig left a comment

sirbrillig Jan 14, 2021

sirbrillig Jan 18, 2021

sirbrillig Jan 14, 2021

nbloomf Apr 21, 2021

sirbrillig Jan 14, 2021

sirbrillig commented Feb 10, 2021

nbloomf commented Apr 21, 2021

sirbrillig left a comment

Billing: Remove unused code not behind new payment methods flag #48494

Billing: Remove unused code not behind new payment methods flag #48494

Conversation

nbloomf commented Dec 18, 2020 • edited Loading

Changes proposed in this Pull Request

Testing instructions

matticbot commented Dec 18, 2020

matticbot commented Dec 18, 2020 • edited Loading

sirbrillig left a comment

Choose a reason for hiding this comment

sirbrillig Jan 14, 2021

Choose a reason for hiding this comment

sirbrillig Jan 18, 2021

Choose a reason for hiding this comment

sirbrillig Jan 14, 2021

Choose a reason for hiding this comment

nbloomf Apr 21, 2021

Choose a reason for hiding this comment

sirbrillig Jan 14, 2021

Choose a reason for hiding this comment

sirbrillig commented Feb 10, 2021

nbloomf commented Apr 21, 2021

sirbrillig left a comment

Choose a reason for hiding this comment

nbloomf commented Dec 18, 2020 •

edited

Loading

matticbot commented Dec 18, 2020 •

edited

Loading