* Add filter to extend product price

* Remove code targeting WC Subscriptions

* Rename filter

* Use extendibility API instead of filters

* Remove __EXPERIMENTAL_CART_ITEM_PRICE_FILTER from docs

* throw errors on validation

* Don't catch filter errors for admins

* Add tests

* wrap filter calls in memo

* pass extensions as top level prop

* abstract errors

* add jsdoc

* update tests

* review

* turn __experimentalApplyCheckoutFilter into a hook and move useMemo inside it

* revert name

* wrap getCheckoutFilters in useMemo

* refactor filter function so memozation is done inside components

* unify true instance

* fix rebase

Co-authored-by: Albert Juhé Lluveras <contact@albertjuhe.com>
This commit is contained in:
Seghir Nadir 2021-02-17 14:01:20 +01:00 committed by GitHub
parent eac22ca2f7
commit 30c2079af4
12 changed files with 322 additions and 92 deletions

View File

@ -6,10 +6,15 @@ import Label from '@woocommerce/base-components/label';
import ProductPrice from '@woocommerce/base-components/product-price';
import ProductName from '@woocommerce/base-components/product-name';
import { getCurrency } from '@woocommerce/price-format';
import { __experimentalApplyCheckoutFilter } from '@woocommerce/blocks-checkout';
import {
__experimentalApplyCheckoutFilter,
mustBeString,
mustContain,
} from '@woocommerce/blocks-checkout';
import PropTypes from 'prop-types';
import Dinero from 'dinero.js';
import { DISPLAY_CART_PRICES_INCLUDING_TAX } from '@woocommerce/block-settings';
import { useCallback, useMemo } from '@wordpress/element';
/**
* Internal dependencies
@ -36,16 +41,27 @@ const OrderSummaryItem = ( { cartItem } ) => {
extensions,
} = cartItem;
const productPriceValidation = useCallback(
( value ) => mustBeString( value ) && mustContain( value, '<price/>' ),
[]
);
const arg = useMemo(
() => ( {
context: 'summary',
cartItem,
} ),
[ cartItem ]
);
const priceCurrency = getCurrency( prices );
const name = __experimentalApplyCheckoutFilter( {
filterName: 'itemName',
defaultValue: initialName,
arg: {
extensions,
context: 'summary',
},
validation: ( value ) => typeof value === 'string',
extensions,
arg,
validation: mustBeString,
} );
const regularPriceSingle = Dinero( {
@ -74,24 +90,18 @@ const OrderSummaryItem = ( { cartItem } ) => {
const subtotalPriceFormat = __experimentalApplyCheckoutFilter( {
filterName: 'subtotalPriceFormat',
defaultValue: '<price/>',
arg: {
lineItem: cartItem,
},
// Only accept strings.
validation: ( value ) =>
typeof value === 'string' && value.includes( '<price/>' ),
extensions,
arg,
validation: productPriceValidation,
} );
// Allow extensions to filter how the price is displayed. Ie: prepending or appending some values.
const productPriceFormat = __experimentalApplyCheckoutFilter( {
filterName: 'cartItemPrice',
defaultValue: '<price/>',
arg: {
cartItem,
block: 'checkout',
},
validation: ( value ) =>
typeof value === 'string' && value.includes( '<price/>' ),
extensions,
arg,
validation: productPriceValidation,
} );
return (

View File

@ -11,6 +11,7 @@ import FormattedMonetaryAmount from '@woocommerce/base-components/formatted-mone
import PropTypes from 'prop-types';
import {
__experimentalApplyCheckoutFilter,
mustBeString,
TotalsItem,
} from '@woocommerce/blocks-checkout';
import { useStoreCart } from '@woocommerce/base-hooks';
@ -28,11 +29,9 @@ const TotalsFooterItem = ( { currency, values } ) => {
const label = __experimentalApplyCheckoutFilter( {
filterName: 'totalLabel',
defaultValue: __( 'Total', 'woo-gutenberg-products-block' ),
arg: {
extensions,
},
extensions,
// Only accept strings.
validation: ( value ) => typeof value === 'string',
validation: mustBeString,
} );
return (

View File

@ -16,9 +16,14 @@ import {
ProductSaleBadge,
} from '@woocommerce/base-components/cart-checkout';
import { getCurrency } from '@woocommerce/price-format';
import { __experimentalApplyCheckoutFilter } from '@woocommerce/blocks-checkout';
import {
__experimentalApplyCheckoutFilter,
mustBeString,
mustContain,
} from '@woocommerce/blocks-checkout';
import Dinero from 'dinero.js';
import { DISPLAY_CART_PRICES_INCLUDING_TAX } from '@woocommerce/block-settings';
import { useCallback, useMemo } from '@wordpress/element';
/**
* @typedef {import('@woocommerce/type-defs/cart').CartItem} CartItem
@ -94,16 +99,24 @@ const CartLineItemRow = ( { lineItem = {} } ) => {
isPendingDelete,
} = useStoreCartItemQuantity( lineItem );
const productPriceValidation = useCallback(
( value ) => mustBeString( value ) && mustContain( value, '<price/>' ),
[]
);
const arg = useMemo(
() => ( {
context: 'cart',
cartItem: lineItem,
} ),
[ lineItem ]
);
const priceCurrency = getCurrency( prices );
const name = __experimentalApplyCheckoutFilter( {
filterName: 'itemName',
defaultValue: initialName,
arg: {
extensions,
context: 'cart',
},
validation: ( value ) => typeof value === 'string',
extensions,
arg,
validation: mustBeString,
} );
const regularAmountSingle = Dinero( {
@ -132,37 +145,29 @@ const CartLineItemRow = ( { lineItem = {} } ) => {
catalogVisibility === 'hidden' || catalogVisibility === 'search';
// Allow extensions to filter how the price is displayed. Ie: prepending or appending some values.
const productPriceFormat = __experimentalApplyCheckoutFilter( {
filterName: 'cartItemPrice',
defaultValue: '<price/>',
arg: {
cartItem: lineItem,
block: 'cart',
},
validation: ( value ) =>
typeof value === 'string' && value.includes( '<price/>' ),
extensions,
arg,
validation: productPriceValidation,
} );
const subtotalPriceFormat = __experimentalApplyCheckoutFilter( {
filterName: 'subtotalPriceFormat',
defaultValue: '<price/>',
arg: {
lineItem,
},
// Only accept strings.
validation: ( value ) =>
typeof value === 'string' && value.includes( '<price/>' ),
extensions,
arg,
validation: productPriceValidation,
} );
const saleBadgePriceFormat = __experimentalApplyCheckoutFilter( {
filterName: 'saleBadgePriceFormat',
defaultValue: '<price/>',
arg: {
lineItem,
},
// Only accept strings.
validation: ( value ) =>
typeof value === 'string' && value.includes( '<price/>' ),
extensions,
arg,
validation: productPriceValidation,
} );
return (

View File

@ -3545,6 +3545,20 @@
"@testing-library/dom": "^7.28.1"
}
},
"@testing-library/react-hooks": {
"version": "5.0.3",
"resolved": "https://registry.npmjs.org/@testing-library/react-hooks/-/react-hooks-5.0.3.tgz",
"integrity": "sha512-UrnnRc5II7LMH14xsYNm/WRch/67cBafmrSQcyFh0v+UUmSf1uzfB7zn5jQXSettGwOSxJwdQUN7PgkT0w22Lg==",
"dev": true,
"requires": {
"@babel/runtime": "^7.12.5",
"@types/react": ">=16.9.0",
"@types/react-dom": ">=16.9.0",
"@types/react-test-renderer": ">=16.9.0",
"filter-console": "^0.1.1",
"react-error-boundary": "^3.1.0"
}
},
"@testing-library/user-event": {
"version": "12.6.3",
"resolved": "https://registry.npmjs.org/@testing-library/user-event/-/user-event-12.6.3.tgz",
@ -3948,6 +3962,15 @@
"@types/react": "*"
}
},
"@types/react-test-renderer": {
"version": "17.0.1",
"resolved": "https://registry.npmjs.org/@types/react-test-renderer/-/react-test-renderer-17.0.1.tgz",
"integrity": "sha512-3Fi2O6Zzq/f3QR9dRnlnHso9bMl7weKCviFmfF6B4LS1Uat6Hkm15k0ZAQuDz+UBq6B3+g+NM6IT2nr5QgPzCw==",
"dev": true,
"requires": {
"@types/react": "*"
}
},
"@types/reactcss": {
"version": "1.2.3",
"resolved": "https://registry.npmjs.org/@types/reactcss/-/reactcss-1.2.3.tgz",
@ -15160,6 +15183,12 @@
}
}
},
"filter-console": {
"version": "0.1.1",
"resolved": "https://registry.npmjs.org/filter-console/-/filter-console-0.1.1.tgz",
"integrity": "sha512-zrXoV1Uaz52DqPs+qEwNJWJFAWZpYJ47UNmpN9q4j+/EYsz85uV0DC9k8tRND5kYmoVzL0W+Y75q4Rg8sRJCdg==",
"dev": true
},
"finalhandler": {
"version": "1.1.2",
"resolved": "https://registry.npmjs.org/finalhandler/-/finalhandler-1.1.2.tgz",
@ -27306,6 +27335,15 @@
}
}
},
"react-error-boundary": {
"version": "3.1.0",
"resolved": "https://registry.npmjs.org/react-error-boundary/-/react-error-boundary-3.1.0.tgz",
"integrity": "sha512-lmPrdi5SLRJR+AeJkqdkGlW/CRkAUvZnETahK58J4xb5wpbfDngasEGu+w0T1iXEhVrYBJZeW+c4V1hILCnMWQ==",
"dev": true,
"requires": {
"@babel/runtime": "^7.12.5"
}
},
"react-error-overlay": {
"version": "6.0.9",
"resolved": "https://registry.npmjs.org/react-error-overlay/-/react-error-overlay-6.0.9.tgz",

View File

@ -83,6 +83,7 @@
"@storybook/react": "6.0.28",
"@testing-library/jest-dom": "5.11.9",
"@testing-library/react": "11.2.5",
"@testing-library/react-hooks": "^5.0.3",
"@testing-library/user-event": "12.6.3",
"@types/jest": "26.0.20",
"@types/react": "16.14.3",
@ -103,7 +104,7 @@
"@wordpress/env": "3.0.0",
"@wordpress/html-entities": "2.8.0",
"@wordpress/i18n": "3.15.0",
"@wordpress/is-shallow-equal": "1.8.0",
"@wordpress/is-shallow-equal": "^1.8.0",
"@wordpress/scripts": "13.0.1",
"autoprefixer": "10.2.3",
"axios": "0.21.1",
@ -139,7 +140,7 @@
"progress-bar-webpack-plugin": "2.1.0",
"promptly": "3.2.0",
"puppeteer": "npm:puppeteer-core@5.5.0",
"react-test-renderer": "17.0.1",
"react-test-renderer": "^17.0.1",
"request-promise": "4.2.6",
"rimraf": "3.0.2",
"sass-loader": "10.1.0",

View File

@ -1,5 +1,6 @@
export * from './totals';
export * from './shipping';
export * from './utils';
export * from './slot';
export * from './registry';
export { default as ExperimentalOrderMeta } from './order-meta';

View File

@ -1,3 +1,14 @@
/**
* External dependencies
*/
import { useMemo } from '@wordpress/element';
import { CURRENT_USER_IS_ADMIN } from '@woocommerce/block-settings';
/**
* Internal dependencies
*/
import { returnTrue } from '../';
let checkoutFilters = {};
/**
@ -32,33 +43,42 @@ const getCheckoutFilters = ( filterName ) => {
/**
* Apply a filter.
*
* @param {Object} o Object of arguments.
* @param {string} o.filterName Name of the filter to apply.
* @param {any} o.defaultValue Default value to filter.
* @param {any} [o.arg] Argument to pass to registered functions. If
* several arguments need to be passed, use an
* object.
* @param {Function} [o.validation] Function that needs to return true when the
* filtered value is passed in order for the
* filter to be applied.
* @param {Object} o Object of arguments.
* @param {string} o.filterName Name of the filter to apply.
* @param {any} o.defaultValue Default value to filter.
* @param {Object} [o.extensions] Values extend to REST API response.
* @param {any} [o.arg] Argument to pass to registered functions.
* If several arguments need to be passed, use
* an object.
* @param {Function} [o.validation] Function that needs to return true when
* the filtered value is passed in order for
* the filter to be applied.
* @return {any} Filtered value.
*/
export const __experimentalApplyCheckoutFilter = ( {
filterName,
defaultValue,
extensions,
arg = null,
validation = () => true,
validation = returnTrue,
} ) => {
const filters = getCheckoutFilters( filterName );
let value = defaultValue;
filters.forEach( ( filter ) => {
try {
const newValue = filter( value, arg );
value = validation( newValue ) ? newValue : value;
} catch ( e ) {
// eslint-disable-next-line no-console
console.log( e );
}
} );
return value;
return useMemo( () => {
const filters = getCheckoutFilters( filterName );
let value = defaultValue;
filters.forEach( ( filter ) => {
try {
const newValue = filter( value, extensions, arg );
value = validation( newValue ) ? newValue : value;
} catch ( e ) {
if ( CURRENT_USER_IS_ADMIN ) {
throw e;
} else {
// eslint-disable-next-line no-console
console.error( e );
}
}
} );
return value;
}, [ filterName, defaultValue, extensions, arg, validation ] );
};

