Refactor payment status (#8110)

* WIP * Change payment status from pristine to idle * Deprecate isPaymentStarted and isPaymentFinished * Correct comments * Deprecate isPaymentPristine and undeprecate isPaymentStarted * Set payment status to FAILED or SUCCESS when the storeAPI fetch returns * Remove FINISHED as a status * Remove ready status * Revert "Remove FINISHED as a status" This reverts commit 38d66ed. * Add payment status READY * Update use-payment-interface * Removed payment statuses pristine, failed and success * Remove deprecated selectors and update docs * Deprecate isPaymentStarted in favour of isExpressPaymentStarted * Fix tests * Update assets/js/base/context/providers/cart-checkout/payment-events/index.tsx Co-authored-by: Mike Jolley <[email protected]> * Mikes suggestions * Change since version * Fix tests --------- Co-authored-by: Mike Jolley <[email protected]>
woocommerce · Feb 14, 2023 · cffafeb · cffafeb
1 parent 5acac1f
commit cffafeb
Show file tree

Hide file tree

Showing 21 changed files with 284 additions and 194 deletions.
diff --git a/assets/js/base/context/hooks/payment-methods/use-payment-method-interface.ts b/assets/js/base/context/hooks/payment-methods/use-payment-method-interface.ts
@@ -60,13 +60,46 @@ export const usePaymentMethodInterface = (): PaymentMethodInterface => {
 			return {
 				// The paymentStatus is exposed to third parties via the payment method interface so the API must not be changed
 				paymentStatus: {
-					isPristine: store.isPaymentPristine(),
-					isStarted: store.isPaymentStarted(),
+					get isPristine() {
+						deprecated( 'isPristine', {
+							since: '9.6.0',
+							alternative: 'isIdle',
+							plugin: 'WooCommerce Blocks',
+							link: 'https://github.com/woocommerce/woocommerce-blocks/pull/8110',
+						} );
+						return store.isPaymentIdle();
+					}, // isPristine is the same as isIdle
+					isIdle: store.isPaymentIdle(),
+					isStarted: store.isExpressPaymentStarted(),
 					isProcessing: store.isPaymentProcessing(),
-					isFinished: store.isPaymentFinished(),
+					get isFinished() {
+						deprecated( 'isFinished', {
+							since: '9.6.0',
+							plugin: 'WooCommerce Blocks',
+							link: 'https://github.com/woocommerce/woocommerce-blocks/pull/8110',
+						} );
+						return (
+							store.hasPaymentError() || store.isPaymentReady()
+						);
+					},
 					hasError: store.hasPaymentError(),
-					hasFailed: store.isPaymentFailed(),
-					isSuccessful: store.isPaymentSuccess(),
+					get hasFailed() {
+						deprecated( 'hasFailed', {
+							since: '9.6.0',
+							plugin: 'WooCommerce Blocks',
+							link: 'https://github.com/woocommerce/woocommerce-blocks/pull/8110',
+						} );
+						return store.hasPaymentError();
+					},
+					get isSuccessful() {
+						deprecated( 'isSuccessful', {
+							since: '9.6.0',
+							plugin: 'WooCommerce Blocks',
+							link: 'https://github.com/woocommerce/woocommerce-blocks/pull/8110',
+						} );
+						return store.isPaymentReady();
+					},
+					isReady: store.isPaymentReady(),
 					isDoingExpressPayment: store.isExpressPaymentMethodActive(),
 				},
 				activePaymentMethod: store.getActivePaymentMethod(),

diff --git a/assets/js/base/context/providers/cart-checkout/checkout-processor.ts b/assets/js/base/context/providers/cart-checkout/checkout-processor.ts
@@ -92,7 +92,7 @@ const CheckoutProcessor = () => {
 		paymentMethodData,
 		isExpressPaymentMethodActive,
 		hasPaymentError,
-		isPaymentSuccess,
+		isPaymentReady,
 		shouldSavePayment,
 	} = useSelect( ( select ) => {
 		const store = select( PAYMENT_STORE_KEY );
@@ -102,7 +102,7 @@ const CheckoutProcessor = () => {
 			paymentMethodData: store.getPaymentMethodData(),
 			isExpressPaymentMethodActive: store.isExpressPaymentMethodActive(),
 			hasPaymentError: store.hasPaymentError(),
-			isPaymentSuccess: store.isPaymentSuccess(),
+			isPaymentReady: store.isPaymentReady(),
 			shouldSavePayment: store.getShouldSavePaymentMethod(),
 		};
 	}, [] );
@@ -130,7 +130,7 @@ const CheckoutProcessor = () => {
 	const paidAndWithoutErrors =
 		! checkoutHasError &&
 		! checkoutWillHaveError &&
-		( isPaymentSuccess || ! cartNeedsPayment ) &&
+		( isPaymentReady || ! cartNeedsPayment ) &&
 		checkoutIsProcessing;
 
 	// Determine if checkout has an error.

diff --git a/assets/js/base/context/providers/cart-checkout/payment-events/index.tsx b/assets/js/base/context/providers/cart-checkout/payment-events/index.tsx
@@ -61,16 +61,18 @@ export const PaymentEventsProvider = ( {
 			isCalculating: store.isCalculating(),
 		};
 	} );
-	const { isPaymentSuccess, isPaymentFinished, isPaymentProcessing } =
-		useSelect( ( select ) => {
-			const store = select( PAYMENT_STORE_KEY );
+	const { isPaymentReady } = useSelect( ( select ) => {
+		const store = select( PAYMENT_STORE_KEY );
 
-			return {
-				isPaymentSuccess: store.isPaymentSuccess(),
-				isPaymentFinished: store.isPaymentFinished(),
-				isPaymentProcessing: store.isPaymentProcessing(),
-			};
-		} );
+		return {
+			// The PROCESSING status represents befor the checkout runs the observers
+			// registered for the payment_setup event.
+			isPaymentProcessing: store.isPaymentProcessing(),
+			// the READY status represents when the observers have finished processing and payment data
+			// synced with the payment store, ready to be sent to the StoreApi
+			isPaymentReady: store.isPaymentReady(),
+		};
+	} );
 
 	const { setValidationErrors } = useDispatch( VALIDATION_STORE_KEY );
 	const [ observers, observerDispatch ] = useReducer( emitReducer, {} );
@@ -84,59 +86,50 @@ export const PaymentEventsProvider = ( {
 
 	const {
 		__internalSetPaymentProcessing,
-		__internalSetPaymentPristine,
+		__internalSetPaymentIdle,
 		__internalEmitPaymentProcessingEvent,
 	} = useDispatch( PAYMENT_STORE_KEY );
 
-	// flip payment to processing if checkout processing is complete, there are no errors, and payment status is started.
+	// flip payment to processing if checkout processing is complete and there are no errors
 	useEffect( () => {
 		if (
 			checkoutIsProcessing &&
 			! checkoutHasError &&
-			! checkoutIsCalculating &&
-			! isPaymentFinished
+			! checkoutIsCalculating
 		) {
 			__internalSetPaymentProcessing();
+
+			// Note: the nature of this event emitter is that it will bail on any
+			// observer that returns a response that !== true. However, this still
+			// allows for other observers that return true for continuing through
+			// to the next observer (or bailing if there's a problem).
+			__internalEmitPaymentProcessingEvent(
+				currentObservers.current,
+				setValidationErrors
+			);
 		}
 	}, [
 		checkoutIsProcessing,
 		checkoutHasError,
 		checkoutIsCalculating,
-		isPaymentFinished,
 		__internalSetPaymentProcessing,
+		__internalEmitPaymentProcessingEvent,
+		setValidationErrors,
 	] );
 
-	// When checkout is returned to idle, set payment status to pristine but only if payment status is already not finished.
+	// When checkout is returned to idle, and the payment setup has not completed, set payment status to idle
 	useEffect( () => {
-		if ( checkoutIsIdle && ! isPaymentSuccess ) {
-			__internalSetPaymentPristine();
+		if ( checkoutIsIdle && ! isPaymentReady ) {
+			__internalSetPaymentIdle();
 		}
-	}, [ checkoutIsIdle, isPaymentSuccess, __internalSetPaymentPristine ] );
+	}, [ checkoutIsIdle, isPaymentReady, __internalSetPaymentIdle ] );
 
-	// if checkout has an error sync payment status back to pristine.
+	// if checkout has an error sync payment status back to idle.
 	useEffect( () => {
-		if ( checkoutHasError && isPaymentSuccess ) {
-			__internalSetPaymentPristine();
+		if ( checkoutHasError && isPaymentReady ) {
+			__internalSetPaymentIdle();
 		}
-	}, [ checkoutHasError, isPaymentSuccess, __internalSetPaymentPristine ] );
-
-	// Emit the payment processing event
-	useEffect( () => {
-		// Note: the nature of this event emitter is that it will bail on any
-		// observer that returns a response that !== true. However, this still
-		// allows for other observers that return true for continuing through
-		// to the next observer (or bailing if there's a problem).
-		if ( isPaymentProcessing ) {
-			__internalEmitPaymentProcessingEvent(
-				currentObservers.current,
-				setValidationErrors
-			);
-		}
-	}, [
-		isPaymentProcessing,
-		setValidationErrors,
-		__internalEmitPaymentProcessingEvent,
-	] );
+	}, [ checkoutHasError, isPaymentReady, __internalSetPaymentIdle ] );
 
 	const paymentContextData = {
 		onPaymentProcessing,

diff --git a/assets/js/blocks/cart-checkout-shared/payment-methods/express-payment-methods.js b/assets/js/blocks/cart-checkout-shared/payment-methods/express-payment-methods.js
@@ -36,8 +36,8 @@ const ExpressPaymentMethods = () => {
 	);
 	const {
 		__internalSetActivePaymentMethod,
-		__internalSetPaymentStarted,
-		__internalSetPaymentPristine,
+		__internalSetExpressPaymentStarted,
+		__internalSetPaymentIdle,
 		__internalSetPaymentError,
 		__internalSetPaymentMethodData,
 		__internalSetExpressPaymentError,
@@ -58,14 +58,14 @@ const ExpressPaymentMethods = () => {
 		( paymentMethodId ) => () => {
 			previousActivePaymentMethod.current = activePaymentMethod;
 			previousPaymentMethodData.current = paymentMethodData;
-			__internalSetPaymentStarted();
+			__internalSetExpressPaymentStarted();
 			__internalSetActivePaymentMethod( paymentMethodId );
 		},
 		[
 			activePaymentMethod,
 			paymentMethodData,
 			__internalSetActivePaymentMethod,
-			__internalSetPaymentStarted,
+			__internalSetExpressPaymentStarted,
 		]
 	);
 
@@ -75,12 +75,12 @@ const ExpressPaymentMethods = () => {
 	 * This restores the active method and returns the state to pristine.
 	 */
 	const onExpressPaymentClose = useCallback( () => {
-		__internalSetPaymentPristine();
+		__internalSetPaymentIdle();
 		__internalSetActivePaymentMethod(
 			previousActivePaymentMethod.current,
 			previousPaymentMethodData.current
 		);
-	}, [ __internalSetActivePaymentMethod, __internalSetPaymentPristine ] );
+	}, [ __internalSetActivePaymentMethod, __internalSetPaymentIdle ] );
 
 	/**
 	 * onExpressPaymentError should be triggered when the express payment process errors.

diff --git a/assets/js/data/checkout/constants.ts b/assets/js/data/checkout/constants.ts
@@ -12,8 +12,6 @@ import { CheckoutResponseSuccess } from '@woocommerce/types';
 export const STORE_KEY = 'wc/store/checkout';
 
 export enum STATUS {
-	// Checkout is in its initialized state.
-	PRISTINE = 'pristine',
 	// When checkout state has changed but there is no activity happening.
 	IDLE = 'idle',
 	// After the AFTER_PROCESSING event emitters have completed. This status triggers the checkout redirect.

diff --git a/assets/js/data/checkout/reducers.ts b/assets/js/data/checkout/reducers.ts
@@ -166,14 +166,6 @@ const reducer = ( state = defaultState, action: CheckoutAction ) => {
 			}
 			break;
 	}
-
-	if (
-		newState !== state &&
-		action.type !== types.SET_PRISTINE &&
-		newState?.status === STATUS.PRISTINE
-	) {
-		newState.status = STATUS.IDLE;
-	}
 	return newState;
 };
 

diff --git a/assets/js/data/checkout/test/reducer.ts b/assets/js/data/checkout/test/reducer.ts
@@ -26,7 +26,6 @@ describe.only( 'Checkout Store Reducer', () => {
 		const expectedState = {
 			...defaultState,
 			redirectUrl: 'https://example.com',
-			status: STATUS.IDLE,
 		};
 
 		expect(
@@ -97,12 +96,15 @@ describe.only( 'Checkout Store Reducer', () => {
 	} );
 
 	it( 'should handle SET_HAS_ERROR when status is anything else', () => {
-		const initialState = { ...defaultState, status: STATUS.PRISTINE };
+		const initialState = {
+			...defaultState,
+			status: STATUS.AFTER_PROCESSING,
+		};
 
 		const expectedState = {
 			...defaultState,
 			hasError: false,
-			status: STATUS.IDLE,
+			status: STATUS.AFTER_PROCESSING,
 		};
 
 		expect(
@@ -135,7 +137,6 @@ describe.only( 'Checkout Store Reducer', () => {
 	it( 'should handle INCREMENT_CALCULATING', () => {
 		const expectedState = {
 			...defaultState,
-			status: STATUS.IDLE,
 			calculatingCount: 1,
 		};
 
@@ -152,7 +153,6 @@ describe.only( 'Checkout Store Reducer', () => {
 
 		const expectedState = {
 			...defaultState,
-			status: STATUS.IDLE,
 			calculatingCount: 0,
 		};
 
@@ -164,7 +164,6 @@ describe.only( 'Checkout Store Reducer', () => {
 	it( 'should handle SET_CUSTOMER_ID', () => {
 		const expectedState = {
 			...defaultState,
-			status: STATUS.IDLE,
 			customerId: 1,
 		};
 
@@ -176,7 +175,6 @@ describe.only( 'Checkout Store Reducer', () => {
 	it( 'should handle SET_USE_SHIPPING_AS_BILLING', () => {
 		const expectedState = {
 			...defaultState,
-			status: STATUS.IDLE,
 			useShippingAsBilling: false,
 		};
 
@@ -191,7 +189,6 @@ describe.only( 'Checkout Store Reducer', () => {
 	it( 'should handle SET_SHOULD_CREATE_ACCOUNT', () => {
 		const expectedState = {
 			...defaultState,
-			status: STATUS.IDLE,
 			shouldCreateAccount: true,
 		};
 
@@ -206,7 +203,6 @@ describe.only( 'Checkout Store Reducer', () => {
 	it( 'should handle SET_ORDER_NOTES', () => {
 		const expectedState = {
 			...defaultState,
-			status: STATUS.IDLE,
 			orderNotes: 'test',
 		};
 
@@ -225,7 +221,6 @@ describe.only( 'Checkout Store Reducer', () => {
 			};
 			const expectedState = {
 				...defaultState,
-				status: STATUS.IDLE,
 				extensionData: mockExtensionData,
 			};
 			expect(
@@ -247,7 +242,6 @@ describe.only( 'Checkout Store Reducer', () => {
 			};
 			const expectedState = {
 				...defaultState,
-				status: STATUS.IDLE,
 				extensionData: mockExtensionData,
 			};
 			const firstState = reducer(
@@ -272,7 +266,6 @@ describe.only( 'Checkout Store Reducer', () => {
 			};
 			const expectedState = {
 				...defaultState,
-				status: STATUS.IDLE,
 				extensionData: mockExtensionData,
 			};
 			const firstState = reducer(

diff --git a/assets/js/data/payment/action-types.ts b/assets/js/data/payment/action-types.ts
@@ -1,10 +1,9 @@
 export enum ACTION_TYPES {
-	SET_PAYMENT_PRISTINE = 'SET_PAYMENT_PRISTINE',
-	SET_PAYMENT_STARTED = 'SET_PAYMENT_STARTED',
+	SET_PAYMENT_IDLE = 'SET_PAYMENT_IDLE',
+	SET_EXPRESS_PAYMENT_STARTED = 'SET_EXPRESS_PAYMENT_STARTED',
+	SET_PAYMENT_READY = 'SET_PAYMENT_READY',
 	SET_PAYMENT_PROCESSING = 'SET_PAYMENT_PROCESSING',
-	SET_PAYMENT_FAILED = 'SET_PAYMENT_FAILED',
 	SET_PAYMENT_ERROR = 'SET_PAYMENT_ERROR',
-	SET_PAYMENT_SUCCESS = 'SET_PAYMENT_SUCCESS',
 	SET_PAYMENT_METHODS_INITIALIZED = 'SET_PAYMENT_METHODS_INITIALIZED',
 	SET_EXPRESS_PAYMENT_METHODS_INITIALIZED = 'SET_EXPRESS_PAYMENT_METHODS_INITIALIZED',
 	SET_ACTIVE_PAYMENT_METHOD = 'SET_ACTIVE_PAYMENT_METHOD',