fix(pleno-vue): rollback partial add-on sync when one POST rejects (#289)

Closes the mobile POS step 2 bug where only some of the selected primary product add-ons were persisted to the order.

Root cause: syncCurrentTransactionToOrder fired every add-on / additional-item POST in parallel via Promise.all. A single rejection short-circuited the batch while the rows that already landed stayed on the server; the operator saw only a generic error popup and on retry the half-synced state was visible.

Fix: extract the sync logic into a dedicated helper that uses Promise.allSettled, collects per-product failures, and rolls back every order_items row created in this attempt via Promise.allSettled before throwing OrderItemsPartialSyncError. The existing error-popup wiring from PR #282 surfaces the message unchanged. Also strips related_item_id from the idempotency comparison shapes so the placeholder "__PRIMARY__" does not break the short-circuit (every Fuldfør click previously rebuilt every order_items row).

Files:
- src/components/displays/department/pos/steps/mobile/utils/syncMobileOrderItems.js (new)
- src/components/displays/department/pos/steps/mobile/PosDepartmentStepMobile2.vue
- tests/unit/pos-mobile-step-2-addon-sync.spec.js (new, 13 tests)
- tests/e2e/pos-mobile-order-flow.spec.js (2 regression tests)
- tests/e2e/support/mobilePos.js (failureBudget.orderItemCreateForProductId knob)

Backend api was reviewed and confirmed correct; no api change is required.

Admin override used: E2E-pr-smoke-chromium-{desktop,mobile} Playwright containers hung past the documented 90-minute flake window — same known flake as PR #280 and PR #286. All other Required CI (format, lint, i18n, build, unit-fast, unit-serial, E2E-pr-changed/ct/pr both browsers, Qodana) passed.
This commit is contained in:
Jeppe B
2026-08-12 16:34:01 +02:00
committed by GitHub
parent 0b7efc3be5
commit 9c74c4d477
5 changed files with 953 additions and 173 deletions
+160
View File
@@ -3447,6 +3447,166 @@ test.describe("POS mobile order flow", () => {
expect(createdProductIds).toEqual([53, 71, 91]);
});
test("manual step 2 persists every selected primary product add-on (multi-addon happy path)", async ({ page }) => {
// Regression test for the bug where only some of the selected primary
// product add-ons were persisted to the order during mobile POS step 2.
// We select two add-ons on the primary and one standalone additional
// item, click Fuldfør, and assert that all four POST /order/items calls
// landed and that the server-side rows are linked correctly.
const orderId = 9426;
const fixture = createMobilePosFixture({
ordersById: {
[orderId]: buildRegularOrder(orderId, {
reference: "STEP2-MULTI-ADDON-REF",
reg_1: "ZZ00000",
}),
},
orderItemsByOrderId: {
[orderId]: [],
},
});
await setupMobilePosPage(page, fixture, {
token: "mobile-step2-multi-addon-happy-token",
seedState: {
customerId: REGULAR_CUSTOMER_ID,
reg: "ZZ00000",
reference: "STEP2-MULTI-ADDON-REF",
includePrimaryItem: false,
vehicleType: null,
lastOrderId: null,
},
route: {
step: 2,
orderId,
customerId: REGULAR_CUSTOMER_ID,
},
});
await selectPrimaryProduct(page, 53);
// Addons 71 and 41 are both attached to fixture product 53.
await expect(page.getByTestId("pos-mobile-addon-71")).toBeVisible({ timeout: 10_000 });
await page.getByTestId("pos-mobile-addon-71").click();
await expect(page.getByTestId("pos-mobile-addon-71-value")).toHaveText("1", { timeout: 10_000 });
await expect(page.getByTestId("pos-mobile-addon-41")).toBeVisible({ timeout: 10_000 });
await page.getByTestId("pos-mobile-addon-41").click();
await expect(page.getByTestId("pos-mobile-addon-41-value")).toHaveText("1", { timeout: 10_000 });
// One standalone additional item (id 91) so we also exercise the
// additionalItems path alongside the add-ons.
await longPressAdditionalItems(page);
await expect(page.getByTestId("pos-mobile-additional-items-selection")).toBeVisible({ timeout: 10_000 });
await page.getByTestId("pos-mobile-category-8").click();
await page.waitForTimeout(2_000);
await page.getByTestId("pos-mobile-product-91").click();
await expect
.poll(
async () => {
const snapshot = await getStoredPosSnapshot(page);
return (snapshot?.transactionItems?.additionalItems || [])
.map((item) => Number(item?.id))
.sort((left, right) => left - right);
},
{ timeout: 10_000 }
)
.toEqual([91]);
await page.getByTestId("pos-mobile-next-step").click();
// 1 primary + 2 addons + 1 additional = 4 POST /order/items calls.
await expect.poll(() => fixture.requestCounters.orderItemsPost, { timeout: 10_000 }).toBe(4);
await expect.poll(() => (fixture.orderItemsByOrderId[orderId] || []).length, { timeout: 10_000 }).toBe(4);
const createdProductIds = fixture.requestLog.orderItemCreates.map((entry) => Number(entry.product_id));
// The order of POSTs is: primary, then add-ons (in fixture order), then
// additional items. Add-ons 71 and 41 are listed before the additional
// item 91.
expect(createdProductIds).toEqual([53, 71, 41, 91]);
// The add-on rows must be linked to the primary by related_item_id.
const persistedItems = fixture.orderItemsByOrderId[orderId] || [];
const primary = persistedItems.find((item) => item.product_id === 53 && item.related_item_id === null);
expect(primary).toBeTruthy();
const addon71 = persistedItems.find((item) => item.product_id === 71);
const addon41 = persistedItems.find((item) => item.product_id === 41);
expect(addon71?.related_item_id).toBe(primary.id);
expect(addon41?.related_item_id).toBe(primary.id);
// The standalone additional item must NOT be linked to the primary.
const additional91 = persistedItems.find((item) => item.product_id === 91);
expect(additional91?.related_item_id).toBeNull();
});
test("mobile POS rolls back partial sync when one primary add-on POST fails", async ({ page }) => {
// Regression test for the bug where a single add-on POST rejection left
// the order with the primary and a subset of add-ons on the server while
// the operator only saw a generic failure popup. The sync helper now
// collects per-product failures via Promise.allSettled and rolls back
// every order_items row it created during this attempt.
const orderId = 9427;
const fixture = createMobilePosFixture({
failureBudget: {
orderItemCreateForProductId: { 41: 1 },
},
ordersById: {
[orderId]: buildRegularOrder(orderId, {
reference: "STEP2-PARTIAL-ROLLBACK-REF",
reg_1: "ZZ00000",
}),
},
orderItemsByOrderId: {
[orderId]: [],
},
});
await setupMobilePosPage(page, fixture, {
token: "mobile-step2-partial-rollback-token",
seedState: {
customerId: REGULAR_CUSTOMER_ID,
reg: "ZZ00000",
reference: "STEP2-PARTIAL-ROLLBACK-REF",
includePrimaryItem: false,
vehicleType: null,
lastOrderId: null,
},
route: {
step: 2,
orderId,
customerId: REGULAR_CUSTOMER_ID,
},
});
await selectPrimaryProduct(page, 53);
await expect(page.getByTestId("pos-mobile-addon-71")).toBeVisible({ timeout: 10_000 });
await page.getByTestId("pos-mobile-addon-71").click();
await expect(page.getByTestId("pos-mobile-addon-71-value")).toHaveText("1", { timeout: 10_000 });
await expect(page.getByTestId("pos-mobile-addon-41")).toBeVisible({ timeout: 10_000 });
await page.getByTestId("pos-mobile-addon-41").click();
await expect(page.getByTestId("pos-mobile-addon-41-value")).toHaveText("1", { timeout: 10_000 });
await page.getByTestId("pos-mobile-next-step").click();
// The error popup should appear with the per-product failure message.
const errorPopup = page.locator('[data-testid="pos-mobile-popup"][data-popup-id="error"]');
await expect(errorPopup).toBeVisible({ timeout: 10_000 });
await expect(errorPopup).toContainText("Product 41 rejected");
// The failure budget forced exactly one rejection on add-on 41.
expect(fixture.failureBudget.orderItemCreateForProductId[41]).toBe(0);
// Rollback runs after the partial failure: every order_items row that
// landed in this attempt must have been deleted, so the order ends
// up empty (no half-synced state) and ready for a clean retry.
await expect.poll(() => (fixture.orderItemsByOrderId[orderId] || []).length, { timeout: 10_000 }).toBe(0);
await expect.poll(() => fixture.requestCounters.orderItemsDelete, { timeout: 10_000 }).toBeGreaterThanOrEqual(2);
// The order is still open (not marked completed) because the failure
// happened before completion could run.
await expect.poll(() => fixture.requestCounters.markAsCompleted, { timeout: 1_000 }).toBe(0);
});
test("manual step 2 disables exact add-on and standalone product controls with tap tooltips", async ({ page }) => {
const orderId = 9414;
const fixture = createMobilePosFixture({
+23
View File
@@ -396,6 +396,13 @@ function createFailureBudget(overrides = {}) {
orderDelete: 0,
customerAttributesGet: 0,
orderItemCreate: 0,
// Per-product-id order-item creation failure budget. Keys are product
// ids, values are how many POST /order/items calls for that product
// should be rejected with 400. Used by regression tests for the mobile
// POS step 2 partial-sync rollback (see
// tests/unit/pos-mobile-step-2-addon-sync.spec.js for the matching
// unit-level contract).
orderItemCreateForProductId: {},
...overrides,
};
}
@@ -2002,6 +2009,22 @@ export async function mockMobilePosApi(page, fixture) {
}
const orderId = toPositiveInteger(body.order_id);
const productId = toPositiveInteger(body.product_id);
const perProductBudget = fixture.failureBudget.orderItemCreateForProductId || {};
if (perProductBudget[productId] > 0) {
perProductBudget[productId] -= 1;
await route.fulfill(
json(
{
success: false,
data: {
message: `Product ${productId} rejected`,
},
},
400
)
);
return;
}
const order = fixture.ordersById[orderId];
const product = getProductById(fixture, productId);
if (!order || !product) {
@@ -0,0 +1,325 @@
// @vitest-environment jsdom
import { beforeEach, describe, expect, it, vi } from "vitest";
import {
OrderItemsPartialSyncError,
buildDesiredOrderItemShapes,
extractErrorMessage,
formatFailureFragment,
normalizeExistingOrderItemShapes,
syncMobileOrderItems,
} from "@/components/displays/department/pos/steps/mobile/utils/syncMobileOrderItems.js";
const buildPrimary = (overrides = {}) => ({
id: 53,
name: "Trækker",
price: 599,
notes: "",
skip_price_override: false,
addons: [
{
id: 71,
quantity: 1,
product: { id: 71, price: 99 },
},
{
id: 41,
quantity: 2,
product: { id: 41, price: 25 },
},
],
...overrides,
});
const buildAdditional = (overrides = {}) => ({
id: 91,
price: 120,
quantity: 1,
...overrides,
});
const makeApi = ({ createOrderItem, getOrderItems, removeOrderItem } = {}) => ({
createOrderItem: createOrderItem ?? vi.fn(),
getOrderItems: getOrderItems ?? vi.fn(),
removeOrderItem: removeOrderItem ?? vi.fn(async () => ({})),
});
describe("normalizeExistingOrderItemShapes", () => {
it("treats the first related_item_id=null row as primary and links children", () => {
const shapes = normalizeExistingOrderItemShapes(
[
{ id: 100, product_id: 53, product: { id: 53 }, quantity: 1, related_item_id: null, price: 599, notes: "" },
{ id: 101, product_id: 71, product: { id: 71 }, quantity: 1, related_item_id: 100, price: 99, notes: "" },
{ id: 102, product_id: 41, product: { id: 41 }, quantity: 2, related_item_id: 100, price: 25, notes: "" },
{ id: 103, product_id: 91, product: { id: 91 }, quantity: 1, related_item_id: null, price: 120, notes: "" },
],
53
);
// related_item_id is intentionally excluded from the comparison shape —
// both the desired-shape placeholder and the real numeric id map to the
// same idempotency bucket.
expect(shapes).toEqual([
{ product_id: 53, quantity: 1, price: 599, notes: "" },
{ product_id: 71, quantity: 1, price: 99, notes: "" },
{ product_id: 41, quantity: 2, price: 25, notes: "" },
{ product_id: 91, quantity: 1, price: 120, notes: "" },
]);
});
it("returns an empty list when there is no primary row", () => {
expect(normalizeExistingOrderItemShapes([{ id: 101, product_id: 71, related_item_id: 999 }], 53)).toEqual([]);
});
});
describe("buildDesiredOrderItemShapes", () => {
it("filters out add-ons with quantity 0 and uses the placeholder related_item_id", () => {
const primary = buildPrimary({
addons: [
{ id: 71, quantity: 0, product: { id: 71, price: 99 } },
{ id: 41, quantity: 2, product: { id: 41, price: 25 } },
],
});
const shapes = buildDesiredOrderItemShapes({ primaryItem: primary, additionalItems: [] });
expect(shapes).toHaveLength(2); // primary + 1 addon
expect(shapes[1]).toMatchObject({
kind: "addon",
product_id: 41,
quantity: 2,
related_item_id: "__PRIMARY__",
price: 25,
});
});
it("includes standalone additional items with quantity > 0", () => {
const shapes = buildDesiredOrderItemShapes({
primaryItem: buildPrimary(),
additionalItems: [buildAdditional({ id: 91, quantity: 1 })],
});
expect(shapes).toHaveLength(4); // primary + 2 addons + 1 additional
expect(shapes.at(-1)).toMatchObject({
kind: "additional",
product_id: 91,
related_item_id: null,
});
});
it("honours the restriction predicates", () => {
const isAddonRestricted = (addon) => addon.id === 41;
const shapes = buildDesiredOrderItemShapes({
primaryItem: buildPrimary(),
additionalItems: [],
isAddonRestricted,
});
expect(shapes).toHaveLength(2); // primary + addon 71 only
expect(shapes.map((shape) => shape.product_id)).toEqual([53, 71]);
});
});
describe("extractErrorMessage and formatFailureFragment", () => {
it("unwraps axios-shaped error responses", () => {
expect(
extractErrorMessage({
response: { data: { data: { message: "Notes is required for this product" } } },
})
).toBe("Notes is required for this product");
});
it("falls back to plain Error messages when the response shape is unknown", () => {
expect(extractErrorMessage(new Error("boom"))).toBe("boom");
expect(extractErrorMessage(undefined)).toBe("Unknown error");
});
it("prefixes the product id when one is available", () => {
expect(formatFailureFragment({ productId: 41, message: "rejected" })).toBe("Product 41: rejected");
expect(formatFailureFragment({ productId: 0, message: "no id" })).toBe("no id");
});
});
describe("syncMobileOrderItems", () => {
beforeEach(() => {
vi.clearAllMocks();
});
it("creates the primary and every add-on plus additional item on the happy path", async () => {
const createdIds = { value: 100 };
const api = makeApi({
getOrderItems: vi.fn(async () => ({ data: { data: [] } })),
createOrderItem: vi.fn(async (_orderId, _productId) => {
const id = ++createdIds.value;
return { data: { data: { id } } };
}),
removeOrderItem: vi.fn(async () => ({})),
});
const result = await syncMobileOrderItems({
orderId: 9402,
primaryItem: buildPrimary(),
additionalItems: [buildAdditional({ id: 91, quantity: 1 })],
api,
});
// 1 primary + 2 addons + 1 additional = 4 POSTs
expect(api.createOrderItem).toHaveBeenCalledTimes(4);
const postedProductIds = api.createOrderItem.mock.calls.map(([, productId]) => productId);
expect(postedProductIds).toEqual([53, 71, 41, 91]);
// Addons pass the created primary id as related_item_id
expect(api.createOrderItem.mock.calls[1][3]).toBe(result.createdPrimaryItemId);
expect(api.createOrderItem.mock.calls[2][3]).toBe(result.createdPrimaryItemId);
// Standalone additional passes null
expect(api.createOrderItem.mock.calls[3][3]).toBeNull();
expect(api.removeOrderItem).not.toHaveBeenCalled();
expect(result.createdIds).toHaveLength(4);
});
it("rolls back every order item it created when one add-on POST rejects", async () => {
let call = 0;
const api = makeApi({
getOrderItems: vi.fn(async () => ({ data: { data: [] } })),
createOrderItem: vi.fn(async () => {
call += 1;
// call 1 = primary (id 101), call 2 = addon 71 (id 102),
// call 3 = addon 41 (reject), call 4+ never happen.
if (call === 3) {
const reason = new Error("request failed");
reason.response = { data: { data: { message: "Product 41 rejected" } } };
throw reason;
}
return { data: { data: { id: 100 + call } } };
}),
removeOrderItem: vi.fn(async () => ({})),
});
let captured;
try {
await syncMobileOrderItems({
orderId: 9403,
primaryItem: buildPrimary(),
additionalItems: [],
api,
});
} catch (error) {
captured = error;
}
expect(captured).toBeInstanceOf(OrderItemsPartialSyncError);
// 1 primary + 2 addons attempted = 3 POSTs.
expect(api.createOrderItem).toHaveBeenCalledTimes(3);
// Rollback runs against every row we created in this attempt:
// the primary (101) AND the first add-on (102).
const rolledBack = api.removeOrderItem.mock.calls.map(([id]) => id).sort((a, b) => a - b);
expect(rolledBack).toEqual([101, 102]);
expect(captured.message).toBe("Product 41: Product 41 rejected");
expect(captured.failures).toEqual([{ productId: 41, message: "Product 41 rejected" }]);
expect(captured.rolledBackIds).toEqual(expect.arrayContaining([101, 102]));
});
it("still throws when the primary POST rejects — the failure surfaces in the error", async () => {
const api = makeApi({
getOrderItems: vi.fn(async () => ({ data: { data: [] } })),
createOrderItem: vi.fn(async () => {
const reason = new Error("primary failed");
reason.response = { data: { data: { message: "Primary blocked" } } };
throw reason;
}),
removeOrderItem: vi.fn(async () => ({})),
});
await expect(
syncMobileOrderItems({
orderId: 9404,
primaryItem: buildPrimary(),
additionalItems: [],
api,
})
).rejects.toBeInstanceOf(OrderItemsPartialSyncError);
});
it("is idempotent: skips work when the server already matches the desired shapes", async () => {
const primaryRow = {
id: 500,
product_id: 53,
product: { id: 53 },
quantity: 1,
related_item_id: null,
price: 599,
notes: "",
};
const api = makeApi({
getOrderItems: vi.fn(async () => ({
data: {
data: [
primaryRow,
{
id: 501,
product_id: 71,
product: { id: 71 },
quantity: 1,
related_item_id: 500,
price: 99,
notes: "",
},
{
id: 502,
product_id: 41,
product: { id: 41 },
quantity: 2,
related_item_id: 500,
price: 25,
notes: "",
},
],
},
})),
createOrderItem: vi.fn(),
removeOrderItem: vi.fn(),
});
const result = await syncMobileOrderItems({
orderId: 9405,
primaryItem: buildPrimary(),
additionalItems: [],
api,
});
expect(api.createOrderItem).not.toHaveBeenCalled();
expect(api.removeOrderItem).not.toHaveBeenCalled();
expect(result.createdPrimaryItemId).toBeNull();
});
it("forces a recreate when any desired shape carries skip_price_override=true", async () => {
const primaryRow = {
id: 600,
product_id: 53,
product: { id: 53 },
quantity: 1,
related_item_id: null,
price: 599,
notes: "",
};
const api = makeApi({
getOrderItems: vi.fn(async () => ({
data: {
data: [primaryRow],
},
})),
createOrderItem: vi.fn(async () => ({ data: { data: { id: 700 } } })),
removeOrderItem: vi.fn(async () => ({})),
});
await syncMobileOrderItems({
orderId: 9406,
primaryItem: buildPrimary({ skip_price_override: true, price: 599 }),
additionalItems: [],
api,
});
expect(api.removeOrderItem).toHaveBeenCalledWith(600);
expect(api.createOrderItem).toHaveBeenCalled();
});
});