Commit 11ac9d8619c for woocommerce
commit 11ac9d8619c5b18d39c5726b2d16e1ba51d965fd
Author: Luis Herranz <luisherranz@gmail.com>
Date: Fri Jul 31 12:25:27 2026 +0200
Return a structured outcome from the cart store addCartItem action (#66687)
* Widen addCartItem to return AddCartItemOutcome captured at the request boundary
addCartItem now resolves a structured per-call outcome (`{ success: true }` or
`{ success: false, error: { code?, message } }`) captured immediately after its
own Store API request settles, so a later throw in post-success processing
(notices, a11y announcement) can never downgrade an already-captured success.
The Store type's addCartItem signature and the test harness are widened in
lockstep; removeCartItem and batchAddCartItems are unchanged.
* Rewire saved-for-later Move to cart onto the addCartItem outcome
onClickMoveToCart now gates the source-entry removal on the resolved
AddCartItemOutcome from addCartItem instead of summing cart-line quantities
before and after the request. This removes sumMatchingCartQuantity and its
supporting CartLine type, the doesCartItemMatchAttributes dependency, and the
now-unused cartState binding, since the outcome tells the block directly
whether the add was accepted.
* Rewire wishlist Add to cart onto the addCartItem outcome
Gate the wishlist entry's removal on addCartItem's resolved
AddCartItemOutcome instead of comparing cart quantity totals before
and after the add, removing the sumMatchingCartQuantity helper and
its variation-matching dependency now that success/failure comes
straight from the request boundary.
* Document the addCartItem outcome contract in the cart store README
Add a tightly-scoped `woocommerce/cart` store section covering the
AddCartItemOutcome/AddCartItemError shape, the fire-and-forget guarantee,
per-call attribution under batching/looping, keyed/keyless coverage, and
the removeCartItem/batchAddCartItems non-parity guardrail, with pointers
to the saved-for-later and wishlist read sites.
* Update stale addCartItem comment in mutation-batcher e2e suite
The inline comment said addCartItem's promises merely "resolve," which
no longer tells the whole story now that each call resolves with a
structured per-product outcome. Spell out the { success: true } /
{ success: false, error } shape so a reader of this mixed accept/reject
scenario (products 15, 16 accepted, 999999 rejected) understands the
outcome is distinguishable programmatically, not just observable via
cart state.
* Add changelog entry
---------
Co-authored-by: luisherranz <luis.herranz@automattic.com>
diff --git a/plugins/woocommerce/changelog/add-addcartitem-outcome b/plugins/woocommerce/changelog/add-addcartitem-outcome
new file mode 100644
index 00000000000..fb0615e26bb
--- /dev/null
+++ b/plugins/woocommerce/changelog/add-addcartitem-outcome
@@ -0,0 +1,4 @@
+Significance: patch
+Type: dev
+
+Add a structured outcome to the cart store addCartItem action so consumers can read add success/failure directly instead of counting cart lines.
diff --git a/plugins/woocommerce/client/blocks/assets/js/base/stores/woocommerce/README.md b/plugins/woocommerce/client/blocks/assets/js/base/stores/woocommerce/README.md
index 02d752b0700..3fed9dd18c3 100644
--- a/plugins/woocommerce/client/blocks/assets/js/base/stores/woocommerce/README.md
+++ b/plugins/woocommerce/client/blocks/assets/js/base/stores/woocommerce/README.md
@@ -5,7 +5,7 @@ This folder contains the Interactivity API (iAPI) stores that WooCommerce blocks
Stores in this folder:
- [`woocommerce/products`](#woocommerceproducts-store) — server-populated cache of product and variation data in Store API format.
-- `woocommerce/cart` — cart state and actions (with mutation batching for performance).
+- [`woocommerce/cart`](#woocommercecart-store) — cart state and actions (with mutation batching for performance). `addCartItem` resolves with a structured per-call outcome (`AddCartItemOutcome`) instead of `void` — see below.
---
@@ -232,3 +232,60 @@ For variable products, `findProduct` returns `null` when no variation matches th
- **Local context beats state.** If a block is wrapped in a `data-wp-context="woocommerce/products::{ ... }"` element, its `productId` / `variationId` override any globally-set values for descendants of that element. See `test/products.test.ts` for the exact precedence rules — notably, a context that has `productId` but no `variationId` key does **not** fall back to the global `variationId`.
- **Keep the consent string in sync.** The literal string is defined in `ProductsStore::$consent_statement` (PHP) and `universalLock` (JS). They are intentionally different (loaders vs. store lock); copy-paste from this README or the source files.
- **Do not extend this store from third-party code.** It is `lock: true` and private by design; anything here can change or disappear without notice.
+
+---
+
+## `woocommerce/cart` store
+
+The `woocommerce` Interactivity API store (`base/stores/woocommerce/cart.ts`) holds cart state and actions, with mutation batching for performance. This section only covers the `addCartItem` outcome contract below — it is not a write-up of the store's full state shape, its other actions, or the mutation batcher's internals.
+
+### The `addCartItem` outcome contract
+
+`addCartItem` returns `Promise< AddCartItemOutcome >` — a structured per-call outcome — instead of `Promise< void >`:
+
+```ts
+export type AddCartItemError = {
+ /** Server error code for a per-item rejection (e.g. `woocommerce_rest_product_out_of_stock`),
+ * or the batcher's `unknown_error` fallback. Absent on whole-batch/transport failures. */
+ code?: string;
+ /** Human-readable failure description. Always present. */
+ message: string;
+};
+
+export type AddCartItemOutcome =
+ | { success: true }
+ | { success: false; error: AddCartItemError };
+```
+
+Both types are exported from the cart store module and reach consumers through the existing type-only import path:
+
+```ts
+import type { AddCartItemOutcome } from '@woocommerce/stores/woocommerce/cart';
+```
+
+**`addCartItem` stays fire-and-forget.** It never rejects because of a request or server failure — every request path, whether the Store API accepts or rejects it, resolves with an outcome instead of throwing. The action still throws synchronously for its two programmer-error guards (both `quantity` and `quantityToAdd` supplied together, or a keyless call — no `key` — that passes an absolute `quantity` instead of a `quantityToAdd` delta) — those are argument-validation bugs, unrelated to the request outcome below.
+
+Read the outcome from a consuming generator:
+
+```ts
+const outcome = ( yield cartActions.addCartItem( {
+ id,
+ quantityToAdd,
+} ) ) as AddCartItemOutcome;
+
+if ( ! outcome.success ) {
+ // outcome.error.code — may be absent (e.g. transport/whole-batch failures)
+ // outcome.error.message — always present
+ return;
+}
+```
+
+**`success: true` means the Store API accepted the request — nothing more needs checking.** That includes a brand-new standalone cart line (e.g. a bundle/booking/add-on "meta" line) as much as an incremented existing line, and a silently server-normalized quantity (capped to a maximum, bumped to a minimum/multiple, or forced to one for a sold-individually product) as much as an exact one. A caller never needs to inspect or diff cart lines to interpret the outcome.
+
+**Attribution is per-call, never per-batch.** When several `addCartItem` calls are dispatched within the same tick and coalesced into one `wc/store/v1/batch` request, or issued one after another in a loop, each call's outcome reflects only that call's own product result — never a shared or last-write-wins value from a sibling call, even when calls in the same batch land different outcomes (one product accepted, another rejected).
+
+**Both call shapes share the same request path.** A keyless call (no `key`, posts to `add-item`) and a keyed call (`key` supplied, posts to `update-item`) both resolve through the same single request and outcome capture — there is no separate contract for either shape.
+
+**`removeCartItem` and `batchAddCartItems` do not share this contract.** Both remain typed `Promise<void>` and swallow their own errors the way `addCartItem` used to. Don't assume parity when touching either of them — extending this contract to other cart actions is a separate decision, not something the store does today.
+
+For a worked example, see `onClickMoveToCart` in `saved-for-later/frontend.ts` and `onClickAddToCart` in `wishlist/frontend.ts` — both read the outcome and gate a destructive list-removal on `outcome.success`.
diff --git a/plugins/woocommerce/client/blocks/assets/js/base/stores/woocommerce/cart.ts b/plugins/woocommerce/client/blocks/assets/js/base/stores/woocommerce/cart.ts
index 5799e5add94..bedacbfb783 100644
--- a/plugins/woocommerce/client/blocks/assets/js/base/stores/woocommerce/cart.ts
+++ b/plugins/woocommerce/client/blocks/assets/js/base/stores/woocommerce/cart.ts
@@ -60,6 +60,30 @@ export type ClientCartItem = Omit<
type CartUpdateOptions = { showCartUpdatesNotices?: boolean };
+/**
+ * The failure detail carried by a rejected `AddCartItemOutcome`.
+ */
+export type AddCartItemError = {
+ /**
+ * Server error code for a per-item rejection (e.g.
+ * `woocommerce_rest_product_out_of_stock`), or the batcher's
+ * `unknown_error` fallback. Absent on whole-batch/transport failures.
+ */
+ code?: string;
+ /** Human-readable failure description. Always non-empty. */
+ message: string;
+};
+
+/**
+ * The per-call outcome `addCartItem` resolves with, captured at the moment
+ * its own request settles (accepted or rejected). A later throw in
+ * post-success processing (notices, a11y announcement) never downgrades an
+ * already-captured success into a failure.
+ */
+export type AddCartItemOutcome =
+ | { success: true }
+ | { success: false; error: AddCartItemError };
+
export type Store = {
state: {
errorMessages?: {
@@ -82,7 +106,7 @@ export type Store = {
addCartItem: (
args: ClientCartItem,
options?: CartUpdateOptions
- ) => Promise< void >;
+ ) => Promise< AddCartItemOutcome >;
batchAddCartItems: (
items: ClientCartItem[],
options?: CartUpdateOptions
@@ -288,11 +312,14 @@ const getInfoNoticesFromCartUpdates = (
// Items auto-removed by the server (stock change, product deleted, etc.).
// We pass the optimistic snapshot as oldCart, so user-initiated removals
// are already absent and do not generate spurious notices here.
- const autoDeletedToNotify = oldItems.filter(
- ( old ) =>
- isCartItem( old ) &&
- ! newItems.some( ( item ) => old.key === item.key )
- );
+ // Filtering on the `isCartItem` type guard first (rather than folding it
+ // into a single predicate) narrows the result to `CartItem[]`, so `.name`
+ // below type-checks without a cast.
+ const autoDeletedToNotify = oldItems
+ .filter( isCartItem )
+ .filter(
+ ( old ) => ! newItems.some( ( item ) => old.key === item.key )
+ );
// Items whose quantity was adjusted by the server (stock cap, sold-individually).
// By default a line is compared optimistic → server, so intentional user
@@ -492,7 +519,7 @@ const { actions } = store< Store >(
*addCartItem(
{ id, key, quantity, quantityToAdd, variation }: ClientCartItem,
{ showCartUpdatesNotices = true }: CartUpdateOptions = {}
- ): AsyncAction< void > {
+ ): AsyncAction< AddCartItemOutcome > {
if ( quantity !== undefined && quantityToAdd !== undefined ) {
throw new Error(
'addCartItem: pass either quantity or quantityToAdd, not both.'
@@ -643,6 +670,13 @@ const { actions } = store< Store >(
} );
}
+ // Captured at the request-settlement boundary (the line right
+ // after the request-sending yield resolves/throws) and never
+ // overwritten afterwards. A throw from later post-success
+ // processing (notices, a11y announcement) must not downgrade an
+ // already-captured success.
+ let outcome: AddCartItemOutcome | undefined;
+
try {
const result = ( yield sendCartRequest( state, {
path: `/wc/store/v1/cart/${ endpoint }`,
@@ -694,6 +728,12 @@ const { actions } = store< Store >(
},
} ) ) as TypeYield< typeof sendCartRequest >;
+ // The request settled successfully. Capture the outcome
+ // immediately so any later throw in this block (a11y
+ // chunk-load failure, notices import rejection, response-shape
+ // assertion) cannot downgrade it to a failure.
+ outcome = { success: true };
+
// Success - handle side effects that don't trigger refreshCartItems
const cart = result.data as Cart;
@@ -738,7 +778,25 @@ const { actions } = store< Store >(
} catch ( error ) {
// Show error notice
void actions.showNoticeError( error as Error );
+
+ // Only record a failure outcome if the request-settlement
+ // boundary above did not already capture a success — a throw
+ // after a successful request must not overwrite it.
+ outcome ??= {
+ success: false,
+ error: {
+ ...( ( error as ApiErrorResponse )?.code && {
+ code: ( error as ApiErrorResponse ).code,
+ } ),
+ message:
+ ( error instanceof Error && error.message ) ||
+ String( error ) ||
+ 'Request failed',
+ },
+ };
}
+
+ return outcome as AddCartItemOutcome;
},
*batchAddCartItems(
diff --git a/plugins/woocommerce/client/blocks/assets/js/base/stores/woocommerce/test/cart.ts b/plugins/woocommerce/client/blocks/assets/js/base/stores/woocommerce/test/cart.ts
index c7f6abbb0cc..c1769771e67 100644
--- a/plugins/woocommerce/client/blocks/assets/js/base/stores/woocommerce/test/cart.ts
+++ b/plugins/woocommerce/client/blocks/assets/js/base/stores/woocommerce/test/cart.ts
@@ -7,7 +7,7 @@ import type { Notice } from '@woocommerce/stores/store-notices';
/**
* Internal dependencies
*/
-import type { Store, OptimisticCartItem } from '../cart';
+import type { Store, OptimisticCartItem, AddCartItemOutcome } from '../cart';
type MockStore = { state: Store[ 'state' ]; actions: Store[ 'actions' ] };
@@ -62,10 +62,10 @@ type CapturedRequest = {
/**
* Drives an Interactivity API async action generator to completion.
*
- * Async actions are typed as `void` for consumers but are generators
- * internally. Each yielded value is awaited (resolving the batched cart
- * request and any dynamic imports) and the resolved value is fed back into the
- * generator until it is done.
+ * Async actions are typed as `void` (or a resolved value) for consumers but
+ * are generators internally. Each yielded value is awaited (resolving the
+ * batched cart request and any dynamic imports) and the resolved value is fed
+ * back into the generator until it is done.
*
* When a yielded promise rejects, the rejection is routed back into the
* generator via `iterator.throw()` (mirroring the real Interactivity runtime),
@@ -74,9 +74,9 @@ type CapturedRequest = {
* catch re-throws here so `await runAction(...)` still rejects.
*
* @param action The async action return value cast to a generator.
- * @return A promise that resolves once the generator has finished.
+ * @return A promise resolving to the generator's final (`done`) return value.
*/
-async function runAction( action: unknown ): Promise< void > {
+async function runAction( action: unknown ): Promise< unknown > {
const iterator = action as Generator< unknown, unknown, unknown >;
let next = iterator.next();
while ( ! next.done ) {
@@ -89,6 +89,7 @@ async function runAction( action: unknown ): Promise< void > {
next = iterator.throw( error );
}
}
+ return next.value;
}
/**
@@ -323,6 +324,86 @@ function mockBatchFetchFailing( {
return captured;
}
+/**
+ * Installs a `global.fetch` mock whose batch responses reject only the
+ * mutation(s) targeting a specific product id, letting every other mutation
+ * in the same batch succeed.
+ *
+ * Unlike {@link mockBatchFetchFailing} (which fails every request matching a
+ * given path), this discriminates by the posted product id so two mutations
+ * hitting the same endpoint (e.g. two `add-item` calls) can settle
+ * differently within one batch — reproducing one accepted and one rejected
+ * product in the same request cycle.
+ *
+ * @param options Failure configuration.
+ * @param options.failForId The product id whose mutation(s) should fail.
+ * @param options.status The HTTP status to report for the failed mutation.
+ * @param options.code The error code carried in the failed response body.
+ * @param options.message The human-readable error message in the body.
+ */
+function mockBatchFetchFailingProduct( {
+ failForId,
+ status = 400,
+ code = 'woocommerce_rest_cart_product_no_stock',
+ message = 'You cannot add that amount to the cart.',
+}: {
+ failForId: number;
+ status?: number;
+ code?: string;
+ message?: string;
+} ): void {
+ global.fetch = jest.fn(
+ async ( _url: RequestInfo | URL, init?: RequestInit ) => {
+ // The GET refresh has no body; reply with an empty cart and a nonce.
+ if ( ! init?.body ) {
+ return new Response(
+ JSON.stringify( { items: [], totals: {}, errors: [] } ),
+ { headers: { Nonce: 'test-nonce-123' } }
+ );
+ }
+ const parsed = JSON.parse( init.body as string ) as {
+ requests: CapturedRequest[];
+ };
+ const serverCart = JSON.parse( JSON.stringify( mockState.cart ) );
+ const responses = parsed.requests.map( ( request ) =>
+ request.body.id === failForId
+ ? { status, body: { code, message } }
+ : { status: 200, body: serverCart }
+ );
+ return new Response( JSON.stringify( { responses } ), {
+ headers: { Nonce: 'test-nonce-123' },
+ } );
+ }
+ ) as unknown as typeof fetch;
+}
+
+/**
+ * Installs a `global.fetch` mock whose batch request fails entirely — a
+ * non-2xx response to the outer `/batch` POST itself, not a per-item
+ * rejection — reproducing a whole-batch/transport failure with no
+ * per-product server error code.
+ *
+ * The GET refresh still resolves the nonce gate. The batch POST resolves with
+ * a non-2xx HTTP status, which the mutation queue reports as one `Error`
+ * (lacking a `code` property) shared by every tracked request in the cycle.
+ */
+function mockBatchFetchWholeBatchFailure(): void {
+ global.fetch = jest.fn(
+ async ( _url: RequestInfo | URL, init?: RequestInit ) => {
+ // The GET refresh has no body; reply with an empty cart and a nonce.
+ if ( ! init?.body ) {
+ return new Response(
+ JSON.stringify( { items: [], totals: {}, errors: [] } ),
+ { headers: { Nonce: 'test-nonce-123' } }
+ );
+ }
+ // The whole batch request fails at the HTTP level (no per-item
+ // responses are ever parsed).
+ return new Response( 'Internal Server Error', { status: 500 } );
+ }
+ ) as unknown as typeof fetch;
+}
+
/**
* Builds a minimal server-confirmed cart line carrying a key.
*
@@ -598,7 +679,7 @@ describe( 'WooCommerce Cart Interactivity API Store', () => {
type: 'simple',
} )
)
- ).resolves.toBeUndefined();
+ ).resolves.toEqual( { success: true } );
expect( captured ).toHaveLength( 1 );
expect( captured[ 0 ].path ).toBe( '/wc/store/v1/cart/add-item' );
@@ -620,7 +701,7 @@ describe( 'WooCommerce Cart Interactivity API Store', () => {
type: 'simple',
} )
)
- ).resolves.toBeUndefined();
+ ).resolves.toEqual( { success: true } );
} );
it( 'still throws when both quantity and quantityToAdd are passed together', async () => {
@@ -641,6 +722,161 @@ describe( 'WooCommerce Cart Interactivity API Store', () => {
} );
} );
+ describe( 'addCartItem resolved outcome', () => {
+ it( 'resolves { success: true } when the Store API accepts the request', async () => {
+ mockBatchFetch();
+ const actions = await loadCartStore();
+ seedCart( [ makeKeyedLine( { id: 42, quantity: 3 } ) ] );
+
+ const outcome = await runAction(
+ actions.addCartItem( {
+ id: 42,
+ quantityToAdd: 1,
+ type: 'simple',
+ } )
+ );
+
+ expect( outcome ).toEqual( { success: true } );
+ } );
+
+ it( 'resolves { success: false, error: { code, message } } carrying the server-supplied code and message on a per-item rejection, without the promise rejecting', async () => {
+ mockBatchFetchFailing( {
+ failForPath: '/wc/store/v1/cart/add-item',
+ status: 400,
+ code: 'woocommerce_rest_product_out_of_stock',
+ message: 'You cannot add that amount to the cart.',
+ } );
+ const actions = await loadCartStore();
+ seedCart( [ makeKeyedLine( { id: 42, quantity: 3 } ) ] );
+ spyOnShowNoticeError();
+
+ const outcome = await runAction(
+ actions.addCartItem( {
+ id: 42,
+ quantityToAdd: 1,
+ type: 'simple',
+ } )
+ );
+
+ expect( outcome ).toEqual( {
+ success: false,
+ error: {
+ code: 'woocommerce_rest_product_out_of_stock',
+ message: 'You cannot add that amount to the cart.',
+ },
+ } );
+ } );
+
+ it( 'resolves { success: false, error } with a non-empty message and no code on a whole-batch/transport failure', async () => {
+ mockBatchFetchWholeBatchFailure();
+ const actions = await loadCartStore();
+ seedCart( [ makeKeyedLine( { id: 42, quantity: 3 } ) ] );
+ spyOnShowNoticeError();
+
+ const outcome = ( await runAction(
+ actions.addCartItem( {
+ id: 42,
+ quantityToAdd: 1,
+ type: 'simple',
+ } )
+ ) ) as AddCartItemOutcome;
+
+ if ( outcome.success ) {
+ throw new Error(
+ 'expected a failure outcome for a whole-batch/transport failure'
+ );
+ }
+ expect( typeof outcome.error.message ).toBe( 'string' );
+ expect( outcome.error.message.length ).toBeGreaterThan( 0 );
+ expect( outcome.error.code ).toBeUndefined();
+ } );
+
+ it( 'resolves each call in a shared batch with only its own product outcome, never a shared or last-write-wins value', async () => {
+ mockBatchFetchFailingProduct( {
+ failForId: 99,
+ code: 'woocommerce_rest_cart_product_no_stock',
+ message: 'You cannot add that amount to the cart.',
+ } );
+ const actions = await loadCartStore();
+ seedCart( [] );
+ spyOnShowNoticeError();
+
+ const [ acceptedOutcome, rejectedOutcome ] = await Promise.all( [
+ runAction(
+ actions.addCartItem( {
+ id: 42,
+ quantityToAdd: 1,
+ type: 'simple',
+ } )
+ ),
+ runAction(
+ actions.addCartItem( {
+ id: 99,
+ quantityToAdd: 1,
+ type: 'simple',
+ } )
+ ),
+ ] );
+
+ expect( acceptedOutcome ).toEqual( { success: true } );
+ expect( rejectedOutcome ).toEqual( {
+ success: false,
+ error: {
+ code: 'woocommerce_rest_cart_product_no_stock',
+ message: 'You cannot add that amount to the cart.',
+ },
+ } );
+ } );
+
+ it( 'still resolves { success: true } when a step after the successful request throws (post-success client bug)', async () => {
+ mockBatchFetch();
+ const actions = await loadCartStore();
+ seedCart( [ makeKeyedLine( { id: 42, quantity: 3 } ) ] );
+ spyOnShowNoticeError();
+ // Simulate a client bug in post-success processing (e.g. a notices
+ // import rejection or a11y chunk-load failure): the captured
+ // success outcome must survive this throw.
+ actions.updateNotices = jest.fn( () => {
+ throw new Error( 'post-success client bug' );
+ } ) as unknown as Store[ 'actions' ][ 'updateNotices' ];
+
+ const outcome = await runAction(
+ actions.addCartItem( {
+ id: 42,
+ quantityToAdd: 1,
+ type: 'simple',
+ } )
+ );
+
+ expect( outcome ).toEqual( { success: true } );
+ } );
+
+ it( 'resolves { success: true } and omits unrelated cart.errors from the outcome', async () => {
+ mockBatchFetchReturning( {
+ items: [ makeKeyedLine( { id: 42, quantity: 4 } ) ],
+ totals: {},
+ errors: [
+ {
+ code: 'woocommerce_rest_cart_coupon_error',
+ message: 'The coupon has expired.',
+ },
+ ],
+ } as unknown as Cart );
+ const actions = await loadCartStore();
+ seedCart( [ makeKeyedLine( { id: 42, quantity: 3 } ) ] );
+
+ const outcome = await runAction(
+ actions.addCartItem( {
+ id: 42,
+ quantityToAdd: 1,
+ type: 'simple',
+ } )
+ );
+
+ expect( outcome ).toEqual( { success: true } );
+ } );
+ } );
+
describe( 'batchAddCartItems endpoint selection', () => {
it( 'issues add-item (never update-item) for a keyless batch item that matches a keyed line by product id', async () => {
const captured = mockBatchFetch();
diff --git a/plugins/woocommerce/client/blocks/assets/js/blocks/saved-for-later/frontend.ts b/plugins/woocommerce/client/blocks/assets/js/blocks/saved-for-later/frontend.ts
index 616a668a9f7..37f5ba8f1ec 100644
--- a/plugins/woocommerce/client/blocks/assets/js/blocks/saved-for-later/frontend.ts
+++ b/plugins/woocommerce/client/blocks/assets/js/blocks/saved-for-later/frontend.ts
@@ -15,16 +15,11 @@ import type {
Store as ShopperListsStore,
} from '@woocommerce/stores/woocommerce/shopper-lists';
import type {
- SelectedAttributes,
+ AddCartItemOutcome,
Store as WooCommerce,
} from '@woocommerce/stores/woocommerce/cart';
import { sanitizeHTML } from '@woocommerce/sanitize';
-/**
- * Internal dependencies
- */
-import { doesCartItemMatchAttributes } from '../../base/utils/variations/does-cart-item-match-attributes';
-
const universalLock =
'I acknowledge that using a private store means my plugin will inevitably break on the next store release.';
@@ -114,7 +109,7 @@ const { state: shopperListsState, actions: shopperListsActions } =
{ lock: universalLock }
);
-const { state: cartState, actions: cartActions } = store< WooCommerce >(
+const { actions: cartActions } = store< WooCommerce >(
'woocommerce',
{},
{ lock: universalLock }
@@ -142,51 +137,6 @@ const formatVariationLabel = ( item: RawShopperListItem ): string => {
const getList = ( slug: string ) => shopperListsState.lists[ slug ] ?? null;
-/** A single line of the shared cart store, optimistic or server-confirmed. */
-type CartLine = WooCommerce[ 'state' ][ 'cart' ][ 'items' ][ number ];
-
-/**
- * Sums the quantity across every cart line that represents the given product.
- *
- * The "did the add succeed?" check for Move to cart cannot read a single line:
- * the server owns cart-line identity for adds, so a product already present as
- * one line (e.g. a meta line) can be resolved into a new standalone line rather
- * than an in-place bump. Reading one id-matched line would then see no growth
- * and misread a correct add as a failure. Summing the quantity over all
- * matching lines makes the comparison order-independent and robust to the
- * server splitting or merging lines.
- *
- * Matching mirrors the cart store's `findItemInCart` product-matching: a simple
- * product matches by `id`; a variation additionally requires the same number of
- * variation attributes and the same attribute values, evaluated with
- * `doesCartItemMatchAttributes` (the selector's own semantics).
- *
- * @param items The current cart lines (`cartState.cart.items`).
- * @param id The product id to match.
- * @param variation Selected variation attributes (cart shape). Empty/omitted
- * for a simple product; provided to match variation lines.
- * @return The total quantity across all matching lines (0 when none match).
- */
-const sumMatchingCartQuantity = (
- items: readonly CartLine[],
- id: number,
- variation?: SelectedAttributes[]
-): number =>
- items.reduce( ( total, cartItem ) => {
- let matches: boolean;
- if ( cartItem.type === 'variation' ) {
- matches =
- id === cartItem.id &&
- Array.isArray( cartItem.variation ) &&
- Array.isArray( variation ) &&
- cartItem.variation.length === variation.length &&
- doesCartItemMatchAttributes( cartItem, variation );
- } else {
- matches = id === cartItem.id;
- }
- return matches ? total + cartItem.quantity : total;
- }, 0 );
-
store< BlockStore >(
'woocommerce/saved-for-later',
{
@@ -314,45 +264,21 @@ store< BlockStore >(
);
const isVariation = listItem.variation_id > 0;
- // `cartActions.addCartItem` catches its own errors and
- // surfaces them as store notices, so the yield resolves
- // the same way on success and failure. To tell success from
- // failure we have to read the cart ourselves. The server owns
- // cart-line identity for adds, so a successful add can land as
- // a brand new standalone line rather than bumping an existing
- // one — e.g. when this product is in the cart only as a meta
- // line (a bundle child, booking, or add-on configuration). A
- // single id-matched read would misjudge that case: it can
- // resolve to the unchanged pre-existing line both times, see no
- // growth, and conclude the add failed even though a new line
- // was correctly created. So instead, compare the total quantity
- // across every line matching this product before and after the
- // add, and only remove from the saved list when that total
- // grew. Both reads observe the server-reconciled cart because
- // `addCartItem` resolves only after the mutation batcher commits
- // it, so the comparison is order-independent.
- const beforeSum = sumMatchingCartQuantity(
- cartState.cart.items,
- listItem.id,
- isVariation ? variation : undefined
- );
-
pendingKeys[ listItem.key ] = true;
try {
- yield cartActions.addCartItem( {
+ // `addCartItem` resolves an `AddCartItemOutcome` captured
+ // at the moment its own request settles (accepted or
+ // rejected), so `outcome.success` tells us directly
+ // whether to drop the source entry — no need to read or
+ // sum the cart ourselves.
+ const outcome = ( yield cartActions.addCartItem( {
id: listItem.id,
quantityToAdd: listItem.quantity,
type: isVariation ? 'variation' : 'simple',
...( isVariation && { variation } ),
- } );
-
- const afterSum = sumMatchingCartQuantity(
- cartState.cart.items,
- listItem.id,
- isVariation ? variation : undefined
- );
+ } ) ) as AddCartItemOutcome;
- if ( afterSum <= beforeSum ) {
+ if ( ! outcome.success ) {
return;
}
diff --git a/plugins/woocommerce/client/blocks/assets/js/blocks/saved-for-later/test/frontend.ts b/plugins/woocommerce/client/blocks/assets/js/blocks/saved-for-later/test/frontend.ts
index 3115571772a..d08acabdfdb 100644
--- a/plugins/woocommerce/client/blocks/assets/js/blocks/saved-for-later/test/frontend.ts
+++ b/plugins/woocommerce/client/blocks/assets/js/blocks/saved-for-later/test/frontend.ts
@@ -1,13 +1,9 @@
/**
* External dependencies
*/
-import type {
- OptimisticCartItem,
- Store as WooCommerce,
-} from '@woocommerce/stores/woocommerce/cart';
+import type { AddCartItemOutcome } from '@woocommerce/stores/woocommerce/cart';
import type { RawShopperListItem } from '@woocommerce/stores/woocommerce/shopper-lists';
-type CartItems = WooCommerce[ 'state' ][ 'cart' ][ 'items' ];
type BlockActions = {
onClickMoveToCart: () => Generator< unknown, void >;
};
@@ -22,22 +18,14 @@ let mockContext: {
pendingKeys: Record< string, true >;
};
-// The cart store's mutable line list and selector, controlled per test.
-let mockCartItems: CartItems;
-let mockFindItemInCart: jest.Mock;
-
-// Captured cart-store action spies.
-let mockAddCartItem: jest.Mock;
+// Captured cart-store action spy; resolves an `AddCartItemOutcome` per test.
+let mockAddCartItem: jest.Mock< Promise< AddCartItemOutcome > >;
// Captured shopper-lists `removeItem` spy and the block store's registered
// actions, populated when `frontend.ts` calls the mocked `store()`.
let mockRemoveItem: jest.Mock;
let mockBlockActions: BlockActions | null;
-// Stands in for `doesCartItemMatchAttributes`; a test controls which seeded
-// lines count as variation matches.
-let mockAttributeMatcher: ( cartItem: OptimisticCartItem ) => boolean;
-
jest.mock(
'@wordpress/interactivity',
() => ( {
@@ -53,16 +41,7 @@ jest.mock(
};
}
if ( name === 'woocommerce' ) {
- return {
- state: {
- get cart() {
- return { items: mockCartItems };
- },
- findItemInCart: ( ...args: unknown[] ) =>
- mockFindItemInCart( ...args ),
- },
- actions: { addCartItem: mockAddCartItem },
- };
+ return { actions: { addCartItem: mockAddCartItem } };
}
// woocommerce/shopper-lists
return {
@@ -85,16 +64,6 @@ jest.mock( '@woocommerce/sanitize', () => ( { sanitizeHTML: jest.fn() } ), {
virtual: true,
} );
-// Matching is delegated to the cart store's `doesCartItemMatchAttributes`
-// semantics; the test controls which lines match via `mockAttributeMatcher`.
-jest.mock(
- '../../../base/utils/variations/does-cart-item-match-attributes',
- () => ( {
- doesCartItemMatchAttributes: ( cartItem: OptimisticCartItem ) =>
- mockAttributeMatcher( cartItem ),
- } )
-);
-
/**
* Drives an Interactivity API async action generator to completion.
*
@@ -143,23 +112,6 @@ function makeListItem(
};
}
-/**
- * Builds a minimal cart line for seeding `cart.items`.
- *
- * @param overrides Partial cart-line fields overriding the defaults.
- * @return A cart line.
- */
-function makeCartLine(
- overrides: Partial< OptimisticCartItem > = {}
-): OptimisticCartItem {
- return {
- id: 42,
- type: 'simple',
- quantity: 1,
- ...overrides,
- } as OptimisticCartItem;
-}
-
/**
* Loads a fresh copy of the saved-for-later frontend module so it registers its
* block store against the mocked `store()` and exposes its actions.
@@ -175,36 +127,19 @@ function loadBlockStore(): BlockActions {
return mockBlockActions;
}
-describe( 'Saved-for-later onClickMoveToCart success detection', () => {
+describe( 'Saved-for-later onClickMoveToCart', () => {
beforeEach( () => {
mockContext = { pendingKeys: {} };
- mockCartItems = [];
- mockFindItemInCart = jest.fn();
mockRemoveItem = jest.fn( () => undefined );
- mockAttributeMatcher = () => false;
- // By default the add resolves without mutating the cart; individual
- // tests install an `addCartItem` that mutates `mockCartItems` to model
- // the server-reconciled cart.
- mockAddCartItem = jest.fn( () => Promise.resolve() );
+ mockAddCartItem = jest.fn( () => Promise.resolve( { success: true } ) );
} );
afterEach( () => {
jest.clearAllMocks();
} );
- it( 'removes the entry when a meta-only product gains a new standalone line (before-sum 1, after-sum 2)', async () => {
- // The product exists only as a single meta line at quantity 1; the add
- // resolves server-side as a separate standalone line, so the total
- // quantity across matching lines goes 1 -> 2.
- mockContext.listItem = makeListItem( { id: 42, quantity: 1 } );
- mockCartItems = [ makeCartLine( { id: 42, quantity: 1 } ) ];
- mockAddCartItem = jest.fn( () => {
- mockCartItems = [
- makeCartLine( { id: 42, quantity: 1 } ),
- makeCartLine( { id: 42, quantity: 1, key: 'new-line' } ),
- ];
- return Promise.resolve();
- } );
+ it( 'removes the entry when addCartItem resolves a successful outcome', async () => {
+ mockContext.listItem = makeListItem();
const actions = loadBlockStore();
await runAction( actions.onClickMoveToCart() );
@@ -215,13 +150,13 @@ describe( 'Saved-for-later onClickMoveToCart success detection', () => {
);
} );
- it( 'removes the entry when an existing standalone line is incremented (before-sum 1, after-sum 2)', async () => {
- mockContext.listItem = makeListItem( { id: 42, quantity: 1 } );
- mockCartItems = [ makeCartLine( { id: 42, quantity: 1 } ) ];
- mockAddCartItem = jest.fn( () => {
- mockCartItems = [ makeCartLine( { id: 42, quantity: 2 } ) ];
- return Promise.resolve();
- } );
+ it( 'removes the entry when the add succeeds for a product already in the cart as a meta line', async () => {
+ // A meta line (e.g. a bundle child) resolves the add into a brand-new
+ // standalone cart line rather than bumping the existing one; the
+ // outcome the request itself settles with is what decides removal,
+ // not the shape the cart ends up in.
+ mockContext.listItem = makeListItem();
+ mockAddCartItem = jest.fn( () => Promise.resolve( { success: true } ) );
const actions = loadBlockStore();
await runAction( actions.onClickMoveToCart() );
@@ -232,34 +167,12 @@ describe( 'Saved-for-later onClickMoveToCart success detection', () => {
);
} );
- it( 'preserves the entry when the server rejects the add and the cart is unchanged (before-sum equals after-sum)', async () => {
- mockContext.listItem = makeListItem( { id: 42, quantity: 1 } );
- mockCartItems = [ makeCartLine( { id: 42, quantity: 1 } ) ];
- // addCartItem swallows the error and leaves the cart untouched.
- mockAddCartItem = jest.fn( () => Promise.resolve() );
-
- const actions = loadBlockStore();
- await runAction( actions.onClickMoveToCart() );
-
- expect( mockRemoveItem ).not.toHaveBeenCalled();
- } );
-
- it( 'sums over all matching lines rather than a single id-matched line', async () => {
- // Two matching lines exist before (1 + 2 = 3). The add bumps one of them
- // so the total becomes 4. A single-line read of just one matched line
- // could miss the growth; summing all matching lines detects it.
- mockContext.listItem = makeListItem( { id: 42, quantity: 1 } );
- mockCartItems = [
- makeCartLine( { id: 42, quantity: 1, key: 'a' } ),
- makeCartLine( { id: 42, quantity: 2, key: 'b' } ),
- ];
- mockAddCartItem = jest.fn( () => {
- mockCartItems = [
- makeCartLine( { id: 42, quantity: 2, key: 'a' } ),
- makeCartLine( { id: 42, quantity: 2, key: 'b' } ),
- ];
- return Promise.resolve();
- } );
+ it( 'removes the entry when the add succeeds with a server-normalized quantity', async () => {
+ // The server may resolve the requested quantity into a different
+ // final line quantity than requested; that is still a success and
+ // must not block removal.
+ mockContext.listItem = makeListItem( { quantity: 2 } );
+ mockAddCartItem = jest.fn( () => Promise.resolve( { success: true } ) );
const actions = loadBlockStore();
await runAction( actions.onClickMoveToCart() );
@@ -270,159 +183,53 @@ describe( 'Saved-for-later onClickMoveToCart success detection', () => {
);
} );
- it( 'compares total quantity, not the cart-line count', async () => {
- // The line count stays at 1 across the add (an in-place increment), so a
- // count-based check would read no change. The total quantity grows
- // 1 -> 3, so a quantity sum correctly detects success.
- mockContext.listItem = makeListItem( { id: 42, quantity: 2 } );
- mockCartItems = [ makeCartLine( { id: 42, quantity: 1 } ) ];
- mockAddCartItem = jest.fn( () => {
- mockCartItems = [ makeCartLine( { id: 42, quantity: 3 } ) ];
- return Promise.resolve();
- } );
-
- const actions = loadBlockStore();
- await runAction( actions.onClickMoveToCart() );
-
- expect( mockRemoveItem ).toHaveBeenCalledWith(
- 'saved-for-later',
- 'list-key-1'
+ it( 'keeps the entry when addCartItem resolves a failed outcome', async () => {
+ mockContext.listItem = makeListItem();
+ mockAddCartItem = jest.fn( () =>
+ Promise.resolve( {
+ success: false,
+ error: { message: 'Out of stock.' },
+ } )
);
- } );
-
- it( 'is independent of cart-line ordering', async () => {
- // Same lines as the multi-line case but seeded in reverse order before
- // and after; the result must not depend on iteration order.
- mockContext.listItem = makeListItem( { id: 42, quantity: 1 } );
- mockCartItems = [
- makeCartLine( { id: 99, quantity: 5, key: 'other' } ),
- makeCartLine( { id: 42, quantity: 1, key: 'a' } ),
- ];
- mockAddCartItem = jest.fn( () => {
- mockCartItems = [
- makeCartLine( { id: 42, quantity: 1, key: 'new' } ),
- makeCartLine( { id: 99, quantity: 5, key: 'other' } ),
- makeCartLine( { id: 42, quantity: 1, key: 'a' } ),
- ];
- return Promise.resolve();
- } );
const actions = loadBlockStore();
await runAction( actions.onClickMoveToCart() );
- expect( mockRemoveItem ).toHaveBeenCalledWith(
- 'saved-for-later',
- 'list-key-1'
- );
+ expect( mockRemoveItem ).not.toHaveBeenCalled();
} );
- it( 'ignores non-matching product lines when summing', async () => {
- // A different product (id 99) churns its quantity across the add while
- // the saved product (id 42) is unchanged. Summing only matching lines
- // must read no growth and preserve the entry.
- mockContext.listItem = makeListItem( { id: 42, quantity: 1 } );
- mockCartItems = [
- makeCartLine( { id: 42, quantity: 1 } ),
- makeCartLine( { id: 99, quantity: 1, key: 'other' } ),
- ];
- mockAddCartItem = jest.fn( () => {
- mockCartItems = [
- makeCartLine( { id: 42, quantity: 1 } ),
- makeCartLine( { id: 99, quantity: 8, key: 'other' } ),
- ];
- return Promise.resolve();
- } );
+ it( 'does not call addCartItem when the row is not purchasable', async () => {
+ mockContext.listItem = makeListItem( { is_purchasable: false } );
const actions = loadBlockStore();
await runAction( actions.onClickMoveToCart() );
+ expect( mockAddCartItem ).not.toHaveBeenCalled();
expect( mockRemoveItem ).not.toHaveBeenCalled();
} );
- it( 'sums over lines matching id and variation attributes for a variation entry', async () => {
- mockContext.listItem = makeListItem( {
- id: 7,
- variation_id: 7,
- quantity: 1,
- variation: [
- {
- raw_attribute: 'attribute_pa_color',
- attribute: 'Color',
- value: 'blue',
- },
- ],
- } );
- // Variation lines carry a same-length `variation` array (matching the
- // selector's length guard); the attribute matcher decides which one is
- // the saved product. One variation line of the saved product plus a
- // same-id line with a different (non-matching) variation.
- const variationAttr = [
- {
- raw_attribute: 'attribute_pa_color',
- attribute: 'Color',
- value: 'x',
- },
- ];
- const matchingLine = makeCartLine( {
- id: 7,
- type: 'variation',
- quantity: 1,
- key: 'match',
- variation: variationAttr,
- } );
- const otherVariationLine = makeCartLine( {
- id: 7,
- type: 'variation',
- quantity: 4,
- key: 'other-variation',
- variation: variationAttr,
- } );
- mockCartItems = [ matchingLine, otherVariationLine ];
- // Only the matching variation line satisfies the attribute matcher.
- mockAttributeMatcher = ( cartItem ) => cartItem.key === 'match';
- mockAddCartItem = jest.fn( () => {
- mockCartItems = [
- makeCartLine( {
- id: 7,
- type: 'variation',
- quantity: 2,
- key: 'match',
- variation: variationAttr,
- } ),
- makeCartLine( {
- id: 7,
- type: 'variation',
- quantity: 4,
- key: 'other-variation',
- variation: variationAttr,
- } ),
- ];
- return Promise.resolve();
- } );
+ it( 'does not call addCartItem when the row is already pending', async () => {
+ mockContext.listItem = makeListItem();
+ mockContext.pendingKeys[ 'list-key-1' ] = true;
const actions = loadBlockStore();
await runAction( actions.onClickMoveToCart() );
- expect( mockRemoveItem ).toHaveBeenCalledWith(
- 'saved-for-later',
- 'list-key-1'
- );
+ expect( mockAddCartItem ).not.toHaveBeenCalled();
} );
- it( 'does not call findItemInCart for the success check (consumer sums cart.items directly)', async () => {
- // The before/after read sums `cart.items` itself; `findItemInCart` must
- // not be the mechanism for the success comparison.
- mockContext.listItem = makeListItem( { id: 42, quantity: 1 } );
- mockCartItems = [ makeCartLine( { id: 42, quantity: 1 } ) ];
- mockAddCartItem = jest.fn( () => {
- mockCartItems = [ makeCartLine( { id: 42, quantity: 2 } ) ];
- return Promise.resolve();
- } );
+ it( 'clears the pending flag once the add settles, whether it succeeds or fails', async () => {
+ mockContext.listItem = makeListItem();
+ mockAddCartItem = jest.fn( () =>
+ Promise.resolve( {
+ success: false,
+ error: { message: 'Out of stock.' },
+ } )
+ );
const actions = loadBlockStore();
await runAction( actions.onClickMoveToCart() );
- expect( mockFindItemInCart ).not.toHaveBeenCalled();
- expect( mockRemoveItem ).toHaveBeenCalled();
+ expect( mockContext.pendingKeys[ 'list-key-1' ] ).toBeUndefined();
} );
} );
diff --git a/plugins/woocommerce/client/blocks/assets/js/blocks/wishlist/frontend.ts b/plugins/woocommerce/client/blocks/assets/js/blocks/wishlist/frontend.ts
index ae777eb6523..26412c093f9 100644
--- a/plugins/woocommerce/client/blocks/assets/js/blocks/wishlist/frontend.ts
+++ b/plugins/woocommerce/client/blocks/assets/js/blocks/wishlist/frontend.ts
@@ -15,16 +15,11 @@ import type {
Store as ShopperListsStore,
} from '@woocommerce/stores/woocommerce/shopper-lists';
import type {
- SelectedAttributes,
+ AddCartItemOutcome,
Store as WooCommerce,
} from '@woocommerce/stores/woocommerce/cart';
import { sanitizeHTML } from '@woocommerce/sanitize';
-/**
- * Internal dependencies
- */
-import { doesCartItemMatchAttributes } from '../../base/utils/variations/does-cart-item-match-attributes';
-
const universalLock =
'I acknowledge that using a private store means my plugin will inevitably break on the next store release.';
@@ -106,7 +101,7 @@ const { state: shopperListsState, actions: shopperListsActions } =
{ lock: universalLock }
);
-const { state: cartState, actions: cartActions } = store< WooCommerce >(
+const { actions: cartActions } = store< WooCommerce >(
'woocommerce',
{},
{ lock: universalLock }
@@ -134,51 +129,6 @@ const formatVariationLabel = ( item: RawShopperListItem ): string => {
const getList = ( slug: string ) => shopperListsState.lists[ slug ] ?? null;
-/** A single line of the shared cart store, optimistic or server-confirmed. */
-type CartLine = WooCommerce[ 'state' ][ 'cart' ][ 'items' ][ number ];
-
-/**
- * Sums the quantity across every cart line that represents the given product.
- *
- * The "did the add succeed?" check for Add to cart cannot read a single line:
- * the server owns cart-line identity for adds, so a product already present as
- * one line (e.g. a meta line) can be resolved into a new standalone line rather
- * than an in-place bump. Reading one id-matched line would then see no growth
- * and misread a correct add as a failure. Summing the quantity over all
- * matching lines makes the comparison order-independent and robust to the
- * server splitting or merging lines.
- *
- * Matching mirrors the cart store's `findItemInCart` product-matching: a simple
- * product matches by `id`; a variation additionally requires the same number of
- * variation attributes and the same attribute values, evaluated with
- * `doesCartItemMatchAttributes` (the selector's own semantics).
- *
- * @param items The current cart lines (`cartState.cart.items`).
- * @param id The product id to match.
- * @param variation Selected variation attributes (cart shape). Empty/omitted
- * for a simple product; provided to match variation lines.
- * @return The total quantity across all matching lines (0 when none match).
- */
-const sumMatchingCartQuantity = (
- items: readonly CartLine[],
- id: number,
- variation?: SelectedAttributes[]
-): number =>
- items.reduce( ( total, cartItem ) => {
- let matches: boolean;
- if ( cartItem.type === 'variation' ) {
- matches =
- id === cartItem.id &&
- Array.isArray( cartItem.variation ) &&
- Array.isArray( variation ) &&
- cartItem.variation.length === variation.length &&
- doesCartItemMatchAttributes( cartItem, variation );
- } else {
- matches = id === cartItem.id;
- }
- return matches ? total + cartItem.quantity : total;
- }, 0 );
-
store< BlockStore >(
'woocommerce/wishlist',
{
@@ -291,45 +241,21 @@ store< BlockStore >(
const isVariation = listItem.variation_id > 0;
// Wishlist always adds quantity 1 (no quantity column).
- // `cartActions.addCartItem` catches its own errors and
- // surfaces them as store notices, so the yield resolves
- // the same way on success and failure. To tell success from
- // failure we have to read the cart ourselves. The server owns
- // cart-line identity for adds, so a successful add can land as
- // a brand new standalone line rather than bumping an existing
- // one — e.g. when this product is in the cart only as a meta
- // line (a bundle child, booking, or add-on configuration). A
- // single id-matched read would misjudge that case: it can
- // resolve to the unchanged pre-existing line both times, see no
- // growth, and conclude the add failed even though a new line
- // was correctly created. So instead, compare the total quantity
- // across every line matching this product before and after the
- // add, and only remove from the wishlist when that total grew.
- // Both reads observe the server-reconciled cart because
- // `addCartItem` resolves only after the mutation batcher commits
- // it, so the comparison is order-independent.
- const beforeSum = sumMatchingCartQuantity(
- cartState.cart.items,
- listItem.id,
- isVariation ? variation : undefined
- );
-
pendingKeys[ listItem.key ] = true;
try {
- yield cartActions.addCartItem( {
+ // `addCartItem` resolves an `AddCartItemOutcome` captured
+ // at the moment its own request settles (accepted or
+ // rejected), so `outcome.success` tells us directly
+ // whether to drop the source entry — no need to read or
+ // sum the cart ourselves.
+ const outcome = ( yield cartActions.addCartItem( {
id: listItem.id,
quantityToAdd: 1,
type: isVariation ? 'variation' : 'simple',
...( isVariation && { variation } ),
- } );
-
- const afterSum = sumMatchingCartQuantity(
- cartState.cart.items,
- listItem.id,
- isVariation ? variation : undefined
- );
+ } ) ) as AddCartItemOutcome;
- if ( afterSum <= beforeSum ) {
+ if ( ! outcome.success ) {
return;
}
diff --git a/plugins/woocommerce/client/blocks/assets/js/blocks/wishlist/test/frontend.ts b/plugins/woocommerce/client/blocks/assets/js/blocks/wishlist/test/frontend.ts
index b58bc180eaa..8c44df5827d 100644
--- a/plugins/woocommerce/client/blocks/assets/js/blocks/wishlist/test/frontend.ts
+++ b/plugins/woocommerce/client/blocks/assets/js/blocks/wishlist/test/frontend.ts
@@ -1,13 +1,9 @@
/**
* External dependencies
*/
-import type {
- OptimisticCartItem,
- Store as WooCommerce,
-} from '@woocommerce/stores/woocommerce/cart';
+import type { AddCartItemOutcome } from '@woocommerce/stores/woocommerce/cart';
import type { RawShopperListItem } from '@woocommerce/stores/woocommerce/shopper-lists';
-type CartItems = WooCommerce[ 'state' ][ 'cart' ][ 'items' ];
type BlockActions = {
onClickAddToCart: () => Generator< unknown, void >;
};
@@ -22,22 +18,14 @@ let mockContext: {
pendingKeys: Record< string, true >;
};
-// The cart store's mutable line list and selector, controlled per test.
-let mockCartItems: CartItems;
-let mockFindItemInCart: jest.Mock;
-
-// Captured cart-store action spies.
-let mockAddCartItem: jest.Mock;
+// Captured cart-store action spy; resolves an `AddCartItemOutcome` per test.
+let mockAddCartItem: jest.Mock< Promise< AddCartItemOutcome > >;
// Captured shopper-lists `removeItem` spy and the block store's registered
// actions, populated when `frontend.ts` calls the mocked `store()`.
let mockRemoveItem: jest.Mock;
let mockBlockActions: BlockActions | null;
-// Stands in for `doesCartItemMatchAttributes`; a test controls which seeded
-// lines count as variation matches.
-let mockAttributeMatcher: ( cartItem: OptimisticCartItem ) => boolean;
-
jest.mock(
'@wordpress/interactivity',
() => ( {
@@ -53,16 +41,7 @@ jest.mock(
};
}
if ( name === 'woocommerce' ) {
- return {
- state: {
- get cart() {
- return { items: mockCartItems };
- },
- findItemInCart: ( ...args: unknown[] ) =>
- mockFindItemInCart( ...args ),
- },
- actions: { addCartItem: mockAddCartItem },
- };
+ return { actions: { addCartItem: mockAddCartItem } };
}
// woocommerce/shopper-lists
return {
@@ -85,16 +64,6 @@ jest.mock( '@woocommerce/sanitize', () => ( { sanitizeHTML: jest.fn() } ), {
virtual: true,
} );
-// Matching is delegated to the cart store's `doesCartItemMatchAttributes`
-// semantics; the test controls which lines match via `mockAttributeMatcher`.
-jest.mock(
- '../../../base/utils/variations/does-cart-item-match-attributes',
- () => ( {
- doesCartItemMatchAttributes: ( cartItem: OptimisticCartItem ) =>
- mockAttributeMatcher( cartItem ),
- } )
-);
-
/**
* Drives an Interactivity API async action generator to completion.
*
@@ -143,23 +112,6 @@ function makeListItem(
};
}
-/**
- * Builds a minimal cart line for seeding `cart.items`.
- *
- * @param overrides Partial cart-line fields overriding the defaults.
- * @return A cart line.
- */
-function makeCartLine(
- overrides: Partial< OptimisticCartItem > = {}
-): OptimisticCartItem {
- return {
- id: 42,
- type: 'simple',
- quantity: 1,
- ...overrides,
- } as OptimisticCartItem;
-}
-
/**
* Loads a fresh copy of the wishlist frontend module so it registers its block
* store against the mocked `store()` and exposes its actions.
@@ -175,36 +127,19 @@ function loadBlockStore(): BlockActions {
return mockBlockActions;
}
-describe( 'Wishlist onClickAddToCart success detection', () => {
+describe( 'Wishlist onClickAddToCart', () => {
beforeEach( () => {
mockContext = { pendingKeys: {} };
- mockCartItems = [];
- mockFindItemInCart = jest.fn();
mockRemoveItem = jest.fn( () => undefined );
- mockAttributeMatcher = () => false;
- // By default the add resolves without mutating the cart; individual
- // tests install an `addCartItem` that mutates `mockCartItems` to model
- // the server-reconciled cart.
- mockAddCartItem = jest.fn( () => Promise.resolve() );
+ mockAddCartItem = jest.fn( () => Promise.resolve( { success: true } ) );
} );
afterEach( () => {
jest.clearAllMocks();
} );
- it( 'removes the entry when a meta-only product gains a new standalone line (before-sum 1, after-sum 2)', async () => {
- // The product exists only as a single meta line at quantity 1; the add
- // resolves server-side as a separate standalone line, so the total
- // quantity across matching lines goes 1 -> 2.
- mockContext.listItem = makeListItem( { id: 42 } );
- mockCartItems = [ makeCartLine( { id: 42, quantity: 1 } ) ];
- mockAddCartItem = jest.fn( () => {
- mockCartItems = [
- makeCartLine( { id: 42, quantity: 1 } ),
- makeCartLine( { id: 42, quantity: 1, key: 'new-line' } ),
- ];
- return Promise.resolve();
- } );
+ it( 'removes the entry when addCartItem resolves a successful outcome', async () => {
+ mockContext.listItem = makeListItem();
const actions = loadBlockStore();
await runAction( actions.onClickAddToCart() );
@@ -215,13 +150,13 @@ describe( 'Wishlist onClickAddToCart success detection', () => {
);
} );
- it( 'removes the entry when an existing standalone line is incremented (before-sum 1, after-sum 2)', async () => {
- mockContext.listItem = makeListItem( { id: 42 } );
- mockCartItems = [ makeCartLine( { id: 42, quantity: 1 } ) ];
- mockAddCartItem = jest.fn( () => {
- mockCartItems = [ makeCartLine( { id: 42, quantity: 2 } ) ];
- return Promise.resolve();
- } );
+ it( 'removes the entry when the add succeeds for a product already in the cart as a meta line', async () => {
+ // A meta line (e.g. a bundle child) resolves the add into a brand-new
+ // standalone cart line rather than bumping the existing one; the
+ // outcome the request itself settles with is what decides removal,
+ // not the shape the cart ends up in.
+ mockContext.listItem = makeListItem();
+ mockAddCartItem = jest.fn( () => Promise.resolve( { success: true } ) );
const actions = loadBlockStore();
await runAction( actions.onClickAddToCart() );
@@ -232,34 +167,13 @@ describe( 'Wishlist onClickAddToCart success detection', () => {
);
} );
- it( 'preserves the entry when the server rejects the add and the cart is unchanged (before-sum equals after-sum)', async () => {
- mockContext.listItem = makeListItem( { id: 42 } );
- mockCartItems = [ makeCartLine( { id: 42, quantity: 1 } ) ];
- // addCartItem swallows the error and leaves the cart untouched.
- mockAddCartItem = jest.fn( () => Promise.resolve() );
-
- const actions = loadBlockStore();
- await runAction( actions.onClickAddToCart() );
-
- expect( mockRemoveItem ).not.toHaveBeenCalled();
- } );
-
- it( 'sums over all matching lines rather than a single id-matched line', async () => {
- // Two matching lines exist before (1 + 2 = 3). The add bumps one of them
- // so the total becomes 4. A single-line read of just one matched line
- // could miss the growth; summing all matching lines detects it.
- mockContext.listItem = makeListItem( { id: 42 } );
- mockCartItems = [
- makeCartLine( { id: 42, quantity: 1, key: 'a' } ),
- makeCartLine( { id: 42, quantity: 2, key: 'b' } ),
- ];
- mockAddCartItem = jest.fn( () => {
- mockCartItems = [
- makeCartLine( { id: 42, quantity: 2, key: 'a' } ),
- makeCartLine( { id: 42, quantity: 2, key: 'b' } ),
- ];
- return Promise.resolve();
- } );
+ it( 'removes the entry when the add succeeds with a server-normalized quantity', async () => {
+ // Wishlist always requests quantityToAdd: 1, but the server may still
+ // resolve the line to a different final quantity (e.g. merging with
+ // an existing line); that is still a success and must not block
+ // removal.
+ mockContext.listItem = makeListItem();
+ mockAddCartItem = jest.fn( () => Promise.resolve( { success: true } ) );
const actions = loadBlockStore();
await runAction( actions.onClickAddToCart() );
@@ -270,158 +184,53 @@ describe( 'Wishlist onClickAddToCart success detection', () => {
);
} );
- it( 'compares total quantity, not the cart-line count', async () => {
- // The line count stays at 1 across the add (an in-place increment), so a
- // count-based check would read no change. The total quantity grows
- // 1 -> 2, so a quantity sum correctly detects success.
- mockContext.listItem = makeListItem( { id: 42 } );
- mockCartItems = [ makeCartLine( { id: 42, quantity: 1 } ) ];
- mockAddCartItem = jest.fn( () => {
- mockCartItems = [ makeCartLine( { id: 42, quantity: 2 } ) ];
- return Promise.resolve();
- } );
-
- const actions = loadBlockStore();
- await runAction( actions.onClickAddToCart() );
-
- expect( mockRemoveItem ).toHaveBeenCalledWith(
- 'wishlist',
- 'list-key-1'
+ it( 'keeps the entry when addCartItem resolves a failed outcome', async () => {
+ mockContext.listItem = makeListItem();
+ mockAddCartItem = jest.fn( () =>
+ Promise.resolve( {
+ success: false,
+ error: { message: 'Out of stock.' },
+ } )
);
- } );
-
- it( 'is independent of cart-line ordering', async () => {
- // Same lines as the multi-line case but seeded in mixed order before and
- // after; the result must not depend on iteration order.
- mockContext.listItem = makeListItem( { id: 42 } );
- mockCartItems = [
- makeCartLine( { id: 99, quantity: 5, key: 'other' } ),
- makeCartLine( { id: 42, quantity: 1, key: 'a' } ),
- ];
- mockAddCartItem = jest.fn( () => {
- mockCartItems = [
- makeCartLine( { id: 42, quantity: 1, key: 'new' } ),
- makeCartLine( { id: 99, quantity: 5, key: 'other' } ),
- makeCartLine( { id: 42, quantity: 1, key: 'a' } ),
- ];
- return Promise.resolve();
- } );
const actions = loadBlockStore();
await runAction( actions.onClickAddToCart() );
- expect( mockRemoveItem ).toHaveBeenCalledWith(
- 'wishlist',
- 'list-key-1'
- );
+ expect( mockRemoveItem ).not.toHaveBeenCalled();
} );
- it( 'ignores non-matching product lines when summing', async () => {
- // A different product (id 99) churns its quantity across the add while
- // the wishlisted product (id 42) is unchanged. Summing only matching
- // lines must read no growth and preserve the entry.
- mockContext.listItem = makeListItem( { id: 42 } );
- mockCartItems = [
- makeCartLine( { id: 42, quantity: 1 } ),
- makeCartLine( { id: 99, quantity: 1, key: 'other' } ),
- ];
- mockAddCartItem = jest.fn( () => {
- mockCartItems = [
- makeCartLine( { id: 42, quantity: 1 } ),
- makeCartLine( { id: 99, quantity: 8, key: 'other' } ),
- ];
- return Promise.resolve();
- } );
+ it( 'does not call addCartItem when the row is not purchasable', async () => {
+ mockContext.listItem = makeListItem( { is_purchasable: false } );
const actions = loadBlockStore();
await runAction( actions.onClickAddToCart() );
+ expect( mockAddCartItem ).not.toHaveBeenCalled();
expect( mockRemoveItem ).not.toHaveBeenCalled();
} );
- it( 'sums over lines matching id and variation attributes for a variation entry', async () => {
- mockContext.listItem = makeListItem( {
- id: 7,
- variation_id: 7,
- variation: [
- {
- raw_attribute: 'attribute_pa_color',
- attribute: 'Color',
- value: 'blue',
- },
- ],
- } );
- // Variation lines carry a same-length `variation` array (matching the
- // selector's length guard); the attribute matcher decides which one is
- // the wishlisted product. One variation line of the wishlisted product
- // plus a same-id line with a different (non-matching) variation.
- const variationAttr = [
- {
- raw_attribute: 'attribute_pa_color',
- attribute: 'Color',
- value: 'x',
- },
- ];
- const matchingLine = makeCartLine( {
- id: 7,
- type: 'variation',
- quantity: 1,
- key: 'match',
- variation: variationAttr,
- } );
- const otherVariationLine = makeCartLine( {
- id: 7,
- type: 'variation',
- quantity: 4,
- key: 'other-variation',
- variation: variationAttr,
- } );
- mockCartItems = [ matchingLine, otherVariationLine ];
- // Only the matching variation line satisfies the attribute matcher.
- mockAttributeMatcher = ( cartItem ) => cartItem.key === 'match';
- mockAddCartItem = jest.fn( () => {
- mockCartItems = [
- makeCartLine( {
- id: 7,
- type: 'variation',
- quantity: 2,
- key: 'match',
- variation: variationAttr,
- } ),
- makeCartLine( {
- id: 7,
- type: 'variation',
- quantity: 4,
- key: 'other-variation',
- variation: variationAttr,
- } ),
- ];
- return Promise.resolve();
- } );
+ it( 'does not call addCartItem when the row is already pending', async () => {
+ mockContext.listItem = makeListItem();
+ mockContext.pendingKeys[ 'list-key-1' ] = true;
const actions = loadBlockStore();
await runAction( actions.onClickAddToCart() );
- expect( mockRemoveItem ).toHaveBeenCalledWith(
- 'wishlist',
- 'list-key-1'
- );
+ expect( mockAddCartItem ).not.toHaveBeenCalled();
} );
- it( 'does not call findItemInCart for the success check (consumer sums cart.items directly)', async () => {
- // The before/after read sums `cart.items` itself; `findItemInCart` must
- // not be the mechanism for the success comparison.
- mockContext.listItem = makeListItem( { id: 42 } );
- mockCartItems = [ makeCartLine( { id: 42, quantity: 1 } ) ];
- mockAddCartItem = jest.fn( () => {
- mockCartItems = [ makeCartLine( { id: 42, quantity: 2 } ) ];
- return Promise.resolve();
- } );
+ it( 'clears the pending flag once the add settles, whether it succeeds or fails', async () => {
+ mockContext.listItem = makeListItem();
+ mockAddCartItem = jest.fn( () =>
+ Promise.resolve( {
+ success: false,
+ error: { message: 'Out of stock.' },
+ } )
+ );
const actions = loadBlockStore();
await runAction( actions.onClickAddToCart() );
- expect( mockFindItemInCart ).not.toHaveBeenCalled();
- expect( mockRemoveItem ).toHaveBeenCalled();
+ expect( mockContext.pendingKeys[ 'list-key-1' ] ).toBeUndefined();
} );
} );