dev to main - #834
Merged
Merged
dev to main#834
Conversation
A regular orders the same thing. Reading their own history, finding five dishes and tapping each one back in is work the phone can do, and every ordering app in the world does it. This one did not. AT TODAY'S PRICES, NEVER THE REMEMBERED ONE. The kept line says what they paid last week; putting that number in the basket quotes a price the shop is not offering, and the customer sees it confirmed at checkout. So the catalogue product is what goes in the cart and the remembered line is used only for what it is: which dish, how many, and how they asked for it. AND IT SAYS WHAT IT COULD NOT ADD. A basket that quietly comes back with three of five dishes is worse than one that refuses: they check out believing they ordered what they ordered last week. Anything off the menu, sold out, or waiting on a price the shop has not set today is named, and the page waits for them to read it before it moves. It adds rather than replaces - whatever is in the basket was put there deliberately a moment ago - and it carries the note and the spice level back, because those are part of "the same again". Not a spice level for a dish whose picker the shop has since switched off: that would print on a ticket for a choice the menu no longer makes. The button appears only where the shop answered and only for this shop. The remembered order carries a name and a quantity and NO item id - the id comes from the shop's own copy - and matching a dish by its name is how a customer ends up with the wrong one. An id from another shop's menu either misses or, worse, hits something else. Four new strings, with Tamil, in both copies of the dictionary.
`build:quick` set ELECTRON_BUILDER_COMPRESSION_LEVEL=fast. app-builder-lib takes a single digit 0-9 there and throws on anything else, so that script could never produce an installer. It packaged the whole app, signed every binary, and then died at the archive step with ELECTRON_BUILDER_COMPRESSION_LEVEL must be a single digit 0-9 after about two minutes of looking exactly like a build in progress. Run through a pipe the shell reports the exit code of the last command in the pipeline, so it reads as a SUCCESS and leaves whatever was in dist/ from the last real build sitting there. That is how somebody ends up testing an installer from a week ago. It also called cross-env, which nothing in this repo depends on and which is not installed, so on a clean checkout it would have failed on the first word instead. Both go away together: electron-builder has a `compression` setting whose own documentation says `store` is what to use to rapidly test a build, and a config override needs no environment variable and no cross-env. Four tests, over every script that invokes electron-builder rather than the one that happened to be wrong: a compression level is a digit, a script that sets an env var has cross-env to set it with, and cross-env is installed if anything calls it. The first test asserts there ARE build scripts, because an assertion over an empty list passes for ever. Verified by building the installer.
A build reached a shop with server.js asking for ./handset-slots and the file not beside it. The require sat INSIDE the per-request path, and that path is an override of server.emit, so the throw did not become a 500 - it escaped as an uncaught exception and the request was never answered. The handset held an open socket until it timed out. Nothing appeared on the till. The shop was told the captain app keeps disconnecting, which is the same sentence this whole area exists because of. [MobileTracker] External request: 192.168.1.2 -> GET /api [FATAL] Uncaught Exception: Cannot find module './handset-slots' Localhost stayed instant throughout, because the slots are only consulted for non-loopback callers. So everything anybody tried on the machine itself worked. Two changes. The module is required ONCE, at the top of the file, inside a try. A require in the request path is evaluated per request, and a failure there is invisible; at the top it is one line in the log at startup. The decision goes through admitHandset, which never throws and always answers. Gone, broken, or answering nonsense: the handset is admitted. The cap exists so a shop is not silently serving a stranger's phone; it is not a security boundary, the API's authentication is. Against a till that stops answering its handsets mid-service that is the smaller harm, and every outage handset-slots.js documents came from refusing a handset that should have been served. 11 tests. admitHandset is lifted out of the source and driven directly, because the caller is an emit override and cannot be. The source-level ones pin where the require lives and that nothing calls .admit inside the request path again - checked by putting that call back and watching it fail. Not the cause of the shop's bad build: the require and the module landed in the same commit (ec6077e) and the file has never been deleted, so no clean checkout produces what is installed there. This makes that class of failure answer something instead of hanging.
This is the actual cause of the till that answered nothing, and it is a
packaging fact rather than a code one.
src/server.js is not in build.files. It is in extraResources, copied to
the resources root and FLATTENED:
{ "from": "src/server.js", "to": "server.js" }
So at runtime it is resources/server.js, and require('./handset-slots')
resolves to resources/handset-slots.js - not to
resources/app.asar/src/handset-slots.js, which is a different directory a
relative require cannot reach.
src/handset-slots.js WAS listed, in build.files, so it went into the
asar. Beside main.js, which does not use it. Never beside server.js,
which does.
AND A TEST ALREADY CLAIMED TO PREVENT THIS:
assert.ok(pkg.build.files.includes('src/handset-slots.js'))
True, meaningless, and green for the whole life of the bug it names.
Twelfth in the silent no-op family and the most expensive so far.
One line added to extraResources. The new test DERIVES the requirement
instead of restating a filename: for every file flattened out of src/ to
the resources root, every relative require it makes must land at that
same level. It also refuses a require that climbs out of the directory,
which cannot work once flattened, and asserts there are entries to check
at all. Verified by deleting the new line and watching it name the exact
failure the shop saw.
The old assertion now checks both places and says why they are not
interchangeable.
2779 desktop tests, 0 failures.
Owner, reading an Azure bill: "if walk in customer no need to show. (remove it)". It was supposed to have been removed already. bill-payload.js had a guard for it: if (name && !/^walk[\s-]?in$/i.test(name)) out.push(name); That matches "walkin", "walk in" and "walk-in". The name this system actually writes - in five places - is "Walk-in Customer", which it does not match. The guard was real, the intent was right, and it had never once matched the thing it existed to catch. Every cash bill printed the name of a customer who does not exist. The writer and the reader each knew a string, and the strings were not the same string. utils/walk-in.js now holds the one name and the one question; the sale repository writes WALK_IN_NAME and the bill asks isWalkIn. It stays deliberately tolerant - walk in / walkin / walk_in, alone or followed by customer, guest or client - and deliberately anchored, so a real customer called Walkinshaw keeps their name on their own bill. A false positive costs a name nobody gave; a false negative is this bug. 21 tests, and the one that matters takes the name the WRITER uses and asks the READER about it, rather than a string typed into a test. A test that invents its own input can only confirm what its author believed the input looked like, and the belief was the bug. It also scans the source for any other spelling and fails if one is not recognised. Checked by restoring the old regex and watching it fail.
The walk-in name is one name
The same order again
A build script that cannot build
The till has priced extra cheese from the shop's own option documents since the handset learned about it, and the storefront sends the option sets to every client. This page named none of them, so somebody ordering from the table could not ask for a thing the waiter standing next to them could ring up. Three parts, and the middle one is the risk. THE CATALOGUE KEEPS THEM. The bundle's whole catalogue is one object literal, and a field it does not NAME never reaches the page however correctly the server sent it. That is the fourth time this literal has cost something. A BASKET LINE IS A DISH AND A CHOICE, NOT A DISH. Two dosas, one with extra cheese, are two lines. The basket has always been keyed by the dish's id, so choosing cheese on the second would have silently changed the first and the kitchen would have made two cheesy dosas for somebody who asked for one. The key is now the dish plus what was chosen, sorted so the same two options picked in either order are one line, and the dish's real id rides alongside as `item_id`. A line written before this change carries no item_id and still answers, because everything reads item_id first and falls back to id. AND IT NEVER SENDS A PRICE. The deltas are drawn on the sheet, because a charge that appears at checkout without having been shown is what people write bad reviews about - but what goes back to the shop is which options were chosen, by name. The shop prices its own extras. A page that could name the price of cheese could name a discount nobody agreed to. A group the shop capped at one draws radios and anything else draws ticks, because a tick that silently refuses to stay ticked is worse than one that visibly replaces another. An option that costs nothing shows no price: "+0" reads as a charge somebody has to work out is not one. The note and the spice level are set against the line the choice made, not against the dish - setCartItemNote returns quietly when it finds no line, so the dish's id would have lost them without a word.
The kitchen tells the floor before it tells anybody with a keyboard. A waiter who heard "no more fish" had to find whoever runs the till, and in the minutes that took, three more tables ordered it, three more tickets printed, and three tables were told no after they had chosen. NOT BY TOUCHING STOCK, which is the whole design. "Sold out" on the menu has always meant available_quantity <= 0: counted inventory, the figures the shop buys against. A waiter zeroing that to warn the floor would be writing a lie into purchasing to solve a problem that lasts one evening. So it is its own flag with its own lifetime. Set during service, gone by the next trading day, which begins at seven in the morning like the fish prices - a dish taken off during Friday service comes back on Saturday morning rather than at one a.m. in the middle of the last push. A kitchen that finds another crate can put it straight back, because whether the dish is off is a fact about tonight and the floor is where that is known. Only a dish the branch actually sells: a handset holds one shop's menu and has no business marking another shop's fish off, even by accident. Behind the ordinary session like every other write here, because taking a dish off the menu is a shop decision made by a person, even when that person is standing at a table holding a phone. THE SEVEN O'CLOCK RULE NOW HAS ONE HOME. It was written inside _pricedToday, whose own comment said four screens carried the same constant, and this would have been the fifth copy. A rule copied five times is five different rules the first time somebody changes one. It moved to utils/trading-day.js and _pricedToday delegates; the 21 existing daily-price tests pass unchanged, which is what makes that safe to say. Nine new tests, and 171 across the suites this touches.
…extras The customer can choose the extras the shop already charges for
A till that answers nothing
Three CI failures, all mine, all worth more than a green tick. THE HOUR GUARD CAUGHT THE REFACTOR, which is exactly its job. A test pins "every screen turns its day at the same hour, and it is seven" by reading each surface's source, and moving the till's copy into utils/trading-day left it looking at a file that no longer holds the constant. It now asks the question in two parts: the shared rule says seven, and the till still USES it rather than having quietly grown its own again. That second half is new, and it is the half that would catch somebody undoing this. THE DOCS GENERATOR ASKED FOR SOMETHING BETTER than a regenerate. Its placeholder said the endpoint "has no validation middleware, so its shape cannot be derived from the source. Adding validation improves both runtime safety and this document." So /items/soldOut now validates: which dish, whether it is off or back, which shop. The document describes a real body instead of an apology, and a caller who reads it can call it. And prettier on the one file it complained about - my lines only, no whole-file rewrite.
A waiter can say it has run out, without lying about the stock
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Acceptance criteria covered
How was it tested?
npm startfor the desktop app, ornpm run devfor a browser at http://localhost:3000)npm run build) if build/packaging was toucheddocs/CONTRIBUTOR_QUICKSTART.mdChecklist
git commit -s, DCO)