Skip to content
This repository has been archived by the owner on Feb 23, 2024. It is now read-only.

Replace filters with an extendibility API to hook into Cart and Checkout blocks #3774

Merged
merged 16 commits into from
Feb 9, 2021
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ import { createInterpolateElement } from 'wordpress-element';
import FormattedMonetaryAmount from '@woocommerce/base-components/formatted-monetary-amount';
import PropTypes from 'prop-types';
import {
__EXPERIMENTAL_TOTAL_LABEL_FILTER,
__experimentalApplyCheckoutFilter,
TotalsItem,
} from '@woocommerce/blocks-checkout';
import { applyFilters } from '@wordpress/hooks';
import { useStoreCart } from '@woocommerce/base-hooks';

/**
* Internal dependencies
Expand All @@ -24,10 +24,16 @@ const SHOW_TAXES = TAXES_ENABLED && DISPLAY_CART_PRICES_INCLUDING_TAX;

const TotalsFooterItem = ( { currency, values } ) => {
const { total_price: totalPrice, total_tax: totalTax } = values;
const label = applyFilters(
__EXPERIMENTAL_TOTAL_LABEL_FILTER,
__( 'Total', 'woo-gutenberg-products-block' )
);
const { extensions } = useStoreCart();
const label = __experimentalApplyCheckoutFilter( {
filterName: 'totalLabel',
defaultValue: __( 'Total', 'woo-gutenberg-products-block' ),
arg: {
extensions,
},
// Only accept strings.
validation: ( value ) => typeof value === 'string',
} );

return (
<TotalsItem
Expand Down
2 changes: 1 addition & 1 deletion docs/blocks/feature-flags-and-experimental-interfaces.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,5 +57,5 @@ We also have individual features or code blocks behind a feature flag, this is a
- `__experimental_woocommerce_blocks_checkout_order_processed` hook when order has completed processing and is ready for payment ([experimental hook](https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/accd1bbf402e043b9fc322f118ab614ba7437c92/src/StoreApi/Routes/Checkout.php#L237)).
- `__experimentalDeRegisterPaymentMethod` function used to deregister a payment method, only used in tests ([experimental function](https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/b07883b8b76feeb439d655b255507b24fc59e091/assets/js/blocks-registry/payment-methods/registry.js#L70)).
- `__experimentalDeRegisterExpressPaymentMethod` function used to deregister an express payment method, only used in tests ([experimental function](https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/b07883b8b76feeb439d655b255507b24fc59e091/assets/js/blocks-registry/payment-methods/registry.js#L74)).
- `__EXPERIMENTAL_TOTAL_LABEL_FILTER` constant which maps to the `wcBlocks.__experimental_total_label_filter` hook name. Used to filter the _Total_ label in the Cart and Checkout blocks ([experimental constant](https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/9c4288b0ee46960bdc2bf8ef351d05ac23073b0c/packages/checkout/index.js#L8-L9)).
- `__experimentalRegisterCheckoutFilters` and `__experimentalApplyCheckoutFilter` methods included with `@woocommerce/blocks-checkout` package. They allow registering and applying a filter to certain parts of the Cart and Checkout blocks ([experimental method 1](https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/3e59ec9842464f783f6e087947e717fa0b0a7b1b/packages/checkout/registry/index.js#L2) | [experimental method 2](https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/3e59ec9842464f783f6e087947e717fa0b0a7b1b/packages/checkout/registry/index.js#L17)).
- `__experimental_woocommerce_blocks_hidden` property in a Cart item data array that allows overwriting the `hidden` property. This is useful to make some cart item data visible/hidden depending if it needs to be displayed in Blocks or shortcode ([experimental property](https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/9c4288b0ee46960bdc2bf8ef351d05ac23073b0c/src/StoreApi/Schemas/CartItemSchema.php#L439-L441)).
4 changes: 1 addition & 3 deletions packages/checkout/index.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
export * from './totals';
export * from './shipping';
export * from './slot';
export * from './registry';
export { default as ExperimentalOrderMeta } from './order-meta';
export { default as ExperimentalOrderShippingPackages } from './order-shipping-packages';
export { default as Panel } from './panel';
export { SlotFillProvider } from 'wordpress-components';

export const __EXPERIMENTAL_TOTAL_LABEL_FILTER =
'wcBlocks.__experimental_total_label_filter';
64 changes: 64 additions & 0 deletions packages/checkout/registry/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
let checkoutFilters = {};

/**
* Register filters for a specific extension.
*
* @param {string} namespace Name of the extension namespace.
* @param {Object} filters Object of filters for that namespace. Each key of
* the object is the name of a filter.
*/
export const __experimentalRegisterCheckoutFilters = ( namespace, filters ) => {
checkoutFilters = {
...checkoutFilters,
[ namespace ]: filters,
};
};

