Repository navigation
feat: add orders, payment and an outbox to the shop plugin, with no route yet - #2
Merged
Merged
Conversation
Phase 1B waits for extension points Mallok has not released. This is the part of the payment phase that needs none of them: tables and logic, with no route, page or admin screen. Nothing in the deployed Worker calls it yet. Placing an order writes the order and its line snapshots in one batch and leaves stock alone. Taking stock when an order is placed would let scripted, unpaid orders empty the catalogue. Confirming a payment is one batch: the event, the stock of the stock-tracked lines, the ledger, the outbox row and the order's status commit together or not at all. The previous implementation decremented line by line and put stock back by hand; a crash in between left stock taken for an unpaid order, and two deliveries arriving together could both decrement. Three things make it safe to call any number of times, at any moment: - Every statement is conditional on the order still being in the status that was read, so a batch that arrives second finds nothing to do. - A payment is on record by its event id and by its type with the payment intent. Stripe can send two events about one payment. - What to do is read from the data, and read again after a write that failed: is the payment on record, what status is the order in, is the stock there. Never from the text of an error. An order whose stock has gone is marked oversold with nothing taken. A payment the order cannot take, because it was cancelled or another payment settled it, is recorded as refused with the payment to refund; the order and the stock are left alone. The outbox is what keeps a follow-up from being lost. An email sent after the batch, on the strength of a return value, is gone whenever the Worker stops in between or the batch's answer does not come back, and every later delivery then sees a paid order. So each change to an order writes what it owes in the same batch. Nothing drains the outbox yet. Shipping, delivery, cancellation and refund go through the state machine and are written only if the order is still where it was read. A refund returns what the ledger says the payment took. Money columns check their storage type: SQLite keeps 99.5 in an INTEGER column rather than refuse it.
The webhook is the only trustworthy word on whether an order was paid, so this is where a forged "payment succeeded" has to be stopped. No route calls it yet: Mallok's plugin routes parse the body before the handler runs, and a signature can only be checked against the bytes as received. Verification is HMAC-SHA256 over the timestamp and the raw body, on WebCrypto, with Stripe's five-minute tolerance. Only v1 signatures are read, several are accepted while a secret is being rolled, the timestamp is digits and nothing else, and an unset secret refuses everything. The decision takes the raw body, the header and the secret and returns what happened with the status to answer. The status follows one question: could delivering this again change the result? Stripe redelivers anything that is not a 2xx for three days, so only a database failure answers 5xx. An event type the shop does not act on, a payment with no order id, a payment naming an order this database does not have, and a payment for an order that cannot take it all answer 200. The last is recorded for a refund rather than signalled by failing. That differs from the previous route in two ways that matter on a shared Stripe account. A payment without an order id answered 400, and one for an unknown order answered 500; every shop on an account sees every payment of that account, so each would have been redelivered for days. The two Stripe calls, opening a hosted Checkout session and refunding a payment, are ported over fetch with no SDK. A failure is described by Stripe's identifiers and the status, never by its free-text message, which nothing stops from repeating a value that was sent. Stripe is never asked to charge nothing: a session for a total of zero creates no payment intent, and the order would wait for ever for an event that cannot come.
The two emails an order sends its buyer, in the language the order was placed in: English, German, French and Spanish, with a regional variant using its language and anything else falling back to English. Each has a plain-text part beside the HTML, since HTML alone renders blank in some clients. They are built in code with Mallok's escapeHtml, the way Mallok's own inquiry plugin builds its emails. Everything a person typed, product names, SKUs and tracking numbers, is escaped before it becomes markup in a buyer's inbox. A lead time is stated only when the order has one to state. The previous template could print one, and nothing supplied it. These build content only. Nothing sends them yet.
The design gains the order tables as built, the webhook flow as built, and a section on what changed while porting and why. It also records what an independent review found afterwards, five defects with their fixes, so the reasoning behind the outbox, the recorded refusals and the choice of webhook status is not left in a commit message. Twelve questions the routes will have to answer are listed and left undecided, among them who drains the outbox, what a free order does, and whether Stripe's Adaptive Pricing stays on. Two gaps on the Mallok side were found on the way and added to the design's list: a plugin cannot add its price to a page's structured data, and a job enqueued after a plugin's batch can be lost with the Worker. The README, the security policy and the contributor rules say what exists now: the logic and its tests, and no route that reaches them.
SQLite does not refuse 99.5 in an INTEGER column; it stores a real number. The first migration's price, stock, minimum-order and cart-quantity columns therefore accepted fractions, and only the code writing them stood between a rounding slip and a half-cent price in the catalogue. Each column now checks its own storage type, as the order tables do. A whole number arriving as text, the way a form field does, is still converted and accepted. The migration is edited in place rather than followed by a new one. A CHECK cannot be added to an existing table, and Mallok's migrator splits on semicolons, which rules out a trigger. Nothing has been deployed from this migration, so no database is left behind by the edit; a local development database created before it keeps the old columns until it is recreated.
Rows were read back oldest first by created_at, and that time is whatever the caller made the change with. Three changes to one order carrying the same time came back as delivered, paid, shipped: sorted by name. A consumer sending notices in that order would tell a buyer the parcel had arrived before it had left. The table now has a sequence of its own, and rows are read back by it. Within a batch that is the order of the statements, and across batches the order they committed in. The migration is edited rather than followed by another: it has not left this branch.
A write that threw was never repeated: if the next reading called for the same write, the first error was rethrown. That assumed the same write would fail the same way, which does not hold when the data moved in between. Stock taken by another order just before the batch, and returned by its refund just after, left a payable order answered with an error and waiting for Stripe to deliver the event again. A write that throws is now tried once more when the next reading still calls for it. The same write throwing twice running is a real failure, and the passes stay bounded at four. Tests are added for what a second review found untested: four passes that all lose, and a write that commits while its answer is lost, for an oversold order, a refused payment, a shipment and a refund. In each the change is made once and what it owes is in the outbox.
The reworked code went back to the reviewer. Nothing was found against the properties it has to hold; three smaller things were, two of which are fixed and one of which is recorded as it stands. Two more questions join the list left open for the routes: whether a payment naming an unknown order should leave a record, and what deleting a variant does to the orders that name it.
1 of 2 tasks
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.
Follows #1, which is merged. This now targets
mainand contains only its own eight commits.What this changes
Adds the part of the payment phase that needs nothing new from Mallok: tables and logic, with their tests. No route, page or admin screen calls any of it, so nothing changes for a visitor or an administrator.
migrations/0002_orders.sql): orders, order lines, payment events, the stock ledger and an outbox.lib/orders.ts): placing an order with line snapshots, reading one, and confirming a payment.lib/order-fulfilment.ts): ship, deliver, cancel, refund.lib/outbox.ts): every change to an order writes what it still owes — an email, a purge — in the same batch.lib/stripe-signature.ts,lib/stripe-client.ts,lib/stripe-webhook.ts): signature verification on WebCrypto, the two API calls overfetch, and the decision of what a delivery means and which status to answer.lib/order-email.ts): payment confirmation and shipping notice in four languages.Why
The next phase of the catalogue waits for Mallok. This is the work that does not have to wait, and it finishes the port list in the design's §8.
The properties it is built to hold:
How it was verified
npm run lintnpm run typechecknpm test— 10 project checks, 280 tests inside workerd, 150 of them newnpm run smoke:shop(when the change touches the plugin, the theme or the content) — andnpm run smoke,npm run buildThe tests were written first. Then 100 guards were broken one at a time and each turned a test red, including the ones that only show when two calls run at the same moment.
The code was then reviewed twice by a reviewer given the properties and not the implementation. The first pass found five defects, each fixed with a test that fails without the fix; the second found nothing against the properties and three smaller things, two fixed and one recorded. Both are written up in the design's §14.
Not verified. No purchase has been made against Stripe, in test mode or otherwise. Nothing has run on a Cloudflare account. Nothing drains the outbox yet.
Spec impact
Design §4.2 (the tables as built), §5.4 (the webhook flow as built) and a new §14: what changed while porting, what the reviews found, and fourteen questions left open for the routes.