Fix flaw with event emitters and complete checkout flow for the chequ…

…e payment method. (#2108)

* Allow for priority to be set on event emitters

* Add payment event emitters

* add new actions/status to checkout state context

* implement event emitters in payment data context

* refactor checkout state context to use new actions in event emitters

* refactor checkout processor to handle new event emitters

* fix type-defs for registered payment methods

* register observer for cheque payment method integration

* add inline todo

* fix sort

* fix tests and add test for priority usage in event emitters

* remove todo and just add explanatory comment

* condense sort logic

* lowercase test description

* abstract emitter callback to reduce code

* don’t process passed in errors if it’s undefined

* improve error response expectation for payment processing event observer

assets/js/base/context/cart-checkout/checkout-state/actions.js

@@ -6,6 +6,7 @@ import { TYPES } from './constants';
 const {
 	SET_PRISTINE,
 	SET_PROCESSING,
+	SET_PROCESSING_COMPLETE,
 	SET_REDIRECT_URL,
 	SET_COMPLETE,
 	SET_HAS_ERROR,
@@ -32,6 +33,9 @@ export const actions = {
 	setComplete: () => ( {
 		type: SET_COMPLETE,
 	} ),
+	setProcessingComplete: () => ( {
+		type: SET_PROCESSING_COMPLETE,
+	} ),
 	setHasError: ( hasError = true ) => {
 		const type = hasError ? SET_HAS_ERROR : SET_NO_ERROR;
 		return { type };

assets/js/base/context/cart-checkout/checkout-state/constants.js

@@ -12,6 +12,7 @@ export const STATUS = {
 	CALCULATING: 'calculating',
 	PROCESSING: 'processing',
 	COMPLETE: 'complete',
+	PROCESSING_COMPLETE: 'processing_complete',
 };
 
 const checkoutData = getSetting( 'checkoutData', { order_id: 0 } );
@@ -31,6 +32,7 @@ export const TYPES = {
 	SET_PRISTINE: 'set_pristine',
 	SET_REDIRECT_URL: 'set_redirect_url',
 	SET_COMPLETE: 'set_checkout_complete',
+	SET_PROCESSING_COMPLETE: 'set_processing_complete',
 	SET_PROCESSING: 'set_checkout_is_processing',
 	SET_HAS_ERROR: 'set_checkout_has_error',
 	SET_NO_ERROR: 'set_checkout_no_error',

assets/js/base/context/cart-checkout/checkout-state/event-emit.js

@@ -1,7 +1,12 @@
 /**
  * Internal dependencies
  */
-import { actions, reducer, emitEvent, emitEventWithAbort } from '../event-emit';
+import {
+	emitterCallback,
+	reducer,
+	emitEvent,
+	emitEventWithAbort,
+} from '../event-emit';
 
 const EMIT_TYPES = {
 	CHECKOUT_COMPLETE_WITH_SUCCESS: 'checkout_complete',
@@ -23,51 +28,18 @@ const EMIT_TYPES = {
  * @return {Object} An object with the `onCheckoutComplete` emmitter registration
  */
 const emitterSubscribers = ( dispatcher ) => ( {
-	onCheckoutCompleteSuccess: ( callback ) => {
-		const action = actions.addEventCallback(
-			EMIT_TYPES.CHECKOUT_COMPLETE_WITH_SUCCESS,
-			callback
-		);
-		dispatcher( action );
-		return () => {
-			dispatcher(
-				actions.removeEventCallback(
-					EMIT_TYPES.CHECKOUT_COMPLETE_WITH_SUCCESS,
-					action.id
-				)
-			);
-		};
-	},
-	onCheckoutCompleteError: ( callback ) => {
-		const action = actions.addEventCallback(
-			EMIT_TYPES.CHECKOUT_COMPLETE_WITH_ERROR,
-			callback
-		);
-		dispatcher( action );
-		return () => {
-			dispatcher(
-				actions.removeEventCallback(
-					EMIT_TYPES.CHECKOUT_COMPLETE_WITH_ERROR,
-					action.id
-				)
-			);
-		};
-	},
-	onCheckoutProcessing: ( callback ) => {
-		const action = actions.addEventCallback(
-			EMIT_TYPES.CHECKOUT_PROCESSING,
-			callback
-		);
-		dispatcher( action );
-		return () => {
-			dispatcher(
-				actions.removeEventCallback(
-					EMIT_TYPES.CHECKOUT_PROCESSING,
-					action.id
-				)
-			);
-		};
-	},
+	onCheckoutCompleteSuccess: emitterCallback(
+		EMIT_TYPES.CHECKOUT_COMPLETE_WITH_SUCCESS,
+		dispatcher
+	),
+	onCheckoutCompleteError: emitterCallback(
+		EMIT_TYPES.CHECKOUT_COMPLETE_WITH_ERROR,
+		dispatcher
+	),
+	onCheckoutProcessing: emitterCallback(
+		EMIT_TYPES.CHECKOUT_PROCESSING,
+		dispatcher
+	),
 } );
 
 export {

assets/js/base/context/cart-checkout/checkout-state/index.js

@@ -38,6 +38,7 @@ const CheckoutContext = createContext( {
 	isIdle: false,
 	isCalculating: false,
 	isProcessing: false,
+	isProcessingComplete: false,
 	hasError: false,
 	redirectUrl: '',
 	orderId: 0,
@@ -48,6 +49,7 @@ const CheckoutContext = createContext( {
 		resetCheckout: () => void null,
 		setRedirectUrl: ( url ) => void url,
 		setHasError: ( hasError ) => void hasError,
+		setComplete: () => void null,
 		incrementCalculating: () => void null,
 		decrementCalculating: () => void null,
 		setOrderId: ( id ) => void id,
@@ -124,6 +126,9 @@ export const CheckoutStateProvider = ( {
 				void dispatch( actions.decrementCalculating() ),
 			setOrderId: ( orderId ) =>
 				void dispatch( actions.setOrderId( orderId ) ),
+			setComplete: () => {
+				void dispatch( actions.setComplete() );
+			},
 		} ),
 		[]
 	);
@@ -141,7 +146,7 @@ export const CheckoutStateProvider = ( {
 					setValidationErrors( response );
 					dispatchActions.setHasError();
 				}
-				dispatch( actions.setComplete() );
+				dispatch( actions.setProcessingComplete() );
 			} );
 		}
 		if ( status === STATUS.COMPLETE ) {
@@ -156,20 +161,15 @@ export const CheckoutStateProvider = ( {
 					currentObservers.current,
 					EMIT_TYPES.CHECKOUT_COMPLETE_WITH_SUCCESS,
 					{}
-				).then( () => {
-					// all observers have done their thing so let's redirect
-					// (if no error).
-					if ( ! checkoutState.hasError ) {
-						window.location = checkoutState.redirectUrl;
-					}
-				} );
+				);
 			}
 		}
 	}, [
 		checkoutState.status,
 		checkoutState.hasError,
 		checkoutState.isComplete,
 		checkoutState.redirectUrl,
+		setValidationErrors,
 	] );
 
 	const onSubmit = () => {
@@ -186,6 +186,8 @@ export const CheckoutStateProvider = ( {
 		isIdle: checkoutState.status === STATUS.IDLE,
 		isCalculating: checkoutState.status === STATUS.CALCULATING,
 		isProcessing: checkoutState.status === STATUS.PROCESSING,
+		isProcessingComplete:
+			checkoutState.status === STATUS.PROCESSING_COMPLETE,
 		hasError: checkoutState.hasError,
 		redirectUrl: checkoutState.redirectUrl,
 		onCheckoutCompleteSuccess,

assets/js/base/context/cart-checkout/checkout-state/reducer.js

@@ -6,6 +6,7 @@ import { TYPES, DEFAULT_STATE, STATUS } from './constants';
 const {
 	SET_PRISTINE,
 	SET_PROCESSING,
+	SET_PROCESSING_COMPLETE,
 	SET_REDIRECT_URL,
 	SET_COMPLETE,
 	SET_HAS_ERROR,
@@ -15,7 +16,14 @@ const {
 	SET_ORDER_ID,
 } = TYPES;
 
-const { PRISTINE, IDLE, CALCULATING, PROCESSING, COMPLETE } = STATUS;
+const {
+	PRISTINE,
+	IDLE,
+	CALCULATING,
+	PROCESSING,
+	PROCESSING_COMPLETE,
+	COMPLETE,
+} = STATUS;
 
 /**
  * Reducer for the checkout state
@@ -76,6 +84,23 @@ export const reducer = ( state = DEFAULT_STATE, { url, type, orderId } ) => {
 					? newState
 					: { ...newState, hasError: false };
 			break;
+		case SET_PROCESSING_COMPLETE:
+			status = PROCESSING_COMPLETE;
+			nextStatus = state.status;
+			if ( state.status === CALCULATING ) {
+				status = CALCULATING;
+				nextStatus = PROCESSING_COMPLETE;
+			}
+			newState =
+				status !== state.status
+					? {
+							...state,
+							status,
+							nextStatus,
+							hasError: false,
+					  }
+					: state;
+			break;
 		case SET_HAS_ERROR:
 			newState = state.hasError
 				? state
@@ -84,7 +109,8 @@ export const reducer = ( state = DEFAULT_STATE, { url, type, orderId } ) => {
 						hasError: true,
 				  };
 			newState =
-				state.status === PROCESSING
+				state.status === PROCESSING ||
+				state.status === PROCESSING_COMPLETE
 					? {
 							...newState,
 							status: IDLE,

assets/js/base/context/cart-checkout/checkout/processor/index.js

@@ -22,7 +22,10 @@ const CheckoutProcessor = () => {
 	const {
 		hasError,
 		onCheckoutProcessing,
+		onCheckoutCompleteSuccess,
 		dispatchActions,
+		redirectUrl,
+		isProcessingComplete: checkoutIsProcessingComplete,
 	} = useCheckoutContext();
 	const { hasValidationErrors } = useValidationContext();
 	const { shippingAddress } = useShippingDataContext();
@@ -38,7 +41,10 @@ const CheckoutProcessor = () => {
 
 	const withErrors = hasValidationErrors();
 	const paidAndWithoutErrors =
-		! hasError && ! withErrors && currentPaymentStatus.isSuccessful;
+		! hasError &&
+		! withErrors &&
+		currentPaymentStatus.isSuccessful &&
+		checkoutIsProcessingComplete;
 
 	useEffect( () => {
 		currentBillingData.current = billingData;
@@ -59,16 +65,7 @@ const CheckoutProcessor = () => {
 		return ! withErrors;
 	}, [ withErrors ] );
 
-	/**
-	 * Process an order via the /wc/store/checkout endpoint.
-	 *
-	 * @return {boolean} True if everything was successful.
-	 */
-	useEffect( () => {
-		if ( ! paidAndWithoutErrors ) {
-			return;
-		}
-
+	const processOrder = useCallback( () => {
 		triggerFetch( {
 			path: '/wc/store/checkout',
 			method: 'POST',
@@ -106,32 +103,54 @@ const CheckoutProcessor = () => {
 								}
 							);
 						}
+						dispatchActions.setHasError();
 					}
 
 					dispatchActions.setRedirectUrl(
 						response.payment_result.redirect_url
 					);
+					dispatchActions.setComplete();
 				} );
 			} )
 			.catch( ( error ) => {
 				addErrorNotice( error.message, {
 					id: 'checkout',
 				} );
+				dispatchActions.setHasError();
+				dispatchActions.setComplete();
 			} );
 	}, [
 		addErrorNotice,
 		activePaymentMethod,
 		currentBillingData,
 		currentShippingAddress,
-		paidAndWithoutErrors,
 	] );
-
+	// setup checkout processing event observers.
 	useEffect( () => {
-		const unsubscribeProcessing = onCheckoutProcessing( checkValidation );
+		const unsubscribeValidation = onCheckoutProcessing(
+			checkValidation,
+			0
+		);
+		const unsubscribeRedirect = onCheckoutCompleteSuccess( () => {
+			window.location.href = redirectUrl;
+		}, 999 );
 		return () => {
-			unsubscribeProcessing();
+			unsubscribeValidation();
+			unsubscribeRedirect();
 		};
-	}, [ onCheckoutProcessing, checkValidation ] );
+	}, [
+		onCheckoutProcessing,
+		onCheckoutCompleteSuccess,
+		checkValidation,
+		redirectUrl,
+	] );
+
+	// process order if conditions are good.
+	useEffect( () => {
+		if ( paidAndWithoutErrors ) {
+			processOrder();
+		}
+	}, [ processOrder, paidAndWithoutErrors ] );
 
 	return null;
 };

assets/js/base/context/cart-checkout/event-emit/emitter-callback.js

@@ -0,0 +1,15 @@
+/**
+ * Internal dependencies
+ */
+import { actions } from './reducer';
+
+export const emitterCallback = ( type, dispatcher ) => (
+	callback,
+	priority
+) => {
+	const action = actions.addEventCallback( type, callback, priority );
+	dispatcher( action );
+	return () => {
+		dispatcher( actions.removeEventCallback( type, action.id ) );
+	};
+};

assets/js/base/context/cart-checkout/event-emit/emitters.js

@@ -1,3 +1,11 @@
+const getObserversByPriority = ( observers, eventType ) => {
+	return observers[ eventType ]
+		? Array.from( observers[ eventType ].values() ).sort( ( a, b ) => {
+				return b.priority - a.priority;
+		  } )
+		: [];
+};
+
 /**
  * Emits events on registered observers for the provided type and passes along
  * the provided data.
@@ -15,12 +23,10 @@
  *                   executed.
  */
 export const emitEvent = async ( observers, eventType, data ) => {
-	const observersByType = observers[ eventType ]
-		? observers[ eventType ].values()
-		: [];
+	const observersByType = getObserversByPriority( observers, eventType );
 	for ( const observer of observersByType ) {
 		try {
-			await Promise.resolve( observer( data ) );
+			await Promise.resolve( observer.callback( data ) );
 		} catch ( e ) {
 			// we don't care about errors blocking execution, but will
 			// console.error for troubleshooting.
@@ -45,12 +51,10 @@ export const emitEvent = async ( observers, eventType, data ) => {
  *                   return value of the aborted observer.
  */
 export const emitEventWithAbort = async ( observers, eventType, data ) => {
-	const observersByType = observers[ eventType ]
-		? observers[ eventType ].values()
-		: [];
+	const observersByType = getObserversByPriority( observers, eventType );
 	for ( const observer of observersByType ) {
 		try {
-			const response = await Promise.resolve( observer( data ) );
+			const response = await Promise.resolve( observer.callback( data ) );
 			if ( response !== true ) {
 				return response;
 			}

assets/js/base/context/cart-checkout/event-emit/index.js

@@ -1,2 +1,3 @@
 export * from './reducer';
 export * from './emitters';
+export * from './emitter-callback';