Skip to content

Commit

Permalink
Revise checkout payment statuses to avoid data loss on error (woocomm…
Browse files Browse the repository at this point in the history
…erce#5350)

* Clarify docs for STARTED

* Clarify docs for setActivePaymentMethod

* Remove useActivePaymentMethod hook (this held state for active methods and tokens)

* Update type defs

* Enhance setActivePaymentMethod action to accept method data

* SET_ACTIVE_PAYMENT_METHOD action

* Add setActivePaymentMethod dispatcher and make "started" status only

* Update setActivePaymentMethod usage in express methods

* Set radio control defaults

* Consolodate tokens and methods

* Update assets/js/base/context/providers/cart-checkout/payment-methods/reducer.ts

Co-authored-by: Seghir Nadir <[email protected]>

* Spacing

* Split saved cards tests from regular, since saved cards are checked by default

Co-authored-by: Seghir Nadir <[email protected]>
  • Loading branch information
2 people authored and jonny-bull committed Dec 16, 2021
1 parent ea86ce3 commit 3babe4f
Show file tree
Hide file tree
Showing 14 changed files with 265 additions and 308 deletions.
4 changes: 2 additions & 2 deletions assets/js/base/components/radio-control/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ import RadioControlOption from './option';
import './style.scss';

const RadioControl = ( {
className,
className = '',
instanceId,
id,
selected,
onChange,
onChange = () => {},
options = [],
} ) => {
const radioControlId = id || instanceId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,72 +14,68 @@ import { ACTION, STATUS } from './constants';
export interface ActionType {
type: ACTION | STATUS;
errorMessage?: string;
paymentMethodData?: Record< string, unknown >;
paymentMethodData?: Record< string, unknown > | undefined;
paymentMethods?: PaymentMethods | ExpressPaymentMethods;
paymentMethod?: string;
shouldSavePaymentMethod?: boolean;
}

/**
* All the actions that can be dispatched for payment methods.
*/
export const actions = {
statusOnly: ( type: STATUS ): { type: STATUS } => ( { type } as const ),
error: ( errorMessage: string ): ActionType =>
( {
type: STATUS.ERROR,
errorMessage,
} as const ),
statusOnly: ( type: STATUS ): ActionType => ( {
type,
} ),
error: ( errorMessage: string ): ActionType => ( {
type: STATUS.ERROR,
errorMessage,
} ),
failed: ( {
errorMessage,
paymentMethodData,
}: {
errorMessage: string;
paymentMethodData: Record< string, unknown >;
} ): ActionType =>
( {
type: STATUS.FAILED,
errorMessage,
paymentMethodData,
} as const ),
} ): ActionType => ( {
type: STATUS.FAILED,
errorMessage,
paymentMethodData,
} ),
success: ( {
paymentMethodData,
}: {
paymentMethodData?: Record< string, unknown >;
} ): ActionType =>
( {
type: STATUS.SUCCESS,
paymentMethodData,
} as const ),
started: ( {
} ): ActionType => ( {
type: STATUS.SUCCESS,
paymentMethodData,
}: {
paymentMethodData?: Record< string, unknown >;
} ): ActionType =>
( {
type: STATUS.STARTED,
paymentMethodData,
} as const ),
} ),
setRegisteredPaymentMethods: (
paymentMethods: PaymentMethods
): ActionType =>
( {
type: ACTION.SET_REGISTERED_PAYMENT_METHODS,
paymentMethods,
} as const ),
): ActionType => ( {
type: ACTION.SET_REGISTERED_PAYMENT_METHODS,
paymentMethods,
} ),
setRegisteredExpressPaymentMethods: (
paymentMethods: ExpressPaymentMethods
): ActionType =>
( {
type: ACTION.SET_REGISTERED_EXPRESS_PAYMENT_METHODS,
paymentMethods,
} as const ),
): ActionType => ( {
type: ACTION.SET_REGISTERED_EXPRESS_PAYMENT_METHODS,
paymentMethods,
} ),
setShouldSavePaymentMethod: (
shouldSavePaymentMethod: boolean
): ActionType =>
( {
type: ACTION.SET_SHOULD_SAVE_PAYMENT_METHOD,
shouldSavePaymentMethod,
} as const ),
): ActionType => ( {
type: ACTION.SET_SHOULD_SAVE_PAYMENT_METHOD,
shouldSavePaymentMethod,
} ),
setActivePaymentMethod: (
paymentMethod: string,
paymentMethodData: Record< string, unknown >
): ActionType => ( {
type: ACTION.SET_ACTIVE_PAYMENT_METHOD,
paymentMethod,
paymentMethodData,
} ),
};

