Skip to content

An order answered anywhere stops the alarm, and the rule can answer one at all - #807

Merged
sridharkalaibala merged 1 commit into
developfrom
fix/an-order-answered-anywhere-stops-the-alarm
Sep 16, 2026
Merged

sridharkalaibala merged 1 commit into
developfrom
fix/an-order-answered-anywhere-stops-the-alarm

Conversation

@sridharkalaibala

Copy link
Copy Markdown
Contributor

Two things, both about an order nobody has answered. I went looking after #804 because that fix raised an obvious question: what else raises this alarm and never stops it?

The "auto cancel after ten minutes" rule has never fired

unanswered-orders.js called decideOnOrder with three positional arguments against a signature that destructures an object:

async decideOnOrder({ saleId, decision, reason } = {}) { ... }
// called as
repo.decideOnOrder(String(order._id), decision, verdict.reason);

Destructuring a string gives saleId === undefined, so every sweep got "Enter must correct order id" before touching the database, and the sweep's own if (!done.status) continue swallowed it. Nothing throws, nothing logs.

So in production: an order nobody answered was never auto-cancelled, the customer was never told, and the alarm never stopped. Run against the real repository to be sure rather than reasoning about it:

positional call -> {"status":false,"message":"Enter must correct order id","data":null}

Why the tests never caught it. They inject a fake Repository whose decideOnOrder happily takes positional arguments, so nineteen passing tests were proving the fake. The fake now has the real signature, and there is a test that calls the real method with exactly what the sweep used to send, plus one that fails if the call goes back to positional.

The alarm only stopped if somebody used the queue page

An order arriving for approval raises the repeating, escalating alarm. The only thing that ever silenced it from a person's decision was the Online orders page telling the desktop main process by hand through silence(saleId).

So an order accepted anywhere else left the till nagging about an order already dealt with:

  • from the request dock, which is reachable from every other screen
  • from the captain handset, which never talks to that main process at all, and which is exactly what a restaurant answers orders on

The alarm belongs to the order, not to the screen that answered it. decideOnOrder now announces the resolution, so all three doors work through one implementation. It carries by, so a shop asking later who cancelled an order gets the true answer: "the ten minute rule" and "Amudha" are different answers.

The sweep drops its own announcement. It was a second implementation of the same promise and the two would drift, which is how this whole area got into trouble.

Silence is only ever earned

Gated on the write having matched, not on the method having said yes. decideOnOrder narrows both its read and its write by whatever tenant the process was last serving. The read refuses first, so a mismatch answers no rather than a false yes - but they are separate lines, and a promise that somebody dealt with an order should not rest on them agreeing.

Checks

  • 8 new repository tests against a real MongoDB, and the fake-signature repair. On the current code 10 of these fail.
  • API suite 10,903 of 10,903; root suite 2,729 of 2,732 (the three need frontend/public built, which CI does).
  • scripts/check-locally.js, the same commands the pull request runs, all 7 pass.

Not in this one

Nothing announces a resolution for a cancellation or change request sitting in the queue. Those arrive on an order the shop usually already accepted, so whether they raise the repeating alarm at all needs checking on a real shop before I write code for it.

…ne at all

TWO THINGS, both about an order nobody has answered.

THE RULE HAS NEVER DECIDED ANYTHING. unanswered-orders.js called
decideOnOrder with three positional arguments against a signature that
destructures an object, so `saleId` was undefined on every sweep and the
method answered "Enter must correct order id" before it touched the
database. "Auto cancel after ten minutes" has therefore never fired in
production: the order sat there, the customer was never told, and the
alarm never stopped. The tests passed throughout because they inject a
fake Repository that happily takes positional arguments, so they were
proving the fake. The fake now has the real signature, and a new test
calls the real method with exactly what the sweep used to send.

THE ALARM ONLY STOPPED IF SOMEBODY USED THE QUEUE PAGE. An order arriving
for approval raises the repeating, escalating alarm, and the only thing
that ever silenced it from a person's decision was that page telling the
desktop main process by hand. So an order accepted from the request dock
on any other screen, or from the captain handset - which never talks to
that main process at all, and is exactly what a restaurant answers orders
on - left the till nagging about an order that was already dealt with.

The alarm belongs to the order rather than to the screen that answered
it, so decideOnOrder announces the resolution: one door that a person,
the shop's own rule and the handset all go through. It carries `by` so a
shop asking later who cancelled an order gets the true answer.

Gated on the write having matched. The method narrows both its read and
its write by whatever tenant the process was last serving, and the read
refuses first - but they are separate lines, and a promise that somebody
dealt with an order should not rest on them agreeing. The sweep drops its
own announcement, which was a second implementation of the same promise.
@sridharkalaibala
sridharkalaibala merged commit 1148dbb into develop Sep 16, 2026
9 checks passed
@github-actions github-actions Bot added the ready for QA Merged to develop and live on develop.posnic.io - anyone can test it label Sep 16, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Merged to develop. Anyone can test this - you do not need write access.

Try it at https://develop.posnic.io, or run it yourself:

git fetch origin develop && git checkout develop
npm install && npm --prefix api install
npm run dev   # then http://localhost:3000

When you have tested it, say what you did and what happened, and set
QA passed or QA failed. If you cannot set labels, just comment -
a maintainer will.

Reporting that something is broken is as useful as fixing it. It is
better found here than by a shopkeeper.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for QA Merged to develop and live on develop.posnic.io - anyone can test it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant