Data Stores: Refactor partner portal credit card store to `createReduxStore()` - take 2 #75377

tyxla · 2023-04-06T09:33:39Z

Proposed Changes

This PR migrates the partner portal credit card store to use createReduxStore() and register() instead of registerStore().

Part of #74399. A follow-up to #73890.

Second take of #74656, which was reverted in #75371.

Testing Instructions

Follow testing instructions in Licensing portal: Rename the existing payment methods state #61583.
Follow testing instructions in Revert "Data Stores: Refactor partner portal credit card store to createReduxStore()" #75371.
Verify all checks are green (there are a bunch of unit tests covering the changes).

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

…e to createReduxStore() (#74656)" (#75371)" This reverts commit 41abfc9.

github-actions · 2023-04-06T09:33:53Z

Calypso Live (direct link)

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

Jetpack Cloud live (direct link)

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

matticbot · 2023-04-06T09:44:35Z

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

Sections (~10 bytes added 📈 [gzipped])

name                          parsed_size           gzip_size
jetpack-cloud-partner-portal       -124 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.

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.

tyxla · 2023-04-06T10:53:32Z

client/jetpack-cloud/sections/partner-portal/credit-card-fields/credit-card-element-field.tsx

@@ -19,7 +19,7 @@ export default function CreditCardElementField( {
 	const { formStatus } = useFormStatus();
 	const isDisabled = formStatus !== FormStatus.READY;
 	const { card: cardError } = useSelect(
-		( select ) => ( select( 'credit-card' ) as CreditCardSelectors ).getCardDataErrors(),
+		( select ) => select( creditCardStore ).getCardDataErrors(),


Nothing special about most of the changed instances - we reference the store through the existing store object instead of its name, which is the recommended way in Gutenberg nowadays.

tyxla · 2023-04-06T10:54:44Z

client/jetpack-cloud/sections/partner-portal/credit-card-fields/credit-card-submit-button.tsx

+	const { fields, useAsPrimaryPaymentMethod, errors, incompleteFieldKeys } = useSelect(
+		( select ) => {
+			const store = select( creditCardStore );
+			return {
+				fields: store.getFields(),
+				useAsPrimaryPaymentMethod: store.useAsPrimaryPaymentMethod(),
+				errors: store.getCardDataErrors(),
+				incompleteFieldKeys: store.getIncompleteFieldKeys(),
+			};
+		},


This contains 2 different sets of changes:

Combining all creditCardStore selectors into one useSelect() for brevity and predictability.

Removing some unnecessary types and relying on type inference instead.

tyxla · 2023-04-06T10:55:09Z

client/jetpack-cloud/sections/partner-portal/credit-card-fields/credit-card-submit-button.tsx

@@ -54,30 +53,53 @@ export default function CreditCardSubmitButton( {
 		return __( 'Please wait…' );
 	}, [ formStatus, activeButtonText, __ ] );

+	const { setCardDataError, setFieldValue, setFieldError } = useDispatch( creditCardStore );


We're using useDispatch() now to get mapped action creators.

tyxla · 2023-04-06T10:55:25Z

client/jetpack-cloud/sections/partner-portal/credit-card-fields/credit-card-submit-button.tsx

@@ -54,30 +53,53 @@ export default function CreditCardSubmitButton( {
 		return __( 'Please wait…' );
 	}, [ formStatus, activeButtonText, __ ] );

+	const { setCardDataError, setFieldValue, setFieldError } = useDispatch( creditCardStore );
+
+	const handleButtonClick = () => {


We're inlining this function to be able to use the data from the hooks.

Nice. This is way neater now.

tyxla · 2023-04-06T10:56:47Z

client/jetpack-cloud/sections/partner-portal/credit-card-fields/credit-card-submit-button.tsx

+	const { setCardDataError, setFieldValue, setFieldError } = useDispatch( creditCardStore );
+
+	const handleButtonClick = () => {
+		debug( 'validating credit card fields' );


We're also inlining the credit card validation function here, that way we can take advantage of the existing store values without having to pass the entire store object. We could keep the function separate, but then we'd still need to pass all the various state data and mapped action creators, which is an extra step of unnecessary complication IMHO.

tyxla · 2023-04-06T10:57:00Z

client/jetpack-cloud/sections/partner-portal/credit-card-fields/credit-card-submit-button.tsx

+
+		if ( ! cardholderName?.value.length ) {
+			// Touch the field so it displays a validation error
+			setFieldValue( 'cardholderName', '' );


See how natural it is to dispatch actions that way.

tyxla · 2023-04-06T10:57:37Z

.../jetpack-cloud/sections/partner-portal/credit-card-fields/test/credit-card-submit-button.jsx

 jest.mock( '@stripe/stripe-js', () => ( {
 	loadStripe: () => null,
 } ) );

-jest.mock( '@wordpress/data' );


We don't need all those mocks; we can just let @wordpress/data do its job.

tyxla · 2023-04-06T10:57:53Z

...jetpack-cloud/sections/partner-portal/payment-methods/hooks/use-create-stored-credit-card.ts

 						stripe,
 						stripeConfiguration,
 						activePayButtonText,
 				  } )
 				: null,
-		[ shouldLoadStripeMethod, store, stripe, stripeConfiguration, activePayButtonText ]


We no longer need the store since we reference it directly in the component.

tyxla · 2023-04-06T10:57:57Z

client/jetpack-cloud/sections/partner-portal/payment-methods/stored-credit-card-method.tsx

@@ -23,7 +20,6 @@ export function createStoredCreditCardMethod( {
 		activeContent: <CreditCardFields />,
 		submitButton: (
 			<CreditCardSubmitButton
-				store={ store }


We no longer need the store since we reference it directly in the component.

yashwin

LGTM. Thanks for making these changes and fixing the issue reported. I was able to add a new card as a payment method, and all the validations work as expected.

yashwin · 2023-04-07T02:06:00Z

client/jetpack-cloud/sections/partner-portal/credit-card-fields/credit-card-submit-button.tsx

@@ -54,30 +53,53 @@ export default function CreditCardSubmitButton( {
 		return __( 'Please wait…' );
 	}, [ formStatus, activeButtonText, __ ] );

+	const { setCardDataError, setFieldValue, setFieldError } = useDispatch( creditCardStore );
+
+	const handleButtonClick = () => {


Nice. This is way neater now.

Revert "Revert "Data Stores: Refactor partner portal credit card stor…

be27883

…e to createReduxStore() (#74656)" (#75371)" This reverts commit 41abfc9.

tyxla added 2 commits April 6, 2023 13:32

Access store directly, rely on type inference

e46fe2a

Remove unnecessary mocks to fix tests

c2fa07a

tyxla commented Apr 6, 2023

View reviewed changes

tyxla marked this pull request as ready for review April 6, 2023 11:11

tyxla requested review from yashwin and vitozev April 6, 2023 11:11

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 6, 2023

tyxla self-assigned this Apr 6, 2023

tyxla added [Type] Enhancement Framework Jetpack Jetpack Licensing Portal Data-Store labels Apr 6, 2023

tyxla mentioned this pull request Apr 6, 2023

Revert "Data Stores: Refactor partner portal credit card store to createReduxStore()" #75371

Merged

yashwin approved these changes Apr 7, 2023

View reviewed changes

tyxla merged commit 072a0d8 into trunk Apr 7, 2023

tyxla deleted the revert-75371-revert-74656-refactor/partner-portal-credit-card-store branch April 7, 2023 09:51

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 Apr 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Data Stores: Refactor partner portal credit card store to `createReduxStore()` - take 2 #75377

Data Stores: Refactor partner portal credit card store to `createReduxStore()` - take 2 #75377

tyxla commented Apr 6, 2023

github-actions bot commented Apr 6, 2023 •

edited by matticbot

Loading

matticbot commented Apr 6, 2023 •

edited

Loading

tyxla Apr 6, 2023

tyxla Apr 6, 2023

tyxla Apr 6, 2023

tyxla Apr 6, 2023

yashwin Apr 7, 2023

tyxla Apr 6, 2023

tyxla Apr 6, 2023

tyxla Apr 6, 2023

tyxla Apr 6, 2023

tyxla Apr 6, 2023

yashwin left a comment

yashwin Apr 7, 2023

Data Stores: Refactor partner portal credit card store to createReduxStore() - take 2 #75377

Data Stores: Refactor partner portal credit card store to createReduxStore() - take 2 #75377

Conversation

tyxla commented Apr 6, 2023

Proposed Changes

Testing Instructions

Pre-merge Checklist

github-actions bot commented Apr 6, 2023 • edited by matticbot Loading

matticbot commented Apr 6, 2023 • edited Loading

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

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

yashwin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Data Stores: Refactor partner portal credit card store to `createReduxStore()` - take 2 #75377

Data Stores: Refactor partner portal credit card store to `createReduxStore()` - take 2 #75377

github-actions bot commented Apr 6, 2023 •

edited by matticbot

Loading

matticbot commented Apr 6, 2023 •

edited

Loading