View File

@ -0,0 +1,58 @@
/**
* External dependencies
*/
import { renderHook } from '@testing-library/react-hooks';
/**
* Internal dependencies
*/
import {
__experimentalRegisterCheckoutFilters,
__experimentalApplyCheckoutFilter,
} from '../';
jest.mock( '@woocommerce/block-settings', () => {
const originalModule = jest.requireActual( '@woocommerce/settings' );
return {
// @ts-ignore We know @woocommerce/settings is an object.
...originalModule,
CURRENT_USER_IS_ADMIN: true,
};
} );
describe( 'Checkout registry (as admin user)', () => {
test( 'should throw if the filter throws and user is an admin', () => {
const filterName = 'ErrorTestFilter';
const value = 'Hello World';
__experimentalRegisterCheckoutFilters( filterName, {
[ filterName ]: () => {
throw new Error( 'test error' );
},
} );
const { result } = renderHook( () =>
__experimentalApplyCheckoutFilter( {
filterName,
defaultValue: value,
} )
);
expect( result.error ).toEqual( Error( 'test error' ) );
} );
test( 'should throw if validation throws and user is an admin', () => {
const filterName = 'ValidationTestFilter';
const value = 'Hello World';
__experimentalRegisterCheckoutFilters( filterName, {
[ filterName ]: ( val ) => val,
} );
const { result } = renderHook( () =>
__experimentalApplyCheckoutFilter( {
filterName,
defaultValue: value,
validation: () => {
throw Error( 'validation error' );
},
} )
);
expect( result.error ).toEqual( Error( 'validation error' ) );
} );
} );

