Empty scope means everything
- Published
- System
- Klang Ya Bhaesaj
- Reading time
- 6 min
Two of the worst bugs I've shipped came out of the same file, and neither one crashed. Both were arithmetic that looked completely reasonable.
The promotions engine decides what a customer pays. It runs on cart preview, on quote creation, on every staff edit, and again at approval. If it disagrees with itself between any two of those, somebody gets charged the wrong amount.
Pure function, transactional caller
First decision: evaluatePromotions is pure. It reads candidate promotions, returns a priced result, writes nothing. Recording that a promotion was used, decrementing a usage cap, backfilling an order id — that's the caller's job, inside the caller's transaction.
That's what makes it safe to run the same engine at four different moments. Preview can evaluate all day with no side effects. Approval evaluates inside the same transaction that reserves stock, so price and stock commit or fail together.
If the engine wrote its own usage records, just looking at your cart would burn promotion quota.
Empty scope means everything
A promotion can be scoped to products, to categories, or to neither. That last one is where it gets sharp.
No product scope and no category scope means all items qualify. Empty isn't "nothing". It's "everything".
This has broken features twice, both times because a perfectly reasonable person read an empty list as an empty result. It's a shop-wide 10% promotion — of course it applies to everything. That's what "no restriction" means.
The trap is there are two different empties and they must not be confused:
- no scope rows → unrestricted → every item qualifies
- scope rows that expand to zero items → a real restriction that currently matches nothing → nothing qualifies
A promotion scoped to a category whose products all got delisted is not a shop-wide sale. Merge those two states and you've just given a full discount on the entire catalogue.
Which means the check has to happen before expansion. The flash-sale resolver computes hasScopeRows first, then expands. Anything that expands and then asks "is the list empty?" has already thrown the distinction away.
The clamp that was worth ฿150
Fixed-baht discounts come in two flavours and the difference isn't cosmetic:
| Mode | Meaning | Formula |
|---|---|---|
| per bill | ฿50 off this order | min(value, qualifyingSubtotal) |
| per unit | ฿1 off each item | Σ min(value, item.unitPrice) × quantity |
Look at the per-unit formula carefully. The min is inside the sum, applied per item, before multiplying by quantity.
An earlier version clamped only at the end, against the qualifying subtotal. Looks equivalent. Isn't. A ฿1-per-piece promotion on a cart full of cheap items produced ฿220 off instead of ฿70 — the discount was bigger than the stuff it was discounting, because nothing stopped a per-item discount from exceeding the item.
The rule that falls out: a per-unit discount can never exceed the unit's own price, and you have to enforce that per unit, at the point of multiplication. Clamping the total afterwards is a weaker statement that just happens to be right for most carts.
Which is why the tempting simplification — value × totalQuantity — is wrong and needs to stay wrong.
The display has to be honest too
The two modes need different treatment on the storefront, and this is correctness, not taste.
Per-unit actually changes the effective unit price, so it gets a struck-through price and a /ชิ้น label. Per-bill does not change the unit price at all — it takes a fixed amount off the total. Showing a struck-through unit price there is a lie: the customer multiplies the price they see by the quantity, gets a different number from the total, and now doesn't trust the cart.
So per-bill gets a badge saying ฿X off the bill, and the unit price is left alone.
Codes, and what auto-applies
Three states from two columns:
| promo_code | is_code_required | Behaviour |
|---|---|---|
| null | — | auto-applies only |
| set | false | auto-applies, and can be typed |
| set | true | never auto-applies; must be typed |
Row three is the one that earns its keep. A code you gave one customer over LINE must not quietly apply to everyone else's cart.
Codes that match nothing return 422 with the offending codes listed, rather than being silently dropped. Someone who types a code and sees no error and no discount assumes the site is broken. Codes get trimmed and uppercased before matching, because the cause is almost always a trailing space.
What I took from it
Neither bug was a crash, a type error, or anything a test suite catches for free. Both were rules whose plain-language version was ambiguous — "empty scope", "฿1 off" — and the ambiguity walked straight into the code.
Both rules now live as comments in the schema file, written as the sentence that got misread, not as a description of what the code does. It's the only documentation I've written that has provably stopped a regression.