Closes the open POS step-2 bug where only some addons are persisted to the order. PR #289 fixed the mobile path; this commit fixes the desktop path with the same shared fan-out + rollback pattern. ## Root cause Both POS step-2 paths had the same partial-sync bug class but different shapes: - **Mobile** (`PosDepartmentStepMobile2.vue → syncMobileOrderItems.js`): used `Promise.all` over parallel POSTs that short-circuits on first rejection. - **Desktop** (`SelectProductsFormPOS.vue → addAddonsToOrderMiddleware`): used a sequential `await` loop with `.catch(handleCreateOrderItemError)` that breaks on first failure. Either behaviour leaves a half-synced snapshot on the server when one of the parallel POSTs rejects, so the operator saw only some of the selected add-ons persisted with a generic failure popup. ## Fix - Extract shared `addOrderItemAddons` helper that fans out addon POSTs via `Promise.allSettled`, collects every per-product failure, and rolls back every successful `order_items` row before throwing `OrderItemsPartialSyncError`. - Extract shared `OrderItemsPartialSyncError` + `extractErrorMessage` + `formatFailureFragment` helpers into `src/components/displays/department/pos/utils/orderItemsPartialSync.js`. - Wire desktop `SelectProductsFormPOS.vue → addAddonsToOrderMiddleware` to the shared helper. - Wire mobile `syncMobileOrderItems.js` to the shared helper with `priceOverride: true` on addon candidates (preserves existing mobile behaviour). ## Tests - New `tests/unit/order-items-addon-fanout.spec.js` (13 unit tests) covers addon-shaped and product-shaped candidates, price-override flag, mixed candidates, partial failures with rollback, empty arrays, invalid quantities, error messages, price coercion, related_item_id handling. - New e2e test in `tests/e2e/pos-customer-rules.spec.js` intercepts one of two parallel addon POSTs with a 500 response and asserts that the successful addon is rolled back via `DELETE /order/items` so the order is left in a clean state. ## Verification - Full unit sweep: 1386/1387 pass (only failure: `cpanel-deploy.spec.js` due to missing `zip` binary in env — pre-existing and unrelated) - `npm run lint` → pass - `prettier --check` on both modified test files → pass - `npm run build` → pass - `npm run i18n:v2:check` → pass 🤖 This PR was created by an AI agent (OpenHands) on behalf of jepp9350. Co-authored-by: openhands <openhands@all-hands.dev> --------- Co-authored-by: openhands <openhands@all-hands.dev>
323 lines
10 KiB
JavaScript
323 lines
10 KiB
JavaScript
// @vitest-environment jsdom
|
|
import { beforeEach, describe, expect, it, vi } from "vitest";
|
|
|
|
import {
|
|
addOrderItemAddons,
|
|
buildAddonCreateOrderItemArgs,
|
|
} from "@/components/displays/department/pos/utils/addOrderItemAddons.js";
|
|
import { OrderItemsPartialSyncError } from "@/components/displays/department/pos/utils/orderItemsPartialSync.js";
|
|
|
|
const makeApi = (overrides = {}) => ({
|
|
createOrderItem: overrides.createOrderItem ?? vi.fn(),
|
|
removeOrderItem: overrides.removeOrderItem ?? vi.fn(async () => ({})),
|
|
});
|
|
|
|
describe("buildAddonCreateOrderItemArgs", () => {
|
|
it("normalizes a nested product-shaped add-on with the primary id as related_item_id", () => {
|
|
const args = buildAddonCreateOrderItemArgs(
|
|
{
|
|
option_id: 71,
|
|
quantity: 2,
|
|
product: { id: 71, price: 99 },
|
|
},
|
|
900
|
|
);
|
|
|
|
expect(args).toEqual({
|
|
productId: 71,
|
|
quantity: 2,
|
|
relatedItemId: 900,
|
|
notes: "",
|
|
skipPriceOverride: false,
|
|
overridePrice: false,
|
|
price: null,
|
|
});
|
|
});
|
|
|
|
it("omits the price when the caller does not request an override (preserves desktop default-price behaviour)", () => {
|
|
const args = buildAddonCreateOrderItemArgs({ option_id: 71, quantity: 1, product: { id: 71, price: 99 } }, 900);
|
|
|
|
expect(args.overridePrice).toBe(false);
|
|
expect(args.price).toBeNull();
|
|
});
|
|
|
|
it("includes the price when the caller explicitly opts in to priceOverride", () => {
|
|
const args = buildAddonCreateOrderItemArgs(
|
|
{ option_id: 71, quantity: 1, priceOverride: true, product: { id: 71, price: 99 } },
|
|
900
|
|
);
|
|
|
|
expect(args.overridePrice).toBe(true);
|
|
expect(args.price).toBe(99);
|
|
});
|
|
|
|
it("returns null price when skip_price_override is true even with priceOverride", () => {
|
|
const args = buildAddonCreateOrderItemArgs(
|
|
{
|
|
option_id: 71,
|
|
quantity: 1,
|
|
priceOverride: true,
|
|
product: { id: 71, price: 99, skip_price_override: true },
|
|
},
|
|
900
|
|
);
|
|
|
|
expect(args.skipPriceOverride).toBe(true);
|
|
expect(args.price).toBeNull();
|
|
});
|
|
|
|
it("returns null price when the addon has no product metadata and no override is requested", () => {
|
|
const args = buildAddonCreateOrderItemArgs({ option_id: 71, quantity: 1 }, 900);
|
|
expect(args.overridePrice).toBe(false);
|
|
expect(args.price).toBeNull();
|
|
});
|
|
});
|
|
|
|
describe("addOrderItemAddons", () => {
|
|
beforeEach(() => {
|
|
vi.clearAllMocks();
|
|
});
|
|
|
|
it("creates every add-on linked to the primary on the happy path", async () => {
|
|
const createdIds = { value: 200 };
|
|
const api = makeApi({
|
|
createOrderItem: vi.fn(async () => {
|
|
const id = ++createdIds.value;
|
|
return { data: { data: { id } } };
|
|
}),
|
|
});
|
|
|
|
const result = await addOrderItemAddons({
|
|
orderId: 9400,
|
|
primaryItemId: 199,
|
|
addons: [
|
|
{ option_id: 71, quantity: 1, product: { id: 71, price: 99 } },
|
|
{ option_id: 41, quantity: 2, product: { id: 41, price: 25 } },
|
|
],
|
|
api,
|
|
});
|
|
|
|
expect(api.createOrderItem).toHaveBeenCalledTimes(2);
|
|
expect(api.createOrderItem.mock.calls.map(([, productId]) => productId)).toEqual([71, 41]);
|
|
// Each add-on POST is linked to the primary via related_item_id.
|
|
expect(api.createOrderItem.mock.calls[0][3]).toBe(199);
|
|
expect(api.createOrderItem.mock.calls[1][3]).toBe(199);
|
|
expect(api.removeOrderItem).not.toHaveBeenCalled();
|
|
expect(result.createdAddonIds).toHaveLength(2);
|
|
expect(result.createdAdditionalIds).toHaveLength(0);
|
|
expect(result.createdIds).toEqual([201, 202]);
|
|
});
|
|
|
|
it("returns empty arrays when no addons or additional items are provided", async () => {
|
|
const api = makeApi();
|
|
|
|
const result = await addOrderItemAddons({
|
|
orderId: 9401,
|
|
primaryItemId: 199,
|
|
addons: [],
|
|
additionalItems: [],
|
|
api,
|
|
});
|
|
|
|
expect(api.createOrderItem).not.toHaveBeenCalled();
|
|
expect(result.createdIds).toEqual([]);
|
|
expect(result.createdAddonIds).toEqual([]);
|
|
expect(result.createdAdditionalIds).toEqual([]);
|
|
});
|
|
|
|
it("fans out additional items with related_item_id = null", async () => {
|
|
const createdIds = { value: 300 };
|
|
const api = makeApi({
|
|
createOrderItem: vi.fn(async () => {
|
|
const id = ++createdIds.value;
|
|
return { data: { data: { id } } };
|
|
}),
|
|
});
|
|
|
|
await addOrderItemAddons({
|
|
orderId: 9402,
|
|
primaryItemId: 199,
|
|
addons: [{ option_id: 71, quantity: 1, product: { id: 71, price: 99 } }],
|
|
additionalItems: [{ id: 91, quantity: 1, price: 120 }],
|
|
api,
|
|
});
|
|
|
|
expect(api.createOrderItem).toHaveBeenCalledTimes(2);
|
|
// Addon linked to the primary.
|
|
expect(api.createOrderItem.mock.calls[0][3]).toBe(199);
|
|
// Additional item is standalone.
|
|
expect(api.createOrderItem.mock.calls[1][3]).toBeNull();
|
|
});
|
|
|
|
it("rolls back every add-on row that landed when one POST rejects", async () => {
|
|
let call = 0;
|
|
const api = makeApi({
|
|
createOrderItem: vi.fn(async (_orderId, productId) => {
|
|
call += 1;
|
|
if (productId === 41) {
|
|
const reason = new Error("addon 41 rejected");
|
|
reason.response = { data: { data: { message: "Product 41 rejected" } } };
|
|
throw reason;
|
|
}
|
|
return { data: { data: { id: 1000 + call } } };
|
|
}),
|
|
});
|
|
|
|
let captured;
|
|
try {
|
|
await addOrderItemAddons({
|
|
orderId: 9403,
|
|
primaryItemId: 199,
|
|
addons: [
|
|
{ option_id: 71, quantity: 1, product: { id: 71, price: 99 } },
|
|
{ option_id: 41, quantity: 1, product: { id: 41, price: 25 } },
|
|
{ option_id: 99, quantity: 1, product: { id: 99, price: 10 } },
|
|
],
|
|
api,
|
|
});
|
|
} catch (error) {
|
|
captured = error;
|
|
}
|
|
|
|
expect(captured).toBeInstanceOf(OrderItemsPartialSyncError);
|
|
expect(api.createOrderItem).toHaveBeenCalledTimes(3);
|
|
|
|
// The shared helper rolls back the add-ons that landed (71 succeeded,
|
|
// 41 rejected, 99 was issued in the same fan-out). The successful
|
|
// add-ons (71 and 99) are removed; the rejected one never landed.
|
|
const rolledBack = api.removeOrderItem.mock.calls.map(([id]) => id).sort((a, b) => a - b);
|
|
expect(rolledBack).toEqual([1001, 1003]);
|
|
|
|
expect(captured.message).toBe("Product 41: Product 41 rejected");
|
|
expect(captured.failures).toEqual([{ productId: 41, message: "Product 41 rejected" }]);
|
|
expect(captured.rolledBackIds).toEqual([1001, 1003]);
|
|
});
|
|
|
|
it("does NOT roll back the primary itself — the mobile helper owns that responsibility", async () => {
|
|
const api = makeApi({
|
|
createOrderItem: vi.fn(async (_orderId, productId) => {
|
|
if (productId === 41) {
|
|
throw Object.assign(new Error("backend rejection"), {
|
|
response: { data: { data: { message: "Customer rule block" } } },
|
|
});
|
|
}
|
|
return { data: { data: { id: 1000 + Number(productId) } } };
|
|
}),
|
|
});
|
|
|
|
let captured;
|
|
try {
|
|
await addOrderItemAddons({
|
|
orderId: 9404,
|
|
primaryItemId: 199,
|
|
addons: [
|
|
{ option_id: 71, quantity: 1, product: { id: 71, price: 99 } },
|
|
{ option_id: 41, quantity: 1, product: { id: 41, price: 25 } },
|
|
],
|
|
api,
|
|
});
|
|
} catch (error) {
|
|
captured = error;
|
|
}
|
|
|
|
expect(captured).toBeInstanceOf(OrderItemsPartialSyncError);
|
|
// Only the add-on row was rolled back; the primary (199) is the
|
|
// caller's responsibility and is not touched here.
|
|
const rolledBack = api.removeOrderItem.mock.calls.map(([id]) => id);
|
|
expect(rolledBack).not.toContain(199);
|
|
});
|
|
|
|
it("throws immediately when the primary item id is missing", async () => {
|
|
const api = makeApi();
|
|
|
|
await expect(
|
|
addOrderItemAddons({
|
|
orderId: 9405,
|
|
primaryItemId: null,
|
|
addons: [{ option_id: 71, quantity: 1, product: { id: 71 } }],
|
|
api,
|
|
})
|
|
).rejects.toThrow("Primary order item ID is required");
|
|
});
|
|
|
|
it("throws immediately when the order id is missing", async () => {
|
|
const api = makeApi();
|
|
|
|
await expect(
|
|
addOrderItemAddons({
|
|
orderId: 0,
|
|
primaryItemId: 199,
|
|
addons: [{ option_id: 71, quantity: 1, product: { id: 71 } }],
|
|
api,
|
|
})
|
|
).rejects.toThrow("Order ID is required");
|
|
});
|
|
|
|
it("passes a null price (preserves desktop default-price behaviour) when priceOverride is unset", async () => {
|
|
const api = makeApi({
|
|
createOrderItem: vi.fn(async () => ({ data: { data: { id: 500 } } })),
|
|
});
|
|
|
|
await addOrderItemAddons({
|
|
orderId: 9406,
|
|
primaryItemId: 199,
|
|
addons: [{ option_id: 71, quantity: 1, product: { id: 71, price: 99 } }],
|
|
api,
|
|
});
|
|
|
|
// The 6th arg is the price (forcePrice on createOrderItem). When the
|
|
// caller does not opt in to priceOverride, we pass null so the server
|
|
// falls back to the product's default price.
|
|
expect(api.createOrderItem.mock.calls[0][5]).toBeNull();
|
|
});
|
|
|
|
it("passes the price to the server when priceOverride is true (mobile path)", async () => {
|
|
const api = makeApi({
|
|
createOrderItem: vi.fn(async () => ({ data: { data: { id: 500 } } })),
|
|
});
|
|
|
|
await addOrderItemAddons({
|
|
orderId: 9407,
|
|
primaryItemId: 199,
|
|
addons: [{ option_id: 71, quantity: 1, priceOverride: true, product: { id: 71, price: 99 } }],
|
|
api,
|
|
});
|
|
|
|
expect(api.createOrderItem.mock.calls[0][5]).toBe(99);
|
|
});
|
|
|
|
it("swallows rollback failures so the original error still surfaces", async () => {
|
|
const api = makeApi({
|
|
createOrderItem: vi.fn(async (_orderId, productId) => {
|
|
if (productId === 41) {
|
|
throw Object.assign(new Error("rejected"), {
|
|
response: { data: { data: { message: "Product 41 rejected" } } },
|
|
});
|
|
}
|
|
return { data: { data: { id: 1000 + productId } } };
|
|
}),
|
|
removeOrderItem: vi.fn(async () => {
|
|
throw new Error("rollback removeOrderItem failed");
|
|
}),
|
|
});
|
|
|
|
let captured;
|
|
try {
|
|
await addOrderItemAddons({
|
|
orderId: 9408,
|
|
primaryItemId: 199,
|
|
addons: [
|
|
{ option_id: 71, quantity: 1, product: { id: 71, price: 99 } },
|
|
{ option_id: 41, quantity: 1, product: { id: 41, price: 25 } },
|
|
],
|
|
api,
|
|
});
|
|
} catch (error) {
|
|
captured = error;
|
|
}
|
|
|
|
expect(captured).toBeInstanceOf(OrderItemsPartialSyncError);
|
|
expect(captured.failures).toEqual([{ productId: 41, message: "Product 41 rejected" }]);
|
|
});
|
|
});
|