Skip to content

Commit

Permalink
fix: Changes from PR Review
Browse files Browse the repository at this point in the history
  • Loading branch information
grmartin committed Jun 30, 2023
1 parent bdd666e commit e39dfb4
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 27 deletions.
8 changes: 4 additions & 4 deletions src/payment/PaymentProcessingModal.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 = () => {
/**
Expand Down Expand Up @@ -49,7 +49,7 @@ export const PaymentProcessingModal = () => {
<ModalDialog
title="Your Payment is Processing"
isOpen={isOpen}
onClose={() => { /* Noop, @see updatePaymentState fulfill */ }}
onClose={() => { /* Noop, @see pollPaymentState fulfill */ }}
hasCloseButton={false}
isFullscreenOnMobile={false}
>
Expand Down
2 changes: 1 addition & 1 deletion src/payment/data/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
6 changes: 3 additions & 3 deletions src/payment/data/reducers.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
fetchCaptureKey,
fetchClientSecret,
fetchActiveOrder,
updatePaymentState,
pollPaymentState,
} from './actions';

import { DEFAULT_STATUS } from '../checkout/payment-form/flex-microform/constants';
Expand Down Expand Up @@ -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: {
Expand All @@ -160,7 +160,7 @@ const paymentState = (state = basketInitialState, action = null) => {
},
};

case updatePaymentState.FULFILL:
case pollPaymentState.FULFILL:
return {
...state,
paymentStatePolling: {
Expand Down
8 changes: 4 additions & 4 deletions src/payment/data/redux.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
submitPayment,
fetchBasket,
fetchActiveOrder,
updatePaymentState,
pollPaymentState,
PAYMENT_STATE_DATA_RECEIVED,
} from './actions';
import { currencyDisclaimerSelector, paymentSelector } from './selectors';
Expand Down Expand Up @@ -230,7 +230,7 @@ describe('redux tests', () => {
});
});

describe('updatePaymentState actions', () => {
describe('pollPaymentState actions', () => {
it('Round Trip', () => {
const triggerStore = createStore(
combineReducers({
Expand All @@ -252,15 +252,15 @@ 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);

triggerStore.dispatch({ type: PAYMENT_STATE_DATA_RECEIVED, payload: { state: PAYMENT_STATE.COMPLETED } });
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);
});
Expand Down
12 changes: 6 additions & 6 deletions src/payment/data/sagas.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
clientSecretProcessing,
fetchClientSecret,
paymentStateDataReceived,
updatePaymentState,
pollPaymentState,
} from './actions';

import { STATUS_LOADING } from '../checkout/payment-form/flex-microform/constants';
Expand Down Expand Up @@ -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());
}
}
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}

Expand All @@ -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));
}
Expand All @@ -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);
}
8 changes: 4 additions & 4 deletions src/payment/data/sagas.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import {
submitPayment,
CAPTURE_KEY_START_TIMEOUT,
fetchClientSecret,
updatePaymentState,
pollPaymentState,
} from './actions';
import { clearMessages, MESSAGE_TYPES, addMessage } from '../../feedback';

Expand Down Expand Up @@ -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(() => {
Expand Down Expand Up @@ -703,7 +703,7 @@ describe('saga tests', () => {
clearMessages(),
submitPayment.request(),
submitPayment.success(),
updatePaymentState.trigger(),
pollPaymentState.trigger(),
basketProcessing(false),
submitPayment.fulfill(),
]);
Expand Down Expand Up @@ -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!

Expand Down
14 changes: 9 additions & 5 deletions src/payment/data/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
};
};

0 comments on commit e39dfb4

Please sign in to comment.