Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions api/src/repositories/sale.repository.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 };
}
Expand Down Expand Up @@ -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,
Expand Down
69 changes: 69 additions & 0 deletions tests/qr-order-idempotency.test.js
Original file line number Diff line number Diff line change
@@ -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');
});
Loading