Revert "Ensure local state is not overwritten by server when using `extensionCartUpdate` (#49311)"

This reverts commit 4861ec250e.
This commit is contained in:
Sam Seay 2024-11-08 17:55:09 +08:00
parent 2c7fa7dad5
commit feec5709c5
No known key found for this signature in database
GPG Key ID: 2223711A9151668A
12 changed files with 31 additions and 262 deletions

View File

@ -225,9 +225,6 @@ const CheckoutProcessor = () => {
// Redirect when checkout is complete and there is a redirect url. // Redirect when checkout is complete and there is a redirect url.
useEffect( () => { useEffect( () => {
window.localStorage.removeItem(
'WOOCOMMERCE_CHECKOUT_IS_CUSTOMER_DATA_DIRTY'
);
if ( currentRedirectUrl.current ) { if ( currentRedirectUrl.current ) {
window.location.href = currentRedirectUrl.current; window.location.href = currentRedirectUrl.current;
} }

View File

@ -1,8 +1,12 @@
/** /**
* External dependencies * External dependencies
*/ */
import { debounce } from '@woocommerce/base-utils'; import { debounce, pick } from '@woocommerce/base-utils';
import { CartBillingAddress, CartShippingAddress } from '@woocommerce/types'; import {
CartBillingAddress,
CartShippingAddress,
BillingAddressShippingAddress,
} from '@woocommerce/types';
import { select, dispatch } from '@wordpress/data'; import { select, dispatch } from '@wordpress/data';
import isShallowEqual from '@wordpress/is-shallow-equal'; import isShallowEqual from '@wordpress/is-shallow-equal';
@ -130,11 +134,25 @@ const updateCustomerData = (): void => {
return; 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 ) dispatch( STORE_KEY )
.updateCustomerData( { .updateCustomerData( customerDataToUpdate )
billing_address: localState.customerData.billingAddress,
shipping_address: localState.customerData.shippingAddress,
} )
.then( () => { .then( () => {
localState.dirtyProps.billingAddress = []; localState.dirtyProps.billingAddress = [];
localState.dirtyProps.shippingAddress = []; localState.dirtyProps.shippingAddress = [];

View File

@ -9,7 +9,6 @@ import type { Reducer } from 'redux';
import { ACTION_TYPES as types } from './action-types'; import { ACTION_TYPES as types } from './action-types';
import { defaultCartState, CartState } from './default-state'; import { defaultCartState, CartState } from './default-state';
import { EMPTY_CART_ERRORS } from '../constants'; import { EMPTY_CART_ERRORS } from '../constants';
import { setIsCustomerDataDirty } from './utils';
/** /**
* Reducer for receiving items related to the cart. * Reducer for receiving items related to the cart.
@ -48,14 +47,6 @@ const reducer: Reducer< CartState > = ( state = defaultCartState, action ) => {
} }
break; break;
case types.SET_BILLING_ADDRESS: case types.SET_BILLING_ADDRESS:
const billingAddressChanged = Object.keys(
action.billingAddress
).some( ( key ) => {
return (
action.billingAddress[ key ] !==
state.cartData.billingAddress?.[ key ]
);
} );
state = { state = {
...state, ...state,
cartData: { cartData: {
@ -66,19 +57,8 @@ const reducer: Reducer< CartState > = ( state = defaultCartState, action ) => {
}, },
}, },
}; };
if ( billingAddressChanged ) {
setIsCustomerDataDirty( true );
}
break; break;
case types.SET_SHIPPING_ADDRESS: case types.SET_SHIPPING_ADDRESS:
const shippingAddressChanged = Object.keys(
action.shippingAddress
).some( ( key ) => {
return (
action.shippingAddress[ key ] !==
state.cartData.shippingAddress?.[ key ]
);
} );
state = { state = {
...state, ...state,
cartData: { cartData: {
@ -89,9 +69,6 @@ const reducer: Reducer< CartState > = ( state = defaultCartState, action ) => {
}, },
}, },
}; };
if ( shippingAddressChanged ) {
setIsCustomerDataDirty( true );
}
break; break;
case types.REMOVING_COUPON: case types.REMOVING_COUPON:

View File

@ -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. // 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 ); 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( { 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: { shipping_address: {
first_name: 'John',
last_name: 'Doe',
address_1: '123 Main St',
address_2: '',
city: 'Houston', city: 'Houston',
state: 'TX', state: 'TX',
postcode: 'ABCDEF', 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. // to the server because the previous push failed when they were originally changed.
pushChanges( false ); pushChanges( false );
await expect( updateCustomerDataMock ).toHaveBeenLastCalledWith( { 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: { shipping_address: {
first_name: 'John',
last_name: 'Doe',
address_1: '123 Main St',
address_2: '',
city: 'Houston', city: 'Houston',
state: 'TX', state: 'TX',
postcode: '77058', postcode: '77058',
country: 'US',
phone: '555-555-5555',
}, },
} ); } );
} ); } );

View File

@ -24,7 +24,6 @@ import { notifyQuantityChanges } from './notify-quantity-changes';
import { notifyCartErrors } from './notify-errors'; import { notifyCartErrors } from './notify-errors';
import { CartDispatchFromMap, CartSelectFromMap } from './index'; import { CartDispatchFromMap, CartSelectFromMap } from './index';
import { apiFetchWithHeaders } from '../shared-controls'; 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 * 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 }, data: { namespace: args.namespace, data: args.data },
cache: 'no-store', 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 ); dispatch.receiveCart( response );
return response;
} catch ( error ) { } catch ( error ) {
dispatch.receiveError( isApiErrorResponse( error ) ? error : null ); dispatch.receiveError( isApiErrorResponse( error ) ? error : null );
return Promise.reject( error ); return Promise.reject( error );
@ -405,11 +390,9 @@ export const updateCustomerData =
} else { } else {
dispatch.receiveCart( response ); dispatch.receiveCart( response );
} }
setIsCustomerDataDirty( false );
return response; return response;
} catch ( error ) { } catch ( error ) {
dispatch.receiveError( isApiErrorResponse( error ) ? error : null ); dispatch.receiveError( isApiErrorResponse( error ) ? error : null );
setIsCustomerDataDirty( true );
return Promise.reject( error ); return Promise.reject( error );
} finally { } finally {
dispatch.updatingCustomerData( false ); dispatch.updatingCustomerData( false );

View File

@ -2,7 +2,7 @@
* External dependencies * External dependencies
*/ */
import { select } from '@wordpress/data'; import { select } from '@wordpress/data';
import { camelCaseKeys, debounce } from '@woocommerce/base-utils'; import { camelCaseKeys } from '@woocommerce/base-utils';
import { isEmail } from '@wordpress/url'; import { isEmail } from '@wordpress/url';
import { import {
CartBillingAddress, CartBillingAddress,
@ -116,27 +116,3 @@ export const validateDirtyProps = ( dirtyProps: {
return invalidProps.length === 0; 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
);

View File

@ -211,7 +211,6 @@ export interface CartMeta {
export interface ExtensionCartUpdateArgs { export interface ExtensionCartUpdateArgs {
data: Record< string, unknown >; data: Record< string, unknown >;
namespace: string; namespace: string;
overwriteDirtyCustomerData?: undefined | boolean;
} }
export interface BillingAddressShippingAddress { export interface BillingAddressShippingAddress {

View File

@ -1,4 +1,3 @@
<!-- markdownlint-disable MD024 -->
# Cart Store (`wc/store/cart`) <!-- omit in toc --> # Cart Store (`wc/store/cart`) <!-- omit in toc -->
> 💡 What's the difference between the Cart Store and the Checkout Store? > 💡 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: - _data_ `object`: The data to send to the endpoint with the following keys:
- _key_ `string`: The key of the extension. - _key_ `string`: The key of the extension.
- _value_ `string`: The value 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_ <!-- omit in toc --> #### _Example_ <!-- omit in toc -->

View File

@ -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. `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 | | Attribute | Type | Required | Description |
| ----------- |-----------|----------| ---------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ----------- | -------- | -------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `namespace` | `string` | Yes | The namespace of your extension. This is used to determine which extension's callbacks should be executed. | | `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. | | `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). |
## Putting it all together ## Putting it all together

View File

@ -1,39 +0,0 @@
<?php
/**
* Plugin Name: WooCommerce Blocks Test extensionCartUpdate
* Description: Adds an extensionCartUpdate endpoint.
* Plugin URI: https://github.com/woocommerce/woocommerce
* Author: WooCommerce
*
* @package woocommerce-blocks-test-extension-cart-update
*/
use Automattic\WooCommerce\StoreApi\Schemas\ExtendSchema;
use Automattic\WooCommerce\StoreApi\StoreApi;
add_action(
'woocommerce_init',
function () {
$extend = StoreApi::container()->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();
}
},
)
);
}
}
);

View File

@ -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' );
} );
} );
} );

View File

@ -1,4 +0,0 @@
Significance: patch
Type: fix
Ensure local state is not overwritten by server when using `extensionCartUpdate`