Release: promote develop to main (one place decides what a dish costs) - #886
Merged
Merged
Conversation
kot.js carried THREE copies of the same discount-and-tax arithmetic: the
search results, and the two paths that re-read an order already sitting on a
table. Three copies of a price calculation is how a dish ends up costing two
different amounts on two screens, and nobody notices until a customer is
charged differently for the same thing twice.
They had already drifted, which is the part worth knowing:
the search path reads `selling_price`, and an item carrying tax with no
tax_type is treated as INCLUSIVE
the order paths read `selling_price || item_price`, and the same item is
treated as EXCLUSIVE
On a taxed item with no tax_type those two return different money. So the sum
is shared in _priceFrom and THE DIFFERENCES ARE KEPT, passed in by each
caller through _priceOf and _priceOfOrderLine. Folding the defaults together
would quietly reprice every untyped item on one screen or the other, and that
is a decision about money for the owner to make on purpose rather than
something to slip into a refactor.
Proven rather than eyeballed: the tests run the real functions against the
numbers the old copies produced, on ten combinations of tax, tax type and the
two kinds of discount. The catalogue path was also compared against the
committed version of the file directly - identical on every case. One test
asserts the two defaults still DISAGREE, so that if somebody makes them
agree it fails and asks them to mean it.
The count of copies is asserted too. A fourth will say so before it can
drift.
…-costs One place decides what a dish costs
develop was carrying a failing suite, and the failure was the test's fault twice over. A handset was given its own, longer token lifetime - reasonable, since a phone on a floor all day should not expire like a till - and two assertions pinned to exact source text went red for it: signLegacyToken(recordsFiltered, req) became signLegacyToken(recordsFiltered, req, undefined, handsetSeconds) expiresIn: jwtLifetimeSeconds() became expiresIn: handsetSeconds Neither weakened anything. The credential is still signed from the user who signed in, and the client is still told when it expires. But a suite that fails when its subject improves is a suite people stop reading, and this one had already gone quiet on develop. So both now assert the rule. The first checks that the user and the request are what the token is signed FROM, whatever else follows them. The second reads the lifetime argument out of the call and requires expiresIn to report THAT identifier - which is the property that matters, because a client told a different figure refreshes too often or, worse, too late. Proven by making the handler report a lifetime the token does not carry: it fails and names the one that was signed. Not my area - this is the handset login - and nothing in the controller is touched. A red develop is everybody's problem, so the assertions are fixed rather than left for whoever wrote them to find.
…ned-it The token test checks who signed it, not how it was typed
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
Promotes the pricing unification to main. Nothing here needs the cloud - it is all till-side renderer code - but main should not sit behind develop, and the installer is built from this.
Commits
Verified on develop by content
Merge with a merge commit, never squash.