amadan
Multi-currency: the third review's findings
A currency refusal expires only the intent's own session: the client returns the refused session, and the service checks whose it is before anything is expired. A bound session completed in another currency files the same review once, from the pass, the webhook or a direct binding, and expires nothing. The barrier trigger reads the calculation's version as JSON and counts only book assignments that were active and paid into the month, so a purchase cancelled before its first reconciliation no longer stops this build's own legacy month. The billing page is one form: the country and currency controls, the plans, the postal code and the consent, carrying what the page priced; a post for a country or currency the page did not price is first shown as a quote, a currency shown as the country's default and not chosen reaches the door as none, the quote button is a door of its own that buys nothing, an account that bought in a currency is offered it alone, and the inclusions beside each price are the quoted band's. A ?currency= or ?country= that is present and empty clears its cookie, so the pricing page's choice of nothing is kept. The provisioning verifier applies the console's own currency_options rule and accepts a map with more than the 36 keys; its test fails rather than skips. The design, the dispositions and a test comment say what the code does.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
17 files changed,
+1087
−172
docs/superpowers/specs/2026-10-01-multi-currency-price-book-design.md+8 −8docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review3.md+133 −0docs/superpowers/specs/DISPOSITIONS-multi-currency.md+30 −1docs/superpowers/specs/OPUS-REVIEW-multi-currency-review3.md+142 −0implementation/carloku-paid-launch/stripe-provision.sh+6 −3internal/billingstripe/client.go+7 −4internal/console/billingcurrency.go+19 −9internal/console/billingcurrency_test.go+466 −64internal/console/billinghandlers.go+80 −4internal/console/billingpage.go+114 −31internal/console/billingpage_test.go+5 −5internal/console/billingservice.go+37 −14internal/console/billingstore.go+17 −9internal/console/server.go+1 −0internal/console/templates/billing.html+18 −18internal/console/templates/layout.html+1 −1internal/plans/provision_test.go+3 −1
CI passed — run details
diff --git a/docs/superpowers/specs/2026-10-01-multi-currency-price-book-design.md b/docs/superpowers/specs/2026-10-01-multi-currency-price-book-design.md| index 532568cb7..41b0cec88 100644 |
| --- a/docs/superpowers/specs/2026-10-01-multi-currency-price-book-design.md |
| +++ b/docs/superpowers/specs/2026-10-01-multi-currency-price-book-design.md |
| @@ -144,11 +144,11 @@ func PublicPriceBook(book string) ([]byte, error) // the export below, byte-stab |
| ## Console data contract |
| -New columns, both `TEXT NOT NULL DEFAULT ''`, on `billing_checkout_intents` and `billing_accounts`: `currency`, `price_book`. Empty in both means a legacy row. Assignments of a purchase priced from a book go in a new table, `billing_book_assignments`, with the same columns plus `currency` and `price_book` required non-empty; `billing_assignments` keeps only legacy assignments. The split is for rollback: a build from before books reads `billing_assignments` alone, so it finds no history for a book-priced purchase. For an account with both kinds of history it would find the legacy half, so a database trigger (`billing_months_no_legacy_calc_over_book_history`) refuses to freeze a version 1 calculation for any month a book-priced assignment of the account begins in or before. The environment and the binary change together, and no binary from before books runs against a database with a book-priced row (roll note in the dispositions). |
| +New columns, both `TEXT NOT NULL DEFAULT ''`, on `billing_checkout_intents` and `billing_accounts`: `currency`, `price_book`. Empty in both means a legacy row. Assignments of a purchase priced from a book go in a new table, `billing_book_assignments`, with the same columns plus `currency` and `price_book` required non-empty; `billing_assignments` keeps only legacy assignments. The split is for rollback: a build from before books reads `billing_assignments` alone, so it finds no history for a book-priced purchase. For an account with both kinds of history it would find the legacy half, so a database trigger (`billing_months_no_legacy_calc_over_book_history`) refuses to freeze a calculation whose top-level `v` is not 2 (read with `json_extract`) for any month in which an active book-priced assignment of the account was paid for (began before the month ended, paid through past its start). A book assignment that never counted for a month, a purchase cancelled before its first reconciliation, say, is left out of that, so this build's own version 1 for a legacy-only month after it still goes in. The environment and the binary change together, and no binary from before books runs against a database with a book-priced row (roll note in the dispositions). |
| -- **Checkout** (`POST /a/{sqid}/billing/checkout`): a new optional form field `currency`. Absent or empty: the country's default. Present and not `usd`, `eur` or `gbp`: 400, nothing is created. The intent stores the chosen currency and book. A retry with a different currency while an intent is open is a different intent, exactly as a different plan is today. |
| -- **A currency already held**: checkout is refused while the account's subscription has not ended (`canceled` or `incomplete_expired`), and after it has ended a checkout in any currency but the one bought in is refused, the billing country's default included. A change of currency waits on what Stripe permits for a customer whose subscription ended (test-mode item 4) and is then an operator's door, not the form's. |
| -- **Stripe price identity**: the configured price for (book, plan, band, currency) must have that `currency`, `unit_amount` equal to the book's unit price, no `currency_options`, and metadata `price_book`, `currency`, `terms_version`, `plan`, `band` that all match. The checks that exist today (per-unit, yearly, licensed, exclusive tax, active, mode) stay. |
| +- **Checkout** (`POST /a/{sqid}/billing/checkout`): one form, with the country and currency controls, the plan, the postal code and the consent. The form carries what the page priced (`quoted_country`, `quoted_currency`) and whether the currency shown was a choice (`currency_chosen`). A post whose country or currency is not what the page priced is not a purchase: it is answered with a redirect to the page quoting the posted country (and the posted currency when it is a choice), so nothing is bought at a price the buyer has not seen. A currency shown as the country's default and not chosen is given to the door as no currency, and the billing country's own currency is bought in; a chosen one (the URL's, the pricing page's, the account's, or changed on the form) is the purchase's. A chosen currency that is not `usd`, `eur` or `gbp`: 400, nothing is created. The intent stores the chosen currency and book. A retry with a different currency while an intent is open is a different intent, exactly as a different plan is today. `POST /a/{sqid}/billing/quote` is the form's "Show prices for this country" button: the same form, answered with the same redirect, buying nothing; it asks for the session and the form's token, no step-up. |
| +- **A currency already held**: checkout is refused while the account's subscription has not ended (`canceled` or `incomplete_expired`), and after it has ended a checkout in any currency but the one bought in is refused, the billing country's default included; the page offers that currency alone, as a choice. A change of currency waits on what Stripe permits for a customer whose subscription ended (test-mode item 4) and is then an operator's door, not the form's. |
| +- **Stripe price identity**: the configured price for (book, plan, band, currency) must have that `currency`, `unit_amount` equal to the book's unit price, no `currency_options` but, at most, an entry under its own currency at its own amount and tax behaviour (which an expanded read of a single-currency price may carry), and metadata `price_book`, `currency`, `terms_version`, `plan`, `band` that all match. The checks that exist today (per-unit, yearly, licensed, exclusive tax, active, mode) stay. |
| - **Checkout Session**: created with `currency` and `adaptive_pricing[enabled]=false`, and with `currency`, `price_book` in the session and subscription metadata. A purchase with no book (only a console with no book configured opens one, and an intent from before books is replayed as that build sent it) is asked for exactly as before: no currency, Adaptive Pricing on. Every session the console creates, replays, lists, reads back or binds passes one check against its intent; one that is this intent's and not its purchase (another currency, a conversion, Adaptive Pricing) is expired and the intent retired, and one already paid is left for a person with a review filed once. A subscription whose price is not the configured one for its stored (book, plan, band, currency), or that the provider reports in another currency, is an identity mismatch. |
| - **Configuration**: `CONSOLE_BILLING_PRICE_BOOK` names the book and is required. `CONSOLE_STRIPE_PRICES` holds the 36 entries of the configured book, keyed `"<price_book>/<plan>/<band>/<currency>"`. Beside them it may hold the whole catalog of any other book this build has, and the twelve legacy keys `"<terms>/<plan>/<band>"`, each set all or none; those recognise subscriptions already bought and sell nothing. No price under two keys. Live mode with a draft book is refused. |
| - **Monthly calculation**: a month priced from a book is a version 2 snapshot: `currency`, `price_book`, and amounts under currency-neutral names. A version 1 snapshot is EUR, is read as it was written, and is never priced again. A stored calculation whose currency is not the month's assignment currency is refused. |
| @@ -157,7 +157,7 @@ New columns, both `TEXT NOT NULL DEFAULT ''`, on `billing_checkout_intents` and |
| ## Carrying a choice through sign-in |
| -The pricing page links to the console with `?currency=<code>` and, when the visitor picked a billing country there, `&country=<ISO2>`. The console accepts both on any GET, validates each, and keeps each in a short-lived cookie (`carloku_currency`, `carloku_country`; host-only, `SameSite=Lax`, one hour). An invalid value is dropped and no cookie is set: a currency must be one the book sells in, a country must be two upper-case letters in the deployment's band table, as the pricing page spells it. The billing page preselects the currency from its cookie, else from nothing; it quotes the country its own form names, else the country cookie's, else none: the country the owner then picks decides the default currency. The cookies are suggestions; only the `currency` and `country` fields of the checkout form are choices, and checkout reprices from them. No geo lookup, no external service. |
| +The pricing page links to the console with `?currency=<code or empty>&country=<ISO2 or empty>`, always both: the currency is given only when the visitor chose it (the URL or the control), never from the browser's locale or a country's default, and the country is the one selected. The console accepts both on any GET, validates each, and keeps each in a short-lived cookie (`carloku_currency`, `carloku_country`; host-only, `SameSite=Lax`, one hour); a parameter that is present and empty clears its cookie, so a choice from an earlier visit does not come back. An invalid value is dropped and no cookie is set: a currency must be one the book sells in, a country must be two upper-case letters in the deployment's band table, as the pricing page spells it. The billing page prices from its own URL first (`?country=`, `?currency=`, which its one form's quote button and any changed post lead to), and falls back to a cookie only for a parameter the URL does not name at all. The cookies carry a choice the visitor made and saw; only the fields of the one checkout form buy anything, under the quoted-price rule above. No geo lookup, no external service. |
| The billing page (`billingpage.go`, built on the layout branch's template): `s.billingCurrencies()` gives the currencies to offer, `s.suggestedCurrency(r)` the one to preselect (may be empty), `s.suggestedCountry(r)` the country to quote when the form names none, the prices come from `plans.PriceFor`, and the checkout form posts `currency` beside `country`. |
| @@ -203,12 +203,12 @@ The billing page (`billingpage.go`, built on the layout branch's template): `s.b |
| - `internal/plans/pricebook.go`, `pricebook.json`, `public-pricebook.json`, `cmd/pricebook-export`: the book, its validation, `ChooseCurrency`, `PriceFor`, `OverageFor`, `OverageInvoiceFor`, the export. |
| - `internal/plans/month.go`: assignments and month allowances carry the book and currency; a month in two currencies is refused. |
| - `internal/billingstripe`: a currency on every checkout session, draft invoice and invoice item; Adaptive Pricing off; a session that comes back in another currency, converted, or with Adaptive Pricing on is refused; sessions, subscriptions and items report their currency. |
| -- `internal/console/billingstore.go`: the six columns; an intent and a recorded subscription need whole price terms; a subscription never changes currency or book. |
| +- `internal/console/billingstore.go`: the four columns (two on intents, two on accounts), the `billing_book_assignments` table and the barrier trigger; an intent and a recorded subscription need whole price terms; a subscription never changes currency or book. |
| - `internal/console/billingservice.go`: checkout chooses and stores the currency, checks the provider's price against the book, and reconcile requires the configured price for exactly the stored plan, band, book and currency and a subscription in that currency. |
| - `internal/console/billingcalculation.go`, `billinginvoice.go`: the version 2 calculation; the engine sends and verifies the calculation's own currency; legacy request hashes unchanged. |
| - `internal/console/billingcurrency.go`: the two cookies and the list of currencies offered. |
| -- `internal/console/billingpage.go` and `templates/billing.html`: the currency control in the approved words, prices in the page's currency at the quoted country's band, the checkout form posting the currency it shows. |
| -- `cmd/console/billing.go`: `CONSOLE_BILLING_PRICE_BOOK` is required; 36 prices, or 48 with all twelve legacy keys; live mode refuses a draft book. |
| +- `internal/console/billingpage.go`, `billinghandlers.go` and `templates/billing.html`: one form with the country and currency controls in the approved words, prices and per-member inclusions at the quoted country's band in the page's currency, the quoted-price rule and the quote door, the account's held currency offered alone. |
| +- `cmd/console/billing.go`: `CONSOLE_BILLING_PRICE_BOOK` is required; the 36 prices of the configured book, beside which the twelve legacy keys (all or none) and the whole catalog of any other book this build has may stand; live mode refuses a draft book. |
| With no book configured (`BillingConfig.PriceBook` empty, which only tests and the demo do) the console behaves as it did: EUR, the terms' own figures, rows with no currency. Every billing test from before this change passes in that mode, unedited except for call signatures and fakes that now report a currency. |
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review3.md b/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review3.md| new file mode 100644 |
| index 000000000..d6e3c5f5e |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review3.md |
| @@ -0,0 +1,133 @@ |
| +All platform references are at `de81dd70`; site references are at `15317d2`. This is source-only: no files changed, tests executed or network commands run. Statements about test failures are deductions from the source. |
| + |
| +**Second-review findings, individually** |
| + |
| +| Finding | Closed? | Would its test pass without the fix? | |
| +|---|---|---| |
| +| Astra P1: older console freezes incomplete mixed legacy/book history | **Original case closed**, by `billingstore.go:183`. The trigger also blocks a legitimate current-build calculation; finding 3 below. | **No.** Removing the trigger fails the assertion at `billingcurrency_test.go:1464`. | |
| +| Astra P2: invalid bound session remains payable | **Closed for an open session belonging to the intent**, on both resume paths and the background pass (`billingservice.go:528`, `:583`; `billingpass.go:205`). | Removing expiry fails the tests. Removing the **entire first resume guard still passes**; finding 8. | |
| +| Astra P3: historical catalogs lack regression coverage | **Closed**, `cmd/console/billing_test.go:105`. Two books, incomplete catalogs and shared provider IDs are covered. | **No** if historical-catalog support is removed. Removing only the completeness guard still passes because the used-key count independently rejects partial catalogs. | |
| +| Astra P3 / Opus P3-2: surviving checkout mutations are not redundant | **Partially closed.** Wrong-intent creation and the second resume check now have distinguishing cases. The first resume check remains masked. | **Yes for the first guard; no for the other two.** | |
| +| Opus P2-1: expanded own-currency option rejected | **Closed in checkout**, `billingservice.go:179`. The provisioning verifier retains the old assumption; finding 9. | **No.** The own-only acceptance case at `billingcurrency_test.go:253` fails with the former check. | |
| +| Opus P3-1: paid refused session gets no review | **Partially closed.** Unbound listing recovery files a review; bound completion still does not. Finding 4. | **No** for removing the listing-recovery fix. Those tests nevertheless pass with the bound-completion hole. | |
| +| Opus P3-3: case-variant duplicate keys | **Closed for the reported ASCII case**, `plans/pricebook.go:209`. | **No.** The `Status` duplicate case requires case folding. | |
| +| Opus P3-4: rollback leaves stale account columns | **Addressed by the explicit no-downgrade rule**, `DISPOSITIONS-multi-currency.md:70`, not by preventing those writes. Its mixed-history subcase is covered by the trigger above. | No executable regression for the operational restriction. | |
| +| Opus claim 1: vacuous explicit-EUR golden tail | **Closed**, `billingcurrency_test.go:1271`, with an exact expected-byte comparison. | Reverting the assertion alone still passes correct code, but restores its inability to detect unrelated byte changes. | |
| +| Opus claims 2 and 5: empty-book and background-POST comments | **Corrected**, `cmd/console/billing_test.go:229`, `billingservice.go:1088`. | Not applicable to comment corrections. | |
| +| Astra claims: already-recorded bind; “before the provider is asked”; stale design details | The already-recorded bind exception is documented. **The other corrections remain incomplete**, detailed below. | Existing tests do not enforce those prose claims. | |
| + |
| +**Findings, most severe first** |
| + |
| +1. **P2 — Currency rejection can expire another account’s checkout before checking ownership.** |
| + |
| + **Where:** `internal/billingstripe/client.go:651`; `internal/console/billingservice.go:614`, `:1094`, `:286`. |
| + |
| + **Scenario:** A GBP checkout request receives an inconsistent provider response containing another account’s open EUR session and a valid Stripe URL. `CreateCheckout` returns only `CodeCurrencyMismatch` and its session ID, discarding ownership information. The service calls `refuseSession(..., nil, ...)`, which expires that ID immediately. The ownership checks in `billingstore.go:400` and `:423` happen afterward, too late to prevent cancellation. |
| + |
| + This is a provider-response validation gap, not evidence that an ordinary buyer can induce the response. The existing wrong-intent test uses the correct currency and misses this combined failure. |
| + |
| + **Smallest fix:** Retain or retrieve the rejected session; verify intent, account, customer and mode before expiry. Inspect paid/completed status too. Add a wrong-owner **and** wrong-currency case requiring zero expiry calls. |
| + |
| +2. **P2 — The visible currency and country controls can disagree with checkout.** |
| + |
| + **Where:** `internal/console/templates/billing.html:44`, `:53`, `:69`, `:96`. |
| + |
| + **Scenario:** Load `?country=IN¤cy=gbp`. Change the visible currency selector to USD, then press **Continue to payment** without submitting the separate quote form. Checkout still posts hidden `currency=gbp`. Conversely, changing the checkout country to Ireland leaves the India quote visible: Personal remains shown at £7.70, but checkout prices Ireland at £22. |
| + |
| + A fresh page also posts its default USD as an explicit override even when the buyer only selects Ireland in the checkout country field. The backend therefore never gets an absent currency from that flow. |
| + |
| + **Smallest fix:** Use one authoritative set of checkout controls and require a matching quote before payment continues. Preserve whether currency was explicitly overridden so a country change can otherwise choose its default. Test actual control changes and submission; `TestBillingPagePostsTheCurrencyItShows` only extracts an unchanged hidden field. |
| + |
| +3. **P2 — The rollback trigger rejects a legitimate legacy calculation from this build.** |
| + |
| + **Where:** `internal/console/billingstore.go:183`; `internal/plans/month.go:160`; `internal/console/billingcalculation.go:185`. |
| + |
| + **Scenario:** A legacy subscription ends September 10. An EUR book subscription starts September 12 and is canceled before its first reconciliation. Reconciliation accepts the canceled subscription and records a canceled book assignment dated to creation (`billingservice.go:718`, `:779`). |
| + |
| + `AllowanceForMonth` excludes that nonactive assignment. September’s paid spans are therefore legacy-only, and this build correctly produces version 1. The trigger nevertheless rejects it because **any** earlier book assignment qualifies. Repeated attempts cannot freeze the legitimate legacy bill. |
| + |
| + **Smallest fix:** Distinguish paid book spans overlapping the month from nonpaid history while retaining the old-reader barrier. Add this current-build case alongside the existing mixed-history downgrade regression. |
| + |
| +4. **P2 — A bound session completed in the wrong currency still receives no billing review.** |
| + |
| + **Where:** `internal/console/billingpass.go:187`; `internal/console/billingservice.go:904`. |
| + |
| + **Scenario:** A valid GBP session is saved and offered, then completes and reads back as EUR. The background pass takes its `complete` branch into `bindBillingCheckout`, which returns `errBillingIdentity` directly. It never calls `refuseSession`, so no `billing_checkout_currency_refused` review is filed. The account remains unactivated with a pending intent. A checkout webhook records a rejected event, but does not create the promised account review. |
| + |
| + The new paid-session tests exercise only **unbound listing recovery**. |
| + |
| + **Smallest fix:** Route an owned completed-session mismatch through the common review path during binding/reconciliation. Test a previously bound session across repeated passes and webhook delivery, requiring one review and no expiry. |
| + |
| +5. **P2 — The billing page displays unscaled inclusions beside band-scaled prices.** |
| + |
| + **Where:** `internal/console/templates/billing.html:88`; `internal/console/billingpage.go:333`. |
| + |
| + **Scenario:** `?country=IN¤cy=gbp` correctly quotes Personal at £7.70, but its inclusion row still states 85 awake hours, 2 GB storage and 2 GB transfer. Band D grants **29.75 hours, 0.7 GB and 0.7 GB**. Community similarly displays 17 hours per member where the grant is 5.95. |
| + |
| + The general “35%” note does not make those concrete inclusion figures correct. This concerns the rendered numbers, not the approved wording. |
| + |
| + **Smallest fix:** Populate inclusion figures from the same `PriceFor` result as the price, respecting pooled quantity arithmetic. Add non-A-band assertions for displayed resources. |
| + |
| +6. **P2 — The pricing site displays overage rates different from the exported rates.** |
| + |
| + **Where, site:** `copy-review/pricing/build.mjs:33`; `test/pricing-pricebook.test.mjs:102`. |
| + |
| + **Scenario:** Formatting every rate with `toFixed(3)` turns GBP writes from **£0.0207** into **£0.021 per 1,000**. One million excess writes costs £20.70, while the displayed rate implies £21. USD storage similarly becomes $0.127 instead of $0.1272. |
| + |
| + The test repeats the same rounding, so it accepts the defect. |
| + |
| + **Smallest fix:** Format micro-unit rates exactly, retaining significant digits beyond three decimals. Assert that parsing the displayed rate reproduces the exported micro amount. |
| + |
| +7. **P3 — Clearing the pricing site’s country revives an old console country.** |
| + |
| + **Where:** `internal/console/billingpage.go:193`; site `src/site.js:38`, `:60`. |
| + |
| + **Scenario:** An earlier visit stored `carloku_country=IN`. The visitor returns to pricing, clears the country and selects GBP. The site shows band A, Personal £22, and removes `country` from the signup URL. The console interprets the omission as permission to reuse India, displaying and preselecting band D at £7.70. |
| + |
| + The changed quote is visible before payment, but the handoff does not preserve what the visitor saw. |
| + |
| + **Smallest fix:** Carry an explicit “country cleared” signal that clears the suggestion, while ordinary sign-in redirects continue preserving it. Test the link with an existing country cookie. |
| + |
| +8. **P3 — The first resume guard still lacks an independent regression case.** |
| + |
| + **Where:** `internal/console/billingcurrency_test.go:362`; `internal/console/billingservice_test.go:137`. |
| + |
| + **Scenario:** The test supplies `{"eur"}` or `{"gbp","eur"}`. The fake repeats its last currency forever. Delete the entire first guard at `billingservice.go:528`: in both cases, the second guard still observes EUR and performs the expected expiry. All existing assertions pass. |
| + |
| + **Smallest fix:** Add `{"eur","gbp"}`. Correct code expires after the first read; removing the first guard then permits the second valid response to return the URL. This qualifies the coverage claim; it does not establish what exact mutations were used in the reported run. |
| + |
| +9. **P3 — Provisioning verification still rejects the own-only currency-options shape accepted by checkout.** |
| + |
| + **Where:** `implementation/carloku-paid-launch/stripe-provision.sh:406`, `:411`. |
| + |
| + **Scenario:** An expanded price response contains only its own currency, matching amount and tax behavior. Runtime checkout now accepts it, but the printed verification command requires `currency_options` to be empty and reports `PRICE CHECK FAILED`. |
| + |
| + **Smallest fix:** Apply the same empty-or-matching-own-only rule in the verifier and test both accepted shapes plus an additional-currency rejection. |
| + |
| +Outside those findings, the unchanged IN/GBP form follows the intended money path: posted fields → `startBillingCheckout` → immutable intent → fixed-currency `CreateCheckout` → bind/reconcile → account and book assignments → `billingInvoiceHistory` → `AllowanceForMonth` → version 2 calculation. Drafts, items, recovery, invoice verification, finalization and payment retain the frozen GBP currency. The quantity worker checks both the subscription/item read and the update response. I found no further conversion, historical repricing or request-rehash path. |
| + |
| +Version 1 marshaling retains the legacy bytes; draft/item hashes omit the appended currency field while their HTTP requests retain EUR. Configuration rejects missing books, a selected draft book in live mode, incomplete catalogs and shared price IDs. Startup migration supplies the new schema; the documented prohibition on running older binaries after book purchases remains necessary. |
| + |
| +Checkout reads `PostForm`, so changed cookies in another tab cannot alter an already-rendered submission. Invalid and oversized suggestion values cannot become unsupported stored currencies or countries; valid forged values remain suggestions. No concrete wall-clock or test-order failure was found, and the page tests deliberately use fresh cookie jars. |
| + |
| +The export contains no customer, credential or provider identifiers. The site’s current resource figures match the pooling rule at one member and at the ten-member minimum: all current per-member band products are integral in base units. Initial script-free HTML correctly shows USD/band A with USD and blank country selected. Normal scripted links use lowercase currency and uppercase country consistently. |
| + |
| +**Claims the code contradicts** |
| + |
| +- `DISPOSITIONS-multi-currency.md:57` and design line 152 overstate paid-session handling: bound completion misses the review. “Nothing is asked of the provider again” is also too broad; subsequent passes still perform reads. |
| +- `billingstore.go:180` and dispositions line 55 say this build never produces a version 1 calculation rejected by the trigger. Finding 3 supplies a valid counterexample. |
| +- The first resume guard is not independently protected by the new sequential-response fixtures. |
| +- Dispositions line 66 says the provider-call wording was corrected, but `billingcurrency_test.go:1276` still says nothing is asked of the provider. Catalog and configuration reads precede the guard. |
| +- Dispositions line 68 says stale design details were corrected, but design lines 206 and 211 still say “six columns” and “36 prices, or 48,” omitting the distinction between four added columns, the new table and optional historical catalogs. Its literal “no `currency_options`” rule also needs the own-only exception. |
| +- The site’s “every price, rate and default” provenance claim is broader than its implementation. Free resource figures, team minima and band percentages remain literal strings (`build.mjs:95`, `:138`, `:238`); currency/default-country rules are duplicated in `site.js:24`. These currently agree with the export, but are not derived from it. |
| + |
| +**What must be verified against Stripe test mode before enabling** |
| + |
| +1. **Catalog:** Read all 36 prices using the pinned API version and expanded `currency_options`. Establish the actual empty/own-only shape and verify exact amounts, currencies, recurrence, tax behavior and metadata against the book. |
| +2. **Checkout and completion:** Exercise USD/EUR/GBP with countries whose defaults differ, including IN/GBP. Verify displayed subtotal, disabled Adaptive Pricing, no conversion, and matching session/subscription/item currencies. Deliberately incompatible price/session currencies must fail without a payable checkout. |
| +3. **Overages:** Exercise every meter, rounding boundary and floor in each currency through draft, item creation, finalization and payment. Compare every line, subtotal and tax result with the frozen calculation; include canceled subscriptions. |
| +4. **Recovery:** Lose responses after checkout, draft, line, finalize and pay operations, inside and beyond replay windows. Verify recovery retains original objects, currencies, amounts and keys without duplicate charges. Include legacy checkout requests and version 1 invoice operations. |
| +5. **Quantity and renewal:** Run `TestSandboxQuantityFence`, both proration policies and renewal. Inspect actual generated invoices and proration lines, not merely the updated item, for the stored currency and expected amount. |
| +6. **Customer constraints and discounts:** Establish customer-currency behavior with active/canceled subscriptions and outstanding historical invoices. Exercise percentage, house and incompatible fixed-currency discounts. An old-style Adaptive session must establish whether its resulting subscription remains compatible with the deliberate legacy-EUR restriction. |
| + |
| +**Verdict: reject.** |
| \ No newline at end of file |
diff --git a/docs/superpowers/specs/DISPOSITIONS-multi-currency.md b/docs/superpowers/specs/DISPOSITIONS-multi-currency.md| index b2c6fb97b..383dda39e 100644 |
| --- a/docs/superpowers/specs/DISPOSITIONS-multi-currency.md |
| +++ b/docs/superpowers/specs/DISPOSITIONS-multi-currency.md |
| @@ -54,7 +54,7 @@ Reports: `OPUS-REVIEW-multi-currency-review2.md`, `CODEX-FINDINGS-multi-currency |
| | --- | --- | --- | --- | |
| | Astra P1: the table split did not stop an older build on mixed history. A legacy subscription that ended on the 10th and a book one bought on the 12th: the older build finds the legacy half, prices the month from it alone in EUR, and can waive it. | Fixed, in the database. A trigger refuses to freeze a version 1 calculation for any month a book-priced assignment of the account begins in or before. The older build writes only version 1, so it cannot freeze such a month; this build never writes one for it. A legacy-only month before the purchase, and a legacy-only account, stay the older build's. | `billingstore.go` (`billing_months_no_legacy_calc_over_book_history`) | `TestOlderBuildCannotFreezeAMonthWithBookHistory` | |
| | Astra P2: a bound session that reads back in another currency was refused but left open, so a buyer with its URL could still pay. | Fixed. On the resume paths and in the background pass, a bound session that is this intent's and not its purchase is expired and the intent retired (`refuseSession` handles bound and unbound intents). | `billingservice.go`, `billingpass.go`, `billingstore.go` `retireIntent` | `TestCheckoutNeverOffersASessionInAnotherCurrency` (two new cases, one with a provider that answers in pounds on the first read and euros on the second) | |
| -| Opus P3-1: a refused session that was already paid wedged the intent, was asked to expire on every pass, and nobody was told. | Fixed. A completed or paid session is not expired; a review (`billing_checkout_currency_refused`) is filed once, the intent is left as it is so no second purchase opens beside it, and nothing is asked of the provider again. | `billingservice.go` `refuseSession` | `TestRecoveredSessionMustBeInTheIntentsCurrency` (the two "untouched" cases, across two passes) | |
| +| Opus P3-1: a refused session that was already paid wedged the intent, was asked to expire on every pass, and nobody was told. | Fixed. A completed or paid session is not expired; a review (`billing_checkout_currency_refused`) is filed once, the intent is left as it is so no second purchase opens beside it, and nothing is written at the provider again (later passes still read the session). | `billingservice.go` `refuseSession` | `TestRecoveredSessionMustBeInTheIntentsCurrency` (the two "untouched" cases, across two passes) | |
| | Opus P2-1: `currency_options` may hold the price's own currency when expanded, and the check would then refuse every real price. | Fixed. An entry under the price's own currency, at its own amount and tax behaviour, is accepted; any other entry is refused. Which shape Stripe returns is test-mode item 1; either passes. | `billingservice.go` `priceCurrencyOptionsAreOwn` | `TestCheckoutRefusesAProviderPriceThatIsNotTheBooks` (own-only accepted; own plus another, another alone, own at another amount refused) | |
| | Astra P3: the catalog of another book had no test. | Fixed through a seam: the console's book list, currencies and approval come from package variables a test can set. Two books, the old one whole or not at all, no price serving both. | `cmd/console/billing.go` | `TestBillingConfigKeepsCatalogsOfOtherBooks` | |
| | Opus P3-2, Astra P3: the surviving mutations were not redundant. | Corrected above; each has a case: a created session carrying another intent's id; a provider answering differently on two reads. | tests | as above | |
| @@ -73,3 +73,32 @@ Mutation checks on these fixes, nine in a scratch copy: eight failed their tests |
| The fixes are in the commit that adds this section and have not been read by a reviewer. |
| + |
| +## Review three (at de81dd70; Opus accepted with fixes, Astra rejected) |
| + |
| +Reports: `OPUS-REVIEW-multi-currency-review3.md`, `CODEX-FINDINGS-multi-currency-review3.md`. Both found the second review's findings closed and the GBP purchase carrying one currency end to end; Astra found the first resume guard not independently covered (below). The two disagreed on severity, not on the code: every finding of either is treated as a defect here, and each has a test that fails without its fix. |
| + |
| +| Finding | Disposition | Where | Reproduction | |
| +| --- | --- | --- | --- | |
| +| Astra 1 (P2): a currency refusal expired the session the provider answered with before checking whose it was. | Fixed. The client returns the refused session itself, not its id alone; the service expires it only when `ownSession` says it is this intent's, else refuses the attempt and touches nothing. | `billingstripe/client.go` (`Error.Session`), `billingservice.go` (both create sites) | `TestCurrencyRefusalExpiresOnlyTheIntentsOwnSession` (another intent's session in another currency: zero expiries, the intent kept) | |
| +| Astra 4 (P2): a bound session that completed in another currency was refused in `bindBillingCheckout` with no review. | Fixed. The owned, completed, wrong-currency session files the same review once, through the pass, the webhook and a direct binding; nothing is expired, nothing recorded. | `billingservice.go` `bindBillingCheckout` | `TestBoundSessionCompletedInAnotherCurrencyIsReviewedOnce` | |
| +| Astra 3 (P2): the barrier trigger refused this build's own version 1 for a legacy-only month after a book purchase that was cancelled before its first reconciliation. | Fixed. The trigger counts only book assignments that were active and paid into the month (`status='active'`, not free, began before the month's end, paid through past its start), which is what `AllowanceForMonth` prices from. The claim at `billingstore.go` that this build never writes a refused version 1 is true again. | `billingstore.go` | `TestBarrierTriggerLetsThisBuildsLegacyMonthsThrough` (the cancelled purchase: September freezes; an active one: refused) | |
| +| Opus P3-3: the trigger read the version by substring. | Fixed: `json_extract(... '$.v') IS NOT 2`, with `json_valid` guarding a calculation that is not JSON (nothing writes one; tests do). | `billingstore.go` | same test (a version 2 nested in a version 1 is refused; "not json" over book history is refused, before it is not) | |
| +| Opus P2-1, Astra 2 (P2): the page always posted a currency, so the country's default never applied; the quote form and the checkout form could disagree. | Fixed by one form. The country and currency controls, the plans, the postal code and the consent are one form, which carries what the page priced (`quoted_country`, `quoted_currency`, `currency_chosen`). A post whose country or currency is not what the page priced is redirected to the page quoting it, so nothing is bought unseen; a currency shown as the country's default and not chosen reaches the door as no currency, and the country's own currency is bought in; a chosen one is the purchase's. The quote button posts the same form to `/billing/quote`, which only redirects. | `billing.html`, `billingpage.go`, `billinghandlers.go` | `TestBillingFormPostsWhatItShows` (the form as the page built it, submitted untouched and changed: Ireland with no choice buys in euro after one quote; a currency changed is shown first, then bought as a choice; the quote door; the old-way console; the held currency), `TestCheckoutFormCurrencyField`, `TestBillingPageCurrencyControls` (the `currency_chosen` flag per case) | |
| +| Opus P2-1 (the site's half), Astra 7 (P3): the pricing page sent the locale's guess as `?currency=`, and omitted the country, so the console reused an old one. | Fixed on the site (`2fc6388`): the links always carry both parameters, the currency only when the visitor chose it, the country selected or empty; the console clears a cookie whose parameter is present and empty. | site `src/site.js`; `billingcurrency.go` | site `pricing-currency.test.mjs` ("a locale or country default is shown but never sent as a choice"); `TestCurrencySuggestionCookie`, `TestCountrySuggestionCookie` (present-empty clears) | |
| +| Opus P3-1: a returning buyer was refused with a misleading banner. | Fixed: an account that bought in a currency is offered that currency alone, as a choice, whatever the URL or cookie says. | `billingpage.go` `pageCurrency` | `TestBillingFormPostsWhatItShows` ("offered pounds alone") | |
| +| Opus P3-2: an empty `?country=` brought the cookie's country back. | Fixed: a parameter present and empty quotes nothing and clears the cookie; only an absent one falls back. | `billingpage.go`, `billingcurrency.go` | `TestBillingPageCurrencyControls` ("no currency, said so, over the cookie"), the cookie tests | |
| +| Astra 5 (P2): the billing page stated band A inclusions beside a band D price. | Fixed: the inclusions beside each price are the quoted band's, from the same quote, as exact per-member shares of the pooled allowance; band A reads exactly as the approved sentences. | `billingpage.go` `billingPlanRows`, `formatFigure`; `billing.html` | `TestBillingInclusionsFollowTheQuotedBand` (band A verbatim; band D figures; exactness for every plan, band and quantity) | |
| +| Opus P2-2, Astra 6 (P2): the site rounded USD and GBP overage rates to three decimals. | Fixed on the site: rates are rendered exactly (at least three decimals, more when the figure needs them); a test parses each displayed rate back to the export's micro amount. | site `build.mjs` | site `pricing-pricebook.test.mjs` | |
| +| Opus P3-4: the site publishes a draft book unchecked. | Fixed on the site: a page built from a draft carries a first-line marker; the build refuses to write it without `PRICEBOOK_ALLOW_DRAFT=1`; `npm run check` refuses it (`hack/check-pricebook.mjs`). | site | site test "a page built from a draft book says so" | |
| +| Opus P3-5, Astra claims: figures typed into strings, the site's own currency rules, latent band scaling, a provisioning test that skips, a check script that wants exactly 36 keys; Astra 9: the verifier refused the own-only `currency_options` shape. | Fixed: the site checks the free plan's figures, the team minimum, the band percentages and the currency list against the export and stops on a difference; reads a country's default from its option; builds the country list from the book; checks per-member shares exact at one member and the minimum. The provisioning test fails, not skips, when the script is missing; the check accepts a map with at least the 36 keys and applies the console's own `currency_options` rule. | site `build.mjs`, `src/site.js`; `plans/provision_test.go`; `stripe-provision.sh` | site tests; `TestStripeProvisionScriptMatchesThePriceBook` | |
| +| Astra 8 (P3): the first resume guard had no independent case. | Fixed: the reads `{"eur","gbp"}` (wrong first, right second) refuse only through the first guard. | `billingcurrency_test.go` | `TestCheckoutNeverOffersASessionInAnotherCurrency` | |
| +| Astra claims: "nothing is asked of the provider again" (dispositions, design, a test comment); "six columns"; "36 or 48"; "no `currency_options`". | Corrected: later passes still read a refused session and write nothing more; four columns, one table, one trigger; 36 beside optional legacy keys and other books' catalogs; the own-only exception. | this file, the design, `billingcurrency_test.go` | none | |
| +| Opus claim: the site's `bands.p3` ("The euro price is what is charged") is false under fixed currencies. | Not edited: approved copy. Needs its own review; flagged in `ENABLEMENT.md`. | | | |
| +| Opus E: the quoted band could differ from the charged one if the checkout form's country was changed without re-quoting. | Closed by the one-form rule above: a changed country is quoted before it is bought. | | `TestBillingFormPostsWhatItShows` | |
| + |
| +Not done: the design's "no step-up stands between a member and a price" is the quote door's rule; the checkout door steps up as before. |
| + |
| +Mutation checks on these fixes, twelve in a scratch copy, each run against the test named for it: eleven failed their tests (the ownership check before expiry removed; the review on a bound completion removed; the trigger counting cancelled purchases; the trigger reading the version by substring, once the nested statement was spelled `"v":2,` as a substring reads it; the quote-before-purchase redirect removed; the pricing page's cookie counted as no choice; the held currency not offered alone; an empty `?country=` falling back to the cookie; the inclusions priced at band A; an empty `?currency=` not clearing the cookie; the first resume guard removed, which `{"eur","gbp"}` now catches). One survived and is equivalent by construction: passing the shown currency to the door as a choice when it was a default. Under the quote rule the posted currency equals the shown one, which equals the country's default, so the door's outcome is the same either way; the flag keeps the door's contract honest (no currency means the country's) and changes no result. No claim is made that the flag is tested beyond what `TestBillingFormPostsWhatItShows` observes. |
| + |
| +The fixes are in the commit that adds this section and have not been read by a reviewer. |
diff --git a/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review3.md b/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review3.md| new file mode 100644 |
| index 000000000..9ff908c9e |
| --- /dev/null |
| +++ b/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review3.md |
| @@ -0,0 +1,142 @@ |
| +# Opus authority review: `feat/carloku-multi-currency-20261001` at `de81dd70` |
| + |
| +**Verdict: accept with fixes.** The second review's findings are closed, but the billing page and the pricing page together undo the country-default rule, so that has to be fixed first. |
| + |
| +I read the source only, with `git show`. I ran no tests and changed nothing in either repository. For section K, a sub-agent read the pricing site at `15317d2`, built it and ran its tests in a scratch copy under `$TMPDIR` (15 of 15 pass, no network), then deleted the copy. I checked its two main claims myself. |
| + |
| +## Second-review findings: closed? Would the test pass without the fix? |
| + |
| +| Finding | Closed? | Would its test pass without the fix? | |
| +|---|---|---| |
| +| Astra P1: an older build freezes a mixed legacy and book month from the legacy half | **Closed.** The trigger `billing_months_no_legacy_calc_over_book_history` (`billingstore.go:173-187`) refuses any insert of a calculation that is not version 2 for a month that a book-priced assignment begins in or before. The older build's only write path is `INSERT OR IGNORE` (base `billingstore.go:816`), so the trigger covers it. This build never writes version 1 for such a month: mixed months go to `ErrPriceTerms` and a review. An older build that meets a version 2 month this build already froze stops with a review (`billing_invoice_record_mismatch`: the base code reads `Invoice.TotalCents` as 0). It does not invoice it. | **No.** `TestOlderBuildCannotFreezeAMonthWithBookHistory` fails without the trigger (September is frozen). | |
| +| Astra P2: a bound session that reads back changed stays payable | **Closed** on both resume paths (`billingservice.go:525-534`, `:583-590`) and in the background pass (`billingpass.go:199-209`). `refuseSession` expires the session, then retires the intent through `expireBillingCheckout`. | **No.** The two-read fixture (`{"gbp","eur"}`) separates the two resume checks, and the pass case fails without the code added to `billingpass.go`. | |
| +| Opus P3-1: a paid, refused session is expired on every pass and nobody is told | **Closed** (`billingservice.go:279-285`): no expiry is attempted and the review is filed once. | **No.** The test asserts no expiry calls and exactly one review after two passes. | |
| +| Opus P2-1: `currency_options` may hold the price's own currency | **Closed** (`priceCurrencyOptionsAreOwn`, `billingservice.go:179-198`). It accepts only a single entry under the price's own currency, at the same amount and tax behaviour. That does not weaken the check. | **No.** "Its own currency as its only option" fails under the old `len != 0` check, and the three refusal cases fail if the check is loosened. | |
| +| Astra P3: a second book's catalog had no test | **Closed** through a seam (`cmd/console/billing.go:17-21`). | **No.** Reverting to "the current book plus legacy" fails "the current book and the old one whole". | |
| +| Opus P3-2 / Astra P3: the surviving mutations were not redundant | **Closed.** "A created session that is not this intent's is not kept" covers the branch at `:625-629`. | **No.** Without that branch the session would be bound. | |
| +| Opus P3-3: keys differing only in case loaded | **Closed** (`pricebook.go:206-209`). | **No**, with the case added to `TestPriceBookValidation`. | |
| +| Opus claim 1: the golden test's tail was vacuous | **Closed.** It now asserts exact bytes. | Not applicable. | |
| +| Opus claims 2 and 5, the stale design text, Astra's "before the provider is asked" | **Corrected.** | Not applicable. | |
| +| Opus P3-4 / roll note | **Written** (dispositions, "Roll note"). | Not applicable. | |
| + |
| +The delta since `a88a39e6` does not touch `billinginvoice.go`, `billingcalculation.go`, `billingquantity.go`, `billingstripe/`, `month.go` or the public export. So the second review's results for sections C, D (apart from `currency_options`), F, H and J still stand. |
| + |
| +## Findings, most severe first |
| + |
| +### P2-1: The billing page always posts a currency, so the billing country's default (rule 1) almost never applies. The pricing page's link and the cookie make this worse. |
| + |
| +**Where:** |
| +- `internal/console/templates/billing.html:52-57`: the "Show prices" GET form always includes the currency select. |
| +- `billing.html:69`: the checkout form posts a hidden `currency` field, always. |
| +- `billingpage.go:235-249`: the query wins, then the cookie, then the quoted country's default, then the book's default. |
| +- `billingcurrency.go:45-48`: any GET carrying `currency=` writes the cookie. |
| +- In the site: `src/site.js:63` sets `currency` on every signup link. Its initial value comes from `navigator.language` (`site.js:84-88`), even when the visitor never touched the control. |
| + |
| +**Scenarios:** |
| +1. **No link.** An Irish owner comes in from the console menu, with no cookie and no `?country`. The page renders in USD, with `usd` selected and a hidden `currency=usd`. |
| + - If they pick Ireland and press "Show prices for this country", the form sends `?country=IE¤cy=usd`. |
| + - `CurrencyChosen` becomes true, the cookie is set to `usd`, and the page stays in USD. |
| + - If they instead pick Ireland in the checkout form's own country select, the post still carries `currency=usd`. |
| + - Either way the intent is USD. Ruling 1 says EUR. |
| +2. **The pricing page's link.** A visitor whose browser language is en-GB clicks "Choose Business" without touching the currency control. The link carries `currency=gbp`, which goes into the cookie. On the billing page the owner picks United States, and the page and its post stay GBP. |
| + - The design says the browser's locale only suggests a currency (rule 2), and it ends up choosing one. |
| + - The handler's comment (`billinghandlers.go:65`) and `suggestedCurrency`'s comment (`billingcurrency.go:116-118`) both say "the billing country the owner picks decides the default". Neither the page nor the site ever lets that happen. |
| + |
| +**Why it matters:** the buyer does see the currency before paying, so nothing is converted. But it is locked to the account for good: `billingservice.go:482` refuses any other currency after the subscription ends, and a change is left to an operator. In the ordinary flow, the ruled default is replaced by whatever the first render, the browser's locale or a stale cookie happened to pick. |
| + |
| +**Smallest acceptable fix:** |
| +- Post `currency` only when the owner chose it. Move the currency select into the checkout form, with a first option "the billing country's currency" (`value=""`) that is selected unless the query explicitly chose a currency. |
| +- Have the GET form send `currency` only when it was changed, or give it the same empty option. |
| +- In `site.js`, set `currency=` on links only when the visitor chose it explicitly; otherwise delete it. |
| +- Add a test that submits the page's real GET form and its real checkout form (country only) and expects the country's default. |
| + |
| +### P2-2: The pricing page shows USD and GBP overage rates rounded to three decimals |
| + |
| +**Where:** `carloku-paid-pricing` `copy-review/pricing/build.mjs:33` (`toFixed(3)`). The check at `:58` only compares the EUR texts, which happen to be exact. |
| + |
| +**Scenario:** a US visitor without script sees "$0.127" per GB-month, and the book charges $0.1272. Other cases: transfer $0.497 against $0.4968, writes $0.028 against $0.0276, GBP storage £0.095 against £0.0954. `test/pricing-pricebook.test.mjs:102-103` locks the rounding in. |
| + |
| +**Fix:** print the exact decimal (three to four places, trailing zeros trimmed), or stop the build on an inexact rate the way `figure()` does. |
| + |
| +### P3-1: A returning buyer is refused with a misleading banner |
| + |
| +**Where:** `billingservice.go:482`, and `pageCurrency`, which never looks at the account's held currency. |
| + |
| +**Scenario:** an account bought in GBP and its subscription was cancelled. The page defaults to USD, and the post is refused with `errBillingConflict`. The page then shows "The account changed while you were on this page…". |
| + |
| +**Fix:** when `state.Plan != ""`, preselect the held currency on the page and show only that one. |
| + |
| +### P3-2: An empty country in the quote form brings back the cookie's country |
| + |
| +**Where:** `billingpage.go:193-196`. |
| + |
| +**Scenario:** the owner picks "Choose a country" and submits. That sends `country=` (empty), and the page quotes the cookie's country again. The owner cannot clear a stale suggestion except by waiting out the hour. |
| + |
| +**Fix:** fall back to the cookie only when `country` is absent from the query, not when it is empty. |
| + |
| +### P3-3: The barrier trigger tests the version by substring |
| + |
| +**Where:** `billingstore.go:183` (`NOT LIKE '%"v":2,%'`). |
| + |
| +**Why it matters:** it is sound today, because `usage.Statement` and the evidence nested in a version 1 calculation carry `"v":1`. A future statement version 2 would be embedded as `"statement":{"v":2,…` inside a version 1 calculation and get through the trigger. |
| + |
| +**Fix:** `json_extract(CAST(NEW.calculation AS TEXT), '$.v') IS NOT 2`. |
| + |
| +### P3-4: The pricing page publishes a draft book with no status check |
| + |
| +**Where:** `build.mjs`, which never reads the export's `status`. |
| + |
| +**Scenario:** a deploy of the page while the book is still a draft shows prices that a live console refuses to sell. |
| + |
| +**Fix:** stop the build or the deploy unless `status === "approved"`, or pin it to the deployed `CONSOLE_BILLING_PRICE_BOOK`. |
| + |
| +### P3-5: Smaller problems on the site and in the tests |
| + |
| +- **Numbers typed into strings.json.** The free plan's figures, "Minimum 10", the band percentages and "USD, EUR or GBP" are written into `strings.json` rather than taken from the export. They agree today, but nothing checks them. |
| +- **The site keeps its own copy of the currency rules.** `site.js:24-30` has its own EU list while each option's `data-currency` attribute goes unread. The country list uses `countries.json` bands rather than the book's (`build.mjs:111`); all 199 match today. |
| +- **Band scaling is latent.** The site computes floor(f×bps/10000) at one member and the console floors at the quantity. They agree only because every figure × factor is divisible by 10000 today. Add a build check that it stays so. |
| +- **The provisioning test can skip itself.** `TestStripeProvisionScriptMatchesThePriceBook` calls `t.Skip` if the script is missing, so moving the script turns it into a silent pass. |
| +- **The operator's check script rejects a valid map.** It demands exactly 36 keys (`stripe-provision.sh`), while the console accepts 48 (with the twelve legacy keys) or more (with other books' catalogs). |
| + |
| +## Section by section |
| + |
| +- **A. One GBP purchase, end to end.** With `?country=IN¤cy=gbp`, the page shows £7.70 at band D and posts `gbp`. Checkout uses `gbp` at band D, and the provider price is checked for currency, amount, options and metadata. The session is asked for in `gbp` with Adaptive Pricing off and checked at every read. Reconcile, then the version 2 calculation, then the invoice engine and the quantity worker all stay in the stored currency (unchanged since review 2). No wrong-currency path found. |
| +- **B. Choosing a currency.** |
| + - An explicit unsold currency is refused at the door (`billingservice.go:356-363`). |
| + - On the page, `?currency=jpy` falls back to the default, which is then posted as if chosen (part of P2-1). |
| + - A forged or oversized cookie is harmless: `soldCurrency` and `pricedCountry` validate it, and the post never reads it. |
| + - A form submitted after another tab changed the cookie posts what its own page showed. |
| +- **C. Currency and pricing of what is already recorded.** No change since review 2. Legacy bytes and hashes are as before. |
| +- **D. Checks against the provider.** Nothing weakened. The `currency_options` relaxation accepts only the price's own entry. |
| +- **E. Purchases differing only in currency.** Allowances, bands, quantities and tax codes do not depend on currency. The quoted band can differ from the charged one if the owner changes the checkout form's country without re-quoting. That predates this branch, and Stripe shows the exact price. |
| +- **F. The price book.** Sound. The only change is the case-folded duplicate check. |
| +- **G. Configuration.** Sound. The catalog of another book is now tested. |
| +- **H. Schema and rollout.** The trigger is sound for legacy months before a purchase and for legacy-only accounts (see P3-3 for how it reads the version). An older binary on a version 2 frozen month stops with a review. The roll note covers stale account columns. |
| +- **I. Tests.** `TestBillingPageCurrencyControls` never submits the page's real GET form, which always carries `currency`, so it misses P2-1. Its "a currency not sold falls to the default" case encodes that fallback as correct. The first `fx` in the "reads back in another currency" subtest is unused. I found no ordering or shared-cookie-jar dependence. `TestPricingLinkSurvivesSignIn` shares a cookie jar on purpose. |
| +- **J. The public export.** Unchanged. It discloses nothing beyond prices, bands and allowances. |
| +- **K. The pricing page.** The site's export copy is byte-identical to the platform's. Prices and banded inclusions come from the export. Without script, the page is USD at band A, matching what the controls show selected. All the site's country codes are in the console's band table. Findings: P2-1 (the link always carries a currency), P2-2, P3-4, P3-5. |
| + |
| +## Claims the code contradicts |
| + |
| +- **The design's sign-in section** ("the country the owner then picks decides the default currency"), **`suggestedCurrency`'s comment** and **the handler's comment at `billinghandlers.go:65`**: the page never posts an empty currency (P2-1). |
| +- **Design rule 2** ("a browser locale only suggests"): the site's locale-guessed currency becomes the posted currency through the cookie and the hidden field. |
| +- **The pricing site's `bands.p3`** ("…The euro price is what is charged.", `strings.json:433`) contradicts fixed USD, EUR and GBP billing. Its wording is approved copy and out of scope, but the claim is now false and needs a new copy review. |
| +- **The dispositions** say a refused, paid session leaves "nothing … asked of the provider again". Every pass still reads it (`GetCheckout`); only writes stop. |
| + |
| +## What must be run in Stripe test mode before this is enabled |
| + |
| +1. **`GET /v1/prices/{id}?expand[]=currency_options`** on a single-currency price. This shows whether the expanded field is empty or holds only the price's own currency. Both pass the check today. |
| +2. **Sessions with `currency` and `adaptive_pricing[enabled]=false`**, for each of `usd`, `eur` and `gbp`, with buyers in IE, GB, US and JP. Each should come back with `currency` equal to the price's, Adaptive Pricing off, `currency_conversion` null and `amount_subtotal` equal to unit × quantity. |
| +3. **A session whose `currency` differs from the price's.** Expect a 400 at creation, and note the error code. |
| +4. **The completed subscription.** Its own `currency` and its item's price currency should both be the choice, with `price_book` and `currency` in its metadata. |
| +5. **The customer's currency lock:** |
| + - With a live GBP subscription, a USD checkout should be refused; note at which call. |
| + - After cancelling, is USD allowed on the same customer? |
| + - Is a GBP overage invoice for an old month still accepted afterwards? |
| +6. **Overage invoices in GBP and USD** (draft, items, finalize, pay), for UK and US customers with only the IE registrations. Every line should be in the currency, automatic tax should complete, and the subtotal should equal the calculation. |
| +7. **A GBP quantity change and a GBP renewal.** The proration items and the renewal invoice should be in GBP, and reconcile should record the renewal without an identity error. |
| +8. **Replays after a lost response.** The same key and parameters should return the same session. A legacy request (Adaptive Pricing on, no currency) should still be accepted under its original key. |
| +9. **`ExpireCheckout` on a completed session.** Note the error shape. |
| +10. **Promotion codes.** A EUR `amount_off` code on a GBP session should be refused; the 50% code and the house coupon should apply. |
| + |
| +**Accept with fixes.** P2-1 and P2-2 must be fixed before the billing page or the pricing page goes out with currency controls. The P3s can follow. |
diff --git a/implementation/carloku-paid-launch/stripe-provision.sh b/implementation/carloku-paid-launch/stripe-provision.sh| index eaf2dc868..d4e9db422 100755 |
| --- a/implementation/carloku-paid-launch/stripe-provision.sh |
| +++ b/implementation/carloku-paid-launch/stripe-provision.sh |
| @@ -393,13 +393,15 @@ print_check() { |
| section "Check 3. Every price and its product (the GetPrice and GetProduct fields the console compares)" |
| note "Console rules: active, livemode matches mode, the key's currency, exclusive, year x1, licensed, per_unit, no transform_quantity," |
| - note "no currency_options, unit_amount equals the book, metadata terms_version/price_book/currency/plan/band; product active, livemode, tax_code per plan." |
| + note "currency_options empty or holding the price's own currency alone at its own amount (priceCurrencyOptionsAreOwn), unit_amount equals the book," |
| + note "metadata terms_version/price_book/currency/plan/band; product active, livemode, tax_code per plan. CONSOLE_STRIPE_PRICES may hold more keys" |
| + note "(the twelve legacy keys, other books' catalogs); every one of this book's 36 must be there." |
| local tmpl |
| tmpl=$( |
| cat <<'EOF' |
| declare -A CENTS=(@CENTS@) |
| declare -A TAX=([personal]=txcd_10102001 [community]=txcd_10102001 [business]=txcd_10102000) |
| -[ "$(jq -r 'length' <<<"$CONSOLE_STRIPE_PRICES")" = 36 ] || { echo "CONSOLE_STRIPE_PRICES must have exactly the 36 keys of $BOOK" >&2; exit 1; } |
| +[ "$(jq -r 'length' <<<"$CONSOLE_STRIPE_PRICES")" -ge 36 ] || { echo "CONSOLE_STRIPE_PRICES must hold at least the 36 keys of $BOOK" >&2; exit 1; } |
| for key in "${!CENTS[@]}"; do |
| plan="${key%%/*}"; rest="${key#*/}"; band="${rest%/*}"; currency="${rest#*/}" |
| id="$(jq -er --arg k "$BOOK/$key" '.[$k]' <<<"$CONSOLE_STRIPE_PRICES")" |
| @@ -408,7 +410,8 @@ for key in "${!CENTS[@]}"; do |
| .id == $id and .active == true and .livemode == $live and .currency == $currency |
| and .tax_behavior == "exclusive" and .billing_scheme == "per_unit" and .transform_quantity == null |
| and .unit_amount == $cents and .recurring.interval == "year" and .recurring.interval_count == 1 |
| - and .recurring.usage_type == "licensed" and ((.currency_options // {}) | length) == 0 |
| + and .recurring.usage_type == "licensed" |
| + and ((.currency_options // {}) | to_entries | all(.key == $currency and .value.unit_amount == $cents and ((.value.tax_behavior // "exclusive") == "exclusive"))) |
| and .metadata.terms_version == $terms and .metadata.price_book == $book and .metadata.currency == $currency |
| and .metadata.plan == $plan and .metadata.band == $band' \ |
| <<<"$price" >/dev/null || { echo "PRICE CHECK FAILED: $key $id" >&2; exit 1; } |
diff --git a/internal/billingstripe/client.go b/internal/billingstripe/client.go| index fd21b642d..94e43c55a 100644 |
| --- a/internal/billingstripe/client.go |
| +++ b/internal/billingstripe/client.go |
| @@ -72,10 +72,13 @@ type Error struct { |
| // earlier request under the same idempotency key (its |
| // Idempotent-Replayed header), and ran nothing now. |
| Replayed bool |
| - // SessionID is set only for CodeCurrencyMismatch: the checkout session |
| - // the provider did create, which is not the one that was asked for. |
| - // It exists and is open; the caller expires it. |
| + // SessionID and Session are set only for CodeCurrencyMismatch: the |
| + // checkout session the provider did create, which is not the one that |
| + // was asked for. It exists and is open. The caller decides what to do |
| + // with it from the session itself (whose it is, above all), not from |
| + // the id alone. |
| SessionID string |
| + Session *Session |
| } |
| // RefusedParam reports whether Stripe refused the request outright as |
| @@ -649,7 +652,7 @@ func (c *Client) CreateCheckout(ctx context.Context, p CreateCheckoutParams, ide |
| return nil, err |
| } |
| if !p.LegacyAdaptive && !sess.FixedCurrency(p.Currency) { |
| - return nil, &Error{Code: CodeCurrencyMismatch, SessionID: sess.ID} |
| + return nil, &Error{Code: CodeCurrencyMismatch, SessionID: sess.ID, Session: &sess} |
| } |
| return &sess, nil |
| } |
diff --git a/internal/console/billingcurrency.go b/internal/console/billingcurrency.go| index be10c8200..95fb10f4b 100644 |
| --- a/internal/console/billingcurrency.go |
| +++ b/internal/console/billingcurrency.go |
| @@ -34,21 +34,28 @@ const ( |
| ) |
| // rememberCurrency keeps a valid ?currency= and a valid ?country= from a |
| -// GET in their cookies. It is called for every request before routing; a |
| -// request with no such parameter, or with one that is not a currency sold |
| -// or a country priced, sets nothing for it. |
| +// GET in their cookies, and clears a cookie whose parameter is present |
| +// and empty: that is how the pricing page says the visitor chose nothing |
| +// (or cleared a choice), so a choice from an earlier visit does not come |
| +// back. It is called for every request before routing; a request with no |
| +// such parameter, or with one that is not a currency sold or a country |
| +// priced, changes nothing for it. |
| func (s *Server) rememberCurrency(w http.ResponseWriter, r *http.Request) { |
| if r.Method != http.MethodGet || r.URL.RawQuery == "" { |
| return |
| } |
| q := r.URL.Query() |
| if values, ok := q["currency"]; ok && len(values) == 1 { |
| - if c, ok := s.soldCurrency(values[0]); ok { |
| + if values[0] == "" { |
| + s.clearCookie(w, currencyCookie) |
| + } else if c, ok := s.soldCurrency(values[0]); ok { |
| s.setCookie(w, currencyCookie, c, int(currencyCookieTTL.Seconds())) |
| } |
| } |
| if values, ok := q["country"]; ok && len(values) == 1 { |
| - if iso, ok := s.pricedCountry(values[0]); ok { |
| + if values[0] == "" { |
| + s.clearCookie(w, countryCookie) |
| + } else if iso, ok := s.pricedCountry(values[0]); ok { |
| s.setCookie(w, countryCookie, iso, int(currencyCookieTTL.Seconds())) |
| } |
| } |
| @@ -112,10 +119,13 @@ func (s *Server) billingCurrencies() []string { |
| return out |
| } |
| -// suggestedCurrency is the currency to preselect on the billing page: the |
| -// one the visitor arrived with, if it is still one the console sells in, |
| -// or none. With none, the billing country the owner picks decides the |
| -// default. |
| +// suggestedCurrency is the currency to preselect on the billing page when |
| +// its URL names none: the one the visitor chose on the pricing page, if it |
| +// is still one the console sells in, or none. With none, the billing |
| +// country decides. The pricing page sends a currency only when the |
| +// visitor chose it (and an empty one, which clears this, when not), so |
| +// what is here is a choice the visitor made and saw, never a guess from |
| +// the browser's locale. |
| func (s *Server) suggestedCurrency(r *http.Request) string { |
| c, err := r.Cookie(currencyCookie) |
| if err != nil { |
diff --git a/internal/console/billingcurrency_test.go b/internal/console/billingcurrency_test.go| index d72fd6843..f344ba167 100644 |
| --- a/internal/console/billingcurrency_test.go |
| +++ b/internal/console/billingcurrency_test.go |
| @@ -10,6 +10,7 @@ import ( |
| "net/http" |
| "net/http/httptest" |
| "net/url" |
| + "regexp" |
| "strings" |
| "testing" |
| "time" |
| @@ -168,8 +169,10 @@ func TestCheckoutRefusesACurrencyThatIsNotSold(t *testing.T) { |
| } |
| // Through the door: the form's currency field, posted by an owner who has |
| -// just confirmed who they are. An unknown currency is a 400 and nothing |
| -// else happens. |
| +// just confirmed who they are, with the quoted_* fields the page carries. |
| +// A currency shown as the country's default and not chosen is no |
| +// currency to the door; a chosen one is the purchase's; an unknown one is |
| +// a 400 and nothing else happens. |
| func TestCheckoutFormCurrencyField(t *testing.T) { |
| post := func(t *testing.T, currency string) (*httptest.ResponseRecorder, *consoleFixture, *fakeBillingStripe, *Account) { |
| fx, f, acct := newCurrencyFixture(t) |
| @@ -181,9 +184,17 @@ func TestCheckoutFormCurrencyField(t *testing.T) { |
| if err := fx.st.RecordStepUp(raw, owner, time.Now(), time.Now()); err != nil { |
| t.Fatal(err) |
| } |
| - form := url.Values{"csrf": {raw}, "plan": {"personal"}, "country": {"IE"}, "consent": {plans.CurrentTermsVersion}} |
| - if currency != "-" { |
| + form := url.Values{"csrf": {raw}, "plan": {"personal"}, "country": {"IE"}, "consent": {plans.CurrentTermsVersion}, "quoted_country": {"IE"}, "quoted_currency": {"eur"}, "currency_chosen": {""}} |
| + switch currency { |
| + case "-": |
| + // A form with no currency control (a console selling the old way). |
| + case "shown": |
| + // The page showed Ireland's euro as the default; nobody chose it. |
| + form.Set("currency", "eur") |
| + default: |
| form.Set("currency", currency) |
| + form.Set("quoted_currency", currency) |
| + form.Set("currency_chosen", "1") |
| } |
| r := httptest.NewRequest(http.MethodPost, fx.path("/billing/checkout"), strings.NewReader(form.Encode())) |
| r.Header.Set("Content-Type", "application/x-www-form-urlencoded") |
| @@ -192,7 +203,7 @@ func TestCheckoutFormCurrencyField(t *testing.T) { |
| fx.srv.ServeHTTP(w, r) |
| return w, fx, f, acct |
| } |
| - for currency, want := range map[string]string{"-": "eur", "": "eur", "gbp": "gbp", "usd": "usd"} { |
| + for currency, want := range map[string]string{"-": "eur", "shown": "eur", "gbp": "gbp", "usd": "usd"} { |
| w, fx, _, acct := post(t, currency) |
| if w.Code != http.StatusSeeOther || !strings.HasPrefix(w.Header().Get("Location"), "https://checkout.stripe.com/") { |
| t.Fatalf("currency %q: %d %s", currency, w.Code, w.Header().Get("Location")) |
| @@ -359,7 +370,10 @@ func TestCheckoutNeverOffersASessionInAnotherCurrency(t *testing.T) { |
| // The provider answers in pounds on the first read of the second |
| // click and in euros on the next: the two checks on the resume |
| // path are each the fence for a reading the other did not see. |
| - for _, reads := range [][]string{{"eur"}, {"gbp", "eur"}} { |
| + // {"eur","gbp"} is the first guard's own case: the first read is |
| + // wrong and the second would be right, so only a check on the first |
| + // reading refuses it. |
| + for _, reads := range [][]string{{"eur"}, {"gbp", "eur"}, {"eur", "gbp"}} { |
| fx, f, acct := newCurrencyFixture(t) |
| f.customerCountry = "GB" |
| if dest, err := click(fx, f, acct, 0); err != nil || dest == "" { |
| @@ -1067,11 +1081,16 @@ func TestCurrencySuggestionCookie(t *testing.T) { |
| t.Errorf("GET %s: cookie %+v", target, c) |
| } |
| } |
| - for _, target := range []string{"/", "/?currency=", "/?currency=jpy", "/?currency=GBP", "/?currency=gbp¤cy=usd", "/?currency=gbp%20"} { |
| + for _, target := range []string{"/", "/?currency=jpy", "/?currency=GBP", "/?currency=gbp¤cy=usd", "/?currency=gbp%20"} { |
| if c := cookie(http.MethodGet, target); c != nil { |
| t.Errorf("GET %s set %+v", target, c) |
| } |
| } |
| + // Present and empty: the pricing page says no currency was chosen, and |
| + // an earlier choice is forgotten. |
| + if c := cookie(http.MethodGet, "/?currency="); c == nil || c.Value != "" || c.MaxAge >= 0 { |
| + t.Errorf("GET /?currency= did not clear the cookie: %+v", c) |
| + } |
| if c := cookie(http.MethodPost, "/?currency=gbp"); c != nil { |
| t.Errorf("a POST set %+v", c) |
| } |
| @@ -1274,7 +1293,8 @@ func TestLegacyCalculationBytesAreUnchanged(t *testing.T) { |
| } |
| // While an account has a subscription that has not ended it buys nothing |
| -// more, in any currency, and nothing is asked of the provider. Once it has |
| +// more, in any currency, and no session is asked of the provider (the |
| +// catalog and configuration reads before the refusal still happen). Once it has |
| // ended, the account buys again in the currency it bought in before and in |
| // no other: a change of currency is not something the form does. |
| func TestCheckoutAfterAPurchaseKeepsItsCurrency(t *testing.T) { |
| @@ -1607,40 +1627,42 @@ func TestBillingPageCurrencyControls(t *testing.T) { |
| "Your billing country sets your pricing band. Changing currency does not change your inclusions.", |
| "Your selected currency stays with your subscription and usage bills.", |
| `<select name="currency">`, `<option value="usd" selected>USD</option>`, `<option value="eur">EUR</option>`, `<option value="gbp">GBP</option>`, |
| - `<input type="hidden" name="currency" value="usd">`, |
| + `<input type="hidden" name="quoted_country" value="">`, `<input type="hidden" name="quoted_currency" value="usd">`, `<input type="hidden" name="currency_chosen" value="">`, |
| + `formaction="/a/` + fx.sqid + `/billing/quote" formnovalidate>`, |
| "<strong>$28</strong> a year", "<strong>$6</strong> per member a year", "<strong>$60</strong> per member a year", |
| } { |
| if !strings.Contains(m, want) { |
| t.Fatalf("billing page with no choice made is missing %q in:\n%s", want, m) |
| } |
| } |
| - // The posted field is inside the checkout form, not the GET form. |
| - getForm, _, _ := strings.Cut(m[strings.Index(m, `method="get"`):], "</form>") |
| - if strings.Contains(getForm, `type="hidden" name="currency"`) { |
| - t.Fatal("the hidden currency is in the price preview form") |
| + // One form, one country control, one currency control: what is shown |
| + // selected is what is posted, and nothing else names a country. |
| + if n := strings.Count(m, `<select name="country"`); n != 1 { |
| + t.Fatalf("%d country controls, want one", n) |
| } |
| - postForm, _, _ := strings.Cut(m[strings.Index(m, `method="post" action="/a/`+fx.sqid+`/billing/checkout"`):], "</form>") |
| - if !strings.Contains(postForm, `<input type="hidden" name="currency" value="usd">`) { |
| - t.Fatal("the checkout form does not post the page's currency") |
| + if strings.Contains(m, `method="get"`) { |
| + t.Fatal("a second form prices the page") |
| } |
| for name, tc := range map[string]struct { |
| query, cookie, want, price string |
| - band string |
| + band, chosen string |
| }{ |
| - "the quoted country's default": {"?country=IE", "", "eur", "<strong>€24</strong> a year", "100%"}, |
| - "the United Kingdom's default": {"?country=GB", "", "gbp", "<strong>£22</strong> a year", "100%"}, |
| - "the buyer's choice over the country": {"?country=IE¤cy=gbp", "", "gbp", "<strong>£22</strong> a year", "100%"}, |
| - "the pricing page's suggestion": {"", "gbp", "gbp", "<strong>£22</strong> a year", ""}, |
| - "the buyer's choice over the suggestion": {"?currency=eur", "gbp", "eur", "<strong>€24</strong> a year", ""}, |
| - "the country's default over no suggestion": {"?country=FR", "", "eur", "<strong>€24</strong> a year", "100%"}, |
| - "a band C country in its default": {"?country=PL", "", "eur", "<strong>€12</strong> a year", "50%"}, |
| - "a band D country in pounds": {"?country=IN¤cy=gbp", "", "gbp", "<strong>£7.70</strong> a year", "35%"}, |
| - "a currency not sold falls to the default": {"?currency=jpy", "", "usd", "<strong>$28</strong> a year", ""}, |
| - "a forged cookie falls to the default": {"", "jpy", "usd", "<strong>$28</strong> a year", ""}, |
| + "the quoted country's default": {"?country=IE", "", "eur", "<strong>€24</strong> a year", "100%", ""}, |
| + "the United Kingdom's default": {"?country=GB", "", "gbp", "<strong>£22</strong> a year", "100%", ""}, |
| + "the buyer's choice over the country": {"?country=IE¤cy=gbp", "", "gbp", "<strong>£22</strong> a year", "100%", "1"}, |
| + "the pricing page's choice": {"", "gbp", "gbp", "<strong>£22</strong> a year", "", "1"}, |
| + "the buyer's choice over the pricing page": {"?currency=eur", "gbp", "eur", "<strong>€24</strong> a year", "", "1"}, |
| + "the country's default over no suggestion": {"?country=FR", "", "eur", "<strong>€24</strong> a year", "100%", ""}, |
| + "a band C country in its default": {"?country=PL", "", "eur", "<strong>€12</strong> a year", "50%", ""}, |
| + "a band D country in pounds": {"?country=IN¤cy=gbp", "", "gbp", "<strong>£7.70</strong> a year", "35%", "1"}, |
| + "no currency, said so, over the cookie": {"?country=GB¤cy=", "eur", "gbp", "<strong>£22</strong> a year", "100%", ""}, |
| + "a currency not sold is no choice": {"?currency=jpy", "", "usd", "<strong>$28</strong> a year", "", ""}, |
| + "a currency not sold, over the cookie": {"?country=IE¤cy=jpy", "gbp", "eur", "<strong>€24</strong> a year", "100%", ""}, |
| + "a forged cookie is no choice": {"", "jpy", "usd", "<strong>$28</strong> a year", "", ""}, |
| } { |
| t.Run(name, func(t *testing.T) { |
| m := fetch(t, tc.query, tc.cookie) |
| - for _, want := range []string{`<option value="` + tc.want + `" selected>`, `<input type="hidden" name="currency" value="` + tc.want + `">`, tc.price} { |
| + for _, want := range []string{`<option value="` + tc.want + `" selected>`, `<input type="hidden" name="quoted_currency" value="` + tc.want + `">`, `<input type="hidden" name="currency_chosen" value="` + tc.chosen + `">`, tc.price} { |
| if !strings.Contains(m, want) { |
| t.Fatalf("missing %q in:\n%s", want, m) |
| } |
| @@ -1648,9 +1670,8 @@ func TestBillingPageCurrencyControls(t *testing.T) { |
| if tc.band != "" && !strings.Contains(m, "are "+tc.band+" of the base price") { |
| t.Fatalf("the band line does not say %s", tc.band) |
| } |
| - // The country is selected in both country selects (the preview |
| - // form's and the checkout form's), the currency in one. |
| - if strings.Count(m, " selected>") > 3 { |
| + // One country and one currency selected, at most. |
| + if strings.Count(m, " selected>") > 2 { |
| t.Fatalf("more than one currency or country selected:\n%s", m) |
| } |
| }) |
| @@ -1677,10 +1698,43 @@ func TestBillingPageCurrencyControls(t *testing.T) { |
| } |
| } |
| -// What the page posts is what the door takes: the owner picks a currency on |
| -// the page, the form posts it, and the intent is opened in it. |
| -func TestBillingPagePostsTheCurrencyItShows(t *testing.T) { |
| - fx, f, acct := newCurrencyFixture(t) |
| +// billingFormFields reads the one billing form as a browser would submit it |
| +// untouched: every hidden field, and each select's selected option (or its |
| +// first, empty, option). The caller picks the plan and ticks the consent. |
| +func billingFormFields(t *testing.T, m string) url.Values { |
| + t.Helper() |
| + i := strings.Index(m, `<form class="billing-checkout"`) |
| + if i < 0 { |
| + t.Fatalf("no billing form in:\n%s", m) |
| + } |
| + form, _, ok := strings.Cut(m[i:], "</form>") |
| + if !ok { |
| + t.Fatal("unterminated billing form") |
| + } |
| + v := url.Values{} |
| + for _, h := range regexp.MustCompile(`<input type="hidden" name="([^"]+)" value="([^"]*)">`).FindAllStringSubmatch(form, -1) { |
| + v.Set(h[1], h[2]) |
| + } |
| + for _, sel := range regexp.MustCompile(`(?s)<select name="([^"]+)"[^>]*>(.*?)</select>`).FindAllStringSubmatch(form, -1) { |
| + v.Set(sel[1], "") |
| + if o := regexp.MustCompile(`<option value="([^"]*)" selected>`).FindStringSubmatch(sel[2]); o != nil { |
| + v.Set(sel[1], o[1]) |
| + } |
| + } |
| + return v |
| +} |
| + |
| +// billingBrowser is an owner's signed-in, stepped-up session against a |
| +// fixture, with GET and POST that carry it and no cookie jar: each request |
| +// says exactly what it sends. |
| +type billingBrowser struct { |
| + t *testing.T |
| + fx *consoleFixture |
| + raw string |
| +} |
| + |
| +func newBillingBrowser(t *testing.T, fx *consoleFixture) *billingBrowser { |
| + t.Helper() |
| const owner = "paul@keymail.dev" |
| raw, err := fx.st.CreateSession(owner, "", time.Now()) |
| if err != nil { |
| @@ -1689,33 +1743,189 @@ func TestBillingPagePostsTheCurrencyItShows(t *testing.T) { |
| if err := fx.st.RecordStepUp(raw, owner, time.Now(), time.Now()); err != nil { |
| t.Fatal(err) |
| } |
| - page := httptest.NewRequest(http.MethodGet, fx.path("/billing")+"?country=IE¤cy=gbp", nil) |
| - page.AddCookie(&http.Cookie{Name: sessionCookie, Value: raw}) |
| + return &billingBrowser{t: t, fx: fx, raw: raw} |
| +} |
| + |
| +func (b *billingBrowser) page(query string) (string, url.Values) { |
| + b.t.Helper() |
| + r := httptest.NewRequest(http.MethodGet, b.fx.path("/billing")+query, nil) |
| + r.AddCookie(&http.Cookie{Name: sessionCookie, Value: b.raw}) |
| w := httptest.NewRecorder() |
| - fx.srv.ServeHTTP(w, page) |
| - body := w.Body.String() |
| - i := strings.Index(body, `<input type="hidden" name="currency" value="`) |
| - if i < 0 { |
| - t.Fatalf("no currency field:\n%s", body) |
| + b.fx.srv.ServeHTTP(w, r) |
| + if w.Code != http.StatusOK { |
| + b.t.Fatalf("GET /billing%s = %d:\n%s", query, w.Code, w.Body.String()) |
| } |
| - posted := body[i+len(`<input type="hidden" name="currency" value="`):] |
| - posted = posted[:strings.Index(posted, `"`)] |
| - form := url.Values{"csrf": {raw}, "plan": {"personal"}, "country": {"IE"}, "currency": {posted}, "consent": {plans.CurrentTermsVersion}} |
| - r := httptest.NewRequest(http.MethodPost, fx.path("/billing/checkout"), strings.NewReader(form.Encode())) |
| + m := mainOf(b.t, w.Body.String()) |
| + return m, billingFormFields(b.t, m) |
| +} |
| + |
| +func (b *billingBrowser) post(door string, form url.Values) *httptest.ResponseRecorder { |
| + b.t.Helper() |
| + form.Set("csrf", b.raw) |
| + r := httptest.NewRequest(http.MethodPost, b.fx.path(door), strings.NewReader(form.Encode())) |
| r.Header.Set("Content-Type", "application/x-www-form-urlencoded") |
| - r.AddCookie(&http.Cookie{Name: sessionCookie, Value: raw}) |
| - w = httptest.NewRecorder() |
| - fx.srv.ServeHTTP(w, r) |
| - if w.Code != http.StatusSeeOther { |
| - t.Fatalf("checkout: %d %s", w.Code, w.Body.String()) |
| - } |
| - intent, err := fx.st.billingCheckoutIntent(acct.ID) |
| - if err != nil || intent.Currency != "gbp" || intent.Band != "A" { |
| - t.Fatalf("intent %+v %v; want gbp at Ireland's band", intent, err) |
| - } |
| - if got := f.forms[len(f.forms)-1].Get("currency"); got != "gbp" { |
| - t.Fatalf("the session was asked for in %q", got) |
| - } |
| + r.AddCookie(&http.Cookie{Name: sessionCookie, Value: b.raw}) |
| + w := httptest.NewRecorder() |
| + b.fx.srv.ServeHTTP(w, r) |
| + return w |
| +} |
| + |
| +// submit presses "Continue to payment" on the form as the page built it, |
| +// with a plan picked and the consent ticked. |
| +func (b *billingBrowser) submit(fields url.Values) *httptest.ResponseRecorder { |
| + b.t.Helper() |
| + fields.Set("plan", plans.PlanPersonal) |
| + fields.Set("consent", plans.CurrentTermsVersion) |
| + return b.post("/billing/checkout", fields) |
| +} |
| + |
| +// The one billing form posts what it shows, and buys nothing it has not |
| +// shown. A country picked on the form is first quoted (the page comes back |
| +// priced for it) and only then bought, in its own currency when none was |
| +// chosen; a currency changed on the form is first shown, then bought as a |
| +// choice; what the pricing page's link carried is bought as it was shown. |
| +func TestBillingFormPostsWhatItShows(t *testing.T) { |
| + t.Run("the billing country's own currency, with none chosen", func(t *testing.T) { |
| + fx, f, acct := newCurrencyFixture(t) |
| + b := newBillingBrowser(t, fx) |
| + m, fields := b.page("") |
| + if !strings.Contains(m, `<option value="usd" selected>`) || fields.Get("quoted_country") != "" { |
| + t.Fatalf("a fresh page: %v in:\n%s", fields, m) |
| + } |
| + fields.Set("country", "IE") |
| + if w := b.submit(fields); w.Code != http.StatusSeeOther || w.Header().Get("Location") != fx.path("/billing")+"?country=IE¤cy=" { |
| + t.Fatalf("a country not yet quoted: %d %s", w.Code, w.Header().Get("Location")) |
| + } |
| + if _, err := fx.st.billingCheckoutIntent(acct.ID); err == nil { |
| + t.Fatal("an intent was opened before the price was shown") |
| + } |
| + m, fields = b.page("?country=IE¤cy=") |
| + for _, want := range []string{`<option value="IE" selected>`, `<option value="eur" selected>`, `name="currency_chosen" value=""`, "<strong>€24</strong> a year"} { |
| + if !strings.Contains(m, want) { |
| + t.Fatalf("Ireland quoted: missing %q in:\n%s", want, m) |
| + } |
| + } |
| + if w := b.submit(fields); w.Code != http.StatusSeeOther || !strings.HasPrefix(w.Header().Get("Location"), "https://checkout.stripe.com/") { |
| + t.Fatalf("buying as quoted: %d %s %s", w.Code, w.Header().Get("Location"), w.Body.String()) |
| + } |
| + intent, err := fx.st.billingCheckoutIntent(acct.ID) |
| + if err != nil || intent.Currency != "eur" || intent.Band != plans.BandA { |
| + t.Fatalf("intent %+v %v; want Ireland's euro at band A", intent, err) |
| + } |
| + if got := f.forms[len(f.forms)-1].Get("currency"); got != "eur" { |
| + t.Fatalf("the session was asked for in %q", got) |
| + } |
| + }) |
| + t.Run("the pricing page's country and currency, as shown", func(t *testing.T) { |
| + fx, f, acct := newCurrencyFixture(t) |
| + f.customerCountry = "IN" |
| + b := newBillingBrowser(t, fx) |
| + m, fields := b.page("?country=IN¤cy=gbp") |
| + if !strings.Contains(m, "<strong>£7.70</strong> a year") || fields.Get("currency_chosen") != "1" { |
| + t.Fatalf("India in pounds: %v in:\n%s", fields, m) |
| + } |
| + fields.Set("postal", "110001") |
| + if w := b.submit(fields); w.Code != http.StatusSeeOther || !strings.HasPrefix(w.Header().Get("Location"), "https://checkout.stripe.com/") { |
| + t.Fatalf("%d %s", w.Code, w.Header().Get("Location")) |
| + } |
| + if intent, err := fx.st.billingCheckoutIntent(acct.ID); err != nil || intent.Currency != "gbp" || intent.Band != plans.BandD { |
| + t.Fatalf("intent %+v %v; want pounds at band D", intent, err) |
| + } |
| + }) |
| + t.Run("a currency changed on the form is shown first, then bought as a choice", func(t *testing.T) { |
| + fx, _, acct := newCurrencyFixture(t) |
| + b := newBillingBrowser(t, fx) |
| + _, fields := b.page("?country=IE¤cy=") |
| + fields.Set("currency", "usd") |
| + if w := b.submit(fields); w.Code != http.StatusSeeOther || w.Header().Get("Location") != fx.path("/billing")+"?country=IE¤cy=usd" { |
| + t.Fatalf("a currency not yet shown: %d %s", w.Code, w.Header().Get("Location")) |
| + } |
| + m, fields := b.page("?country=IE¤cy=usd") |
| + if !strings.Contains(m, "<strong>$28</strong> a year") || fields.Get("currency_chosen") != "1" || fields.Get("currency") != "usd" { |
| + t.Fatalf("Ireland in dollars: %v in:\n%s", fields, m) |
| + } |
| + if w := b.submit(fields); w.Code != http.StatusSeeOther || !strings.HasPrefix(w.Header().Get("Location"), "https://checkout.stripe.com/") { |
| + t.Fatalf("%d %s", w.Code, w.Header().Get("Location")) |
| + } |
| + if intent, err := fx.st.billingCheckoutIntent(acct.ID); err != nil || intent.Currency != "usd" { |
| + t.Fatalf("intent %+v %v; want the chosen dollars", intent, err) |
| + } |
| + // And a country changed on the form, with the currency a choice, |
| + // keeps the choice through the quote. |
| + fx2, _, _ := newCurrencyFixture(t) |
| + b2 := newBillingBrowser(t, fx2) |
| + _, fields = b2.page("?country=IE¤cy=usd") |
| + fields.Set("country", "GB") |
| + if w := b2.submit(fields); w.Code != http.StatusSeeOther || w.Header().Get("Location") != fx2.path("/billing")+"?country=GB¤cy=usd" { |
| + t.Fatalf("a country changed with a chosen currency: %d %s", w.Code, w.Header().Get("Location")) |
| + } |
| + _, fields = b2.page("?country=IE") |
| + fields.Set("country", "GB") |
| + if w := b2.submit(fields); w.Code != http.StatusSeeOther || w.Header().Get("Location") != fx2.path("/billing")+"?country=GB¤cy=" { |
| + t.Fatalf("a country changed with the default currency: %d %s", w.Code, w.Header().Get("Location")) |
| + } |
| + }) |
| + t.Run("the quote button", func(t *testing.T) { |
| + fx, _, acct := newCurrencyFixture(t) |
| + b := newBillingBrowser(t, fx) |
| + _, fields := b.page("") |
| + fields.Set("country", "PL") |
| + if w := b.post("/billing/quote", fields); w.Code != http.StatusSeeOther || w.Header().Get("Location") != fx.path("/billing")+"?country=PL¤cy=" { |
| + t.Fatalf("quote with the default currency: %d %s", w.Code, w.Header().Get("Location")) |
| + } |
| + fields.Set("currency", "gbp") |
| + if w := b.post("/billing/quote", fields); w.Code != http.StatusSeeOther || w.Header().Get("Location") != fx.path("/billing")+"?country=PL¤cy=gbp" { |
| + t.Fatalf("quote with a changed currency: %d %s", w.Code, w.Header().Get("Location")) |
| + } |
| + fields.Set("country", "") |
| + if w := b.post("/billing/quote", fields); w.Code != http.StatusSeeOther || w.Header().Get("Location") != fx.path("/billing")+"?country=¤cy=gbp" { |
| + t.Fatalf("quote with the country cleared: %d %s", w.Code, w.Header().Get("Location")) |
| + } |
| + if _, err := fx.st.billingCheckoutIntent(acct.ID); err == nil { |
| + t.Fatal("a quote opened an intent") |
| + } |
| + fields.Set("csrf", "wrong") |
| + r := httptest.NewRequest(http.MethodPost, fx.path("/billing/quote"), strings.NewReader(fields.Encode())) |
| + r.Header.Set("Content-Type", "application/x-www-form-urlencoded") |
| + r.AddCookie(&http.Cookie{Name: sessionCookie, Value: b.raw}) |
| + w := httptest.NewRecorder() |
| + fx.srv.ServeHTTP(w, r) |
| + if w.Code != http.StatusForbidden { |
| + t.Fatalf("a quote without the form's token: %d", w.Code) |
| + } |
| + }) |
| + t.Run("a console selling the old way has no currency control and the same form", func(t *testing.T) { |
| + fx, _, acct := newBillingFixture(t) |
| + b := newBillingBrowser(t, fx) |
| + m, fields := b.page("") |
| + if strings.Contains(m, `name="currency"`) || fields.Get("quoted_currency") != "eur" { |
| + t.Fatalf("the old way: %v in:\n%s", fields, m) |
| + } |
| + fields.Set("country", "IE") |
| + if w := b.submit(fields); w.Code != http.StatusSeeOther || w.Header().Get("Location") != fx.path("/billing")+"?country=IE¤cy=" { |
| + t.Fatalf("%d %s", w.Code, w.Header().Get("Location")) |
| + } |
| + _, fields = b.page("?country=IE¤cy=") |
| + if w := b.submit(fields); w.Code != http.StatusSeeOther || !strings.HasPrefix(w.Header().Get("Location"), "https://checkout.stripe.com/") { |
| + t.Fatalf("%d %s %s", w.Code, w.Header().Get("Location"), w.Body.String()) |
| + } |
| + if intent, err := fx.st.billingCheckoutIntent(acct.ID); err != nil || intent.Currency != "" || intent.PriceBook != "" { |
| + t.Fatalf("intent %+v %v; want legacy terms", intent, err) |
| + } |
| + }) |
| + t.Run("an account that bought in pounds is offered pounds alone", func(t *testing.T) { |
| + fx, _, acct, _ := buyIn(t, "GB", "") |
| + if _, err := fx.st.db.Exec(`UPDATE billing_accounts SET status='canceled' WHERE account_id=?`, acct.ID); err != nil { |
| + t.Fatal(err) |
| + } |
| + b := newBillingBrowser(t, fx) |
| + for _, query := range []string{"", "?currency=usd", "?country=IE¤cy=eur"} { |
| + m, fields := b.page(query) |
| + if strings.Count(m, `<option value="gbp" selected>GBP</option>`) != 1 || strings.Contains(m, `<option value="usd"`) || fields.Get("currency_chosen") != "1" || fields.Get("currency") != "gbp" { |
| + t.Fatalf("%q: %v in:\n%s", query, fields, m) |
| + } |
| + } |
| + }) |
| } |
| // The pricing page's link carries the visitor's currency and billing |
| @@ -1735,8 +1945,8 @@ func TestPricingLinkSurvivesSignIn(t *testing.T) { |
| } |
| m := mainOf(t, body) |
| for _, want := range []string{ |
| - `<option value="gbp" selected>GBP</option>`, `<input type="hidden" name="currency" value="gbp">`, |
| - `<option value="IN" selected>India</option>`, |
| + `<option value="gbp" selected>GBP</option>`, `<input type="hidden" name="quoted_currency" value="gbp">`, `<input type="hidden" name="currency_chosen" value="1">`, |
| + `<option value="IN" selected>India</option>`, `<input type="hidden" name="quoted_country" value="IN">`, |
| "Prices for India are 35% of the base price, and so are the monthly inclusions.", |
| "<strong>£7.70</strong> a year", "<strong>£1.58</strong> per member a year", "<strong>£15.75</strong> per member a year", |
| } { |
| @@ -1744,8 +1954,8 @@ func TestPricingLinkSurvivesSignIn(t *testing.T) { |
| t.Fatalf("billing page after the pricing page's link is missing %q in:\n%s", want, m) |
| } |
| } |
| - if n := strings.Count(m, `<option value="IN" selected>`); n != 2 { |
| - t.Fatalf("India is selected %d times, want both the quote form and the checkout form", n) |
| + if n := strings.Count(m, `<option value="IN" selected>`); n != 1 { |
| + t.Fatalf("India is selected %d times, want once, in the one form", n) |
| } |
| // The page's own form names another country: that is what is quoted, |
| // in the currency still chosen. |
| @@ -1789,11 +1999,14 @@ func TestCountrySuggestionCookie(t *testing.T) { |
| t.Errorf("GET %s: cookie %+v", target, c) |
| } |
| } |
| - for _, target := range []string{"/", "/?country=", "/?country=in", "/?country=XX", "/?country=IND", "/?country=IN&country=IE", "/?country=IN%20", "/?currency=gbp"} { |
| + for _, target := range []string{"/", "/?country=in", "/?country=XX", "/?country=IND", "/?country=IN&country=IE", "/?country=IN%20", "/?currency=gbp"} { |
| if c := cookie(fx.srv, http.MethodGet, target); c != nil { |
| t.Errorf("GET %s set %+v", target, c) |
| } |
| } |
| + if c := cookie(fx.srv, http.MethodGet, "/?country="); c == nil || c.Value != "" || c.MaxAge >= 0 { |
| + t.Errorf("GET /?country= did not clear the cookie: %+v", c) |
| + } |
| if c := cookie(fx.srv, http.MethodPost, "/?country=IN"); c != nil { |
| t.Errorf("a POST set %+v", c) |
| } |
| @@ -1817,3 +2030,192 @@ func TestCountrySuggestionCookie(t *testing.T) { |
| t.Errorf("a console with billing off suggests %q", got) |
| } |
| } |
| + |
| +// A session the provider answers with in another currency is expired only |
| +// when it is this intent's own. A response carrying some other session |
| +// (another account's, another intent's), whatever its currency, expires |
| +// nothing: the console never acts on a session it cannot place. |
| +func TestCurrencyRefusalExpiresOnlyTheIntentsOwnSession(t *testing.T) { |
| + fx, f, acct := newCurrencyFixture(t) |
| + f.customerCountry, f.sessionCurrency, f.createdIntent = "GB", "eur", "0123456789abcdef0123456789abcdef0123456789abcdef" |
| + dest, err := fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now) |
| + if dest != "" || err == nil || errors.Is(err, errBillingIdentity) { |
| + t.Fatalf("dest %q err %v, want no destination and not an identity refusal of a session that is not ours", dest, err) |
| + } |
| + if f.expireCalls != 0 { |
| + t.Fatalf("the console expired a session that is not this intent's %d time(s)", f.expireCalls) |
| + } |
| + if intent, err := fx.st.billingCheckoutIntent(acct.ID); err != nil || intent.SessionID != "" { |
| + t.Fatalf("intent %+v %v; want it kept, unbound, for the next attempt", intent, err) |
| + } |
| + // The same response for the intent's own session is refused and expired. |
| + fx, f, acct = newCurrencyFixture(t) |
| + f.customerCountry, f.sessionCurrency = "GB", "eur" |
| + if dest, err := fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now); dest != "" || !errors.Is(err, errBillingIdentity) || f.expireCalls != 1 { |
| + t.Fatalf("own session: dest %q err %v, %d expiries", dest, err, f.expireCalls) |
| + } |
| +} |
| + |
| +// A session bound to the intent that completes in another currency than it |
| +// was asked for is recorded from in no way, and a person is told once: |
| +// through the background pass, through the webhook, and through a direct |
| +// binding. Nothing is expired: somebody paid. |
| +func TestBoundSessionCompletedInAnotherCurrencyIsReviewedOnce(t *testing.T) { |
| + fx, f, acct := newCurrencyFixture(t) |
| + f.customerCountry = "GB" |
| + if dest, err := fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now); err != nil || dest == "" { |
| + t.Fatalf("first click: dest %q err %v", dest, err) |
| + } |
| + intent, err := fx.st.billingCheckoutIntent(acct.ID) |
| + if err != nil || intent.SessionID == "" || intent.Currency != "gbp" { |
| + t.Fatalf("fixture: intent %+v %v", intent, err) |
| + } |
| + f.mu.Lock() |
| + f.sessionStatus, f.sessionCurrency, f.subscriptionPrice = "complete", "eur", intent.Price |
| + f.mu.Unlock() |
| + reviews := func() int { |
| + var n int |
| + if err := fx.st.db.QueryRow(`SELECT count(*) FROM billing_reviews WHERE account_id=? AND reason=? AND resolved_at=0`, acct.ID, billingCheckoutRefusedReason).Scan(&n); err != nil { |
| + t.Fatal(err) |
| + } |
| + return n |
| + } |
| + for i := 1; i <= 2; i++ { |
| + _ = fx.srv.billingPass(context.Background(), f.now.Add(time.Duration(i)*time.Minute)) |
| + } |
| + if _, err := fx.srv.bindBillingCheckout(context.Background(), intent.SessionID); !errors.Is(err, errBillingIdentity) { |
| + t.Fatalf("binding the completed session directly: %v, want an identity refusal", err) |
| + } |
| + if n := reviews(); n != 1 { |
| + t.Fatalf("%d reviews filed, want exactly one after two passes and a binding", n) |
| + } |
| + state, _ := fx.st.billingAccount(acct.ID) |
| + if state.Subscription != "" || f.expireCalls != 0 || f.sessionStatus != "complete" { |
| + t.Fatalf("state %+v, %d expiries, session %s; want nothing recorded and nothing expired", state, f.expireCalls, f.sessionStatus) |
| + } |
| + if after, err := fx.st.billingCheckoutIntent(acct.ID); err != nil || after.ID != intent.ID { |
| + t.Fatalf("intent %+v %v; want it left as it was so no second purchase opens", after, err) |
| + } |
| +} |
| + |
| +// The barrier trigger stops the older build and only the older build: a |
| +// book-priced assignment that never counted for a month (a purchase |
| +// cancelled before its first reconciliation) leaves the month legacy-only, |
| +// and this build's own version 1 for it goes in. The version is read as JSON, so a version 2 |
| +// nested inside a version 1 is still a version 1; and a calculation that |
| +// is not JSON (which nothing writes) counts as not version 2. |
| +func TestBarrierTriggerLetsThisBuildsLegacyMonthsThrough(t *testing.T) { |
| + at := func(m, d int) time.Time { return time.Date(2026, time.Month(m), d, 0, 0, 0, 0, time.UTC) } |
| + // A version 1 whose nested statement carries a version 2 of its own, |
| + // spelled as a substring check would read it ("v":2 followed by a |
| + // comma): only the JSON version at the top level says what it is. |
| + v1 := []byte(`{"statement_object":{"key":""},"v":1,"arithmetic":"base_units_floor_v1","currency":"eur","statement":{"v":2,"lines":[]}}`) |
| + setup := func(t *testing.T) (*consoleFixture, *Account, billingState) { |
| + fx, _, acct := newCurrencyFixture(t) |
| + legacy := billingState{AccountID: acct.ID, Customer: "cus_one", Subscription: "sub_legacy", Status: "active", Plan: plans.PlanPersonal, Terms: plans.CurrentTermsVersion, Band: plans.BandA, Quantity: 1, PeriodStart: at(7, 1).Unix(), PeriodEnd: at(7, 1).AddDate(1, 0, 0).Unix()} |
| + if err := fx.st.applyBillingState(legacy, 0, "e1", at(7, 1)); err != nil { |
| + t.Fatal(err) |
| + } |
| + legacy.Status = "canceled" |
| + if err := fx.st.applyBillingState(legacy, 1, "e2", at(9, 10)); err != nil { |
| + t.Fatal(err) |
| + } |
| + return fx, acct, legacy |
| + } |
| + t.Run("a book purchase cancelled before it was reconciled", func(t *testing.T) { |
| + fx, acct, _ := setup(t) |
| + book := billingState{AccountID: acct.ID, Customer: "cus_one", Subscription: "sub_book", Status: "canceled", Plan: plans.PlanPersonal, Terms: plans.CurrentTermsVersion, PriceBook: plans.CurrentPriceBook, Currency: "eur", Band: plans.BandA, Quantity: 1, PeriodStart: at(9, 12).Unix(), PeriodEnd: at(9, 12).AddDate(1, 0, 0).Unix()} |
| + if err := fx.st.applyBillingState(book, 2, "e3", at(9, 12)); err != nil { |
| + t.Fatal(err) |
| + } |
| + rows, err := fx.st.billingInvoiceHistory(acct.ID) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + history := make([]plans.Assignment, 0, len(rows)) |
| + for _, r := range rows { |
| + history = append(history, r.Assignment) |
| + } |
| + if m, err := plans.AllowanceForMonth("2026-09", history); err != nil || !m.PriceTerms().Legacy() { |
| + t.Fatalf("September's terms: %+v %v; want legacy, the only paid span", m, err) |
| + } |
| + if _, err := fx.st.freezeBillingMonth(acct.ID, "2026-09", v1); err != nil { |
| + t.Fatalf("this build's version 1 for a legacy-only September: %v", err) |
| + } |
| + }) |
| + t.Run("an active book purchase, and a nested version 2", func(t *testing.T) { |
| + fx, acct, _ := setup(t) |
| + book := billingState{AccountID: acct.ID, Customer: "cus_one", Subscription: "sub_book", Status: "active", Plan: plans.PlanPersonal, Terms: plans.CurrentTermsVersion, PriceBook: plans.CurrentPriceBook, Currency: "eur", Band: plans.BandA, Quantity: 1, PeriodStart: at(9, 12).Unix(), PeriodEnd: at(9, 12).AddDate(1, 0, 0).Unix()} |
| + if err := fx.st.applyBillingState(book, 2, "e3", at(9, 12)); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := fx.st.freezeBillingMonth(acct.ID, "2026-09", v1); err == nil || !strings.Contains(err.Error(), "billing_month_legacy_calculation_over_book_history") { |
| + t.Fatalf("a version 1 with a version 2 nested in it: %v; want the database to refuse it", err) |
| + } |
| + if _, err := fx.st.freezeBillingMonth(acct.ID, "2026-09", []byte("not json")); err == nil { |
| + t.Fatal("a calculation that is not JSON was frozen over book history") |
| + } |
| + if _, err := fx.st.freezeBillingMonth(acct.ID, "2026-08", []byte("not json")); err != nil { |
| + t.Fatalf("a calculation that is not JSON, before the book purchase: %v", err) |
| + } |
| + }) |
| +} |
| + |
| +// The inclusions the billing page states beside a price are the quoted |
| +// band's, from the same quote, as exact per-member shares of what the |
| +// account is pooled; band A reads exactly as the approved sentences. |
| +func TestBillingInclusionsFollowTheQuotedBand(t *testing.T) { |
| + for _, plan := range billingPaidPlans { |
| + def, err := plans.Lookup(plans.CurrentTermsVersion, plan) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + for _, band := range []string{plans.BandA, plans.BandB, plans.BandC, plans.BandD} { |
| + for _, qty := range []int64{def.MinQuantity, def.MinQuantity * 7} { |
| + if qty > def.MaxQuantity { |
| + continue |
| + } |
| + q, err := plans.Quote(plans.CurrentTermsVersion, plan, band, qty) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + for name, pair := range map[string][2]int64{"hours": {q.Allowances.AwakeSeconds, qty * 3600}, "storage": {q.Allowances.StorageByteMonths, qty * 1_000_000_000}, "transfer": {q.Allowances.TransferBytes, qty * 1_000_000_000}, "writes": {q.Allowances.Writes, qty}} { |
| + if (pair[0]*1000)%pair[1] != 0 { |
| + t.Errorf("%s band %s at %d members: %s %d/%d is not exact to three decimals; the page would have to state the pooled figure", plan, band, qty, name, pair[0], pair[1]) |
| + } |
| + } |
| + } |
| + } |
| + } |
| + for _, tc := range []struct { |
| + num, den int64 |
| + want string |
| + }{{306000, 3600, "85"}, {2_000_000_000, 1_000_000_000, "2"}, {107100, 3600, "29.75"}, {700_000_000, 1_000_000_000, "0.7"}, {300000, 10, "30,000"}, {105000, 10, "10,500"}, {1_750_000_000, 10 * 1_000_000_000, "0.175"}, {1, 3, "0.333"}, {1234567, 1, "1,234,567"}} { |
| + if got := formatFigure(tc.num, tc.den); got != tc.want { |
| + t.Errorf("formatFigure(%d, %d) = %q, want %q", tc.num, tc.den, got, tc.want) |
| + } |
| + } |
| + fx, _, _ := newCurrencyFixture(t) |
| + b := newBillingBrowser(t, fx) |
| + m, _ := b.page("?country=IE") |
| + for _, want := range []string{ |
| + "One member. Each month: 85 awake hours, 2 GB storage, 2 GB transfer.", |
| + "Per member, minimum 10. Each member adds, per month: 17 awake hours, 0.5 GB storage, 0.4 GB transfer.", |
| + "Per member, minimum 10. Each member adds, per month: 170 awake hours, 5 GB storage, 3 GB transfer, 30,000 replication writes. Priority support.", |
| + } { |
| + if !strings.Contains(m, want) { |
| + t.Fatalf("band A does not read as approved: missing %q in:\n%s", want, m) |
| + } |
| + } |
| + m, _ = b.page("?country=IN¤cy=gbp") |
| + for _, want := range []string{ |
| + "<strong>£7.70</strong> a year", |
| + "One member. Each month: 29.75 awake hours, 0.7 GB storage, 0.7 GB transfer.", |
| + "Each member adds, per month: 5.95 awake hours, 0.175 GB storage, 0.14 GB transfer.", |
| + "Each member adds, per month: 59.5 awake hours, 1.75 GB storage, 1.05 GB transfer, 10,500 replication writes.", |
| + } { |
| + if !strings.Contains(m, want) { |
| + t.Fatalf("band D: missing %q in:\n%s", want, m) |
| + } |
| + } |
| +} |
diff --git a/internal/console/billinghandlers.go b/internal/console/billinghandlers.go| index 7dc7e4e0a..fd2e94466 100644 |
| --- a/internal/console/billinghandlers.go |
| +++ b/internal/console/billinghandlers.go |
| @@ -10,6 +10,7 @@ import ( |
| "log" |
| "net/http" |
| "net/url" |
| + "strings" |
| "time" |
| "github.com/carlosframework/platform/internal/billingstripe" |
| @@ -62,10 +63,24 @@ func (s *Server) handleBillingCheckout(w http.ResponseWriter, r *http.Request) { |
| http.Redirect(w, r, back+"?error=unavailable", http.StatusSeeOther) |
| return |
| } |
| - // The currency is the form's own field. Absent or empty, the billing |
| - // country decides. Anything else that is not a currency sold is a |
| - // malformed request: nothing is created and no default stands in. |
| - dest, err := s.startBillingCheckout(r.Context(), acct, r.PostForm.Get("plan"), r.PostForm.Get("country"), r.PostForm.Get("postal"), r.PostForm.Get("currency"), time.Now()) |
| + // The form's country and currency must be the ones the page priced |
| + // (the quoted_* fields it carried). Anything else is first shown as a |
| + // quote: nothing is bought at a price the buyer has not seen. |
| + country, currency, chosen := billingFormChoice(r.PostForm) |
| + if country != r.PostForm.Get("quoted_country") || currency != r.PostForm.Get("quoted_currency") { |
| + http.Redirect(w, r, back+billingQuoteQuery(country, currency, chosen), http.StatusSeeOther) |
| + return |
| + } |
| + // The currency is given to the door only when it is a choice (the |
| + // form says whether the one it shows was chosen or a default). With |
| + // none, the billing country decides. A choice that is not a currency |
| + // sold is a malformed request: nothing is created and no default |
| + // stands in. |
| + posted := "" |
| + if chosen { |
| + posted = currency |
| + } |
| + dest, err := s.startBillingCheckout(r.Context(), acct, r.PostForm.Get("plan"), country, r.PostForm.Get("postal"), posted, time.Now()) |
| if errors.Is(err, errBillingCurrency) { |
| http.Error(w, "currency_invalid", http.StatusBadRequest) |
| return |
| @@ -78,6 +93,67 @@ func (s *Server) handleBillingCheckout(w http.ResponseWriter, r *http.Request) { |
| http.Redirect(w, r, dest, http.StatusSeeOther) |
| } |
| +// handleBillingQuote is POST /a/{sqid}/billing/quote: the "Show prices for |
| +// this country" button of the one billing form. It buys nothing and |
| +// records nothing: it sends the member back to the page with the form's |
| +// country and, when the currency is a choice, its currency in the URL, |
| +// which is where the page prices from. It asks for the session and the |
| +// form's token and nothing more: no step-up stands between a member and a |
| +// price. |
| +func (s *Server) handleBillingQuote(w http.ResponseWriter, r *http.Request) { |
| + if s.billing == nil || r.Method != http.MethodPost { |
| + http.NotFound(w, r) |
| + return |
| + } |
| + _, raw, ok := s.session(r) |
| + if !ok { |
| + http.Redirect(w, r, "/", http.StatusFound) |
| + return |
| + } |
| + r.Body = http.MaxBytesReader(w, r.Body, 8192) |
| + if err := r.ParseForm(); err != nil { |
| + w.WriteHeader(http.StatusBadRequest) |
| + return |
| + } |
| + if raw == "" || subtle.ConstantTimeCompare([]byte(r.PostForm.Get("csrf")), []byte(raw)) != 1 { |
| + w.WriteHeader(http.StatusForbidden) |
| + return |
| + } |
| + _, _, acct, _, ok := s.account(w, r) |
| + if !ok { |
| + return |
| + } |
| + country, currency, chosen := billingFormChoice(r.PostForm) |
| + http.Redirect(w, r, "/a/"+url.PathEscape(acct.Sqid)+"/billing"+billingQuoteQuery(country, currency, chosen), http.StatusSeeOther) |
| +} |
| + |
| +// billingFormChoice reads the billing form's country and currency, and |
| +// whether the currency is a choice: the form said the one it showed was |
| +// chosen (currency_chosen), or the buyer changed it from the one shown |
| +// (quoted_currency). A form with no currency control (a console selling |
| +// the old way) shows and chooses nothing. |
| +func billingFormChoice(form url.Values) (country, currency string, chosen bool) { |
| + country = strings.ToUpper(strings.TrimSpace(form.Get("country"))) |
| + if _, has := form["currency"]; !has { |
| + return country, form.Get("quoted_currency"), false |
| + } |
| + currency = strings.ToLower(strings.TrimSpace(form.Get("currency"))) |
| + chosen = form.Get("currency_chosen") == "1" || currency != form.Get("quoted_currency") |
| + return country, currency, chosen |
| +} |
| + |
| +// billingQuoteQuery is the billing page URL's query for a quote: the |
| +// country, and the currency when it is a choice, else an empty currency, |
| +// which the page reads as "the country's own" and which clears the |
| +// pricing page's suggestion on the way in. |
| +func billingQuoteQuery(country, currency string, chosen bool) string { |
| + q := url.Values{"country": {country}, "currency": {""}} |
| + if chosen { |
| + q.Set("currency", currency) |
| + } |
| + return "?" + q.Encode() |
| +} |
| + |
| func (s *Server) handleBillingPortal(w http.ResponseWriter, r *http.Request) { |
| acct, ok := s.billingOwner(w, r) |
| if !ok { |
diff --git a/internal/console/billingpage.go b/internal/console/billingpage.go| index a8b2e12b8..b9d662cbf 100644 |
| --- a/internal/console/billingpage.go |
| +++ b/internal/console/billingpage.go |
| @@ -63,22 +63,33 @@ type billingPageView struct { |
| // Plans is the chooser's rows, priced for the quoted country or, with |
| // none, the base band, in Currency. |
| Plans []billingPlanRow |
| - // Currencies are the currencies the console sells in, in the price |
| - // book's order, each with whether it is the one the page is priced |
| - // in. With one currency (a console with no price book) no choice is |
| - // offered. Currency is the one the page is priced in and the checkout |
| - // form posts: the ?currency= the page was asked for, else the |
| - // suggestion carried from the pricing page, else the quoted country's |
| - // default, else the book's default. CurrencyChosen says it was the |
| - // buyer's own choice (the query), not a default. |
| + // Currencies are the currencies the buyer may choose among, in the |
| + // price book's order, each with whether it is the one the page is |
| + // priced in. With one currency and no purchase behind the account (a |
| + // console with no price book) no choice is offered. Currency is the |
| + // one the page is priced in and the form shows selected: the currency |
| + // the account already bought in, else the ?currency= the page was |
| + // asked for, else the choice carried from the pricing page, else the |
| + // quoted country's default, else the book's default. CurrencyChosen |
| + // says it is a choice (the account's, the URL's or the pricing |
| + // page's) and not a default: the form carries that, and the checkout |
| + // door is given no currency when it is a default, so the billing |
| + // country's own currency is what is bought in. CurrencyHeld says the |
| + // account bought in Currency already and can buy in no other. |
| Currencies []billingCurrencyOption |
| Currency string |
| CurrencyChosen bool |
| - // Quote is the ?country=XX preview, or the country the visitor chose on |
| - // the pricing page when the form has not named one. QuoteError is a ?country the page |
| - // could not price (malformed or not a known band). |
| - Quote *billingQuote |
| - QuoteError bool |
| + CurrencyHeld bool |
| + // Quote is the preview for the quoted country: the ?country=XX the |
| + // page was asked for, else, when the URL names no country at all, the |
| + // country the visitor chose on the pricing page. QuotedCountry is its |
| + // ISO2, or "" with none; the form carries it so that a checkout for |
| + // another country, or in another currency, is first shown as a quote. |
| + // QuoteError is a ?country the page could not price (malformed or not |
| + // a known band). |
| + Quote *billingQuote |
| + QuotedCountry string |
| + QuoteError bool |
| // Error is the ?error= key (unavailable|pending|conflict), "" otherwise. |
| Error string |
| @@ -103,6 +114,13 @@ type billingPlanRow struct { |
| // for team plans. Both are formatted in the page's currency ("€24", |
| // "$8.40", "£1.58"). |
| Price, PerMember string |
| + // Hours, StorageGB, TransferGB and Writes are the plan's monthly |
| + // inclusions for one member at the quoted band, from the same quote |
| + // as the price (the pooled allowance at the charged quantity, per |
| + // member): "85", "2", "0.7", "30,000". The approved sentences hold |
| + // band A's figures; at another band the same sentences carry that |
| + // band's. |
| + Hours, StorageGB, TransferGB, Writes string |
| // Unavailable: the checkout door would refuse this plan for the |
| // account's member count. The row is shown and cannot be chosen. |
| Unavailable bool |
| @@ -144,8 +162,10 @@ func (s *Server) handleBillingPage(w http.ResponseWriter, r *http.Request) { |
| // Billing switched off: every account is on Free, whatever an old row |
| // might say, and the page offers nothing to press. |
| + var state billingState |
| if !v.Closed { |
| - state, err := s.store.billingAccount(acct.ID) |
| + var err error |
| + state, err = s.store.billingAccount(acct.ID) |
| if err != nil { |
| log.Printf("console: billing page state: %v", err) |
| s.renderMessage(w, r, http.StatusInternalServerError, "Error", "Could not read billing. Please try again.") |
| @@ -190,8 +210,20 @@ func (s *Server) handleBillingPage(w http.ResponseWriter, r *http.Request) { |
| v.Countries = s.billingCountries() |
| band := plans.BandA |
| country := "" |
| - q := r.URL.Query().Get("country") |
| - if q == "" { |
| + // A ?country= that is present and empty quotes nothing: the form |
| + // (or the pricing page) said so, and the cookie was cleared on the |
| + // way in. Only a URL that names no country at all falls back to |
| + // the country the visitor chose on the pricing page. |
| + q, present := "", false |
| + if values, ok := r.URL.Query()["country"]; ok { |
| + present = true |
| + if len(values) == 1 { |
| + q = values[0] |
| + } else { |
| + v.QuoteError = true |
| + } |
| + } |
| + if !present { |
| q = s.suggestedCountry(r) |
| } |
| if q != "" { |
| @@ -210,11 +242,12 @@ func (s *Server) handleBillingPage(w http.ResponseWriter, r *http.Request) { |
| hit.Selected = true |
| band, country = hit.Band, hit.ISO2 |
| v.Quote = &billingQuote{ISO2: hit.ISO2, Country: hit.Name, Percent: pct} |
| + v.QuotedCountry = hit.ISO2 |
| } else { |
| v.QuoteError = true |
| } |
| } |
| - terms := s.pageCurrency(r, v, country) |
| + terms := s.pageCurrency(r, v, country, state) |
| v.Plans = billingPlanRows(terms, band, int64(v.Members)) |
| } |
| @@ -223,25 +256,36 @@ func (s *Server) handleBillingPage(w http.ResponseWriter, r *http.Request) { |
| } |
| // pageCurrency decides the currency the page is priced in and fills the |
| -// view's currency fields. The query's ?currency= wins when it is one the |
| -// console sells in (it is the buyer's own choice, and the page says so); |
| -// else the suggestion the pricing page's link left in the cookie; else the |
| -// quoted billing country's default; else the book's default. None of this |
| -// is the purchase's currency: that is what the checkout form posts, and |
| -// the form posts what this chose. |
| -func (s *Server) pageCurrency(r *http.Request, v *billingPageView, country string) plans.PriceTerms { |
| +// view's currency fields. The currency the account already bought in is |
| +// the only one offered (the checkout door refuses any other after a |
| +// purchase). Else the URL's ?currency= when it is one the console sells |
| +// in (a choice); a ?currency= that is present and empty is "no choice", |
| +// and does not fall back to the cookie; else, when the URL names no |
| +// currency at all, the choice the pricing page carried in the cookie; |
| +// else the quoted billing country's default; else the book's default. |
| +// Only the first three are choices (CurrencyChosen); the form carries |
| +// that, and for a default the checkout door is given no currency, so the |
| +// posted country's own currency is bought in. None of this is the |
| +// purchase's currency: that is what the checkout form posts, and the |
| +// form posts what this chose. |
| +func (s *Server) pageCurrency(r *http.Request, v *billingPageView, country string, state billingState) plans.PriceTerms { |
| book := plans.LegacyPriceBook |
| if s.billing != nil { |
| book = s.billing.cfg.PriceBook |
| } |
| + sold := s.billingCurrencies() |
| chosen := "" |
| - if q := r.URL.Query().Get("currency"); q != "" { |
| - if c, ok := s.soldCurrency(q); ok { |
| - chosen, v.CurrencyChosen = c, true |
| + if held, err := plans.ParseCurrency(state.Currency); err == nil && state.PriceBook != "" && state.Subscription != "" { |
| + chosen, v.CurrencyChosen, v.CurrencyHeld = string(held), true, true |
| + sold = []string{chosen} |
| + } else if values, ok := r.URL.Query()["currency"]; ok { |
| + if len(values) == 1 && values[0] != "" { |
| + if c, ok := s.soldCurrency(values[0]); ok { |
| + chosen, v.CurrencyChosen = c, true |
| + } |
| } |
| - } |
| - if chosen == "" { |
| - chosen = s.suggestedCurrency(r) |
| + } else if c := s.suggestedCurrency(r); c != "" { |
| + chosen, v.CurrencyChosen = c, true |
| } |
| if chosen == "" { |
| if c, err := plans.CurrencyForCountry(book, country); err == nil { |
| @@ -251,9 +295,10 @@ func (s *Server) pageCurrency(r *http.Request, v *billingPageView, country strin |
| terms := plans.PriceTerms{TermsVersion: plans.CurrentTermsVersion, PriceBook: book, Currency: plans.Currency(chosen)} |
| if terms.Legacy() { |
| terms.Currency = "" |
| + v.CurrencyChosen, v.CurrencyHeld = false, false |
| } |
| v.Currency = string(terms.Money()) |
| - if sold := s.billingCurrencies(); len(sold) > 1 { |
| + if len(sold) > 1 || v.CurrencyHeld { |
| for _, c := range sold { |
| v.Currencies = append(v.Currencies, billingCurrencyOption{Code: c, Label: strings.ToUpper(c), Selected: c == v.Currency}) |
| } |
| @@ -341,11 +386,49 @@ func billingPlanRows(terms plans.PriceTerms, band string, members int64) []billi |
| if row.Team { |
| row.PerMember = formatMinor(terms.Money(), price.UnitMinor) |
| } |
| + // The inclusions for one member at this band: the pooled allowance |
| + // the quote grants at its quantity, per member. What the account |
| + // is granted is the pooled figure; the page states its per-member |
| + // share, which is exact for every plan and band in the terms |
| + // (TestBillingInclusionsAreExactPerMember keeps it so). |
| + row.Hours = formatFigure(price.Allowances.AwakeSeconds, price.Quantity*3600) |
| + row.StorageGB = formatFigure(price.Allowances.StorageByteMonths, price.Quantity*1_000_000_000) |
| + row.TransferGB = formatFigure(price.Allowances.TransferBytes, price.Quantity*1_000_000_000) |
| + row.Writes = formatFigure(price.Allowances.Writes, price.Quantity) |
| rows = append(rows, row) |
| } |
| return rows |
| } |
| +// formatFigure renders num/den as the approved sentences write a figure: |
| +// the whole part with thousands separators, then up to three decimals |
| +// with trailing zeros dropped ("85", "0.7", "29.75", "30,000"). A share |
| +// that needs more than three decimals is cut at three, never rounded up. |
| +func formatFigure(num, den int64) string { |
| + if den <= 0 || num < 0 { |
| + return "0" |
| + } |
| + whole, rem := num/den, num%den |
| + digits := strconv.FormatInt(whole, 10) |
| + var b strings.Builder |
| + for i, c := range digits { |
| + if i > 0 && (len(digits)-i)%3 == 0 { |
| + b.WriteByte(',') |
| + } |
| + b.WriteRune(c) |
| + } |
| + if rem == 0 { |
| + return b.String() |
| + } |
| + frac := "" |
| + for i := 0; i < 3 && rem != 0; i++ { |
| + rem *= 10 |
| + frac += strconv.FormatInt(rem/den, 10) |
| + rem %= den |
| + } |
| + return b.String() + "." + strings.TrimRight(frac, "0") |
| +} |
| + |
| // formatEuroMicro renders a microEUR amount in whole cents as "€24" or |
| // "€8.40": no trailing ".00". Sub-cent remainders are dropped; plan prices |
| // are whole cents by construction. |
diff --git a/internal/console/billingpage_test.go b/internal/console/billingpage_test.go| index 3ae2e5996..cbc2b7cbd 100644 |
| --- a/internal/console/billingpage_test.go |
| +++ b/internal/console/billingpage_test.go |
| @@ -59,7 +59,7 @@ func TestBillingPageOwnerSeesChooser(t *testing.T) { |
| `name="consent" value="` + plans.CurrentTermsVersion + `"`, |
| `name="plan" value="personal"`, `name="plan" value="community"`, `name="plan" value="business"`, |
| `name="postal"`, `<option value="">Choose a country</option>`, `<option value="IE">Ireland</option>`, |
| - `method="get" action="/a/` + fx.sqid + `/billing"`, |
| + `formaction="/a/` + fx.sqid + `/billing/quote" formnovalidate>`, |
| `href="https://carloku.com/pricing/"`, |
| billingFreeCopy, |
| "<strong>€24</strong> a year", // Personal at the base band |
| @@ -77,10 +77,10 @@ func TestBillingPageOwnerSeesChooser(t *testing.T) { |
| if strings.Contains(m, "Prices for ") { |
| t.Fatal("band line shown without a ?country") |
| } |
| - // The GET price form must never post to checkout: it is its own form. |
| - getForm, _, _ := strings.Cut(m[strings.Index(m, `method="get"`):], "</form>") |
| - if strings.Contains(getForm, "checkout") || strings.Contains(getForm, "csrf") { |
| - t.Fatalf("price preview form leaks into checkout:\n%s", getForm) |
| + // The quote button is a door of its own: it posts the same form to |
| + // /billing/quote, which buys nothing, and the page has no second form. |
| + if strings.Count(m, "<form") != 1 || !strings.Contains(m, `formaction="/a/`+fx.sqid+`/billing/quote" formnovalidate>`) { |
| + t.Fatalf("want one form with a quote button:\n%s", m) |
| } |
| } |
diff --git a/internal/console/billingservice.go b/internal/console/billingservice.go| index 7a6b284f5..876cff0ff 100644 |
| --- a/internal/console/billingservice.go |
| +++ b/internal/console/billingservice.go |
| @@ -271,11 +271,12 @@ func (b *billingRuntime) ownSession(session *billingstripe.Session, intent billi |
| // currency, which the provider should not have let happen. Nothing is |
| // recorded from it, the intent is left as it is so no second purchase is |
| // opened beside it, a review is filed once for the account, and nothing |
| -// is asked of the provider again. A person has to look at what was paid |
| -// for. |
| +// is written at the provider again (later passes still read the session, |
| +// and file nothing more). A person has to look at what was paid for. |
| // |
| -// session is the session as last read, or nil when only its id is known |
| -// (the client refused it as it was created, so it is open). |
| +// session is the session as last read, or as the client refused it when |
| +// it was created. The caller has established that it is this intent's |
| +// own (ownSession): nothing here expires a session on its id alone. |
| func (s *Server) refuseSession(ctx context.Context, intent billingIntent, sessionID string, session *billingstripe.Session, now time.Time) error { |
| if session != nil && (session.Status == "complete" || session.PaymentStatus == "paid") { |
| if err := s.store.fileBillingReviewOnce(intent.AccountID, session.SubscriptionID, billingCheckoutRefusedReason, now); err != nil { |
| @@ -308,13 +309,14 @@ func (s *Server) refuseSession(ctx context.Context, intent billingIntent, sessio |
| const billingCheckoutRefusedReason = "billing_checkout_currency_refused" |
| // refusedByCurrency reports whether err is the provider client refusing a |
| -// session it created because of its currency, and which session. |
| -func refusedByCurrency(err error) (string, bool) { |
| +// session it created because of its currency, and which session, as the |
| +// provider returned it. |
| +func refusedByCurrency(err error) (*billingstripe.Session, bool) { |
| var stripeErr *billingstripe.Error |
| - if errors.As(err, &stripeErr) && stripeErr.Code == billingstripe.CodeCurrencyMismatch && stripeErr.SessionID != "" { |
| - return stripeErr.SessionID, true |
| + if errors.As(err, &stripeErr) && stripeErr.Code == billingstripe.CodeCurrencyMismatch && stripeErr.Session != nil && stripeErr.Session.ID != "" { |
| + return stripeErr.Session, true |
| } |
| - return "", false |
| + return nil, false |
| } |
| // errBillingCurrency: the buyer named a currency that is not one of the |
| @@ -611,8 +613,14 @@ func (s *Server) startBillingCheckout(ctx context.Context, acct *Account, plan, |
| return "", err |
| } |
| session, err := b.client.CreateCheckout(ctx, s.checkoutParams(acct, intent), "carloku-checkout-"+intent.ID) |
| - if id, refused := refusedByCurrency(err); refused { |
| - return "", s.refuseSession(ctx, intent, id, nil, now) |
| + if refusedSession, refused := refusedByCurrency(err); refused { |
| + // The provider answered in another currency. Before that session |
| + // is expired it has to be this intent's own: a response carrying |
| + // some other session, whatever its currency, is not acted on. |
| + if !b.ownSession(refusedSession, intent, acct) { |
| + return "", errBillingUnavailable |
| + } |
| + return "", s.refuseSession(ctx, intent, refusedSession.ID, refusedSession, now) |
| } |
| if err != nil { |
| return "", err |
| @@ -901,7 +909,19 @@ func (s *Server) bindBillingCheckout(ctx context.Context, sessionID string) (str |
| if intent.ID != session.Metadata["checkout_intent"] || intent.Plan != session.Metadata["plan"] || intent.Terms != session.Metadata["terms_version"] || intent.Band != session.Metadata["band"] { |
| return "", errBillingIdentity |
| } |
| - if sessionTerms, err := priceTermsFromMetadata(session.Metadata); err != nil || sessionTerms != intent.priceTerms() || (!sessionTerms.Legacy() && !session.FixedCurrency(string(sessionTerms.Currency))) { |
| + sessionTerms, err := priceTermsFromMetadata(session.Metadata) |
| + if err != nil || sessionTerms != intent.priceTerms() { |
| + return "", errBillingIdentity |
| + } |
| + if !sessionTerms.Legacy() && !session.FixedCurrency(string(sessionTerms.Currency)) { |
| + // This intent's own session, completed in another currency than it |
| + // was asked for. Nothing is recorded from it, and a person is told |
| + // once, the same as when the background pass finds it unbound |
| + // (refuseSession); the intent stays as it is so no second purchase |
| + // opens beside it. |
| + if err := s.store.fileBillingReviewOnce(acct.ID, session.SubscriptionID, billingCheckoutRefusedReason, time.Now()); err != nil { |
| + return "", err |
| + } |
| return "", errBillingIdentity |
| } |
| r, err := s.store.db.Exec(`UPDATE billing_checkout_intents SET session_id=?,expires_at=? WHERE account_id=? AND id=? AND (session_id='' OR session_id=?)`, session.ID, session.ExpiresAt, acct.ID, intent.ID, session.ID) |
| @@ -1091,8 +1111,11 @@ func (s *Server) recoverReadyIntentSession(ctx context.Context, acct *Account, i |
| // intent's and not its purchase, which creates nothing). |
| if allowCreate && within() { |
| session, err := b.client.CreateCheckout(ctx, s.checkoutParams(acct, intent), "carloku-checkout-"+intent.ID) |
| - if id, refused := refusedByCurrency(err); refused { |
| - return nil, s.refuseSession(ctx, intent, id, nil, now) |
| + if refusedSession, refused := refusedByCurrency(err); refused { |
| + if !b.ownSession(refusedSession, intent, acct) { |
| + return nil, errBillingIdentity |
| + } |
| + return nil, s.refuseSession(ctx, intent, refusedSession.ID, refusedSession, now) |
| } |
| if err == nil { |
| if !b.intentSession(session, intent, acct) { |
diff --git a/internal/console/billingstore.go b/internal/console/billingstore.go| index 410b2c364..27ae45fac 100644 |
| --- a/internal/console/billingstore.go |
| +++ b/internal/console/billingstore.go |
| @@ -173,17 +173,25 @@ func (s *Store) migrateBilling() error { |
| // The barrier against a build from before price books. Such a |
| // build reads legacy assignments only, prices what it reads in EUR |
| // and writes the month as a version 1 calculation. For a month in |
| - // which an account held a book-priced subscription, that is a |
| - // month priced without part of its history. The database itself |
| - // refuses it: no version 1 calculation may be frozen for a month |
| - // that any book-priced assignment of the account begins in or |
| - // before. This build never writes one for such a month (it prices |
| - // the month from the book, or refuses it as mixed), so the trigger |
| - // only ever stops the older build. |
| + // which an account held a paid, book-priced subscription, that is |
| + // a month priced without part of its history. The database itself |
| + // refuses it: no calculation whose top-level version is not 2 may |
| + // be frozen for a month in which a book-priced assignment of the |
| + // account was active and paid for (it began before the month |
| + // ended and was paid through past the month's start). A book |
| + // assignment that never counted for the month, a purchase |
| + // cancelled before it was first reconciled, say, is not history |
| + // the month was priced from (plans.AllowanceForMonth skips it), |
| + // and a legacy-only month after it is this build's own version 1. |
| + // The version is read with json_extract, not by substring: a |
| + // statement nested in a version 1 calculation may one day carry a |
| + // version 2 of its own. |
| `CREATE TRIGGER IF NOT EXISTS billing_months_no_legacy_calc_over_book_history BEFORE INSERT ON billing_months |
| - WHEN CAST(NEW.calculation AS TEXT) NOT LIKE '%"v":2,%' |
| + WHEN (CASE WHEN json_valid(CAST(NEW.calculation AS TEXT)) THEN json_extract(CAST(NEW.calculation AS TEXT), '$.v') IS NOT 2 ELSE 1 END) |
| AND EXISTS (SELECT 1 FROM billing_book_assignments b WHERE b.account_id = NEW.account_id |
| - AND b.effective_at < CAST(strftime('%s', NEW.month || '-01', '+1 month') AS INTEGER)) |
| + AND b.status = 'active' AND b.plan <> 'free' |
| + AND b.effective_at < CAST(strftime('%s', NEW.month || '-01', '+1 month') AS INTEGER) |
| + AND b.period_end > CAST(strftime('%s', NEW.month || '-01') AS INTEGER)) |
| BEGIN SELECT RAISE(ABORT, 'billing_month_legacy_calculation_over_book_history'); END`, |
| } { |
| if _, err := s.db.Exec(stmt); err != nil { |
diff --git a/internal/console/server.go b/internal/console/server.go| index 643aee996..e9057011b 100644 |
| --- a/internal/console/server.go |
| +++ b/internal/console/server.go |
| @@ -1148,6 +1148,7 @@ func NewServer(cfg Config, st *Store) (*Server, error) { |
| s.mux.HandleFunc("GET /a/{sqid}/settings", s.handleAccountSettings) |
| s.mux.HandleFunc("GET /a/{sqid}/billing", s.handleBillingPage) |
| s.mux.HandleFunc("/a/{sqid}/billing/checkout", s.handleBillingCheckout) |
| + s.mux.HandleFunc("/a/{sqid}/billing/quote", s.handleBillingQuote) |
| s.mux.HandleFunc("/a/{sqid}/billing/portal", s.handleBillingPortal) |
| s.mux.HandleFunc("/billing/webhook", s.handleBillingWebhook) |
| s.mux.HandleFunc("GET /api/billing/reviews", s.handleAPIBillingReviews) |
diff --git a/internal/console/templates/billing.html b/internal/console/templates/billing.html| index f6abfadd4..0c5a027d2 100644 |
| --- a/internal/console/templates/billing.html |
| +++ b/internal/console/templates/billing.html |
| @@ -41,32 +41,37 @@ |
| <h2 id="billing-choose-heading"><!-- billing.choose.heading -->Choose a plan</h2> |
| <p class="small"><!-- billing.choose.lede -->Prices are per year, before VAT, and depend on your billing country. The exact price is shown before you pay.</p> |
| -<form class="signin billing-quote" method="get" action="{{$.Base}}/billing"> |
| +<form class="billing-checkout" method="post" action="{{$.Base}}/billing/checkout"> |
| + <input type="hidden" name="csrf" value="{{$.CSRF}}"> |
| + {{/* What the page priced: the form's country and currency are checked |
| + against these before anything is bought, and a change is first |
| + shown as a quote. currency_chosen says the currency shown was a |
| + choice, not the country's default. */}} |
| + <input type="hidden" name="quoted_country" value="{{.QuotedCountry}}"> |
| + <input type="hidden" name="quoted_currency" value="{{.Currency}}"> |
| + <input type="hidden" name="currency_chosen" value="{{if .CurrencyChosen}}1{{end}}"> |
| + <div class="signin billing-quote"> |
| <label><!-- billing.country.label -->Billing country |
| - <select name="country"> |
| + <select name="country" required> |
| <option value=""><!-- billing.country.placeholder -->Choose a country</option> |
| {{range .Countries}}<option value="{{.ISO2}}"{{if .Selected}} selected{{end}}>{{.Name}}</option> |
| {{end}} |
| </select> |
| </label> |
| {{if .Currencies}}<label><!-- currency.label (currency review) -->Currency |
| - <select name="currency"> |
| + <select name="currency"{{if .CurrencyHeld}} aria-readonly="true"{{end}}> |
| {{range .Currencies}}<option value="{{.Code}}"{{if .Selected}} selected{{end}}>{{.Label}}</option> |
| {{end}} |
| </select> |
| </label>{{end}} |
| - <button class="linkish" type="submit"><!-- billing.quote.button -->Show prices for this country</button> |
| -</form> |
| + <button class="linkish" type="submit" formaction="{{$.Base}}/billing/quote" formnovalidate><!-- billing.quote.button -->Show prices for this country</button> |
| + </div> |
| {{if .Currencies}} |
| <p class="small"><!-- currency.explanation (currency review) -->Choose USD, EUR or GBP for your subscription and usage bills.</p> |
| <p class="small"><!-- country.explanation (currency review) -->Your billing country sets your pricing band. Changing currency does not change your inclusions.</p> |
| {{end}} |
| {{if .QuoteError}}<p class="small"><!-- billing.quote.unknown (not in strings.json) -->There are no prices for that country. Choose one from the list.</p>{{end}} |
| {{with .Quote}}<p class="small"><!-- billing.quote.band -->Prices for {{.Country}} are {{.Percent}}% of the base price, and so are the monthly inclusions.</p>{{end}} |
| - |
| -<form class="billing-checkout" method="post" action="{{$.Base}}/billing/checkout"> |
| - <input type="hidden" name="csrf" value="{{$.CSRF}}"> |
| - {{if .Currencies}}<input type="hidden" name="currency" value="{{.Currency}}">{{end}} |
| <fieldset class="billing-plan-options" aria-labelledby="billing-choose-heading"> |
| {{range .Plans}} |
| <label class="billing-plan-option{{if .Unavailable}} unavailable{{end}}"> |
| @@ -85,20 +90,15 @@ |
| <th scope="row">{{if eq .Plan "personal"}}<!-- billing.plan.personal.name -->Personal{{else if eq .Plan "community"}}<!-- billing.plan.community.name -->Community{{else if eq .Plan "business"}}<!-- billing.plan.business.name -->Business{{end}}</th> |
| <td>{{if .Team}}<strong>{{.PerMember}}</strong> <!-- billing.price.permember -->per member a year |
| <br><span class="small">{{.Price}} <!-- billing.price.peryear -->a year</span>{{else}}<strong>{{.Price}}</strong> <!-- billing.price.peryear -->a year{{end}}</td> |
| - <td class="small">{{if eq .Plan "personal"}}<!-- billing.plan.personal.blurb -->One member. Each month: 85 awake hours, 2 GB storage, 2 GB transfer.{{else if eq .Plan "community"}}<!-- billing.plan.community.blurb -->Per member, minimum 10. Each member adds, per month: 17 awake hours, 0.5 GB storage, 0.4 GB transfer.{{else if eq .Plan "business"}}<!-- billing.plan.business.blurb -->Per member, minimum 10. Each member adds, per month: 170 awake hours, 5 GB storage, 3 GB transfer, 30,000 replication writes. Priority support.{{end}}{{if and .Unavailable (eq .Plan "personal")}}<br><!-- billing.plan.personal.unavailable -->Personal is for one member. This account has {{$.Billing.Members}}.{{end}}</td> |
| + {{/* The approved blurbs name band A's figures; the figures below are |
| + the quoted band's, from the same quote as the price. */}} |
| + <td class="small">{{if eq .Plan "personal"}}<!-- billing.plan.personal.blurb -->One member. Each month: {{.Hours}} awake hours, {{.StorageGB}} GB storage, {{.TransferGB}} GB transfer.{{else if eq .Plan "community"}}<!-- billing.plan.community.blurb -->Per member, minimum {{$.Billing.MinQuantity}}. Each member adds, per month: {{.Hours}} awake hours, {{.StorageGB}} GB storage, {{.TransferGB}} GB transfer.{{else if eq .Plan "business"}}<!-- billing.plan.business.blurb -->Per member, minimum {{$.Billing.MinQuantity}}. Each member adds, per month: {{.Hours}} awake hours, {{.StorageGB}} GB storage, {{.TransferGB}} GB transfer, {{.Writes}} replication writes. Priority support.{{end}}{{if and .Unavailable (eq .Plan "personal")}}<br><!-- billing.plan.personal.unavailable -->Personal is for one member. This account has {{$.Billing.Members}}.{{end}}</td> |
| </tr> |
| {{end}} |
| </table> |
| </div> |
| - <p class="small"><!-- billing.members.note -->Community and Business are priced per member. You have {{.Members}} members; the minimum is 10.</p> |
| + <p class="small"><!-- billing.members.note -->Community and Business are priced per member. You have {{.Members}} members; the minimum is {{.MinQuantity}}.</p> |
| <div class="billing-address"> |
| - <p><label><!-- billing.country.label -->Billing country |
| - <select name="country" required> |
| - <option value=""><!-- billing.country.placeholder -->Choose a country</option> |
| - {{range .Countries}}<option value="{{.ISO2}}"{{if .Selected}} selected{{end}}>{{.Name}}</option> |
| - {{end}} |
| - </select> |
| - </label></p> |
| <p><label><!-- billing.postal.label -->Postal code |
| <input type="text" name="postal" autocomplete="postal-code"> |
| </label></p> |
diff --git a/internal/console/templates/layout.html b/internal/console/templates/layout.html| index 46ea480fb..a22c5db3a 100644 |
| --- a/internal/console/templates/layout.html |
| +++ b/internal/console/templates/layout.html |
| @@ -554,7 +554,7 @@ |
| .billing-inclusions th, .billing-inclusions td { padding: 1rem; vertical-align: top; } |
| .billing-inclusions th { text-align: left; } |
| .billing-inclusions td:nth-child(2) { min-width: 10rem; } |
| - .billing-address { display: grid; grid-template-columns: 2fr 1fr; gap: 1rem; margin-top: 1.5rem; } |
| + .billing-address { display: grid; grid-template-columns: 1fr 2fr; gap: 1rem; margin-top: 1.5rem; } |
| .billing-address label { display: block; font-weight: 550; } |
| .billing-payment { margin-top: 1.5rem; border-top: 1px solid var(--rule); padding-top: 1.5rem; } |
| .billing-consent { display: block; max-width: 48rem; } |
diff --git a/internal/plans/provision_test.go b/internal/plans/provision_test.go| index 1e3423468..d8e9a1a2a 100644 |
| --- a/internal/plans/provision_test.go |
| +++ b/internal/plans/provision_test.go |
| @@ -14,7 +14,9 @@ import ( |
| func TestStripeProvisionScriptMatchesThePriceBook(t *testing.T) { |
| raw, err := os.ReadFile("../../implementation/carloku-paid-launch/stripe-provision.sh") |
| if err != nil { |
| - t.Skip("the provisioning script is not beside this package") |
| + // Not a skip: a script that moved would otherwise stop being |
| + // checked against the book without anyone noticing. |
| + t.Fatalf("the provisioning script is not beside this package: %v", err) |
| } |
| script := string(raw) |
| book := regexp.MustCompile(`(?m)^readonly BOOK="([^"]+)"`).FindStringSubmatch(script) |