Fixes regression with payment method validation (https://github.com/woocommerce/woocommerce-blocks/pull/2450)

* add typedefs for store notice context

* improve useStoreNotices hook so returned interfaces are fairly constant

* fix dependencies, defaults, and add types

* fix dependencies

* fix dependencies

* improve functions exposed on validation context so they are more constant

* fixing dependencies

* normalize tokenId to string everywhere

Assuming the token is a number is a bad idea because it is feasible that source for some payment methods could be a string. Also when retrieved as an input value, the id will be a string anyways.
This commit is contained in:
Darren Ethier 2020-05-10 19:41:10 -04:00 committed by GitHub
parent ea3d817db1
commit e7781ba407
12 changed files with 195 additions and 110 deletions

View File

@ -64,7 +64,7 @@ const PaymentMethods = () => {
...paymentMethodInterface ...paymentMethodInterface
} = usePaymentMethodInterface(); } = usePaymentMethodInterface();
const currentPaymentMethodInterface = useRef( paymentMethodInterface ); const currentPaymentMethodInterface = useRef( paymentMethodInterface );
const [ selectedToken, setSelectedToken ] = useState( 0 ); const [ selectedToken, setSelectedToken ] = useState( '0' );
const { noticeContexts } = useEmitResponse(); const { noticeContexts } = useEmitResponse();
const { removeNotice } = useStoreNotices(); const { removeNotice } = useStoreNotices();
@ -146,7 +146,7 @@ const PaymentMethods = () => {
); );
return Object.keys( customerPaymentMethods ).length > 0 && return Object.keys( customerPaymentMethods ).length > 0 &&
selectedToken !== 0 selectedToken !== '0'
? renderedSavedPaymentOptions ? renderedSavedPaymentOptions
: renderedTabsAndSavedPaymentOptions; : renderedTabsAndSavedPaymentOptions;
}; };

View File

