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
22 changes: 19 additions & 3 deletions api/src/controllers/sales.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -6436,9 +6436,22 @@ class SalesController extends BaseController {

if (response.status === true) {
return this.success(res, response.data, response.message);
} else {
return this.error(res, response.message, 404);
}

/*
* A CONFLICT IS NOT A MISSING ORDER.
*
* Somebody else saved this order while the caller was looking at it.
* The order is there, the caller is welcome, and the request is simply
* out of date - which is 409, not 404. The client keys on the status
* rather than on the spelling of a message, so this stays true when
* somebody rewords it.
*/
if (response.message === 'order_changed') {
return this.error(res, response.message, 409);
}

return this.error(res, response.message, 404);
} catch (error) {
console.error('Error in salesPaymentClose:', error);
return this.error(res, error.message, 500);
Expand Down Expand Up @@ -7400,6 +7413,9 @@ class SalesController extends BaseController {
table. It rides in the options object rather than as an eleventh
positional argument, because ten is already too many to count. */
const newTableId = req.body.table_id;
/* Which version of the order the caller was looking at. Absent from an
older handset, which is why nothing here requires it. */
const seenAt = req.body.seen_at;
const dineType = req.body.dine_type;
const personCount = req.body.person_count;

Expand All @@ -7415,7 +7431,7 @@ class SalesController extends BaseController {
newTableNo,
dineType,
personCount,
{ SaleModel, newTableId }
{ SaleModel, newTableId, seenAt }
);

if (response.status === true) {
Expand Down
39 changes: 38 additions & 1 deletion api/src/repositories/sale.repository.js
Original file line number Diff line number Diff line change
Expand Up @@ -9538,7 +9538,7 @@ class SalesRepository {
newTableNo,
dineType,
personCount,
{ SaleModel, newTableId } = {}
{ SaleModel, newTableId, seenAt } = {}
) {
try {
const db = await BaseModel.getDb();
Expand All @@ -9555,6 +9555,43 @@ class SalesRepository {
return { status: false, message: 'Order not found', data: [] };
}

/*
* A SAVE WRITTEN AGAINST A VIEW THAT HAS MOVED ON.
*
* The client sends the whole order, and what it does not send is
* deleted - that is how a cancelled dish is cancelled, and it is right.
* It is also why two handsets are dangerous: waiter A adds a biryani at
* 19:00, waiter B saves at 19:01 from a screen opened at 18:58, B's list
* has no biryani, and the till removes it. The kitchen has cooked it and
* the bill no longer has it. Nobody is told.
*
* So a caller may say which version of the order it was looking at, and
* a save written against an older one is refused rather than applied.
* The client reloads, sees what changed, and decides again - which is
* the only safe answer, because only a person knows whether the biryani
* was meant to go.
*
* SILENCE STILL MEANS YES. A caller that sends no `seen_at` is treated
* exactly as before: handsets in the wild are older than this code, and
* refusing their saves would turn a data-loss bug into an outage.
*/
if (seenAt) {
const seen = new Date(seenAt).getTime();
const lastChanged = new Date(
orderDoc.updated_date || orderDoc.created_date || 0
).getTime();

/* An unreadable timestamp is not a conflict. It is a caller this
check cannot help, and blocking it would help nobody either. */
if (Number.isFinite(seen) && Number.isFinite(lastChanged) && lastChanged > seen) {
return {
status: false,
message: 'order_changed',
data: { updated_date: orderDoc.updated_date || orderDoc.created_date || null },
};
}
}

// Get license from order document for proper multi-tenant filtering

// ---------- CANCEL FLOW ----------
Expand Down
3 changes: 2 additions & 1 deletion api/src/services/sale.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -3906,7 +3906,7 @@ module.exports = {
newTableNo,
dineType,
personCount,
{ SaleModel, newTableId } = {}
{ SaleModel, newTableId, seenAt } = {}
) =>
salesRepository.updateOrderModel(
orderId,
Expand All @@ -3922,6 +3922,7 @@ module.exports = {
{
SaleModel: getModel(SaleModel),
newTableId,
seenAt,
}
),
getFrequentItemsForBranch: async (branchId, limit, { SaleModel } = {}) =>
Expand Down
181 changes: 181 additions & 0 deletions api/tests/unit/repositories/an-order-changed-under-you.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
'use strict';

/*
* TWO PHONES, ONE ORDER, AND A DISH THAT VANISHES.
*
* A handset sends the WHOLE order when it saves, and the till keeps only what
* arrives. That is not a mistake: it is how a cancelled dish gets cancelled.
*
* It is also why a floor with several handsets loses food. Waiter A adds a
* biryani at 19:00. Waiter B saves at 19:01 from a screen opened at 18:58, so
* B's list has no biryani in it, and the till removes one the kitchen has
* already cooked. The bill goes out short and nobody is told - not A, not B,
* not the kitchen, not the shop.
*
* So a caller may say which version of the order it was looking at, and a save
* written against an older one is refused. The client reloads and decides
* again, because only a person knows whether that biryani was meant to go.
*
* A caller that says nothing is treated exactly as before. Handsets in the
* wild are older than this code, and refusing their saves would turn a
* data-loss bug into an outage.
*/

const { MongoMemoryServer } = require('mongodb-memory-server');
const mongoose = require('mongoose');

const repo = require('../../../src/repositories/sale.repository');
const BaseModel = require('../../../src/models/base.model');

let mem;
let db;

const BRANCH = new mongoose.Types.ObjectId();
const LICENSE = new mongoose.Types.ObjectId();
const BIRYANI = new mongoose.Types.ObjectId();
const NAAN = new mongoose.Types.ObjectId();

const WAS_OPENED_AT = new Date('2026-09-16T18:58:00.000Z');
const CHANGED_AT = new Date('2026-09-16T19:00:00.000Z');

/** An order with one dish, last changed at a known moment. */
async function anOrder(updatedAt) {
const _id = new mongoose.Types.ObjectId();
await db.collection('sales').insertOne({
_id,
branch_id: BRANCH,
license: LICENSE,
sale_process: 'KOT',
created_date: WAS_OPENED_AT,
updated_date: updatedAt,
table_number: '4',
dine_type: 'Dine-in',
items: [
{ item_id: NAAN, item_name: 'Butter Naan', item_quantity: 1, item_price: 40 },
{ item_id: BIRYANI, item_name: 'Chicken Biryani', item_quantity: 1, item_price: 220 },
],
});
return String(_id);
}

const stored = async (id) =>
db.collection('sales').findOne({ _id: new mongoose.Types.ObjectId(id) });

/** Save an order carrying only the naan: the biryani would be dropped. */
const saveWithoutTheBiryani = (id, options) =>
repo.updateOrderModel(
id,
[{ item_id: String(NAAN), item_name: 'Butter Naan', item_quantity: 1, item_price: 40 }],
40,
null,
null,
null,
null,
'4',
'Dine-in',
2,
options || {}
);

beforeAll(async () => {
mem = await MongoMemoryServer.create();
await mongoose.connect(mem.getUri('posnic'));
db = mongoose.connection.db;
}, 120000);

afterAll(async () => {
await mongoose.disconnect();
if (mem) await mem.stop();
});

beforeEach(async () => {
jest.spyOn(BaseModel, 'getDb').mockResolvedValue(db);
jest.spyOn(console, 'warn').mockImplementation(() => {});
jest.spyOn(console, 'log').mockImplementation(() => {});
jest.spyOn(console, 'error').mockImplementation(() => {});
await db.collection('sales').deleteMany({});
await db.collection('items').deleteMany({});
await db.collection('branches').deleteMany({});
await db.collection('branches').insertOne({ _id: BRANCH, license: LICENSE });
await db.collection('items').insertMany([
{ _id: BIRYANI, license: LICENSE, item_name: 'Chicken Biryani', item_price: 220 },
{ _id: NAAN, license: LICENSE, item_name: 'Butter Naan', item_price: 40 },
]);
});

afterEach(() => jest.restoreAllMocks());

describe('a save written against an order that has moved on', () => {
test('IS REFUSED, and the dish the other waiter added is still there', async () => {
const id = await anOrder(CHANGED_AT);

const out = await saveWithoutTheBiryani(id, { seenAt: WAS_OPENED_AT.toISOString() });

expect(out.status).toBe(false);
expect(out.message).toBe('order_changed');

const after = await stored(id);
expect(after.items.map((i) => i.item_name).sort()).toEqual([
'Butter Naan',
'Chicken Biryani',
]);
});

test('says when the order actually changed, so the client can show it', async () => {
const id = await anOrder(CHANGED_AT);

const out = await saveWithoutTheBiryani(id, { seenAt: WAS_OPENED_AT.toISOString() });

expect(new Date(out.data.updated_date).getTime()).toBe(CHANGED_AT.getTime());
});
});

describe('a save that is not stale', () => {
test('goes through, and a dish left out is still cancelled', async () => {
/*
* The guard must not break the thing it sits in front of. Leaving a dish
* out IS how a waiter cancels it, and that has to keep working.
*/
const id = await anOrder(CHANGED_AT);

const out = await saveWithoutTheBiryani(id, { seenAt: CHANGED_AT.toISOString() });

expect(out.status).not.toBe(false);
const after = await stored(id);
expect(after.items.map((i) => i.item_name)).toEqual(['Butter Naan']);
});

test('a caller that says nothing is served exactly as before', async () => {
/*
* Handsets already in shops are older than this code. Refusing their
* saves would turn a bug that loses a dish into one that takes no orders.
*/
const id = await anOrder(CHANGED_AT);

const out = await saveWithoutTheBiryani(id, {});

expect(out.status).not.toBe(false);
expect((await stored(id)).items.map((i) => i.item_name)).toEqual(['Butter Naan']);
});

test('an unreadable timestamp is not treated as a conflict', async () => {
/* A caller this check cannot help is not a caller to block. */
const id = await anOrder(CHANGED_AT);

const out = await saveWithoutTheBiryani(id, { seenAt: 'yesterday afternoon' });

expect(out.status).not.toBe(false);
});

test('an order nobody has ever updated is judged by when it was created', async () => {
const id = await anOrder(undefined);

const stale = await saveWithoutTheBiryani(id, {
seenAt: new Date(WAS_OPENED_AT.getTime() - 60000).toISOString(),
});
expect(stale.message).toBe('order_changed');

const fresh = await saveWithoutTheBiryani(id, { seenAt: WAS_OPENED_AT.toISOString() });
expect(fresh.status).not.toBe(false);
});
});
Loading