From e39dfb4e43022745faa8b36338314c7a406003a0 Mon Sep 17 00:00:00 2001 From: "Glenn R. Martin" Date: Fri, 30 Jun 2023 11:57:02 -0400 Subject: [PATCH] fix: Changes from PR Review --- src/payment/PaymentProcessingModal.jsx | 8 ++++---- src/payment/data/actions.js | 2 +- src/payment/data/reducers.js | 6 +++--- src/payment/data/redux.test.js | 8 ++++---- src/payment/data/sagas.js | 12 ++++++------ src/payment/data/sagas.test.js | 8 ++++---- src/payment/data/utils.js | 14 +++++++++----- 7 files changed, 31 insertions(+), 27 deletions(-) diff --git a/src/payment/PaymentProcessingModal.jsx b/src/payment/PaymentProcessingModal.jsx index 63c858ddc..4071ad50a 100644 --- a/src/payment/PaymentProcessingModal.jsx +++ b/src/payment/PaymentProcessingModal.jsx @@ -17,10 +17,10 @@ import { POLLING_PAYMENT_STATES } from './data/constants'; * @see paymentProcessStatusSelector * @see paymentProcessStatusIsPollingSelector * - * Primary Event: `updatePaymentState` - * @see updatePaymentState + * Primary Event: `pollPaymentState` + * @see pollPaymentState * - * If you wish to perform an action as this dialog closes, please register for the updatePaymentState fulfill event. + * If you wish to perform an action as this dialog closes, please register for the pollPaymentState fulfill event. */ export const PaymentProcessingModal = () => { /** @@ -49,7 +49,7 @@ export const PaymentProcessingModal = () => { { /* Noop, @see updatePaymentState fulfill */ }} + onClose={() => { /* Noop, @see pollPaymentState fulfill */ }} hasCloseButton={false} isFullscreenOnMobile={false} > diff --git a/src/payment/data/actions.js b/src/payment/data/actions.js index df723decd..a7cf502ae 100644 --- a/src/payment/data/actions.js +++ b/src/payment/data/actions.js @@ -20,7 +20,7 @@ export const fetchActiveOrder = createRoutine('FETCH_ACTIVE_ORDER'); export const addCoupon = createRoutine('ADD_COUPON'); export const removeCoupon = createRoutine('REMOVE_COUPON'); export const updateQuantity = createRoutine('UPDATE_QUANTITY'); -export const updatePaymentState = createRoutine('UPDATE_PAYMENT_STATE'); +export const pollPaymentState = createRoutine('UPDATE_PAYMENT_STATE'); // Actions and their action creators export const BASKET_DATA_RECEIVED = 'BASKET_DATA_RECEIVED'; diff --git a/src/payment/data/reducers.js b/src/payment/data/reducers.js index e126491b8..112918b83 100644 --- a/src/payment/data/reducers.js +++ b/src/payment/data/reducers.js @@ -14,7 +14,7 @@ import { fetchCaptureKey, fetchClientSecret, fetchActiveOrder, - updatePaymentState, + pollPaymentState, } from './actions'; import { DEFAULT_STATUS } from '../checkout/payment-form/flex-microform/constants'; @@ -151,7 +151,7 @@ const paymentState = (state = basketInitialState, action = null) => { if (action !== null && action !== undefined) { switch (action.type) { - case updatePaymentState.TRIGGER: + case pollPaymentState.TRIGGER: return { ...state, paymentStatePolling: { @@ -160,7 +160,7 @@ const paymentState = (state = basketInitialState, action = null) => { }, }; - case updatePaymentState.FULFILL: + case pollPaymentState.FULFILL: return { ...state, paymentStatePolling: { diff --git a/src/payment/data/redux.test.js b/src/payment/data/redux.test.js index 337b17800..262a621e2 100644 --- a/src/payment/data/redux.test.js +++ b/src/payment/data/redux.test.js @@ -8,7 +8,7 @@ import { submitPayment, fetchBasket, fetchActiveOrder, - updatePaymentState, + pollPaymentState, PAYMENT_STATE_DATA_RECEIVED, } from './actions'; import { currencyDisclaimerSelector, paymentSelector } from './selectors'; @@ -230,7 +230,7 @@ describe('redux tests', () => { }); }); - describe('updatePaymentState actions', () => { + describe('pollPaymentState actions', () => { it('Round Trip', () => { const triggerStore = createStore( combineReducers({ @@ -252,7 +252,7 @@ describe('redux tests', () => { }, ); - triggerStore.dispatch(updatePaymentState()); + triggerStore.dispatch(pollPaymentState()); expect(triggerStore.getState().payment.basket.paymentStatePolling.keepPolling).toBe(true); expect(triggerStore.getState().payment.basket.paymentState).toBe(PAYMENT_STATE.PENDING); @@ -260,7 +260,7 @@ describe('redux tests', () => { expect(triggerStore.getState().payment.basket.paymentStatePolling.keepPolling).toBe(false); expect(triggerStore.getState().payment.basket.paymentState).toBe(PAYMENT_STATE.COMPLETED); - triggerStore.dispatch(updatePaymentState.fulfill()); + triggerStore.dispatch(pollPaymentState.fulfill()); expect(triggerStore.getState().payment.basket.paymentStatePolling.keepPolling).toBe(false); expect(triggerStore.getState().payment.basket.paymentState === PAYMENT_STATE.PENDING).toBe(false); }); diff --git a/src/payment/data/sagas.js b/src/payment/data/sagas.js index fba742702..d961e3cdd 100644 --- a/src/payment/data/sagas.js +++ b/src/payment/data/sagas.js @@ -26,7 +26,7 @@ import { clientSecretProcessing, fetchClientSecret, paymentStateDataReceived, - updatePaymentState, + pollPaymentState, } from './actions'; import { STATUS_LOADING } from '../checkout/payment-form/flex-microform/constants'; @@ -145,7 +145,7 @@ export function* handleFetchActiveOrder() { yield put(basketProcessing(false)); // we are done modifying the basket yield put(fetchActiveOrder.fulfill()); // mark the basket as finished loading if (yield select((state) => paymentProcessStatusShouldRunSelector(state))) { - yield put(updatePaymentState()); + yield put(pollPaymentState()); } } } @@ -278,7 +278,7 @@ export function* handleSubmitPayment({ payload }) { yield put(submitPayment.success()); if (yield select((state) => paymentProcessStatusShouldRunSelector(state))) { - yield put(updatePaymentState()); + yield put(pollPaymentState()); } } catch (error) { // Do not handle errors on user aborted actions @@ -328,7 +328,7 @@ export function* handlePaymentState() { // This shouldn't happen. // I don't think we need to banner... shouldn't our parent calls recover? (They invoke this) keepPolling = false; - yield put(updatePaymentState.fulfill()); + yield put(pollPaymentState.fulfill()); return; } @@ -337,7 +337,7 @@ export function* handlePaymentState() { if (!(yield select(state => state.payment.basket.paymentStatePolling.keepPolling))) { keepPolling = false; - yield put(updatePaymentState.fulfill()); + yield put(pollPaymentState.fulfill()); } else { yield delay(SECS_AS_MS(delaySecs)); } @@ -360,5 +360,5 @@ export default function* saga() { yield takeEvery(removeCoupon.TRIGGER, handleRemoveCoupon); yield takeEvery(updateQuantity.TRIGGER, handleUpdateQuantity); yield takeEvery(submitPayment.TRIGGER, handleSubmitPayment); - yield takeEvery(updatePaymentState.TRIGGER, handlePaymentState); + yield takeEvery(pollPaymentState.TRIGGER, handlePaymentState); } diff --git a/src/payment/data/sagas.test.js b/src/payment/data/sagas.test.js index d393eaa4b..fd3634e91 100644 --- a/src/payment/data/sagas.test.js +++ b/src/payment/data/sagas.test.js @@ -33,7 +33,7 @@ import { submitPayment, CAPTURE_KEY_START_TIMEOUT, fetchClientSecret, - updatePaymentState, + pollPaymentState, } from './actions'; import { clearMessages, MESSAGE_TYPES, addMessage } from '../../feedback'; @@ -633,7 +633,7 @@ describe('saga tests', () => { // We reset it back after each run so future tests do not explode. nativeGlobalLocation = global.location; delete global.location; - global.location = jest.fn();// = Object.create(window); + global.location = jest.fn(); }); afterEach(() => { @@ -703,7 +703,7 @@ describe('saga tests', () => { clearMessages(), submitPayment.request(), submitPayment.success(), - updatePaymentState.trigger(), + pollPaymentState.trigger(), basketProcessing(false), submitPayment.fulfill(), ]); @@ -927,7 +927,7 @@ describe('saga tests', () => { expect(gen.next().value).toEqual(takeEvery(removeCoupon.TRIGGER, handleRemoveCoupon)); expect(gen.next().value).toEqual(takeEvery(updateQuantity.TRIGGER, handleUpdateQuantity)); expect(gen.next().value).toEqual(takeEvery(submitPayment.TRIGGER, handleSubmitPayment)); - expect(gen.next().value).toEqual(takeEvery(updatePaymentState.TRIGGER, handlePaymentState)); + expect(gen.next().value).toEqual(takeEvery(pollPaymentState.TRIGGER, handlePaymentState)); // If you find yourself adding something here, there are probably more tests to write! diff --git a/src/payment/data/utils.js b/src/payment/data/utils.js index 1d53bc0d2..107b7499f 100644 --- a/src/payment/data/utils.js +++ b/src/payment/data/utils.js @@ -263,9 +263,13 @@ export const chainReducers = (reducers) => { default: /* No-op, lets continue execution */ break; } - // if we name this it might be clearer when debugging, TODO: GRM: float the idea. - return (initialState, action) => reducers.reduce( - (lastState, reducerFn) => reducerFn(lastState, action), - initialState, - ); + // Using a function so someone with a debugger doesn't see infinite anonymous functions + return function _wrappedSerialChainReducers(initialState, action) { + // This loops through the array of reducers, by reducing the array. + // The return of the inner reducerFn becomes the lastState for the next. + return reducers.reduce( + (lastState, reducerFn) => reducerFn(lastState, action), + initialState, + ); + }; };