Accumulate state.include instead of replacing it

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<OrderState>, 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",
+    ])
+  })
+})

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<typeof import("@commercelayer/core-components")>()
+  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<OrderState>, 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()
+  })
+})

packages/react-components/src/reducers/OrderReducer.ts

@@ -798,7 +798,34 @@ export const orderInitialState: Partial<OrderState> = {
   withoutIncludes: true,
 }
 
-const orderReducer = (state: OrderState, reducer: OrderActions): OrderState =>
-  baseReducer<OrderState, OrderActions, OrderActionType[]>(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<OrderState, OrderActions, OrderActionType[]>(state, reducer, actionType)
+}
 
 export default orderReducer