Skip to content

The ordering catalogue keeps what the page goes on to read - #775

Merged
sridharkalaibala merged 2 commits into
developfrom
fix/the-catalogue-keeps-what-the-page-reads
Sep 15, 2026
Merged

sridharkalaibala merged 2 commits into
developfrom
fix/the-catalogue-keeps-what-the-page-reads

Conversation

@sridharkalaibala

@sridharkalaibala sridharkalaibala commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

#772 fixed one instance of a bug. This looks for the rest of them, and finds one that is about money.

The shape of it

order/indexedDB.js builds the entire /order catalogue in one object literal out of what the storefront sent. It is a whitelist. A field it does not name is dropped in silence: no error, no log, no failing test, because every test on those features reads the source of the feature rather than the source of the catalogue.

Asked of every storefront field rather than the four #772 already knew about:

DROPPED BUT READ  daily_price           indexedDB.js
DROPPED BUT READ  price_set_on          indexedDB.js

What that costs

waitingForTodaysPrice() is the one rule behind both the dish card and the dish sheet - written at the top level precisely so the two cannot disagree:

if (product.daily_price === true && !pricedToday(product.price_set_on)) return true;
return !(Number(product.price) > 0);

Neither field has ever arrived. So the first line has been dead since the daily-price release and the gate could only ever answer "has it got a price at all".

A whole fish, crab or lobster flagged as priced from the morning's market and last priced yesterday was offered on /order at yesterday's rate with an ordinary Add button. /menu, which reads the same endpoint without a local store, said "Market price" correctly. The same asymmetry that hid the dish facts, and this one puts a wrong number on a bill.

Nothing is wrong on a live shop today, because nothing writes daily_price yet - the item form does not offer it. What was broken is the surface that will go on being broken, silently, on the day it does.

The test is the rule, not a third field name

Two features have now been eaten by this literal. Adding daily_price to a third named test would catch nothing new, so:

Every storefront field the ordering bundle reads off a catalogue product must be kept by the catalogue.

It derives both sides from source - what the aggregation pushes, what the consumer scripts read - so a read added tomorrow fails until the literal names it. Consumers are listed rather than swept up, because the bundle holds other shapes that share field names with a product: the thank-you page walks the server's receipt, whose lines carry their own tax, and reading that as a product field is a false alarm. The reads inside the catalogue literal itself are excluded too, since price: parseFloat(item.final_price) is the boundary consuming a server field, not the page reading a stored one.

Verified failing on the parent commit, naming exactly the two fields:

actual: [ 'daily_price (read by indexedDB.js)', 'price_set_on (read by indexedDB.js)' ]

daily_price === true rather than a loose truthy read, because flags have arrived here as the string "false" before and read as ON.

And it runs the thing now, rather than reading it

The whitelist was an anonymous object literal three levels inside a fetch, so the only test possible on it was a regex over its source - and a regex passes on a line that names nutrition and stores the wrong thing.

Second commit lifts it out as catalogueItem(item, categoryName) and the loop calls it. No behaviour change, and the test now drives it over a real payload: a dish with facts keeps them, a dish with nothing entered gets empties rather than undefined (the page does (product.claims || []).forEach in one place and Array.isArray(p.tags) in another), the market-price pair survives, and a flag arriving as the string "false" is still off.

Checks

  • 5 new tests; the three about the dropped fields proven failing on the parent commit, naming exactly those two fields
  • Root suite 2,555 of 2,558; the three remaining need frontend/public built, which CI does

Not changed, deliberately

discount_amount, discount_percentage, negative_stock, open_price and tax_type are sent by the storefront and read by nothing in the bundle, so the catalogue rightly drops them. open_price in particular is deliberately not read: it means the price is settled at the counter, and a guest ordering from a phone has no counter to settle it at.

order/indexedDB.js builds the whole /order catalogue in one object
literal out of what the storefront sent. It is a whitelist, and a field
it does not name is dropped in silence: no error, no log, no failing
test, because every test on those features reads the source of the
feature rather than the source of the catalogue.

That has now cost two features. The dish facts went first: nutrition,
tags, marks and claims sent for three releases and dropped on arrival.
The second is about money. waitingForTodaysPrice() reads daily_price and
price_set_on, and has never once received either, so on /order a whole
fish priced from the morning's market and last priced YESTERDAY was
offered at yesterday's rate with an ordinary Add button, while /menu said
"Market price".

Rather than add a third named field to a third test, this states the
rule: every storefront field the ordering bundle reads off a catalogue
product must be kept by the catalogue. Add a read tomorrow and the test
fails until the literal names it. Consumers are listed rather than swept
up, because the thank-you page walks the server's receipt and its lines
carry their own tax.
It was an anonymous object literal three levels inside a fetch, which is
why the only tests possible on it were regexes over its source - and a
regex passes on a line that names nutrition and stores the wrong thing.

catalogueItem(item, categoryName) is the same literal, lifted out and
called from the loop. The test now drives it over a real payload: a dish
with facts keeps them, a dish with nothing entered gets empties rather
than undefined, the market-price pair survives, and a flag arriving as
the string "false" is still off.

No behaviour change.
@sridharkalaibala
sridharkalaibala merged commit 6aed6db into develop Sep 15, 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 15, 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