export default actions;
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,17 @@ export enum ACTION {
SET_REGISTERED_PAYMENT_METHODS = 'set_registered_payment_methods',
SET_REGISTERED_EXPRESS_PAYMENT_METHODS = 'set_registered_express_payment_methods',
SET_SHOULD_SAVE_PAYMENT_METHOD = 'set_should_save_payment_method',
SET_ACTIVE_PAYMENT_METHOD = 'set_active_payment_method',
}

// Note - if fields are added/shape is changed, you may want to update PRISTINE reducer clause to preserve your new field.
export const DEFAULT_PAYMENT_DATA_CONTEXT_STATE: PaymentMethodDataContextState = {
currentStatus: STATUS.PRISTINE,
shouldSavePaymentMethod: false,
activePaymentMethod: '',
paymentMethodData: {
payment_method: '',
},
hasSavedToken: false,
errorMessage: '',
paymentMethods: {},
expressPaymentMethods: {},
Expand Down Expand Up @@ -61,9 +62,8 @@ export const DEFAULT_PAYMENT_METHOD_DATA: PaymentMethodDataContextType = {
paymentMethodData: {},
errorMessage: '',
activePaymentMethod: '',
setActivePaymentMethod: () => void null,
activeSavedToken: '',
setActiveSavedToken: () => void null,
setActivePaymentMethod: () => void null,
customerPaymentMethods: {},
paymentMethods: {},
expressPaymentMethods: {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
useEffect,
useMemo,
} from '@wordpress/element';
import { objectHasProp } from '@woocommerce/types';

/**
* Internal dependencies
Expand All @@ -29,7 +30,6 @@ import {
useExpressPaymentMethods,
} from './use-payment-method-registration';
import { usePaymentMethodDataDispatchers } from './use-payment-method-dispatchers';
import { useActivePaymentMethod } from './use-active-payment-method';
import { useCheckoutContext } from '../checkout-state';
import { useEditorContext } from '../../editor-context';
import {
Expand Down Expand Up @@ -90,6 +90,7 @@ export const PaymentMethodDataProvider = ( {
reducer,
DEFAULT_PAYMENT_DATA_CONTEXT_STATE
);

const {
dispatchActions,
setPaymentStatus,
Expand All @@ -103,13 +104,6 @@ export const PaymentMethodDataProvider = ( {
dispatchActions.setRegisteredExpressPaymentMethods
);

const {
activePaymentMethod,
activeSavedToken,
setActivePaymentMethod,
setActiveSavedToken,
} = useActivePaymentMethod();

const customerPaymentMethods = useMemo( (): CustomerPaymentMethods => {
if ( isEditor ) {
return getPreviewData(
Expand Down Expand Up @@ -145,7 +139,7 @@ export const PaymentMethodDataProvider = ( {

const isExpressPaymentMethodActive = Object.keys(
paymentData.expressPaymentMethods
).includes( activePaymentMethod );
).includes( paymentData.activePaymentMethod );

const currentStatus = useMemo(
() => ( {
Expand All @@ -167,38 +161,64 @@ export const PaymentMethodDataProvider = ( {
[ paymentData.currentStatus, isExpressPaymentMethodActive ]
);

// Update the active (selected) payment method when it is empty, or invalid.
/**
* Active Gateway Selection
*
* Updates the active (selected) payment method when it is empty, or invalid. This uses the first saved payment
* method found (if applicable), or the first standard gateway.
*/
useEffect( () => {
const paymentMethodKeys = Object.keys( paymentData.paymentMethods );

if ( ! paymentMethodsInitialized || ! paymentMethodKeys.length ) {
return;
}

const allPaymentMethodKeys = [
...paymentMethodKeys,
...Object.keys( paymentData.expressPaymentMethods ),
];
if ( ! paymentMethodsInitialized || ! paymentMethodKeys.length ) {

// Return if current method is valid.
if (
paymentData.activePaymentMethod &&
allPaymentMethodKeys.includes( paymentData.activePaymentMethod )
) {
return;
}

setActivePaymentMethod( ( currentActivePaymentMethod ) => {
// If there's no active payment method, or the active payment method has
// been removed (e.g. COD vs shipping methods), set one as active.
// Note: It's possible that the active payment method might be an
// express payment method. So registered express payment methods are
// included in the check here.
if (
! currentActivePaymentMethod ||
! allPaymentMethodKeys.includes( currentActivePaymentMethod )
) {
setPaymentStatus().pristine();
return Object.keys( paymentData.paymentMethods )[ 0 ];
}
return currentActivePaymentMethod;
} );
setPaymentStatus().pristine();

const customerPaymentMethod =
Object.keys( customerPaymentMethods ).flatMap(
( type ) => customerPaymentMethods[ type ]
)[ 0 ] || undefined;

if ( customerPaymentMethod ) {
const token = customerPaymentMethod.tokenId;
const paymentMethodSlug = customerPaymentMethod.method.gateway;
const savedTokenKey = `wc-${ paymentMethodSlug }-payment-token`;

dispatchActions.setActivePaymentMethod( paymentMethodSlug, {
token,
payment_method: paymentMethodSlug,
[ savedTokenKey ]: token.toString(),
isSavedToken: true,
} );
return;
}

dispatchActions.setActivePaymentMethod(
Object.keys( paymentData.paymentMethods )[ 0 ]
);
}, [
paymentMethodsInitialized,
paymentData.paymentMethods,
paymentData.expressPaymentMethods,
setActivePaymentMethod,
dispatchActions,
setPaymentStatus,
paymentData.activePaymentMethod,
customerPaymentMethods,
] );

// flip payment to processing if checkout processing is complete, there are no errors, and payment status is started.
Expand Down Expand Up @@ -226,21 +246,12 @@ export const PaymentMethodDataProvider = ( {
}
}, [ checkoutIsIdle, currentStatus.isSuccessful, setPaymentStatus ] );

// if checkout has an error and payment is not being made with a saved token and payment status is success, then let's sync payment status back to pristine.
// if checkout has an error sync payment status back to pristine.
useEffect( () => {
if (
checkoutHasError &&
currentStatus.isSuccessful &&
! paymentData.hasSavedToken
) {
if ( checkoutHasError && currentStatus.isSuccessful ) {
setPaymentStatus().pristine();
}
}, [
checkoutHasError,
currentStatus.isSuccessful,
paymentData.hasSavedToken,
setPaymentStatus,
] );
}, [ checkoutHasError, currentStatus.isSuccessful, setPaymentStatus ] );

useEffect( () => {
// Note: the nature of this event emitter is that it will bail on any
Expand Down Expand Up @@ -325,16 +336,21 @@ export const PaymentMethodDataProvider = ( {
addErrorNotice,
] );

const activeSavedToken =
typeof paymentData.paymentMethodData === 'object' &&
objectHasProp( paymentData.paymentMethodData, 'token' )
? paymentData.paymentMethodData.token + ''
: '';

const paymentContextData: PaymentMethodDataContextType = {
setPaymentStatus,
currentStatus,
paymentStatuses: STATUS,
paymentMethodData: paymentData.paymentMethodData,
errorMessage: paymentData.errorMessage,
activePaymentMethod,
setActivePaymentMethod,
activePaymentMethod: paymentData.activePaymentMethod,
activeSavedToken,
setActiveSavedToken,
setActivePaymentMethod: dispatchActions.setActivePaymentMethod,
onPaymentProcessing,
customerPaymentMethods,
paymentMethods: paymentData.paymentMethods,
Expand Down
Loading

0 comments on commit 3babe4f

Please sign in to comment.