View File

@ -1,3 +1,7 @@
/**
* External dependencies
*/
import { renderHook } from '@testing-library/react-hooks';
/**
* Internal dependencies
*/
@ -11,29 +15,32 @@ describe( 'Checkout registry', () => {
test( 'should return default value if there are no filters', () => {
const value = 'Hello World';
const newValue = __experimentalApplyCheckoutFilter( {
filterName,
defaultValue: value,
} );
expect( newValue ).toBe( value );
const { result: newValue } = renderHook( () =>
__experimentalApplyCheckoutFilter( {
filterName,
defaultValue: value,
} )
);
expect( newValue.current ).toBe( value );
} );
test( 'should return filtered value when a filter is registered', () => {
const value = 'Hello World';
__experimentalRegisterCheckoutFilters( filterName, {
[ filterName ]: ( val, args ) =>
[ filterName ]: ( val, extensions, args ) =>
val.toUpperCase() + args.punctuationSign,
} );
const newValue = __experimentalApplyCheckoutFilter( {
filterName,
defaultValue: value,
arg: {
punctuationSign: '!',
},
} );
const { result: newValue } = renderHook( () =>
__experimentalApplyCheckoutFilter( {
filterName,
defaultValue: value,
arg: {
punctuationSign: '!',
},
} )
);
expect( newValue ).toBe( 'HELLO WORLD!' );
expect( newValue.current ).toBe( 'HELLO WORLD!' );
} );
test( 'should not return filtered value if validation failed', () => {
@ -41,12 +48,39 @@ describe( 'Checkout registry', () => {
__experimentalRegisterCheckoutFilters( filterName, {
[ filterName ]: ( val ) => val.toUpperCase(),
} );
const newValue = __experimentalApplyCheckoutFilter( {
filterName,
defaultValue: value,
validation: ( val ) => ! val.includes( 'HELLO' ),
} );
const { result: newValue } = renderHook( () =>
__experimentalApplyCheckoutFilter( {
filterName,
defaultValue: value,
validation: ( val ) => ! val.includes( 'HELLO' ),
} )
);
expect( newValue ).toBe( value );
expect( newValue.current ).toBe( value );
} );
test( 'should catch filter errors if user is not an admin', () => {
const spy = {};
spy.console = jest
.spyOn( console, 'error' )
.mockImplementation( () => {} );
const error = new Error( 'test error' );
const value = 'Hello World';
__experimentalRegisterCheckoutFilters( filterName, {
[ filterName ]: () => {
throw error;
},
} );
const { result: newValue } = renderHook( () =>
__experimentalApplyCheckoutFilter( {
filterName,
defaultValue: value,
} )
);
expect( spy.console ).toHaveBeenCalledWith( error );
expect( newValue.current ).toBe( value );
spy.console.mockRestore();
} );
} );

View File

@ -0,0 +1 @@
export * from './validation';

View File

@ -0,0 +1,62 @@
/**
* External dependencies
*/
import { __, sprintf } from '@wordpress/i18n';
/**
* Checks if value passed is a string, throws an error if not.
*
* @param {string} value Value to be validated.
*
* @return {Error|true} Error if value is not string, true otherwise.
*/
export const mustBeString = ( value ) => {
if ( typeof value !== 'string' ) {
throw Error(
sprintf(
// translators: %s is type of value passed
__(
'Returned value must be a string, you passed "%s"',
'woo-gutenberg-products-block'
),
typeof value
)
);
}
return true;
};
/**
* Checks if value passed contain passed label
*
* @param {string} value Value to be validated.
* @param {string} label Label to be searched for.
*
* @return {Error|true} Error if value contains label, true otherwise.
*/
export const mustContain = ( value, label ) => {
if ( ! value.includes( label ) ) {
throw Error(
sprintf(
// translators: %1$s value passed to filter, %2$s : value that must be included.
__(
'Returned value must include %1$s, you passed "%2$s"',
'woo-gutenberg-products-block'
),
value,
label
)
);
}
return true;
};
/**
* A function that always return true.
* We need to have a single instance of this function so it doesn't
* invalidate our memo comparison.
*
*
* @return {true} Returns true.
*/
export const returnTrue = () => true;

View File

@ -11,6 +11,7 @@ global.wcSettings = {
precision: 2,
symbol: '&#36;',
},
currentUserIsAdmin: false,
date: {
dow: 0,
},