diff --git a/packages/react-components/specs/reducers/OrderReducer.include.spec.ts b/packages/react-components/specs/reducers/OrderReducer.include.spec.ts new file mode 100644 index 00000000..934463b9 --- /dev/null +++ b/packages/react-components/specs/reducers/OrderReducer.include.spec.ts @@ -0,0 +1,96 @@ +import { describe, expect, it, vi } from "vitest" +import orderReducer, { + addResourceToInclude, + type OrderActions, + type OrderState, +} from "#reducers/OrderReducer" + +/** + * `state.include` is the union of the resources every mounted component needs. The API + * cannot distinguish "I didn't ask for this relationship" from "this relationship is + * unset", so a dropped resource silently becomes wrong application state rather than a + * failed request. These specs pin the accumulation behaviour. + */ +describe("orderReducer / setIncludesResource", () => { + const reduce = (state: OrderState, action: OrderActions): OrderState => + orderReducer(state as Required, action) + + it("accumulates resources across dispatches", () => { + let state: OrderState = { include: ["line_items"] } + state = reduce(state, { + type: "setIncludesResource", + payload: { include: ["billing_address"] }, + }) + expect(state.include).toEqual(["line_items", "billing_address"]) + }) + + it("de-duplicates resources already present", () => { + const state = reduce( + { include: ["line_items", "billing_address"] }, + { type: "setIncludesResource", payload: { include: ["billing_address"] } } + ) + expect(state.include).toEqual(["line_items", "billing_address"]) + }) + + it("keeps include undefined when a dispatch only reports includeLoaded", () => { + // `useOrderState` distinguishes `undefined` from `[]`, so this must not become `[]`. + const state = reduce( + {}, + { type: "setIncludesResource", payload: { includeLoaded: { billing_address: true } } } + ) + expect(state.include).toBeUndefined() + expect(state.includeLoaded).toEqual({ billing_address: true }) + }) + + it("treats an explicitly empty list as a reset", () => { + const state = reduce( + { include: ["line_items", "billing_address"] }, + { type: "setIncludesResource", payload: { include: [] } } + ) + expect(state.include).toEqual([]) + }) + + it("leaves other action types to the base reducer", () => { + const state = reduce({ include: ["line_items"] }, { + type: "setLoading", + payload: { loading: false }, + } as OrderActions) + expect(state.include).toEqual(["line_items"]) + expect(state.loading).toBe(false) + }) + + /** + * The real defect: several components dispatch more than once from a single effect + * pass, so every call reads the same pre-update `include` snapshot. Before the reducer + * unioned, the last dispatch replaced the others and their resources were lost — + * including `shipments.shipping_method`, which made a set shipping method look unset. + */ + it("keeps every resource when three call sites dispatch off one stale snapshot", () => { + let state: OrderState = { include: ["line_items"] } + const snapshot = state.include + const dispatch = vi.fn((action: OrderActions) => { + state = reduce(state, action) + }) + + // Block 1 and 2 omit `resourcesIncluded`; block 3 passes the stale snapshot. + addResourceToInclude({ + dispatch, + newResource: ["shipments.available_shipping_methods", "shipments.shipping_method"], + }) + addResourceToInclude({ dispatch, newResource: "billing_address" }) + addResourceToInclude({ + dispatch, + newResource: "shipping_address", + resourcesIncluded: snapshot, + }) + + expect(dispatch).toHaveBeenCalledTimes(3) + expect(state.include).toEqual([ + "line_items", + "shipments.available_shipping_methods", + "shipments.shipping_method", + "billing_address", + "shipping_address", + ]) + }) +}) diff --git a/packages/react-components/specs/reducers/OrderReducer.includeRequest.spec.ts b/packages/react-components/specs/reducers/OrderReducer.includeRequest.spec.ts new file mode 100644 index 00000000..6b466118 --- /dev/null +++ b/packages/react-components/specs/reducers/OrderReducer.includeRequest.spec.ts @@ -0,0 +1,86 @@ +import { beforeEach, describe, expect, it, vi } from "vitest" + +const retrieve = vi.fn().mockResolvedValue({ id: "order-1", editable: true }) + +vi.mock("@commercelayer/core-components", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + getSdk: vi.fn().mockReturnValue({ orders: { retrieve } }), + } +}) + +const { getApiOrder, addResourceToInclude, default: orderReducer } = await import( + "#reducers/OrderReducer" +) +type OrderActions = import("#reducers/OrderReducer").OrderActions +type OrderState = import("#reducers/OrderReducer").OrderState + +/** + * Closes the loop between the reducer and the request actually sent: builds `state.include` + * through the same dispatch sequence `PlaceOrderContainer` performs, then asserts what + * `getApiOrder` puts on the wire. Missing an include is indistinguishable from an unset + * relationship in the response, so this is the assertion that matters in practice. + */ +describe("getApiOrder include", () => { + beforeEach(() => { + retrieve.mockClear() + }) + + function includeAfterPlaceOrderContainerEffect(initial: OrderState): OrderState { + let state = initial + const snapshot = state.include + const dispatch = (action: OrderActions) => { + state = orderReducer(state as Required, action) + } + // Mirrors PlaceOrderContainer's single effect pass: three dispatches, one snapshot. + addResourceToInclude({ + dispatch, + newResource: [ + "shipments.available_shipping_methods", + "shipments.stock_line_items.line_item", + "shipments.shipping_method", + "shipments.stock_transfers.line_item", + "shipments.stock_location", + ], + }) + addResourceToInclude({ dispatch, newResource: "billing_address" }) + addResourceToInclude({ + dispatch, + newResource: "shipping_address", + resourcesIncluded: snapshot, + }) + return state + } + + it("requests shipments.shipping_method after the PlaceOrderContainer effect", async () => { + const state = includeAfterPlaceOrderContainerEffect({ include: ["line_items.item"] }) + + await getApiOrder({ + id: "order-1", + config: { accessToken: "test-token" }, + state, + options: {}, + }) + + expect(retrieve).toHaveBeenCalledTimes(1) + const options = retrieve.mock.calls[0]?.[1] as { include?: string[] } + // The resource whose absence made a set shipping method look unset. + expect(options.include).toContain("shipments.shipping_method") + expect(options.include).toContain("billing_address") + expect(options.include).toContain("shipping_address") + expect(options.include).toContain("line_items.item") + }) + + it("omits include entirely when nothing has been registered", async () => { + await getApiOrder({ + id: "order-1", + config: { accessToken: "test-token" }, + state: {}, + options: {}, + }) + + const options = retrieve.mock.calls[0]?.[1] as { include?: string[] } + expect(options.include).toBeUndefined() + }) +}) diff --git a/packages/react-components/src/reducers/OrderReducer.ts b/packages/react-components/src/reducers/OrderReducer.ts index f0b35c27..2f48004c 100644 --- a/packages/react-components/src/reducers/OrderReducer.ts +++ b/packages/react-components/src/reducers/OrderReducer.ts @@ -798,7 +798,34 @@ export const orderInitialState: Partial = { withoutIncludes: true, } -const orderReducer = (state: OrderState, reducer: OrderActions): OrderState => - baseReducer(state, reducer, actionType) +const orderReducer = (state: OrderState, reducer: OrderActions): OrderState => { + if (reducer.type === "setIncludesResource") { + const { payload } = reducer + // `include` is a union of what every mounted component needs, so it must accumulate + // rather than be replaced. `addResourceToInclude` is a free function with no access + // to the store, so it can only union against the `resourcesIncluded` it is handed — + // and most call sites don't pass it. Several components dispatch more than once from + // a single effect pass, all reading the same stale `include` snapshot, so a plain + // spread would let the last dispatch drop the others' resources. Missing an include + // is indistinguishable from an unset relationship in the API response, which turns + // a dropped resource into wrong application state rather than a failed request. + // + // An explicitly empty list is the one exception: it means "reset" (see the effect + // teardown in `useOrderState`), so it is passed through as-is. + if (payload.include != null && payload.include.length === 0) { + return { ...state, ...payload } + } + const merged = [...new Set([...(state.include ?? []), ...(payload.include ?? [])])] + return { + ...state, + ...payload, + // Keep `include` untouched when there is nothing to merge — callers that only + // report `includeLoaded` omit it, and turning `undefined` into `[]` would flip + // the `include?.length === 0` checks in `useOrderState`. + include: merged.length > 0 ? merged : state.include, + } + } + return baseReducer(state, reducer, actionType) +} export default orderReducer