diff --git a/api/src/repositories/sale.repository.js b/api/src/repositories/sale.repository.js index 00aac6245..86163e24d 100644 --- a/api/src/repositories/sale.repository.js +++ b/api/src/repositories/sale.repository.js @@ -7192,6 +7192,7 @@ class SalesRepository { kiosk_table_id, dine_type, person_count, + idempotencyKey, /* * Where the customer is sitting, read by the page out of the URL it * was opened on. @@ -7218,6 +7219,46 @@ class SalesRepository { if (BaseModel.license) branchSelector.license = BaseModel.license; const branchDoc = await branchCollection.findOne(branchSelector); + /* + * The same order, sent twice, is one order. + * + * A waiter taps send, the Wi-Fi drops before the reply arrives, and the + * app cannot tell "never reached the kitchen" from "reached it and the + * answer was lost". Without this the safe choice is to refuse to retry + * and make somebody check the kitchen screen; with it, sending again is + * free, which is what lets a handset hold an order and send it when the + * network returns. + * + * Both apps have been sending idempotencyKey for as long as they have + * existed. Nothing read it: the field was destructured nowhere and + * stored nowhere, so every retry wrote another ticket. + */ + if (idempotencyKey) { + const already = await db.collection('sales').findOne({ + idempotency_key: String(idempotencyKey), + ...(BaseModel.license ? { license: BaseModel.license } : {}), + }); + if (already) { + return { + status: true, + message: 'Order placed successfully', + data: { + tokenId: already.token_id || already.tokenId || '', + sale_id: already._id.toString(), + sales_id: already.sales_id, + branch_name: already.branch_name, + items: already.items || [], + subtotal: already.sub_total ?? already.subtotal ?? 0, + discount: already.discount ?? 0, + tax: already.tax ?? 0, + total: already.sales_total ?? already.total ?? 0, + payment_status: already.payment_status, + duplicate: true, + }, + }; + } + } + if (!branchDoc) { return { status: false, message: 'Branch not found', data: null }; } @@ -7522,6 +7563,9 @@ class SalesRepository { // Use raw MongoDB insert to bypass Mongoose schema validators const salesCollection = db.collection('sales'); const insertResult = await salesCollection.insertOne({ + /* What makes a resend safe. Absent on orders taken before this + shipped, which is why the lookup above is skipped without one. */ + ...(idempotencyKey ? { idempotency_key: String(idempotencyKey) } : {}), branch: branchObjectId, branch_id: branchObjectId, branch_name: branchName, diff --git a/tests/qr-order-idempotency.test.js b/tests/qr-order-idempotency.test.js new file mode 100644 index 000000000..c769adc30 --- /dev/null +++ b/tests/qr-order-idempotency.test.js @@ -0,0 +1,69 @@ +/* + * The same order, sent twice, is one order. + * + * A waiter taps send, the Wi-Fi drops before the reply arrives, and the app + * cannot tell "never reached the kitchen" from "reached it and the answer was + * lost". Those need opposite responses and look identical from the handset. + * + * Both apps have sent an idempotencyKey for as long as they have existed and + * nothing read it: the field was destructured nowhere and stored nowhere, so + * every retry wrote another ticket and the kitchen cooked it twice. A key that + * is sent and ignored is worse than no key, because it reads like protection. + */ + +const test = require('node:test'); +const assert = require('node:assert'); +const fs = require('node:fs'); +const path = require('node:path'); + +const SOURCE = fs.readFileSync( + path.join(__dirname, '..', 'api', 'src', 'repositories', 'sale.repository.js'), 'utf8'); + +/* The qrOrder path alone: this file holds many order writers and a whole-file + match would pass on the wrong one. */ +function qrOrderSource() { + /* Named createOnlineOrder since the venue work; it is still the handler + behind POST /sales/qrOrder. */ + const start = SOURCE.indexOf('async createOnlineOrder('); + assert.ok(start > -1, 'the online-order writer is gone or renamed again'); + const end = SOURCE.indexOf('\n async ', start + 10); + return SOURCE.slice(start, end > -1 ? end : undefined); +} + +test('the key the apps already send is read', () => { + assert.match(qrOrderSource(), /idempotencyKey,/, + 'the field is sent by both apps; destructuring it is what makes it real'); +}); + +test('a repeat returns the order that already exists', () => { + const source = qrOrderSource(); + assert.match(source, /idempotency_key: String\(idempotencyKey\)/); + assert.match(source, /if \(already\)/, + 'without returning the existing sale, a resend writes a second ticket'); + assert.match(source, /duplicate: true/, + 'the caller should be able to tell a resend from a fresh order'); +}); + +test('the lookup is scoped to the shop', () => { + const source = qrOrderSource(); + const lookup = source.slice(source.indexOf('if (idempotencyKey)'), source.indexOf('if (!branchDoc)')); + assert.match(lookup, /BaseModel\.license/, + 'an unscoped key lookup could match another shop’s order'); +}); + +test('the key is stored, or the lookup can never match', () => { + const source = qrOrderSource(); + const insert = source.slice(source.indexOf('insertOne({')); + assert.match(insert, /idempotency_key: String\(idempotencyKey\)/, + 'checking for a key that is never written is a check that always passes'); +}); + +test('an order sent without a key still works', () => { + /* Orders taken before this shipped have no key, and a handset that does not + send one must not be refused. */ + const source = qrOrderSource(); + assert.match(source, /\.\.\.\(idempotencyKey \? \{ idempotency_key/, + 'the field is written conditionally, so a keyless order is unaffected'); + assert.match(source, /if \(idempotencyKey\) \{/, + 'the lookup is skipped without a key rather than matching everything'); +});