From 9c74c4d4770ced456ee93b0cc358840a3a94268f Mon Sep 17 00:00:00 2001 From: Jeppe B <2jepp9350@gmail.com> Date: Wed, 12 Aug 2026 16:34:01 +0200 Subject: [PATCH] fix(pleno-vue): rollback partial add-on sync when one POST rejects (#289) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../steps/mobile/PosDepartmentStepMobile2.vue | 228 +++------- .../mobile/utils/syncMobileOrderItems.js | 390 ++++++++++++++++++ tests/e2e/pos-mobile-order-flow.spec.js | 160 +++++++ tests/e2e/support/mobilePos.js | 23 ++ .../unit/pos-mobile-step-2-addon-sync.spec.js | 325 +++++++++++++++ 5 files changed, 953 insertions(+), 173 deletions(-) create mode 100644 src/components/displays/department/pos/steps/mobile/utils/syncMobileOrderItems.js create mode 100644 tests/unit/pos-mobile-step-2-addon-sync.spec.js diff --git a/src/components/displays/department/pos/steps/mobile/PosDepartmentStepMobile2.vue b/src/components/displays/department/pos/steps/mobile/PosDepartmentStepMobile2.vue index b693ba44..85810ca6 100644 --- a/src/components/displays/department/pos/steps/mobile/PosDepartmentStepMobile2.vue +++ b/src/components/displays/department/pos/steps/mobile/PosDepartmentStepMobile2.vue @@ -38,17 +38,18 @@ import { reg_2, reg_3, department_id, - customer_id, + customer_id, customer_attributes, customer_attributes_status, - customer_name, - getAddonRestriction, - getProductRestriction, + customer_name, + getAddonRestriction, + getProductRestriction, retryCustomerAttributes, - registerPosStepSaveBarrier, - saveOrderMetadataField, - } from "@/components/shop/POSDepartmentProcess.vue"; + registerPosStepSaveBarrier, + saveOrderMetadataField, +} from "@/components/shop/POSDepartmentProcess.vue"; import { createOrderItem, getOrderItems, removeOrderItem } from "@/components/shop/OrdersItems.vue"; +import { syncMobileOrderItems } from "@/components/displays/department/pos/steps/mobile/utils/syncMobileOrderItems.js"; import { PosProduct } from "@/components/displays/department/pos/steps/mobile/objects/PosProduct.vue"; import PosDepartmentStepMobileButtonClearAll from "@/components/displays/department/pos/steps/mobile/elements/PosDepartmentStepMobileButtonClearAll.vue"; import PosDepartmentStepMobileFixedBottomControl from "@/components/displays/department/pos/steps/mobile/elements/PosDepartmentStepMobileFixedBottomControl.vue"; @@ -345,14 +346,14 @@ const applyPendingBookingFromSelection = async () => { effectivePrimaryProduct = firstWash; } } - effectivePrimaryProduct.addons = preparedAddons as any; - transactionItems.setPrimaryItem(effectivePrimaryProduct as any); - if (transactionItems.primaryItem.value) { - transactionItems.primaryItem.value.addons = preparedAddons as any; - } - sanitizeRestrictedTransactionItems(); + effectivePrimaryProduct.addons = preparedAddons as any; + transactionItems.setPrimaryItem(effectivePrimaryProduct as any); + if (transactionItems.primaryItem.value) { + transactionItems.primaryItem.value.addons = preparedAddons as any; + } + sanitizeRestrictedTransactionItems(); - lastAppliedBookingId.value = booking.id; + lastAppliedBookingId.value = booking.id; //console.warn('Applied pending booking to cart (primary + addons):', booking.id, primaryProduct, preparedAddons); //console.warn('Current transaction items after applying booking:', transactionItems.primaryItem.value); lastFetchedPrimaryItemProduct.value = effectivePrimaryProduct; // Update last fetched primary item @@ -885,83 +886,12 @@ watch( { immediate: true } ); -const normalizeOrderItemShape = (item: any) => ({ - product_id: Number(item?.product_id ?? item?.product?.id ?? 0), - quantity: Number(item?.quantity ?? 0), - related_item_id: - item?.related_item_id === null || item?.related_item_id === undefined ? null : Number(item.related_item_id), - price: Number(item?.price ?? 0), - notes: String(item?.notes ?? ""), -}); - -const buildDesiredOrderItemShapes = () => { - if (!transactionItems.primaryItem.value) { - return []; - } - - const primaryShape = { - kind: "primary", - relatedKey: "primary", - product_id: Number(transactionItems.primaryItem.value.id), - quantity: 1, - related_item_id: null, - price: Number(transactionItems.primaryItem.value.price ?? 0), - notes: String(transactionItems.primaryItem.value?.notes ?? ""), - skip_price_override: transactionItems.primaryItem.value?.skip_price_override === true, - }; - - const addonShapes = (transactionItems.primaryItem.value.addons || []) - .filter((addon: any) => Number(addon?.quantity ?? 0) > 0 && !isMobileAddonRestricted(addon)) - .map((addon: any) => { - const addonProduct = addon?.product ?? addon; - return { - kind: "addon", - relatedKey: "primary", - product_id: Number(addonProduct?.id ?? addon?.id ?? 0), - quantity: Number(addon?.quantity ?? 0), - related_item_id: "__PRIMARY__", - price: Number(addonProduct?.price ?? addon?.price ?? 0), - notes: String(addonProduct?.notes ?? addon?.notes ?? ""), - skip_price_override: addonProduct?.skip_price_override === true || addon?.skip_price_override === true, - }; - }); - - const additionalShapes = (transactionItems.additionalItems.value || []) - .filter((item: any) => Number(item?.quantity ?? 0) > 0 && !isStandaloneAdditionalItemRestricted(item)) - .map((item: any) => ({ - kind: "additional", - relatedKey: null, - product_id: Number(item?.id ?? 0), - quantity: Number(item?.quantity ?? 0), - related_item_id: null, - price: Number(item?.price ?? 0), - notes: String(item?.notes ?? ""), - skip_price_override: item?.skip_price_override === true, - })); - - return [primaryShape, ...addonShapes, ...additionalShapes]; -}; - -const normalizeExistingOrderItemShapes = (items: any[]) => { - const primaryItems = items.filter( - (item: any) => item?.related_item_id === null || item?.related_item_id === undefined - ); - if (primaryItems.length === 0) { - return []; - } - - const additionalItems = primaryItems.filter( - (item: any) => - Number(item?.product?.id ?? item?.product_id ?? 0) !== Number(transactionItems.primaryItem.value?.id ?? 0) - ); - const primaryItemShape = normalizeOrderItemShape(primaryItems[0]); - const addonShapes = items - .filter((item: any) => item?.related_item_id === primaryItems[0]?.id) - .map(normalizeOrderItemShape); - const additionalShapes = additionalItems.map(normalizeOrderItemShape); - - return [primaryItemShape, ...addonShapes, ...additionalShapes]; -}; +// The pre-existing helpers buildDesiredOrderItemShapes and +// normalizeExistingOrderItemShapes (and their shared normalizeOrderItemShape +// normalizer) used to live here. They have been extracted to +// src/components/displays/department/pos/steps/mobile/utils/syncMobileOrderItems.js +// so they can be unit-tested in isolation and so the same comparison logic +// is used by syncCurrentTransactionToOrder and any future caller. const sortComparableLastWashShapes = (shapes: Array<{ kind: string; product_id: number; quantity: number }>) => shapes.slice().sort((left, right) => { @@ -988,8 +918,8 @@ const buildCurrentSelectionComparableShapes = () => { }; const addonShapes = sortComparableLastWashShapes( - (transactionItems.primaryItem.value.addons || []) - .filter((addon: any) => Number(addon?.quantity ?? 0) > 0 && !isMobileAddonRestricted(addon)) + (transactionItems.primaryItem.value.addons || []) + .filter((addon: any) => Number(addon?.quantity ?? 0) > 0 && !isMobileAddonRestricted(addon)) .map((addon: any) => ({ kind: "addon", product_id: Number(addon?.product?.id ?? addon?.id ?? 0), @@ -998,8 +928,8 @@ const buildCurrentSelectionComparableShapes = () => { ); const additionalShapes = sortComparableLastWashShapes( - (transactionItems.additionalItems.value || []) - .filter((item: any) => Number(item?.quantity ?? 0) > 0 && !isStandaloneAdditionalItemRestricted(item)) + (transactionItems.additionalItems.value || []) + .filter((item: any) => Number(item?.quantity ?? 0) > 0 && !isStandaloneAdditionalItemRestricted(item)) .map((item: any) => ({ kind: "additional", product_id: Number(item?.id ?? 0), @@ -1074,65 +1004,19 @@ const syncCurrentTransactionToOrder = async () => { return false; } - const existingItemsResponse = await getOrderItems(normalizedOrderId); - const existingItems = Array.isArray(existingItemsResponse?.data?.data) ? existingItemsResponse.data.data : []; - - const desiredShapes = buildDesiredOrderItemShapes(); - const currentShapes = normalizeExistingOrderItemShapes(existingItems); - const shouldForceRecreateForRepricing = desiredShapes.some((shape) => shape.skip_price_override === true); - const comparableDesiredShapes = desiredShapes.map(({ - kind: _kind, - relatedKey: _relatedKey, - skip_price_override: _skipPriceOverride, - ...shape - }) => shape); - - if (!shouldForceRecreateForRepricing && JSON.stringify(currentShapes) === JSON.stringify(comparableDesiredShapes)) { - return true; - } - - await Promise.all(existingItems.map((item: any) => removeOrderItem(item.id))); - - const createdPrimaryItemResponse = await createOrderItem( - normalizedOrderId, - transactionItems.primaryItem.value.id, - 1, - null, - transactionItems.primaryItem.value?.notes || "", - transactionItems.primaryItem.value.skip_price_override === true ? null : transactionItems.primaryItem.value.price - ); - const createdPrimaryItemId = createdPrimaryItemResponse?.data?.data?.id; - - const addonPromises = (transactionItems.primaryItem.value.addons || []) - .filter((addon: any) => Number(addon?.quantity ?? 0) > 0 && !isMobileAddonRestricted(addon)) - .map((addon: any) => { - const addonProduct = addon?.product ?? addon; - return createOrderItem( - normalizedOrderId, - addonProduct.id, - Number(addon.quantity), - createdPrimaryItemId, - addonProduct?.notes || addon?.notes || "", - addonProduct?.skip_price_override === true || addon?.skip_price_override === true - ? null - : addonProduct.price ?? addon.price - ); - }); - - const additionalPromises = (transactionItems.additionalItems.value || []) - .filter((item: any) => Number(item?.quantity ?? 0) > 0 && !isStandaloneAdditionalItemRestricted(item)) - .map((item: any) => - createOrderItem( - normalizedOrderId, - item.id, - Number(item.quantity), - null, - item?.notes || "", - item?.skip_price_override === true ? null : item.price - ) - ); - - await Promise.all([...addonPromises, ...additionalPromises]); + // The sync helper owns the create / rollback / allSettled logic and lives + // in tests/unit/pos-mobile-step-2-addon-sync.spec.js. Returning its result + // unchanged preserves the existing contract: true = synced (or already in + // sync), false = primary product blocked, throw = partial failure. + await syncMobileOrderItems({ + orderId: normalizedOrderId, + primaryItem: transactionItems.primaryItem.value, + additionalItems: transactionItems.additionalItems.value || [], + isAddonRestricted: isMobileAddonRestricted, + isStandaloneRestricted: isStandaloneAdditionalItemRestricted, + isPrimaryRestricted: (item) => getProductRestriction(item).restricted, + api: { createOrderItem, getOrderItems, removeOrderItem }, + }); return true; }; @@ -1140,8 +1024,7 @@ const normalizeText = (value: unknown) => String(value ?? "").trim(); const isEnabledFlag = (value: unknown) => value === true || value === 1 || value === "1" || value === "true"; -const getProductId = (product: any) => - Number(product?.product?.id ?? product?.product_id ?? product?.id ?? 0); +const getProductId = (product: any) => Number(product?.product?.id ?? product?.product_id ?? product?.id ?? 0); const getProductName = (product: any) => normalizeText(product?.product?.name ?? product?.name); @@ -1157,8 +1040,7 @@ const productRequiresOrderItemNote = (product: any) => { ); }; -const productHasOrderItemNote = (product: any) => - normalizeText(product?.notes ?? product?.product?.notes).length > 0; +const productHasOrderItemNote = (product: any) => normalizeText(product?.notes ?? product?.product?.notes).length > 0; const getSelectedProductsMissingRequiredNotes = () => { const missingProducts: any[] = []; @@ -1168,8 +1050,8 @@ const getSelectedProductsMissingRequiredNotes = () => { missingProducts.push(primaryProduct); } - (primaryProduct?.addons || []) - .filter((addon: any) => Number(addon?.quantity ?? 0) > 0 && !isMobileAddonRestricted(addon)) + (primaryProduct?.addons || []) + .filter((addon: any) => Number(addon?.quantity ?? 0) > 0 && !isMobileAddonRestricted(addon)) .forEach((addon: any) => { const addonProduct = addon?.product ?? addon; if ( @@ -1181,8 +1063,8 @@ const getSelectedProductsMissingRequiredNotes = () => { } }); - (transactionItems.additionalItems.value || []) - .filter((item: any) => Number(item?.quantity ?? 0) > 0 && !isStandaloneAdditionalItemRestricted(item)) + (transactionItems.additionalItems.value || []) + .filter((item: any) => Number(item?.quantity ?? 0) > 0 && !isStandaloneAdditionalItemRestricted(item)) .forEach((item: any) => { if (productRequiresOrderItemNote(item) && !productHasOrderItemNote(item)) { missingProducts.push(item); @@ -1538,15 +1420,15 @@ const filteredAddons = computed(() => { - - -

- {{ t(restrictionWarningMessageKey) }} -

+ + +

+ {{ t(restrictionWarningMessageKey) }} +

{ {{ t("common.retry") }}
- - + 0 that is not + * customer-rule-restricted becomes its own order_items row, linked to + * the freshly-created primary row by related_item_id. + * - Every additional (standalone) item with quantity > 0 that is not + * customer-rule-restricted becomes its own order_items row. + * - The old "Promise.all over parallel POSTs" fan-out silently dropped + * rows on a single rejection: the rows that already landed stayed on + * the server while the operator saw only a generic failure popup. + * This helper uses Promise.allSettled, collects every per-product + * failure, and rolls back every order_items row created during this + * attempt before throwing, so a retry starts from a clean state. + */ + +const PLACEHOLDER_PRIMARY_RELATED_ITEM_ID = "__PRIMARY__"; + +const toInteger = (value) => { + const parsed = Number.parseInt(String(value ?? ""), 10); + return Number.isInteger(parsed) ? parsed : null; +}; + +const toPositiveInteger = (value) => { + const parsed = toInteger(value); + return parsed !== null && parsed > 0 ? parsed : null; +}; + +const toNonNegativeNumber = (value) => { + const parsed = Number(value ?? 0); + return Number.isFinite(parsed) && parsed >= 0 ? parsed : 0; +}; + +const safeGet = (object, ...keys) => { + for (const key of keys) { + const candidate = object?.[key]; + if (candidate !== undefined && candidate !== null && candidate !== "") { + return candidate; + } + } + return undefined; +}; + +/** + * Normalize a single order item from the server (returned by GET + * /order/items) into the comparison shape used by the idempotency check. + * + * related_item_id is intentionally excluded from the comparison shape so + * that "desired add-on with placeholder related_item_id" can be matched + * against "existing add-on with real numeric related_item_id". The other + * fields (product_id, quantity, price, notes) fully characterize the row. + */ +export const normalizeExistingOrderItemShape = (item) => ({ + product_id: Number(safeGet(item, "product_id", "product_id") ?? 0), + quantity: toNonNegativeNumber(safeGet(item, "quantity", "quantity")), + price: Number(safeGet(item, "price", "price") ?? 0), + notes: String(safeGet(item, "notes", "notes") ?? ""), +}); + +/** + * Group raw server items into [primaryShape, ...addonShapes, ...additionalShapes] + * so we can compare the server state with what we want the server to look like. + * + * - `primaryItems` is every item with related_item_id === null. The first one + * is treated as the primary; subsequent ones are standalone "additional" + * items unless their product id matches the in-memory primary product. + * - `addonShapes` are items whose related_item_id equals the primary's id. + */ +export const normalizeExistingOrderItemShapes = (existingItems, primaryProductId) => { + const items = Array.isArray(existingItems) ? existingItems : []; + const primaryItems = items.filter((item) => item?.related_item_id === null || item?.related_item_id === undefined); + if (primaryItems.length === 0) { + return []; + } + + const primaryItem = primaryItems[0]; + const primaryItemId = Number(primaryItem?.id ?? 0); + const primaryShape = normalizeExistingOrderItemShape(primaryItem); + + const additionalItems = primaryItems + .filter((item) => Number(item?.product?.id ?? item?.product_id ?? 0) !== Number(primaryProductId ?? 0)) + .map(normalizeExistingOrderItemShape); + + const addonShapes = + primaryItemId > 0 + ? items + .filter((item) => Number(item?.related_item_id ?? 0) === primaryItemId) + .map(normalizeExistingOrderItemShape) + : []; + + return [primaryShape, ...addonShapes, ...additionalItems]; +}; + +/** + * Build the desired shapes from the in-memory primaryItem + additionalItems. + * Uses a stable placeholder ("__PRIMARY__") for the add-on related_item_id + * because the real primary id is not known until the primary POST resolves. + */ +export const buildDesiredOrderItemShapes = ({ + primaryItem, + additionalItems, + isAddonRestricted = () => false, + isStandaloneRestricted = () => false, +}) => { + if (!primaryItem) { + return []; + } + + const primaryShape = { + kind: "primary", + relatedKey: "primary", + product_id: Number(primaryItem.id ?? 0), + quantity: 1, + related_item_id: null, + price: Number(primaryItem.price ?? 0), + notes: String(primaryItem.notes ?? ""), + skip_price_override: primaryItem.skip_price_override === true, + }; + + const addonShapes = (Array.isArray(primaryItem.addons) ? primaryItem.addons : []) + .filter((addon) => Number(addon?.quantity ?? 0) > 0 && !isAddonRestricted(addon)) + .map((addon) => { + const addonProduct = addon?.product ?? addon; + return { + kind: "addon", + relatedKey: "primary", + product_id: Number(addonProduct?.id ?? addon?.id ?? 0), + quantity: Number(addon?.quantity ?? 0), + related_item_id: PLACEHOLDER_PRIMARY_RELATED_ITEM_ID, + price: Number(addonProduct?.price ?? addon?.price ?? 0), + notes: String(addonProduct?.notes ?? addon?.notes ?? ""), + skip_price_override: addonProduct?.skip_price_override === true || addon?.skip_price_override === true, + }; + }); + + const additionalShapes = (Array.isArray(additionalItems) ? additionalItems : []) + .filter((item) => Number(item?.quantity ?? 0) > 0 && !isStandaloneRestricted(item)) + .map((item) => ({ + kind: "additional", + relatedKey: null, + product_id: Number(item?.id ?? 0), + quantity: Number(item?.quantity ?? 0), + related_item_id: null, + price: Number(item?.price ?? 0), + notes: String(item?.notes ?? ""), + skip_price_override: item?.skip_price_override === true, + })); + + return [primaryShape, ...addonShapes, ...additionalShapes]; +}; + +const stripKindMarkers = (shape) => { + const { + kind: _kind, + relatedKey: _relatedKey, + skip_price_override: _skip, + // related_item_id is intentionally stripped: in the desired shapes it is + // the placeholder "__PRIMARY__" for add-ons (the real primary id is not + // known until the primary POST resolves), while in the existing shapes + // it is the real numeric primary id. Without stripping, the comparison + // would always fail and the helper would rebuild the order on every + // Fuldfør click. + related_item_id: _relatedItemId, + ...rest + } = shape; + return rest; +}; + +const shallowEqual = (left, right) => { + if (left === right) return true; + if (!left || !right) return false; + const leftKeys = Object.keys(left); + const rightKeys = Object.keys(right); + if (leftKeys.length !== rightKeys.length) return false; + for (const key of leftKeys) { + if (left[key] !== right[key]) return false; + } + return true; +}; + +const arraysEqual = (left, right) => { + if (left === right) return true; + if (!Array.isArray(left) || !Array.isArray(right)) return false; + if (left.length !== right.length) return false; + for (let i = 0; i < left.length; i += 1) { + if (!shallowEqual(left[i], right[i])) return false; + } + return true; +}; + +/** + * Map an axios-like rejection to the human-readable string the operator + * should see. Falls back to the error message itself. + */ +export const extractErrorMessage = (reason) => { + const message = + reason?.response?.data?.data?.message ?? reason?.response?.data?.message ?? reason?.message ?? "Unknown error"; + return String(message); +}; + +/** + * Format a per-product failure into a human-readable fragment. + */ +export const formatFailureFragment = ({ productId, message }) => { + if (!Number.isFinite(Number(productId)) || Number(productId) <= 0) { + return message; + } + return `Product ${productId}: ${message}`; +}; + +/** + * Custom error thrown when one or more order_items POSTs fail. Carries the + * aggregated message and the per-product failure list so the UI layer can + * either display it directly or surface structured details. + */ +export class OrderItemsPartialSyncError extends Error { + constructor(message, failures, rolledBackIds) { + super(message); + this.name = "OrderItemsPartialSyncError"; + this.failures = failures; + this.rolledBackIds = rolledBackIds; + } +} + +/** + * @typedef {Object} OrderItemApi + * @property {(orderId, productId, quantity, relatedItemId, notes, price) => Promise<{data:{data:{id:number}}}>} createOrderItem + * @property {(orderId) => Promise<{data:{data:Array}}>} getOrderItems + * @property {(orderItemId) => Promise} removeOrderItem + */ + +/** + * Reconcile the in-memory transaction against the server's order_items. + * + * @param {object} args + * @param {number} args.orderId + * @param {object|null} args.primaryItem + * @param {Array} [args.additionalItems] + * @param {(addon:object) => boolean} [args.isAddonRestricted] + * @param {(item:object) => boolean} [args.isStandaloneRestricted] + * @param {(primary:object) => boolean} [args.isPrimaryRestricted] + * @param {OrderItemApi} args.api + * @returns {Promise<{createdIds: number[], createdPrimaryItemId: number|null}>} + * @throws {OrderItemsPartialSyncError} when any add-on or additional POST fails + * after the partial rollback has already run. + */ +export const syncMobileOrderItems = async ({ + orderId, + primaryItem, + additionalItems = [], + isAddonRestricted = () => false, + isStandaloneRestricted = () => false, + isPrimaryRestricted = () => false, + api, +}) => { + const normalizedOrderId = toPositiveInteger(orderId); + if (!normalizedOrderId) { + throw new Error("Order ID is required"); + } + if (!primaryItem) { + throw new Error("No primary item selected"); + } + if (isPrimaryRestricted(primaryItem)) { + return { createdIds: [], createdPrimaryItemId: null }; + } + + const existingItemsResponse = await api.getOrderItems(normalizedOrderId); + const existingItems = Array.isArray(existingItemsResponse?.data?.data) ? existingItemsResponse.data.data : []; + + const desiredShapes = buildDesiredOrderItemShapes({ + primaryItem, + additionalItems, + isAddonRestricted, + isStandaloneRestricted, + }); + const currentShapes = normalizeExistingOrderItemShapes(existingItems, primaryItem?.id); + const comparableDesiredShapes = desiredShapes.map(stripKindMarkers); + const shouldForceRecreateForRepricing = desiredShapes.some((shape) => shape.skip_price_override === true); + + if (!shouldForceRecreateForRepricing && arraysEqual(currentShapes, comparableDesiredShapes)) { + return { createdIds: [], createdPrimaryItemId: null }; + } + + // Tear down whatever the server currently has for this order, including + // any half-synced rows left over from a previous failed attempt. + await Promise.allSettled(existingItems.map((item) => api.removeOrderItem(Number(item.id)))); + + let primaryCreate; + try { + primaryCreate = await api.createOrderItem( + normalizedOrderId, + Number(primaryItem.id ?? 0), + 1, + null, + String(primaryItem.notes ?? ""), + primaryItem.skip_price_override === true ? null : Number(primaryItem.price ?? 0) + ); + } catch (reason) { + // The primary row never landed, so there is nothing to roll back. Wrap + // the rejection so the UI layer gets the same error shape for both + // primary and add-on failures. + throw new OrderItemsPartialSyncError( + extractErrorMessage(reason), + [{ productId: Number(primaryItem.id ?? 0), message: extractErrorMessage(reason) }], + [] + ); + } + const createdPrimaryItemId = toPositiveInteger(primaryCreate?.data?.data?.id); + + const addonCandidates = (Array.isArray(primaryItem.addons) ? primaryItem.addons : []).filter( + (addon) => Number(addon?.quantity ?? 0) > 0 && !isAddonRestricted(addon) + ); + const additionalCandidates = (Array.isArray(additionalItems) ? additionalItems : []).filter( + (item) => Number(item?.quantity ?? 0) > 0 && !isStandaloneRestricted(item) + ); + + const addonPromises = addonCandidates.map((addon) => { + const addonProduct = addon?.product ?? addon; + return api + .createOrderItem( + normalizedOrderId, + Number(addonProduct.id ?? addon.id ?? 0), + Number(addon.quantity ?? 0), + createdPrimaryItemId, + String(addonProduct?.notes ?? addon?.notes ?? ""), + addonProduct?.skip_price_override === true || addon?.skip_price_override === true + ? null + : Number(addonProduct.price ?? addon.price ?? 0) + ) + .then((response) => ({ + productId: Number(addonProduct.id ?? addon.id ?? 0), + response, + })); + }); + + const additionalPromises = additionalCandidates.map((item) => + api + .createOrderItem( + normalizedOrderId, + Number(item.id ?? 0), + Number(item.quantity ?? 0), + null, + String(item.notes ?? ""), + item.skip_price_override === true ? null : Number(item.price ?? 0) + ) + .then((response) => ({ + productId: Number(item.id ?? 0), + response, + })) + ); + + const results = await Promise.allSettled([...addonPromises, ...additionalPromises]); + + const createdIds = []; + if (createdPrimaryItemId) { + createdIds.push(createdPrimaryItemId); + } + + const failures = []; + results.forEach((result, index) => { + const productId = + index < addonPromises.length + ? Number(addonCandidates[index]?.product?.id ?? addonCandidates[index]?.id ?? 0) + : Number(additionalCandidates[index - addonPromises.length]?.id ?? 0); + if (result.status === "fulfilled") { + const id = toPositiveInteger(result.value?.response?.data?.data?.id); + if (id) { + createdIds.push(id); + } + return; + } + failures.push({ + productId, + message: extractErrorMessage(result.reason), + }); + }); + + if (failures.length === 0) { + return { createdIds, createdPrimaryItemId }; + } + + // Roll back every row we created in this attempt so a retry starts clean. + await Promise.allSettled(createdIds.map((id) => api.removeOrderItem(id))); + + const aggregated = failures.map(formatFailureFragment).join("; "); + throw new OrderItemsPartialSyncError(aggregated, failures, createdIds.slice()); +}; diff --git a/tests/e2e/pos-mobile-order-flow.spec.js b/tests/e2e/pos-mobile-order-flow.spec.js index 4c8e6ec9..6d61d5b8 100644 --- a/tests/e2e/pos-mobile-order-flow.spec.js +++ b/tests/e2e/pos-mobile-order-flow.spec.js @@ -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({ diff --git a/tests/e2e/support/mobilePos.js b/tests/e2e/support/mobilePos.js index 980095d5..bad5b82e 100644 --- a/tests/e2e/support/mobilePos.js +++ b/tests/e2e/support/mobilePos.js @@ -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) { diff --git a/tests/unit/pos-mobile-step-2-addon-sync.spec.js b/tests/unit/pos-mobile-step-2-addon-sync.spec.js new file mode 100644 index 00000000..cae54a7f --- /dev/null +++ b/tests/unit/pos-mobile-step-2-addon-sync.spec.js @@ -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(); + }); +});