From feec5709c5c0eed84e132e76b6c2c99369343528 Mon Sep 17 00:00:00 2001 From: Sam Seay Date: Fri, 8 Nov 2024 17:55:09 +0800 Subject: [PATCH] Revert "Ensure local state is not overwritten by server when using `extensionCartUpdate` (#49311)" This reverts commit 4861ec250ef1789f814f4209755165e8abe7b838. --- .../cart-checkout/checkout-processor.ts | 3 - .../assets/js/data/cart/push-changes.ts | 30 ++++-- .../assets/js/data/cart/reducers.ts | 23 ----- .../assets/js/data/cart/test/push-changes.ts | 38 +------ .../assets/js/data/cart/thunks.ts | 19 +--- .../assets/js/data/cart/utils.ts | 26 +---- .../assets/js/types/type-defs/cart.ts | 1 - .../extensibility/data-store/cart.md | 2 - .../rest-api/extend-rest-api-update-cart.md | 9 +- .../e2e/plugins/extension-cart-update.php | 39 -------- ...-extensibility.shopper.block_theme.spec.ts | 99 ------------------- .../changelog/try-ecu-ignore-empty | 4 - 12 files changed, 31 insertions(+), 262 deletions(-) delete mode 100644 plugins/woocommerce-blocks/tests/e2e/plugins/extension-cart-update.php delete mode 100644 plugins/woocommerce-blocks/tests/e2e/tests/checkout/checkout-block-extensibility.shopper.block_theme.spec.ts delete mode 100644 plugins/woocommerce/changelog/try-ecu-ignore-empty diff --git a/plugins/woocommerce-blocks/assets/js/base/context/providers/cart-checkout/checkout-processor.ts b/plugins/woocommerce-blocks/assets/js/base/context/providers/cart-checkout/checkout-processor.ts index bc6b01e3df5..60035cb4002 100644 --- a/plugins/woocommerce-blocks/assets/js/base/context/providers/cart-checkout/checkout-processor.ts +++ b/plugins/woocommerce-blocks/assets/js/base/context/providers/cart-checkout/checkout-processor.ts @@ -225,9 +225,6 @@ const CheckoutProcessor = () => { // Redirect when checkout is complete and there is a redirect url. useEffect( () => { - window.localStorage.removeItem( - 'WOOCOMMERCE_CHECKOUT_IS_CUSTOMER_DATA_DIRTY' - ); if ( currentRedirectUrl.current ) { window.location.href = currentRedirectUrl.current; } diff --git a/plugins/woocommerce-blocks/assets/js/data/cart/push-changes.ts b/plugins/woocommerce-blocks/assets/js/data/cart/push-changes.ts index 5572af25e85..acd2b125ea0 100644 --- a/plugins/woocommerce-blocks/assets/js/data/cart/push-changes.ts +++ b/plugins/woocommerce-blocks/assets/js/data/cart/push-changes.ts @@ -1,8 +1,12 @@ /** * External dependencies */ -import { debounce } from '@woocommerce/base-utils'; -import { CartBillingAddress, CartShippingAddress } from '@woocommerce/types'; +import { debounce, pick } from '@woocommerce/base-utils'; +import { + CartBillingAddress, + CartShippingAddress, + BillingAddressShippingAddress, +} from '@woocommerce/types'; import { select, dispatch } from '@wordpress/data'; import isShallowEqual from '@wordpress/is-shallow-equal'; @@ -130,11 +134,25 @@ const updateCustomerData = (): void => { return; } + // Find valid data from the list of dirtyProps and prepare to push to the server. + const customerDataToUpdate = {} as Partial< BillingAddressShippingAddress >; + + if ( localState.dirtyProps.billingAddress.length ) { + customerDataToUpdate.billing_address = pick( + localState.customerData.billingAddress, + localState.dirtyProps.billingAddress + ); + } + + if ( localState.dirtyProps.shippingAddress.length ) { + customerDataToUpdate.shipping_address = pick( + localState.customerData.shippingAddress, + localState.dirtyProps.shippingAddress + ); + } + dispatch( STORE_KEY ) - .updateCustomerData( { - billing_address: localState.customerData.billingAddress, - shipping_address: localState.customerData.shippingAddress, - } ) + .updateCustomerData( customerDataToUpdate ) .then( () => { localState.dirtyProps.billingAddress = []; localState.dirtyProps.shippingAddress = []; diff --git a/plugins/woocommerce-blocks/assets/js/data/cart/reducers.ts b/plugins/woocommerce-blocks/assets/js/data/cart/reducers.ts index c494f7d2a71..5617c13931d 100644 --- a/plugins/woocommerce-blocks/assets/js/data/cart/reducers.ts +++ b/plugins/woocommerce-blocks/assets/js/data/cart/reducers.ts @@ -9,7 +9,6 @@ import type { Reducer } from 'redux'; import { ACTION_TYPES as types } from './action-types'; import { defaultCartState, CartState } from './default-state'; import { EMPTY_CART_ERRORS } from '../constants'; -import { setIsCustomerDataDirty } from './utils'; /** * Reducer for receiving items related to the cart. @@ -48,14 +47,6 @@ const reducer: Reducer< CartState > = ( state = defaultCartState, action ) => { } break; case types.SET_BILLING_ADDRESS: - const billingAddressChanged = Object.keys( - action.billingAddress - ).some( ( key ) => { - return ( - action.billingAddress[ key ] !== - state.cartData.billingAddress?.[ key ] - ); - } ); state = { ...state, cartData: { @@ -66,19 +57,8 @@ const reducer: Reducer< CartState > = ( state = defaultCartState, action ) => { }, }, }; - if ( billingAddressChanged ) { - setIsCustomerDataDirty( true ); - } break; case types.SET_SHIPPING_ADDRESS: - const shippingAddressChanged = Object.keys( - action.shippingAddress - ).some( ( key ) => { - return ( - action.shippingAddress[ key ] !== - state.cartData.shippingAddress?.[ key ] - ); - } ); state = { ...state, cartData: { @@ -89,9 +69,6 @@ const reducer: Reducer< CartState > = ( state = defaultCartState, action ) => { }, }, }; - if ( shippingAddressChanged ) { - setIsCustomerDataDirty( true ); - } break; case types.REMOVING_COUPON: diff --git a/plugins/woocommerce-blocks/assets/js/data/cart/test/push-changes.ts b/plugins/woocommerce-blocks/assets/js/data/cart/test/push-changes.ts index ad41091c940..c677ed6a6ce 100644 --- a/plugins/woocommerce-blocks/assets/js/data/cart/test/push-changes.ts +++ b/plugins/woocommerce-blocks/assets/js/data/cart/test/push-changes.ts @@ -130,30 +130,12 @@ describe( 'pushChanges', () => { // Push these changes to the server, the `updateCustomerData` mock is set to reject (in the original mock at the top of the file), to simulate a server error. pushChanges( false ); - // Check that the mock was called with full address data. + // Check that the mock was called with only the updated data. await expect( updateCustomerDataMock ).toHaveBeenCalledWith( { - billing_address: { - first_name: 'John', - last_name: 'Doe', - address_1: '123 Main St', - address_2: '', - city: 'New York', - state: 'NY', - postcode: '10001', - country: 'US', - email: 'john.doe@mail.com', - phone: '555-555-5555', - }, shipping_address: { - first_name: 'John', - last_name: 'Doe', - address_1: '123 Main St', - address_2: '', city: 'Houston', state: 'TX', postcode: 'ABCDEF', - country: 'US', - phone: '555-555-5555', }, } ); @@ -195,28 +177,10 @@ describe( 'pushChanges', () => { // to the server because the previous push failed when they were originally changed. pushChanges( false ); await expect( updateCustomerDataMock ).toHaveBeenLastCalledWith( { - billing_address: { - first_name: 'John', - last_name: 'Doe', - address_1: '123 Main St', - address_2: '', - city: 'New York', - state: 'NY', - postcode: '10001', - country: 'US', - email: 'john.doe@mail.com', - phone: '555-555-5555', - }, shipping_address: { - first_name: 'John', - last_name: 'Doe', - address_1: '123 Main St', - address_2: '', city: 'Houston', state: 'TX', postcode: '77058', - country: 'US', - phone: '555-555-5555', }, } ); } ); diff --git a/plugins/woocommerce-blocks/assets/js/data/cart/thunks.ts b/plugins/woocommerce-blocks/assets/js/data/cart/thunks.ts index d9248bea69c..51da2292fbe 100644 --- a/plugins/woocommerce-blocks/assets/js/data/cart/thunks.ts +++ b/plugins/woocommerce-blocks/assets/js/data/cart/thunks.ts @@ -24,7 +24,6 @@ import { notifyQuantityChanges } from './notify-quantity-changes'; import { notifyCartErrors } from './notify-errors'; import { CartDispatchFromMap, CartSelectFromMap } from './index'; import { apiFetchWithHeaders } from '../shared-controls'; -import { getIsCustomerDataDirty, setIsCustomerDataDirty } from './utils'; /** * A thunk used in updating the store with the cart items retrieved from a request. This also notifies the shopper @@ -100,22 +99,8 @@ export const applyExtensionCartUpdate = data: { namespace: args.namespace, data: args.data }, cache: 'no-store', } ); - if ( args.overwriteDirtyCustomerData === true ) { - dispatch.receiveCart( response ); - return response; - } - if ( getIsCustomerDataDirty() ) { - // If the customer data is dirty, we don't want to overwrite it with the response. - // Remove shipping and billing address from the response and then receive the cart. - const { - shipping_address: _, - billing_address: __, - ...responseWithoutShippingOrBilling - } = response; - dispatch.receiveCart( responseWithoutShippingOrBilling ); - return response; - } dispatch.receiveCart( response ); + return response; } catch ( error ) { dispatch.receiveError( isApiErrorResponse( error ) ? error : null ); return Promise.reject( error ); @@ -405,11 +390,9 @@ export const updateCustomerData = } else { dispatch.receiveCart( response ); } - setIsCustomerDataDirty( false ); return response; } catch ( error ) { dispatch.receiveError( isApiErrorResponse( error ) ? error : null ); - setIsCustomerDataDirty( true ); return Promise.reject( error ); } finally { dispatch.updatingCustomerData( false ); diff --git a/plugins/woocommerce-blocks/assets/js/data/cart/utils.ts b/plugins/woocommerce-blocks/assets/js/data/cart/utils.ts index 6aa4829465e..40d0aef5096 100644 --- a/plugins/woocommerce-blocks/assets/js/data/cart/utils.ts +++ b/plugins/woocommerce-blocks/assets/js/data/cart/utils.ts @@ -2,7 +2,7 @@ * External dependencies */ import { select } from '@wordpress/data'; -import { camelCaseKeys, debounce } from '@woocommerce/base-utils'; +import { camelCaseKeys } from '@woocommerce/base-utils'; import { isEmail } from '@wordpress/url'; import { CartBillingAddress, @@ -116,27 +116,3 @@ export const validateDirtyProps = ( dirtyProps: { return invalidProps.length === 0; }; - -/** - * Gets the localStorage flag to indicate whether the customer data is dirty. - */ -export const getIsCustomerDataDirty = () => { - return ( - window.localStorage.getItem( - 'WOOCOMMERCE_CHECKOUT_IS_CUSTOMER_DATA_DIRTY' - ) === 'true' - ); -}; - -/** - * Sets a flag in localStorage to indicate whether the customer data has been modified. - */ -export const setIsCustomerDataDirty = debounce( - ( isCustomerDataDirty: boolean ) => { - window.localStorage.setItem( - 'WOOCOMMERCE_CHECKOUT_IS_CUSTOMER_DATA_DIRTY', - isCustomerDataDirty ? 'true' : 'false' - ); - }, - 300 -); diff --git a/plugins/woocommerce-blocks/assets/js/types/type-defs/cart.ts b/plugins/woocommerce-blocks/assets/js/types/type-defs/cart.ts index bf856ece2ed..ddc070a2f75 100644 --- a/plugins/woocommerce-blocks/assets/js/types/type-defs/cart.ts +++ b/plugins/woocommerce-blocks/assets/js/types/type-defs/cart.ts @@ -211,7 +211,6 @@ export interface CartMeta { export interface ExtensionCartUpdateArgs { data: Record< string, unknown >; namespace: string; - overwriteDirtyCustomerData?: undefined | boolean; } export interface BillingAddressShippingAddress { diff --git a/plugins/woocommerce-blocks/docs/third-party-developers/extensibility/data-store/cart.md b/plugins/woocommerce-blocks/docs/third-party-developers/extensibility/data-store/cart.md index 3e0e192be6a..30fee9643aa 100644 --- a/plugins/woocommerce-blocks/docs/third-party-developers/extensibility/data-store/cart.md +++ b/plugins/woocommerce-blocks/docs/third-party-developers/extensibility/data-store/cart.md @@ -1,4 +1,3 @@ - # Cart Store (`wc/store/cart`) > 💡 What's the difference between the Cart Store and the Checkout Store? @@ -373,7 +372,6 @@ This action is used to send POSTs request to the /cart/extensions endpoint with - _data_ `object`: The data to send to the endpoint with the following keys: - _key_ `string`: The key of the extension. - _value_ `string`: The value of the extension. - - _overwriteDirtyCustomerData_ `boolean`: Whether to overwrite the customer data in the client with the data returned from the server, even if it is dirty (i.e. it hasn't been pushed to the server yet). #### _Example_ diff --git a/plugins/woocommerce-blocks/docs/third-party-developers/extensibility/rest-api/extend-rest-api-update-cart.md b/plugins/woocommerce-blocks/docs/third-party-developers/extensibility/rest-api/extend-rest-api-update-cart.md index c84e444f443..79988c3a714 100644 --- a/plugins/woocommerce-blocks/docs/third-party-developers/extensibility/rest-api/extend-rest-api-update-cart.md +++ b/plugins/woocommerce-blocks/docs/third-party-developers/extensibility/rest-api/extend-rest-api-update-cart.md @@ -123,11 +123,10 @@ If you try to register again, under the same namespace, the previously registere `extensionCartUpdate`: Used to signal that you want your registered callback to be executed, and to pass data to the callback. It takes an object as its only argument. -| Attribute | Type | Required | Description | -| ----------- |-----------|----------| ---------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `namespace` | `string` | Yes | The namespace of your extension. This is used to determine which extension's callbacks should be executed. | -| `data` | `Object` | No | The data you want to pass to your callback. Anything in the `data` key will be passed as the first (and only) argument to your callback as an associative array. | -| `overwriteDirtyCustomerData` | `boolean` | No | Whether to overwrite the customer data in the client with the data returned from the server, even if it is dirty (i.e. it hasn't been pushed to the server yet). | +| Attribute | Type | Required | Description | +| ----------- | -------- | -------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `namespace` | `string` | Yes | The namespace of your extension. This is used to determine which extension's callbacks should be executed. | +| `data` | `Object` | No | The data you want to pass to your callback. Anything in the `data` key will be passed as the first (and only) argument to your callback as an associative array. | ## Putting it all together diff --git a/plugins/woocommerce-blocks/tests/e2e/plugins/extension-cart-update.php b/plugins/woocommerce-blocks/tests/e2e/plugins/extension-cart-update.php deleted file mode 100644 index c33e8b3f90e..00000000000 --- a/plugins/woocommerce-blocks/tests/e2e/plugins/extension-cart-update.php +++ /dev/null @@ -1,39 +0,0 @@ -get( ExtendSchema::class ); - if ( - is_callable( - array( - $extend, - 'register_update_callback', - ) - ) - ) { - $extend->register_update_callback( - array( - 'namespace' => 'woocommerce-blocks-test-extension-cart-update', - 'callback' => function ( $data ) { - if ( ! empty( $data['test-name-change'] ) ) { - WC()->cart->get_customer()->set_shipping_first_name( 'Mr. Test' ); - WC()->cart->get_customer()->save(); - } - }, - ) - ); - } - } -); diff --git a/plugins/woocommerce-blocks/tests/e2e/tests/checkout/checkout-block-extensibility.shopper.block_theme.spec.ts b/plugins/woocommerce-blocks/tests/e2e/tests/checkout/checkout-block-extensibility.shopper.block_theme.spec.ts deleted file mode 100644 index 2f32833452b..00000000000 --- a/plugins/woocommerce-blocks/tests/e2e/tests/checkout/checkout-block-extensibility.shopper.block_theme.spec.ts +++ /dev/null @@ -1,99 +0,0 @@ -/** - * External dependencies - */ -import { expect, test as base, guestFile } from '@woocommerce/e2e-utils'; - -/** - * Internal dependencies - */ -import { REGULAR_PRICED_PRODUCT_NAME } from './constants'; -import { CheckoutPage } from './checkout.page'; - -const test = base.extend< { checkoutPageObject: CheckoutPage } >( { - checkoutPageObject: async ( { page }, use ) => { - const pageObject = new CheckoutPage( { - page, - } ); - await use( pageObject ); - }, -} ); - -test.describe( 'Shopper → Extensibility', () => { - test.use( { storageState: guestFile } ); - - test.beforeEach( async ( { requestUtils, frontendUtils } ) => { - await requestUtils.rest( { - method: 'PUT', - path: 'wc/v3/settings/account/woocommerce_enable_guest_checkout', - data: { value: 'yes' }, - } ); - await requestUtils.rest( { - method: 'PUT', - path: 'wc/v3/settings/account/woocommerce_enable_checkout_login_reminder', - data: { value: 'yes' }, - } ); - await requestUtils.activatePlugin( - 'woocommerce-blocks-test-extensioncartupdate' - ); - - await frontendUtils.goToShop(); - await frontendUtils.addToCart( REGULAR_PRICED_PRODUCT_NAME ); - await frontendUtils.goToCheckout(); - } ); - test.describe( 'extensionCartUpdate', () => { - test( 'Unpushed data is/is not overwritten depending on arg', async ( { - checkoutPageObject, - } ) => { - // First test by only partially filling in the address form. - await checkoutPageObject.page - .getByLabel( 'Country/Region' ) - .selectOption( 'United Kingdom (UK)' ); - await checkoutPageObject.page.getByLabel( 'Country/Region' ).blur(); - - await checkoutPageObject.page.evaluate( - "wc.blocksCheckout.extensionCartUpdate( { namespace: 'woocommerce-blocks-test-extension-cart-update' } )" - ); - await expect( - checkoutPageObject.page.getByLabel( 'Country/Region' ) - ).toHaveValue( 'GB' ); - await checkoutPageObject.page.evaluate( - "wc.blocksCheckout.extensionCartUpdate( { namespace: 'woocommerce-blocks-test-extension-cart-update', overwriteDirtyCustomerData: true } )" - ); - await expect( - checkoutPageObject.page.getByLabel( 'Country/Region' ) - ).not.toHaveValue( 'GB' ); - - // Next fully test the address form (so it pushes), then run extensionCartUpdate with - // overwriteDirtyCustomerData: true so overwriting is possible, but since the address pushed it should not - // be overwritten. - await checkoutPageObject.fillInCheckoutWithTestData(); - await expect( - checkoutPageObject.page.getByLabel( 'Country/Region' ) - ).toHaveValue( 'US' ); - await checkoutPageObject.page.evaluate( - "wc.blocksCheckout.extensionCartUpdate( { namespace: 'woocommerce-blocks-test-extension-cart-update', overwriteDirtyCustomerData: true } )" - ); - await expect( - checkoutPageObject.page.getByLabel( 'Country/Region' ) - ).toHaveValue( 'US' ); - } ); - test( 'Cart data can be modified by extensions', async ( { - checkoutPageObject, - } ) => { - await checkoutPageObject.fillInCheckoutWithTestData(); - await checkoutPageObject.page.waitForFunction( () => { - return ( - window.localStorage.getItem( - 'WOOCOMMERCE_CHECKOUT_IS_CUSTOMER_DATA_DIRTY' - ) === 'false' - ); - } ); - await checkoutPageObject.page.evaluate( - "wc.blocksCheckout.extensionCartUpdate( { namespace: 'woocommerce-blocks-test-extension-cart-update', data: { 'test-name-change': true } } )" - ); - await expect( - checkoutPageObject.page.getByLabel( 'First name' ) - ).toHaveValue( 'Mr. Test' ); - } ); - } ); -} ); diff --git a/plugins/woocommerce/changelog/try-ecu-ignore-empty b/plugins/woocommerce/changelog/try-ecu-ignore-empty deleted file mode 100644 index 88460359dff..00000000000 --- a/plugins/woocommerce/changelog/try-ecu-ignore-empty +++ /dev/null @@ -1,4 +0,0 @@ -Significance: patch -Type: fix - -Ensure local state is not overwritten by server when using `extensionCartUpdate`