/**
* Get all filters with a specific name.
*
* @param {string} filterName Name of the filter to search for.
* @return {Function[]} Array of functions that are registered for that filter
* name.
*/
const getCheckoutFilters = ( filterName ) => {
const namespaces = Object.keys( checkoutFilters );
const filters = namespaces
.map( ( namespace ) => checkoutFilters[ namespace ][ filterName ] )
.filter( Boolean );
return filters;
};

/**
* 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.
* @return {any} Filtered value.
*/
export const __experimentalApplyCheckoutFilter = ( {
filterName,
defaultValue,
senadir marked this conversation as resolved.
Show resolved Hide resolved
arg = null,
validation = () => true,
} ) => {
const filters = getCheckoutFilters( filterName );
let value = defaultValue;
senadir marked this conversation as resolved.
Show resolved Hide resolved
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 );
Comment on lines +59 to +60
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logging errors is not very practical, it wouldn't trigger an error on the console (easily missed) and would happen to all users or admins.
I will write a P2 to suggest how we can catch and expose errors, for now, how about you check if the current use is an admin, fi so, you throw, or else you neglect the error and move on.

Suggested change
// eslint-disable-next-line no-console
console.log( e );
if ( CURRENT_USER_IS_ADMIN ) {
throw e;
}

Copy link
Contributor Author

@Aljullu Aljullu Feb 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will probably need to revisit this after the P2 discussion, but for now I made the change you suggested (throw when the user is an admin) but still kept the console.error for costumers: 1712766. Let me know how it looks.

}
} );
return value;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works in this case because we have one filter: totalLabel.
But if there were more filters the value returned would be of the last filter applied. Is this the expected result, or should the filters have some sort of priority?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, that's an accepted tradeoff, we don't plan on introducing priority for the time.

};
52 changes: 52 additions & 0 deletions packages/checkout/registry/test/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/**
* Internal dependencies
*/
import {
__experimentalRegisterCheckoutFilters,
__experimentalApplyCheckoutFilter,
} from '../';

describe( 'Checkout registry', () => {
const filterName = 'loremIpsum';

test( 'should return default value if there are no filters', () => {
const value = 'Hello World';
const newValue = __experimentalApplyCheckoutFilter( {
filterName,
defaultValue: value,
} );

expect( newValue ).toBe( value );
} );
Comment on lines +12 to +20
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another test we can add (we need to mock CURRENT_USER_IS_ADMIN) is passing in invalid code (that throws an exception) and making sure previous filter still passed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a bad time mocking CURRENT_USER_IS_ADMIN so it had different values in two different tests. I ended up splitting them in two files: 1d5b636. There might better ways of doing this (let me know if you find how!), but couldn't figure it out how after spending some time on this.


test( 'should return filtered value when a filter is registered', () => {
const value = 'Hello World';
__experimentalRegisterCheckoutFilters( filterName, {
[ filterName ]: ( val, args ) =>
val.toUpperCase() + args.punctuationSign,
} );
const newValue = __experimentalApplyCheckoutFilter( {
filterName,
defaultValue: value,
arg: {
punctuationSign: '!',
},
} );

expect( newValue ).toBe( 'HELLO WORLD!' );
} );

test( 'should not return filtered value if validation failed', () => {
const value = 'Hello World';
__experimentalRegisterCheckoutFilters( filterName, {
[ filterName ]: ( val ) => val.toUpperCase(),
} );
const newValue = __experimentalApplyCheckoutFilter( {
filterName,
defaultValue: value,
validation: ( val ) => ! val.includes( 'HELLO' ),
} );

expect( newValue ).toBe( value );
} );
} );