@ -20,7 +20,7 @@ import RadioControl from '@woocommerce/base-components/radio-control';
*/ */
const getCcOrEcheckPaymentMethodOption = ( { method, expires, tokenId } ) => { const getCcOrEcheckPaymentMethodOption = ( { method, expires, tokenId } ) => {
return { return {
value: tokenId, value: tokenId + '',
label: sprintf( label: sprintf(
__( __(
'%1$s ending in %2$s (expires %3$s)', '%1$s ending in %2$s (expires %3$s)',
@ -43,7 +43,7 @@ const getCcOrEcheckPaymentMethodOption = ( { method, expires, tokenId } ) => {
*/ */
const getDefaultPaymentMethodOptions = ( { method, tokenId } ) => { const getDefaultPaymentMethodOptions = ( { method, tokenId } ) => {
return { return {
value: tokenId, value: tokenId + '',
label: sprintf( label: sprintf(
__( 'Saved token for %s', 'woo-gutenberg-products-block' ), __( 'Saved token for %s', 'woo-gutenberg-products-block' ),
method.gateway method.gateway
@ -59,7 +59,7 @@ const SavedPaymentMethodOptions = ( { onSelect } ) => {
customerPaymentMethods, customerPaymentMethods,
activePaymentMethod, activePaymentMethod,
} = usePaymentMethodDataContext(); } = usePaymentMethodDataContext();
const [ selectedToken, setSelectedToken ] = useState( null ); const [ selectedToken, setSelectedToken ] = useState( '' );
/** /**
* @type {Object} Options * @type {Object} Options
@ -77,11 +77,9 @@ const SavedPaymentMethodOptions = ( { onSelect } ) => {
paymentMethods.map( ( paymentMethod ) => { paymentMethods.map( ( paymentMethod ) => {
if ( if (
paymentMethod.is_default && paymentMethod.is_default &&
selectedToken === null selectedToken === ''
) { ) {
setSelectedToken( setSelectedToken( paymentMethod.tokenId + '' );
parseInt( paymentMethod.tokenId, 10 )
);
} }
return type === 'cc' || type === 'echeck' return type === 'cc' || type === 'echeck'
? getCcOrEcheckPaymentMethodOption( ? getCcOrEcheckPaymentMethodOption(
@ -96,7 +94,7 @@ const SavedPaymentMethodOptions = ( { onSelect } ) => {
} ); } );
currentOptions.current = options; currentOptions.current = options;
currentOptions.current.push( { currentOptions.current.push( {
value: 0, value: '0',
label: __( label: __(
'Use a new payment method', 'Use a new payment method',
'woo-gutenberg-product-blocks' 'woo-gutenberg-product-blocks'
@ -117,10 +115,10 @@ const SavedPaymentMethodOptions = ( { onSelect } ) => {
} else { } else {
setPaymentStatus().started(); setPaymentStatus().started();
} }
setSelectedToken( parseInt( token, 10 ) ); setSelectedToken( token );
onSelect( parseInt( token, 10 ) ); onSelect( token );
}, },
[ setSelectedToken, setPaymentStatus, onSelect ] [ setSelectedToken, setPaymentStatus, onSelect, activePaymentMethod ]
); );
useEffect( () => { useEffect( () => {
if ( selectedToken && currentOptions.current.length > 0 ) { if ( selectedToken && currentOptions.current.length > 0 ) {
@ -129,7 +127,7 @@ const SavedPaymentMethodOptions = ( { onSelect } ) => {
}, [ selectedToken, updateToken ] ); }, [ selectedToken, updateToken ] );
// In the editor, show `Use a new payment method` option as selected. // In the editor, show `Use a new payment method` option as selected.
const selectedOption = isEditor ? 0 : selectedToken; const selectedOption = isEditor ? '0' : selectedToken + '';
return currentOptions.current.length > 0 ? ( return currentOptions.current.length > 0 ? (
<RadioControl <RadioControl
id={ 'wc-payment-method-stripe-saved-tokens' } id={ 'wc-payment-method-stripe-saved-tokens' }

View File

@ -173,7 +173,7 @@ export const CheckoutStateProvider = ( {
// emit events. // emit events.
useEffect( () => { useEffect( () => {
const { status } = checkoutState; const status = checkoutState.status;
if ( status === STATUS.BEFORE_PROCESSING ) { if ( status === STATUS.BEFORE_PROCESSING ) {
removeNotices( 'error' ); removeNotices( 'error' );
emitEvent( emitEvent(
@ -196,7 +196,13 @@ export const CheckoutStateProvider = ( {
} }
} ); } );
} }
}, [ checkoutState.status, setValidationErrors ] ); }, [
checkoutState.status,
setValidationErrors,
addErrorNotice,
removeNotices,
dispatch,
] );
useEffect( () => { useEffect( () => {
if ( checkoutState.status === STATUS.AFTER_PROCESSING ) { if ( checkoutState.status === STATUS.AFTER_PROCESSING ) {
@ -292,6 +298,10 @@ export const CheckoutStateProvider = ( {
checkoutState.customerNote, checkoutState.customerNote,
checkoutState.processingResponse, checkoutState.processingResponse,
dispatchActions, dispatchActions,
addErrorNotice,
isErrorResponse,
isFailResponse,
isSuccessResponse,
] ); ] );
const onSubmit = () => { const onSubmit = () => {

View File

@ -100,6 +100,7 @@ const CheckoutProcessor = () => {
checkoutIsProcessing, checkoutIsProcessing,
checkoutIsBeforeProcessing, checkoutIsBeforeProcessing,
expressPaymentMethodActive, expressPaymentMethodActive,
dispatchActions,
] ); ] );
const paidAndWithoutErrors = const paidAndWithoutErrors =
@ -234,6 +235,8 @@ const CheckoutProcessor = () => {
paymentMethodId, paymentMethodId,
paymentMethodData, paymentMethodData,
cartNeedsPayment, cartNeedsPayment,
receiveCart,
dispatchActions,
] ); ] );
// redirect when checkout is complete and there is a redirect url. // redirect when checkout is complete and there is a redirect url.
useEffect( () => { useEffect( () => {

View File

@ -170,47 +170,6 @@ export const PaymentMethodDataProvider = ( { children } ) => {
[ paymentData.currentStatus ] [ paymentData.currentStatus ]
); );
// flip payment to processing if checkout processing is complete, there are
// no errors, and payment status is started.
useEffect( () => {
if (
checkoutIsProcessing &&
! checkoutHasError &&
! checkoutIsCalculating &&
! currentStatus.isFinished
) {
setPaymentStatus().processing();
}
}, [
checkoutIsProcessing,
checkoutHasError,
checkoutIsCalculating,
currentStatus.isFinished,
] );
// when checkout is returned to idle, set payment status to pristine.
useEffect( () => {
dispatch( statusOnly( PRISTINE ) );
}, [ checkoutIsIdle ] );
// set initial active payment method if it's undefined.
useEffect( () => {
const paymentMethodKeys = Object.keys( paymentData.paymentMethods );
if (
paymentMethodsInitialized &&
! activePaymentMethod &&
paymentMethodKeys.length > 0
) {
setActivePaymentMethod(
Object.keys( paymentData.paymentMethods )[ 0 ]
);
}
}, [
activePaymentMethod,
paymentMethodsInitialized,
paymentData.paymentMethods,
] );
/** /**
* @type {PaymentStatusDispatch} * @type {PaymentStatusDispatch}
*/ */
@ -267,9 +226,52 @@ export const PaymentMethodDataProvider = ( { children } ) => {
); );
}, },
} ), } ),
[ dispatch ] [ dispatch, setBillingData, setShippingAddress ]
); );
// flip payment to processing if checkout processing is complete, there are
// no errors, and payment status is started.
useEffect( () => {
if (
checkoutIsProcessing &&
! checkoutHasError &&
! checkoutIsCalculating &&
! currentStatus.isFinished
) {
setPaymentStatus().processing();
}
}, [
checkoutIsProcessing,
checkoutHasError,
checkoutIsCalculating,
currentStatus.isFinished,
setPaymentStatus,
] );
// when checkout is returned to idle, set payment status to pristine.
useEffect( () => {
dispatch( statusOnly( PRISTINE ) );
}, [ checkoutIsIdle ] );
// set initial active payment method if it's undefined.
useEffect( () => {
const paymentMethodKeys = Object.keys( paymentData.paymentMethods );
if (
paymentMethodsInitialized &&
! activePaymentMethod &&
paymentMethodKeys.length > 0
) {
setActivePaymentMethod(
Object.keys( paymentData.paymentMethods )[ 0 ]
);
}
}, [
activePaymentMethod,
paymentMethodsInitialized,
paymentData.paymentMethods,
setActivePaymentMethod,
] );
// emit events. // emit events.
useEffect( () => { useEffect( () => {
// Note: the nature of this event emitter is that it will bail on any // Note: the nature of this event emitter is that it will bail on any
@ -323,7 +325,17 @@ export const PaymentMethodDataProvider = ( { children } ) => {
} }
} ); } );
} }
}, [ currentStatus.isProcessing, setValidationErrors, setPaymentStatus ] ); }, [
currentStatus.isProcessing,
setValidationErrors,
setPaymentStatus,
removeNotice,
noticeContexts.PAYMENTS,
isSuccessResponse,
isFailResponse,
isErrorResponse,
addErrorNotice,
] );
/** /**
* @type {PaymentMethodDataContext} * @type {PaymentMethodDataContext}

View File

@ -81,19 +81,22 @@ export const ValidationContextProvider = ( { children } ) => {
* @param {string} property The name of the property to clear if exists in * @param {string} property The name of the property to clear if exists in
* validation error state. * validation error state.
*/ */
const clearValidationError = ( property ) => { const clearValidationError = useCallback( ( property ) => {
updateValidationErrors( ( prevErrors ) => { updateValidationErrors( ( prevErrors ) => {
if ( ! prevErrors[ property ] ) { if ( ! prevErrors[ property ] ) {
return prevErrors; return prevErrors;
} }
return omit( prevErrors, [ property ] ); return omit( prevErrors, [ property ] );
} ); } );
}; }, [] );
/** /**
* Clears the entire validation error state. * Clears the entire validation error state.
*/ */
const clearAllValidationErrors = () => void updateValidationErrors( {} ); const clearAllValidationErrors = useCallback(
() => void updateValidationErrors( {} ),
[]
);
/** /**
* Used to record new validation errors in the state. * Used to record new validation errors in the state.
@ -132,7 +135,7 @@ export const ValidationContextProvider = ( { children } ) => {
* @param {string} property The name of the property to update. * @param {string} property The name of the property to update.
* @param {Object} newError New validation error object. * @param {Object} newError New validation error object.
*/ */
const updateValidationError = ( property, newError ) => { const updateValidationError = useCallback( ( property, newError ) => {
updateValidationErrors( ( prevErrors ) => { updateValidationErrors( ( prevErrors ) => {
if ( ! prevErrors.hasOwnProperty( property ) ) { if ( ! prevErrors.hasOwnProperty( property ) ) {
return prevErrors; return prevErrors;
@ -148,7 +151,7 @@ export const ValidationContextProvider = ( { children } ) => {
[ property ]: updatedError, [ property ]: updatedError,
}; };
} ); } );
}; }, [] );
/** /**
* Given a property name and if an associated error exists, it sets its * Given a property name and if an associated error exists, it sets its
@ -157,10 +160,13 @@ export const ValidationContextProvider = ( { children } ) => {
* @param {string} property The name of the property to set the `hidden` * @param {string} property The name of the property to set the `hidden`
* value to true. * value to true.
*/ */
const hideValidationError = ( property ) => const hideValidationError = useCallback(
void updateValidationError( property, { ( property ) =>
hidden: true, void updateValidationError( property, {
} ); hidden: true,
} ),
[ updateValidationError ]
);
/** /**
* Given a property name and if an associated error exists, it sets its * Given a property name and if an associated error exists, it sets its
@ -169,36 +175,42 @@ export const ValidationContextProvider = ( { children } ) => {
* @param {string} property The name of the property to set the `hidden` * @param {string} property The name of the property to set the `hidden`
* value to false. * value to false.
*/ */
const showValidationError = ( property ) => const showValidationError = useCallback(
void updateValidationError( property, { ( property ) =>
hidden: false, void updateValidationError( property, {
} ); hidden: false,
} ),
[ updateValidationError ]
);
/** /**
* Sets the `hidden` value of all errors to `false`. * Sets the `hidden` value of all errors to `false`.
*/ */
const showAllValidationErrors = () => const showAllValidationErrors = useCallback(
void updateValidationErrors( ( prevErrors ) => { () =>
const updatedErrors = {}; void updateValidationErrors( ( prevErrors ) => {
const updatedErrors = {};
Object.keys( prevErrors ).forEach( ( property ) => { Object.keys( prevErrors ).forEach( ( property ) => {
if ( prevErrors[ property ].hidden ) { if ( prevErrors[ property ].hidden ) {
updatedErrors[ property ] = { updatedErrors[ property ] = {
...prevErrors[ property ], ...prevErrors[ property ],
hidden: false, hidden: false,
}; };
}
} );
if ( Object.values( updatedErrors ).length === 0 ) {
return prevErrors;
} }
} );
if ( Object.values( updatedErrors ).length === 0 ) { return {
return prevErrors; ...prevErrors,
} ...updatedErrors,
};
return { } ),
...prevErrors, []
...updatedErrors, );
};
} );
const context = { const context = {
getValidationError, getValidationError,

View File

@ -9,14 +9,23 @@ import {
SnackbarNoticesContainer, SnackbarNoticesContainer,
} from '@woocommerce/base-components/store-notices-container'; } from '@woocommerce/base-components/store-notices-container';
/**
* @typedef {import('@woocommerce/type-defs/contexts').NoticeContext} NoticeContext
*/
const StoreNoticesContext = createContext( { const StoreNoticesContext = createContext( {
notices: [], notices: [],
createNotice: ( status, text, props ) => void { status, text, props }, createNotice: ( status, text, props ) => void { status, text, props },
createSnackBarNotice: () => void null, createSnackbarNotice: ( content, options ) => void { content, options },
removeNotice: ( id, ctxt ) => void { id, ctxt }, removeNotice: ( id, ctxt ) => void { id, ctxt },
context: 'wc/core', context: 'wc/core',
} ); } );
/**
* Returns the notices context values.
*
* @return {NoticeContext} The notice context value from the notice context.
*/
export const useStoreNoticesContext = () => { export const useStoreNoticesContext = () => {
return useContext( StoreNoticesContext ); return useContext( StoreNoticesContext );
}; };
@ -53,7 +62,7 @@ export const StoreNoticesProvider = ( {
( id, ctxt = context ) => { ( id, ctxt = context ) => {
removeNotice( id, ctxt ); removeNotice( id, ctxt );
}, },
[ createNotice, context ] [ removeNotice, context ]
); );
const createSnackbarNotice = useCallback( const createSnackbarNotice = useCallback(

View File

@ -2,7 +2,7 @@
* External dependencies * External dependencies
*/ */
import { useStoreNoticesContext } from '@woocommerce/base-context'; import { useStoreNoticesContext } from '@woocommerce/base-context';
import { useMemo } from '@wordpress/element'; import { useMemo, useRef, useEffect } from '@wordpress/element';
export const useStoreNotices = () => { export const useStoreNotices = () => {
const { const {
@ -11,8 +11,36 @@ export const useStoreNotices = () => {
removeNotice, removeNotice,
createSnackbarNotice, createSnackbarNotice,
} = useStoreNoticesContext(); } = useStoreNoticesContext();
// Added to a ref so the surface for notices doesn't change frequently
// and thus can be used as dependencies on effects.
const currentNotices = useRef( notices );
// Update notices ref whenever they change
useEffect( () => {
currentNotices.current = notices;
}, [ notices ] );
const noticesApi = useMemo( const noticesApi = useMemo(
() => ( {
hasNoticesOfType: ( type ) => {
return currentNotices.current.some(
( notice ) => notice.type === type
);
},
removeNotices: ( status = null ) => {
currentNotices.current.map( ( notice ) => {
if ( status === null || notice.status === status ) {
removeNotice( notice.id );
}
return true;
} );
},
removeNotice,
} ),
[ removeNotice ]
);
const noticeCreators = useMemo(
() => ( { () => ( {
addDefaultNotice: ( text, noticeProps = {} ) => addDefaultNotice: ( text, noticeProps = {} ) =>
void createNotice( 'default', text, { void createNotice( 'default', text, {
@ -34,27 +62,16 @@ export const useStoreNotices = () => {
void createNotice( 'success', text, { void createNotice( 'success', text, {
...noticeProps, ...noticeProps,
} ), } ),
hasNoticesOfType: ( type ) => {
return notices.some( ( notice ) => notice.type === type );
},
removeNotices: ( status = null ) => {
notices.map( ( notice ) => {
if ( status === null || notice.status === status ) {
removeNotice( notice.id );
}
return true;
} );
},
removeNotice,
addSnackbarNotice: ( text, noticeProps = {} ) => { addSnackbarNotice: ( text, noticeProps = {} ) => {
createSnackbarNotice( text, noticeProps ); createSnackbarNotice( text, noticeProps );
}, },
} ), } ),
[ createNotice, createSnackbarNotice, notices ] [ createNotice, createSnackbarNotice ]
); );
return { return {
notices, notices,
...noticesApi, ...noticesApi,
...noticeCreators,
}; };
}; };

View File

@ -50,7 +50,7 @@ const CreditCardComponent = ( {
if ( paymentEvent.error ) { if ( paymentEvent.error ) {
onStripeError( paymentEvent ); onStripeError( paymentEvent );
} }
setSourceId( 0 ); setSourceId( '0' );
}; };
const renderedCardElement = getStripeServerData().inline_cc_form ? ( const renderedCardElement = getStripeServerData().inline_cc_form ? (
<InlineCard <InlineCard

View File

@ -84,7 +84,7 @@ export const usePaymentProcessing = (
}; };
} }
// use token if it's set. // use token if it's set.
if ( parseInt( sourceId, 10 ) !== 0 ) { if ( sourceId !== '' && sourceId !== '0' ) {
return { return {
type: emitResponse.responseTypes.SUCCESS, type: emitResponse.responseTypes.SUCCESS,
meta: { meta: {

View File

@ -8,7 +8,7 @@ export const previewSavedPaymentMethods = {
}, },
expires: '12/20', expires: '12/20',
is_default: false, is_default: false,
tokenId: 1, tokenId: '1',
}, },
], ],
}; };

View File

@ -305,4 +305,28 @@
* @property {boolean} hasValidationErrors True if there is at least one error. * @property {boolean} hasValidationErrors True if there is at least one error.
*/ */
/**
* @typedef StoreNoticeObject
*
* @property {string} type The type of notice.
* @property {string} status The status of the notice.
* @property {string} id The id of the notice.
*/
/**
* @typedef NoticeContext
*
* @property {Array<StoreNoticeObject>} notices An array of notice
* objects.
* @property {function(string,string,any):undefined} createNotice Creates a notice for the
* given arguments.
* @property {function(string, any):undefined} createSnackbarNotice Creates a snackbar notice
* type.
* @property {function(string,string=):undefined} removeNotice Removes a notice with the
* given id and context
* @property {string} context The current context
* identifier for the notice
* provider
*/
export {}; export {};