amadan
Multi-currency: the first review's findings
A session the provider created in another currency was refused once and then found again by replay or listing and offered to the buyer (both reviewers). Every session now passes one check against its intent however it was come by, and one that fails is expired and its intent retired. Book-priced assignments live in their own table so a build from before books finds no history to price in EUR (Astra P1). A checkout after a purchase keeps its currency and a live subscription refuses any (Opus P2-2). A month bought in two currencies is reviewed, not retried quietly (Opus P2-3). The configuration keeps whole catalogs for other books. Preflight checks the item's currency; the quantity update's answer is checked for its price and currency; recovered drafts and lines are verified. The book's validation uses checked arithmetic, knows its countries and refuses duplicate keys. A purchase with no book is asked for exactly as before. Legacy bytes are pinned to a golden the base commit's own code produced.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
20 files changed,
+1465
−121
cmd/console/billing.go+60 −27cmd/console/billing_test.go+27 −0docs/superpowers/specs/2026-10-01-multi-currency-price-book-design.md+4 −4docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review1.md+96 −0docs/superpowers/specs/DISPOSITIONS-multi-currency.md+34 −6docs/superpowers/specs/OPUS-REVIEW-multi-currency-review1.md+120 −0internal/billingstripe/client.go+33 −8internal/billingstripe/client_test.go+52 −2internal/console/billingcurrency_test.go+487 −21internal/console/billinginvoice.go+26 −3internal/console/billinginvoice_test.go+25 −1internal/console/billinginvoicestore.go+2 −2internal/console/billingquantity.go+6 −0internal/console/billingquantity_test.go+43 −10internal/console/billingservice.go+140 −10internal/console/billingservice_test.go+25 −1internal/console/billingstore.go+72 −5internal/plans/month.go+20 −8internal/plans/pricebook.go+97 −8internal/plans/pricebook_test.go+96 −5
CI passed — run details
diff --git a/cmd/console/billing.go b/cmd/console/billing.go| index 76cb3cf76..a7afb9e90 100644 |
| --- a/cmd/console/billing.go |
| +++ b/cmd/console/billing.go |
| @@ -95,50 +95,83 @@ func billingConfig(getenv func(string) string) (*console.BillingConfig, error) { |
| // not been approved can be configured in test mode and never in live |
| // mode. |
| cfg.PriceBook = getenv("CONSOLE_BILLING_PRICE_BOOK") |
| - currencies, err := plans.BookCurrencies(cfg.PriceBook) |
| - if cfg.PriceBook == plans.LegacyPriceBook || err != nil { |
| + if _, err := plans.BookCurrencies(cfg.PriceBook); cfg.PriceBook == plans.LegacyPriceBook || err != nil { |
| return nil, errBillingConfiguration |
| } |
| if approved, err := plans.BookIsApproved(cfg.PriceBook); err != nil || (cfg.LiveMode && !approved) { |
| return nil, errBillingConfiguration |
| } |
| // One provider price for every plan, band and currency the book sells |
| - // in, keyed "<book>/<plan>/<band>/<currency>". The twelve keys from |
| - // before books, "<terms>/<plan>/<band>", may be given as well, all of |
| - // them or none: they are only ever used to recognise a subscription |
| - // bought before books. Nothing else may be in the map, and no price |
| - // may appear under two keys. |
| + // in, keyed "<book>/<plan>/<band>/<currency>". |
| + // |
| + // A subscription stays on the book it was bought under for as long as |
| + // it lasts, so the map may also hold the prices of other books this |
| + // build has, and the twelve prices from before books, keyed |
| + // "<terms>/<plan>/<band>". Each of those catalogs is given whole or not |
| + // at all: they are only ever used to recognise a subscription already |
| + // bought, and half a catalog would recognise some and refuse others. |
| + // Nothing else may be in the map, and no price may appear under two |
| + // keys. |
| paid := []string{plans.PlanPersonal, plans.PlanCommunity, plans.PlanBusiness} |
| bands := []string{plans.BandA, plans.BandB, plans.BandC, plans.BandD} |
| raw := getenv("CONSOLE_STRIPE_PRICES") |
| - if len(raw) > 32*1024 || json.Unmarshal([]byte(raw), &cfg.Prices) != nil { |
| - return nil, errBillingConfiguration |
| - } |
| - want := len(paid) * len(bands) * len(currencies) |
| - legacy := len(paid) * len(bands) |
| - if len(cfg.Prices) != want && len(cfg.Prices) != want+legacy { |
| + if len(raw) > 64*1024 || json.Unmarshal([]byte(raw), &cfg.Prices) != nil { |
| return nil, errBillingConfiguration |
| } |
| seen := map[string]bool{} |
| - check := func(key string) bool { |
| - id := cfg.Prices[key] |
| - if !strings.HasPrefix(id, "price_") || len(id) <= 6 || seen[id] { |
| - return false |
| + used := 0 |
| + // catalog checks one book's keys (or the legacy keys, for book ""): |
| + // all present, or, when it is not required, all absent. |
| + catalog := func(book string, required bool) bool { |
| + terms := plans.PriceTerms{TermsVersion: plans.CurrentTermsVersion, PriceBook: book} |
| + sold := []plans.Currency{""} |
| + if book != plans.LegacyPriceBook { |
| + var err error |
| + if sold, err = plans.BookCurrencies(book); err != nil { |
| + return false |
| + } |
| } |
| - seen[id] = true |
| - return true |
| - } |
| - for _, plan := range paid { |
| - for _, band := range bands { |
| - for _, currency := range currencies { |
| - if !check(cfg.PriceBook + "/" + plan + "/" + band + "/" + string(currency)) { |
| - return nil, errBillingConfiguration |
| + var keys []string |
| + for _, plan := range paid { |
| + for _, band := range bands { |
| + for _, currency := range sold { |
| + key := terms.TermsVersion + "/" + plan + "/" + band |
| + if book != plans.LegacyPriceBook { |
| + key = book + "/" + plan + "/" + band + "/" + string(currency) |
| + } |
| + keys = append(keys, key) |
| } |
| } |
| - if len(cfg.Prices) == want+legacy && !check(plans.CurrentTermsVersion+"/"+plan+"/"+band) { |
| - return nil, errBillingConfiguration |
| + } |
| + present := 0 |
| + for _, key := range keys { |
| + if _, ok := cfg.Prices[key]; ok { |
| + present++ |
| } |
| } |
| + if present == 0 && !required { |
| + return true |
| + } |
| + if present != len(keys) { |
| + return false |
| + } |
| + for _, key := range keys { |
| + id := cfg.Prices[key] |
| + if !strings.HasPrefix(id, "price_") || len(id) <= 6 || seen[id] { |
| + return false |
| + } |
| + seen[id] = true |
| + } |
| + used += len(keys) |
| + return true |
| + } |
| + for _, book := range plans.PriceBooks() { |
| + if !catalog(book, book == cfg.PriceBook) { |
| + return nil, errBillingConfiguration |
| + } |
| + } |
| + if !catalog(plans.LegacyPriceBook, false) || used != len(cfg.Prices) { |
| + return nil, errBillingConfiguration |
| } |
| return cfg, nil |
| } |
diff --git a/cmd/console/billing_test.go b/cmd/console/billing_test.go| index 937b37495..fb6452822 100644 |
| --- a/cmd/console/billing_test.go |
| +++ b/cmd/console/billing_test.go |
| @@ -147,6 +147,33 @@ func TestBillingConfigPriceBookAndCatalog(t *testing.T) { |
| } |
| } |
| }, |
| + // With no book named, the keys an empty book name would call for: |
| + // twelve of them, and with the legacy twelve as well. Nothing but |
| + // the explicit refusal of an empty book stands in the way of |
| + // these, in live mode too. |
| + "no book, and keys under an empty book name": func(env map[string]string, prices map[string]string) { |
| + delete(env, "CONSOLE_BILLING_PRICE_BOOK") |
| + for k := range prices { |
| + delete(prices, k) |
| + } |
| + for _, plan := range []string{"personal", "community", "business"} { |
| + for _, band := range []string{"A", "B", "C", "D"} { |
| + prices["/"+plan+"/"+band+"/eur"] = "price_empty_" + plan + band |
| + } |
| + } |
| + }, |
| + "no book, keys under an empty book name, and the legacy keys": func(env map[string]string, prices map[string]string) { |
| + delete(env, "CONSOLE_BILLING_PRICE_BOOK") |
| + for k := range prices { |
| + delete(prices, k) |
| + } |
| + for _, plan := range []string{"personal", "community", "business"} { |
| + for _, band := range []string{"A", "B", "C", "D"} { |
| + prices["/"+plan+"/"+band+"/eur"] = "price_empty_" + plan + band |
| + prices[plans.CurrentTermsVersion+"/"+plan+"/"+band] = "price_" + plan + band |
| + } |
| + } |
| + }, |
| "some of the legacy keys": func(_ map[string]string, prices map[string]string) { |
| prices[plans.CurrentTermsVersion+"/personal/A"] = "price_legacy_personalA" |
| }, |
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 d03ef7aa8..5d55e63f6 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,10 +144,10 @@ func PublicPriceBook(book string) ([]byte, error) // the export below, byte-stab |
| ## Console data contract |
| -New columns, all `TEXT NOT NULL DEFAULT ''`, on `billing_checkout_intents`, `billing_accounts` and `billing_assignments`: `currency`, `price_book`. Empty in both means a legacy row. |
| +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 and cannot price its months in EUR. |
| - **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 in another currency is refused while the account's subscription has not ended (`canceled` or `incomplete_expired`). |
| +- **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 Session**: created with `adaptive_pricing[enabled]=false`, and with `currency`, `price_book` in the session and subscription metadata. The session that comes back, and the subscription reconciled later, must be in the intent's currency; a subscription whose price is not the configured one for its stored (book, plan, band, currency) is an identity mismatch, as a wrong price is today. |
| - **Configuration**: `CONSOLE_BILLING_PRICE_BOOK` names the book and is required. `CONSOLE_STRIPE_PRICES` holds 36 entries keyed `"<price_book>/<plan>/<band>/<currency>"`. The twelve legacy keys `"<terms>/<plan>/<band>"` may be given as well, all or none, and are used only to reconcile legacy subscriptions. Live mode with a draft book is refused. |
| @@ -157,7 +157,7 @@ New columns, all `TEXT NOT NULL DEFAULT ''`, on `billing_checkout_intents`, `bil |
| ## Carrying a choice through sign-in |
| -The pricing page links to the console with `?currency=<code>`. The console's sign-in entry points accept it, validate it, and keep it in a short-lived cookie (`carloku_currency`, host-only, `SameSite=Lax`, one hour). An invalid value is dropped and no cookie is set. The billing page preselects from the cookie, else from nothing: the country the owner then picks decides the default. The cookie is a suggestion; only the `currency` field of the checkout form is a choice. No geo lookup, no external service. |
| +The pricing page links to the console with `?currency=<code>`. The console accepts it on any GET, validates it, and keeps it in a short-lived cookie (`carloku_currency`, host-only, `SameSite=Lax`, one hour). An invalid value is dropped and no cookie is set. The billing page preselects from the cookie, else from nothing: the country the owner then picks decides the default. The cookie is a suggestion; only the `currency` field of the checkout form is a choice. No geo lookup, no external service. |
| Interface for the billing page (the layout branch owns the template and the view): `s.billingCurrencies()` gives the currencies to offer, `s.suggestedCurrency(r)` the one to preselect (may be empty), the prices come from `plans.PriceFor` or the export below, and the checkout form posts `currency`. |
| @@ -196,7 +196,7 @@ Interface for the billing page (the layout branch owns the template and the view |
| } |
| ``` |
| -`country_currency` lists only countries whose default is not `default_currency`. `country_band` is every country the book sells to. `plans` is in display order: free, personal, community, business; free has zero prices. `resources_per_member` is band A; another band's allowance is that figure times the band factor, divided by 10000, rounded down, after multiplying by the quantity. A micro is one millionth of the currency's major unit. Keys are sorted and the output is byte-stable. |
| +`country_currency` lists only countries whose default is not `default_currency`. `country_band` is every country the book sells to. `plans` is in display order: free, personal, community, business; free has zero prices. `resources_per_member` is the plan's own figure. For a plan with `band_scaled` true, a band's pooled allowance is the figure times the quantity times the band factor, divided by 10000, rounded down. For the free plan (`band_scaled` false) the band changes nothing. A micro is one millionth of the currency's major unit. Keys are sorted and the output is byte-stable. |
| ## What was built, and where |
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review1.md b/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review1.md| new file mode 100644 |
| index 000000000..aa58ed430 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review1.md |
| @@ -0,0 +1,96 @@ |
| +All references are at `9d104f1d`. This was a static review against `12e70d95`; no files were changed, tests run, or network commands used. |
| + |
| +## Findings |
| + |
| +1. **P1 — Checkout recovery can offer a session already rejected for its currency.** |
| + `internal/console/billingservice.go:903`, `:920`, `:348`, `:405`. |
| + |
| + **Scenario:** A GBP checkout creation returns an EUR session, or one with Adaptive Pricing enabled. `CreateCheckout` correctly rejects it, leaving a ready intent without a session ID. On retry, replay fails again, but listing recovery accepts that same session using only intent, account and mode. The caller saves it and returns its URL without calling `FixedCurrency`. Binding may reject the mismatch only after the buyer pays. |
| + |
| + **Smallest fix:** Apply one intent/session identity validator—including price terms, currency and absence of Adaptive Pricing/conversion—to recovered and resumed sessions before saving or returning their URLs. Test the rejection followed by recovery, not just the first creation response. |
| + |
| +2. **P1 — An older console can freeze GBP history as EUR after migration.** |
| + `internal/console/billingstore.go:191`; `internal/console/billinginvoicestore.go:530`; `internal/console/billinginvoice.go:307`, `:481`. |
| + |
| + **Scenario:** Record a GBP purchase, then run the pre-change console against the migrated database. Its assignment query ignores the added columns and its calculation prices that history as legacy EUR. It freezes the incorrect version 1 calculation before provider preflight. It can waive the month without contacting Stripe, or attempt an EUR invoice. Stripe refusing that invoice does not undo the incorrect frozen calculation; this build subsequently refuses the history mismatch. |
| + |
| + **Smallest fix:** Enforce exclusive operation by upgraded billing processes and prevent downgrade against databases containing book-priced purchases. A compatibility barrier must stop old **readers**, including the invoice worker; guarding only new writes is insufficient. |
| + |
| +3. **P2 — Configuration cannot retain prices for historical nonlegacy books.** |
| + `cmd/console/billing.go:117`, `:131`; `internal/console/billingservice.go:523`. |
| + |
| + **Scenario:** Publish a new book version while subscriptions under the previous version remain active. Configuration accepts only the selected book’s 36 keys, optionally with twelve legacy keys. Both nonlegacy catalogs fail validation; supplying only the new catalog makes reconciliation and quantity changes reject subscriptions whose stored book is the previous version. |
| + |
| + **Smallest fix:** Require the selected book’s complete catalog while permitting complete catalogs for retained historical books. Preserve provider-price uniqueness across all supplied catalogs. |
| + |
| +4. **P2 — Subscription-item currency checks are incomplete at monetary-operation boundaries.** |
| + `internal/console/billinginvoice.go:552`; `internal/console/billingquantity.go:1099`. |
| + |
| + **Scenario:** Invoice preflight receives a GBP subscription whose item currency is missing or EUR. It checks only the subscription’s currency and proceeds. Separately, a quantity update returning the expected item ID and quantity but another price/currency is recorded as confirmed without checking those fields. The preceding quantity read and subsequent reconciliation do perform currency checks, but the update response itself bypasses them. |
| + |
| + **Smallest fix:** Validate item presence and currency during invoice preflight, and the returned item’s price ID and currency before confirming a quantity operation. Keep historical-invoice checks tied to the relevant purchase identity. |
| + |
| + These are missing required refusal checks; source inspection does **not** establish that Stripe permits an internally inconsistent subscription to debit another currency. |
| + |
| +5. **P2 — Price-book validation can accept prices that violate the rounding rule through integer overflow.** |
| + `internal/plans/pricebook.go:233`. |
| + |
| + **Scenario:** Set Personal/USD `annual_base_minor` to `4611686018427390704`—`2800 + 2^62`—and retain its existing unit prices. Every band factor is divisible by four, so the unchecked `int64` multiplication wraps to the original product. Validation accepts the book despite its exact mathematical prices being different. |
| + |
| + **Smallest fix:** Use checked multiplication or `big.Int` for validation’s multiplication and rounding. Add this boundary case. |
| + |
| +6. **P3 — The legacy snapshot test shares the nested types whose compatibility it should protect.** |
| + `internal/console/billingcurrency_test.go:555`, `:584`. |
| + |
| + **Scenario:** Removing `omitempty` from the added assignment or allowance currency fields changes both the “old” and new representations in this test. The comparison still passes although historical serialized bytes and digests change. |
| + |
| + **Smallest fix:** Compare against pre-change JSON and digest fixtures, or reconstruct the old nested types independently. The implementation at this commit appears compatible; the regression test does not fully protect that compatibility. |
| + |
| +7. **P3 — Non-EUR overage tests lack independent expectations for three rates.** |
| + `internal/plans/pricebook_test.go:249`; `internal/console/billingcurrency_test.go:601`. |
| + |
| + **Scenario:** Using EUR storage, transfer and write rates for GBP/USD would still satisfy these checks: only awake time has a chosen-currency expected rate, while the other amounts are checked through their sum. The console test obtains its expectation by calling `OverageFor` again. |
| + |
| + **Smallest fix:** Assert independent GBP/USD amounts for every meter, including rounding and overflow boundaries. |
| + |
| +The ordinary GBP path carries the stored book and currency through intents, accounts, assignments, calculations and invoice requests. Runtime arithmetic appears exact and checked, with legacy EUR results preserved. Currency does not select bands, resources, quantities, tax codes or entitlement resources. Checkout uses `PostForm`, so neither the cookie nor query string directly supplies the purchase currency. |
| + |
| +For actual legacy snapshots, the appended `omitempty` fields and unchanged field order preserve version 1 JSON bytes. Version 1 draft and line request hashes retain the old encoding, and their HTTP amounts remain EUR. Startup migration adds all six columns with `TEXT NOT NULL DEFAULT ''`; the new binary migrates an old database before using them. Current command configuration rejects missing books, draft books in live mode, incomplete catalogs and reused provider-price IDs. |
| + |
| +## Claims the code contradicts |
| + |
| +- **The three surviving mutations are not all redundant.** |
| + `docs/superpowers/specs/DISPOSITIONS-multi-currency.md:11`. |
| + |
| + The two intent-currency comparisons are redundant on the ordinary checkout path **under the validated unique-price configuration**. That does not independently test the store’s invariant. |
| + |
| + The empty-book guard is **not redundant**. Without `cmd/console/billing.go:99`, twelve unique keys such as `/personal/A/eur` through `/business/D/eur` pass the remaining checks. `BookCurrencies("")` returns EUR and `BookIsApproved("")` returns true (`internal/plans/pricebook.go:361`, `:373`). Even live configuration would pass. The current guard correctly prevents this; the mutation survived because the fixtures retained incompatible catalog keys. |
| + |
| +- **“Verified … on every read” overstates recovery validation.** |
| + Besides the checkout finding, invoice listing recovery matches metadata and mode, then acknowledges the recovered draft without `verifyInvoice` (`internal/console/billinginvoice.go:648`, `:917`). Recovered line matching likewise does not check currency before acknowledgement (`:1001`). Later validation stops mismatched invoices before further billing steps or finalization; these paths do not prove a wrong-currency payment. Verify recovered objects before acknowledging them. |
| + |
| +- **Legacy checkout replay does not preserve the original request parameters.** |
| + `internal/console/billingservice.go:874`, `:903`. |
| + |
| + A pre-upgrade intent is replayed under its old key with explicit EUR and Adaptive Pricing disabled, unlike the base request. Stripe can reject the changed parameters. Listing fallback may recover it, so this is not by itself proof of a stranded purchase; the replay comment and verification must distinguish that fallback. |
| + |
| +- **The public allowance-formatting rule omits the free-plan exception.** |
| + `internal/plans/pricebook.go:630`; design document `:199`. |
| + |
| + Applying the documented band factor to a free plan in band D displays 35 awake hours instead of 100. Qualify the formula with `band_scaled`. The export itself correctly supplies `band_scaled:false`, zero free prices and the unscaled resources. I found no private customer/provider data in it. |
| + |
| +The new tests use wall-clock authentication fixtures, but I found no concrete new calendar- or ordering-dependent failure. |
| + |
| +## Required Stripe test-mode verification |
| + |
| +1. **Catalog and checkout:** Verify all 36 prices’ currency, amount, metadata and absence of currency options. Include GBP Community B at 338 pence and D at 158 pence. Create, retrieve and complete sessions in each currency; verify Adaptive Pricing is disabled, no conversion appears, and subscription/item currencies match the stored choice. |
| + |
| +2. **GBP monthly overages:** Exercise every meter through draft creation, line creation, finalization and payment. Inspect currency, untaxed amounts, tax amounts and totals on the resulting provider objects. |
| + |
| +3. **Lost responses and upgrade recovery:** Lose responses separately after checkout, draft, line, finalize and pay requests. Verify recovery finds the original objects without duplicate charges. Include a pre-upgrade checkout replay and version 1 invoice operations with their original keys and hashes. |
| + |
| +4. **Renewals and quantity changes:** Exercise both proration policies and renewal. Inspect the actual prorated/upcoming invoices and lines for currency and amounts. The quantity worker does not inspect those invoices (`internal/console/billingquantity.go:1167`), so its local checks do not establish their contents. Run `TestSandboxQuantityFence` before enabling its configuration switch. |
| + |
| +5. **Existing customer currency:** Cancel an EUR subscription and attempt GBP checkout with both settled and outstanding EUR invoices/items. Establish Stripe’s acceptance/refusal behavior and verify historical invoices remain EUR. `Customer` does not decode currency (`internal/billingstripe/customer.go:79`); checkout and invoice preflight currently rely on provider enforcement of customer-currency compatibility. |
| + |
| +**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 96985342f..85d3b497f 100644 |
| --- a/docs/superpowers/specs/DISPOSITIONS-multi-currency.md |
| +++ b/docs/superpowers/specs/DISPOSITIONS-multi-currency.md |
| @@ -1,19 +1,47 @@ |
| # Multi-currency price book: what was checked before review |
| -Branch `feat/carloku-multi-currency-20261001`. Design and contract: `2026-10-01-multi-currency-price-book-design.md`. Billing is off; nothing here writes to Stripe. Not yet read by a reviewer. |
| +Branch `feat/carloku-multi-currency-20261001`. Design and contract: `2026-10-01-multi-currency-price-book-design.md`. Billing is off; nothing here writes to Stripe. |
| -## Mutation checks |
| +## Mutation checks before review one (at 9d104f1d) |
| Run in a scratch copy of the tree, each mutation alone, against the tests named for it. Forty-three in all. |
| Forty failed their tests: checkout ignoring the buyer's choice; an unknown currency falling back to the default; the provider price's currency, its amount, and its own metadata unchecked; Adaptive Pricing not switched off; the session that comes back unchecked; reconcile not comparing the subscription's currency; a subscription changing currency in the store; a book month priced at the legacy rates; the invoice's, an item's and a line's currency unverified; the invoice preflight ignoring the subscription's currency; a legacy request hashed with its currency; the frozen month's currency not compared with the recorded assignment; the cookie keeping any value; a forged cookie believed; a month paid for in two currencies; the price key ignoring the currency; any stored calculation counted well formed; charges invoiced in another currency; the quantity worker ignoring the stored currency; the book's rounding rule unenforced; a book's EUR differing from the terms'; live mode accepting a draft book; the EUR floor used for every currency; a session bound to its intent with its currency unchecked; an intent with a book and no currency opened; the session metadata omitting the currency; reconcile not checking the completed session's currency; reconcile not comparing the intent's price terms; binding not comparing the session's price terms with the intent's; a recorded state needing no valid price terms. |
| -Three survived, each because another check refuses the same thing, so the mutated line is redundant today and stays as a second fence: |
| - |
| -- the comparison of an open intent's currency with the new request's, and the same comparison when an intent is begun: a different currency is always a different provider price, which both already compare; |
| -- the configuration's explicit refusal of an empty book name: with no book the price map cannot have the keys it then requires. |
| +Three survived. Two are redundant (the comparison of an open intent's currency with the new request's, and the same when an intent is begun: a different currency is always a different provider price, which both already compare). The third was not redundant, as review one found: the configuration's refusal of an empty book name was the only thing stopping a price map keyed under an empty book, in live mode too. It has its own test now. |
| ## What the tests cannot show |
| - Stripe itself. Every provider answer here is a fake's. That a session created with `adaptive_pricing[enabled]=false` and `currency` comes back in that currency, that Stripe refuses a price in another currency, and what Stripe does when a customer whose subscription ended starts one in another currency, are read from documentation and need test mode, which is not authorized. |
| - The legacy request hashes are shown equal to the hash of the parameter shapes as they were, reconstructed in the test. No operation journaled by an earlier build exists to replay. |
| + |
| +## Review one (at 9d104f1d; Opus accepted with fixes, Astra rejected) |
| + |
| +Reports: `OPUS-REVIEW-multi-currency-review1.md`, `CODEX-FINDINGS-multi-currency-review1.md`. Both followed a GBP purchase end to end and found the ordinary path carries one currency throughout, legacy bytes and hashes unchanged, and allowances, bands, quantities and tax codes the same in every currency. Both found the same first fault. |
| + |
| +| Finding | Disposition | Where | Reproduction | |
| +| --- | --- | --- | --- | |
| +| Astra P1, Opus P2-1: a session the provider created in another currency was refused on the click that created it, then found again by the replay or the listing and offered to the buyer. | Fixed. Every session the console binds, offers or records a purchase from passes one check against its intent (`intentSession`: this account's, this intent's, on the intent's price terms, in its currency with no Adaptive Pricing or conversion), however it was come by. A session that is this intent's and fails it is expired at the provider and the intent retired (`refuseSession`); the client names the session it refuses. A paid one the provider will not expire is left unbound for a person. | `billingservice.go`, `billingstore.go` `retireRefusedCheckout`, `billingstripe/client.go` | `TestCheckoutNeverOffersASessionInAnotherCurrency` (four cases: the click that created it, a failed expiry and a second click, the background pass, a bound session that reads back changed), `TestRecoveredSessionMustBeInTheIntentsCurrency` | |
| +| Astra P1: an older build run against a migrated database reads a GBP assignment from the one table it knows, prices the month in EUR and freezes that. | Fixed. Assignments of a purchase priced from a book are kept in their own table (`billing_book_assignments`, with the currency and book required non-empty); the older build's own history query finds nothing for such a purchase, so it has no history to price. This build reads both tables as one. | `billingstore.go` `billingAllAssignments` | `TestBookPricedHistoryIsInvisibleToAnOlderBuild` (runs the older build's queries word for word) | |
| +| Opus P2-2: the design's refusal of a checkout in another currency while a subscription is live, or after one in a different currency, was not implemented. | Fixed. A live subscription refuses any checkout before the provider is asked; after it ends, a checkout in a currency other than the one bought in is refused, the country's default included. A change of currency is an operator's to make once Stripe's behaviour is known (test-mode list). | `billingservice.go` `startBillingCheckout` | `TestCheckoutAfterAPurchaseKeepsItsCurrency` | |
| +| Opus P2-3: a month whose paid spans differ in currency or book was held quietly, retried for ever, nobody told. | Fixed. `plans.ErrPriceTerms` is its own sentinel; the engine files `billing_invoice_price_terms:<month>` and freezes nothing. | `plans/month.go`, `billinginvoice.go` | `TestAllowanceForMonthKeepsOneCurrency`, `TestInvoiceEngineReviewsAMonthBoughtInTwoCurrencies` | |
| +| Astra P2: configuration could not keep prices for a book no longer current, so publishing a new book would refuse subscriptions on the old one. | Fixed. The map holds the current book's catalog and may hold any other embedded book's and the legacy twelve, each whole or absent, no price under two keys. | `cmd/console/billing.go` | `TestBillingConfigPriceBookAndCatalog` | |
| +| Astra P2, Opus P3-3: the invoice preflight checked the subscription's currency and not its item's; the quantity update's answer was confirmed without checking its price or currency. | Fixed: preflight requires exactly one item in the currency for a book purchase; the update's item must be on the account's own price in its currency. | `billinginvoice.go`, `billingquantity.go` | `TestSubscriptionAndItemCurrencyAreEachChecked`, `TestQuantityWorkerDoesNotConfirmAnUpdateOnAnotherPrice` | |
| +| Astra P2, Opus P3-4: the rounding rule was checked with an unchecked multiplication (2800 plus 2^62 wraps to the real product); a country code the terms do not know loaded; duplicate JSON keys were merged silently. | Fixed, each with a case in `TestPriceBookValidation`. | `plans/pricebook.go` | `TestPriceBookValidation` | |
| +| Astra P3: the legacy snapshot test shared this build's nested types. | Fixed: the golden bytes and digest come from the base commit's own code, run in a scratch copy, and are a literal in the test. | `billingcurrency_test.go` | `TestLegacyCalculationBytesAreUnchanged` | |
| +| Astra P3: overage tests checked one rate independently. | Fixed: all four meters in all three currencies against figures worked out by hand. | `plans/pricebook_test.go` | `TestOverageRatesInEachCurrencyByHand` | |
| +| Astra, claims: recovery of a draft or line acknowledged what it found without verifying it. | Fixed: a recovered draft goes through `verifyInvoice`; a recovered line must be in the calculation's currency. | `billinginvoice.go` | `TestInvoiceRecoveryChecksTheCurrencyOfWhatItFinds` | |
| +| Astra, claims: a legacy intent replayed under its old key was sent with a currency and Adaptive Pricing off, a different request. | Fixed. A purchase with no book is asked for exactly as before (`LegacyAdaptive`, which must be set on purpose), both at creation and on replay, from one builder. | `billingstripe/client.go`, `billingservice.go` `checkoutParams` | `TestCreateCheckout_LegacyAdaptiveIsTheOldRequest`, `TestCheckoutRefusesACurrencyThatIsNotSold` | |
| +| Opus P3-5, Astra: the export's allowance rule omitted the free plan. | Corrected in the doc and the design. | `plans/pricebook.go` | none | |
| +| Opus P3-6: the two currency checks (subscription, item) masked each other in the fakes. | Fixed: the fakes set them apart, and each has a case. | tests | `TestSubscriptionAndItemCurrencyAreEachChecked` | |
| +| Opus, claim 4: an unknown currency was reported after other faults. | Fixed: it is checked first. | `billingservice.go` | `TestCheckoutRefusesACurrencyThatIsNotSold` | |
| +| Opus, claim 5: the empty-book refusal was called redundant and is not. | Corrected above, with a test that passes a map keyed under an empty book. | `cmd/console/billing_test.go` | `TestBillingConfigPriceBookAndCatalog` | |
| +| Opus P3-1: a Stripe customer reused across currencies may strand the old currency's last invoice. | Covered by the refusal of a currency change (P2-2) until Stripe's behaviour is known. | | | |
| +| Opus P3-2: rollback needs the environment changed with the binary; an older binary on a 36-key map exits. | Accepted. The older build's blindness to book-priced history (above) removes the mispricing; the roll note is still owed when this is rolled. Billing is off everywhere. | | | |
| +| Opus P3-7: a legacy subscription Stripe created in a non-EUR currency through Adaptive Pricing is now refused everywhere. | Accepted as correct; none should exist (no catalog). Test-mode item 8. | | | |
| +| Opus, claim 6: the cookie is set on any GET, not only sign-in entry points. | Accepted; the design now says so. | | | |
| +| Opus, claim 3: a console with no book writes rows with no currency. | Accepted: only tests and the demo run without a book; `cmd/console` requires one. | | | |
| + |
| +Mutation checks after these fixes, twenty-five in a scratch copy: twenty-two failed their tests. Three survived: the two checks of a bound session when it is resumed (each is the other's second fence in the same function, and one of them always refuses); and the refusal of a created session that is not this intent's and not expirable (unreachable through the client, which refuses such a session first). |
| + |
| +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-review1.md b/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review1.md| new file mode 100644 |
| index 000000000..043f6a80d |
| --- /dev/null |
| +++ b/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review1.md |
| @@ -0,0 +1,120 @@ |
| +# Review: feat/carloku-multi-currency-20261001 at 9d104f1d (base 12e70d95) |
| + |
| +## Findings |
| + |
| +### P2-1: A session that comes back in the wrong currency is refused on the first click and handed to the buyer on the second |
| +- **Where:** `internal/console/billingservice.go:916-928` (the recovery listing), `billingservice.go:409` (the open-session resume) and `internal/billingstripe/client.go:626`. |
| +- **Scenario:** a GB buyer starts a GBP checkout. Stripe creates the session but returns it in EUR, or with Adaptive Pricing or a `currency_conversion` on it. `CreateCheckout` returns `currency_mismatch`, the intent is CustomerReady and has no session id, and the session stays **open** at Stripe. |
| + - The buyer clicks again. `recoverReadyIntentSession` replays the create under the same key, which fails the same way, then falls through to `ListCheckoutsForCustomer`. |
| + - The listing returns that session after checking only its intent metadata, live mode and client reference. `saveBillingCheckout` stores it, and the `case "open"` path returns `intent.URL` without a currency check. |
| + - The buyer pays the converted amount. `bindBillingCheckout` and reconcile (`:569`) then refuse it. Stripe now bills a subscription the console never records, and only a refund by hand fixes it. |
| + - The background pass (allowCreate=false) binds the same session by the same listing. |
| +- **Test gap:** `TestCheckoutRefusesASessionInAnotherCurrency` only tests the first click. |
| +- **Fix:** |
| + - Require `session.FixedCurrency(intent money)` (and the session's price terms equal to the intent's) in the listing loop and before returning `intent.URL`. |
| + - When `CreateCheckout` reports `CodeCurrencyMismatch`, expire the session it created (`ExpireCheckout` under `carloku-expire-<intent>`). |
| + - Add a two-click test. |
| + |
| +### P2-2: The rule that refuses checkout in another currency while a subscription is live is not implemented |
| +- **Where:** `billingservice.go:314`. `state` is read but only `House` and `Customer` are used. The handler has no guard either. |
| +- **Scenario:** an account has an active GBP subscription. The owner POSTs `/billing/checkout` with `currency=usd`, or the form's default resolves to USD. |
| + - The console prices, verifies and opens a USD session on the same Stripe customer. |
| + - If Stripe lets that customer pay, reconcile refuses the new subscription because one is already active (`:539`). The buyer is charged for a second subscription that the console never records. |
| + - If Stripe refuses, the buyer gets an opaque `unavailable` error. |
| +- **Contradiction:** the design says "checkout in another currency is refused while the account's subscription has not ended… The console refuses a change earlier than Stripe would." Nothing in the code does this. |
| +- **Fix:** in `startBillingCheckout`, after reading `state`, refuse when `state.Subscription != ""` and its status is not `canceled` or `incomplete_expired`. Refuse also when the requested price terms differ from `state.priceTerms()` and the old subscription's last months are not yet invoiced (see P2-3). Add a test. |
| + |
| +### P2-3: A month whose paid spans differ in currency or book is held silently, not held for review |
| +- **Where:** `internal/plans/month.go:167` returns `ErrAssignmentHistory`. `billinginvoice.go:448` (freeze) passes it up, and `billinginvoice.go:187` lists it as quiet. |
| +- **Scenario:** a GBP subscription is cancelled on 10 September and a USD one is bought on 12 September. `AllowanceForMonth("2026-09")` fails with `ErrAssignmentHistory`. No review is filed: the month backs off and is retried for ever, and nobody is told that September's usage was never billed. |
| + - The same happens to a legacy EUR subscription followed by a book EUR one in the same month (same currency, different book). |
| + - Before this change, a cancel and repurchase inside one month reached `errBillingIdentityMixed`, which files a review. Adding the currency check moved that case onto the quiet path. |
| +- **Contradiction:** design rule 6 says "held for review, never priced". |
| +- **Fix:** give the mixed currency or book case its own sentinel (for example `plans.ErrPriceTermsMixed`) and map it in `freeze` to `r.review("billing_invoice_currency_mixed")`, like the identity and tax-code mixes. |
| + |
| +### P3-1: A Stripe customer reused across currencies can strand the old currency's last overage invoice |
| +- **Where:** `billingservice.go:322` (`Customer: state.Customer`), the invoice engine's draft step, and P2-2. |
| +- **Scenario:** the GBP subscription ends on 30 September and a USD one starts on 2 October on the same `cus_`. September is frozen in GBP after the grace period, and its GBP draft invoice goes to a customer Stripe may now hold in USD. If Stripe refuses, the month errors or is reviewed indefinitely. |
| +- **Fix:** either refuse the currency change until every month of the old subscription is terminal (part of P2-2's fix), or prove in test mode that Stripe accepts it (Stripe item 4 below). |
| + |
| +### P3-2: Rollback and roll-forward need the environment changed with the binary |
| +- **Where:** `cmd/console/billing.go:99` and the length check that follows. `main.go:232-234` exits on error. |
| +- **Scenario:** |
| + - This binary on a box whose env still has only the 12 legacy keys and no `CONSOLE_BILLING_PRICE_BOOK` exits with `log.Fatal`, so the console crash-loops. |
| + - The previous binary given 36 or 48 keys also exits. |
| + - If a rollback reverts the env to 12 keys, the old binary runs the invoice engine on GBP rows. Its `AllowanceForMonth` ignores currency, so it freezes a V1 EUR calculation and sends an EUR draft. This build later refuses that frozen month for ever (`invoiceIdentityFromHistory`), and it needs repair by hand. |
| +- No row is misread in either direction otherwise: the columns default to `''`, the old binary's reconcile and quantity checks fail on price id, and the assignment copy is an INSERT…SELECT. |
| +- **Fix:** add a roll note saying no binary rollback below this commit once any non-legacy row exists, and billing env and binary change together. Billing is off everywhere today, so this is documentation. |
| + |
| +### P3-3: The quantity worker does not check the update response's price or currency |
| +- **Where:** `internal/console/billingquantity.go:1099`. |
| +- **Scenario:** the update response's item has a different price (and so currency). The worker confirms the operation anyway, and the next reconcile catches it. |
| +- **Fix:** also require `updated.PriceID == op's configured price` and `billingCurrencyAgrees(state.priceTerms(), updated.PriceCurrency)`. |
| + |
| +### P3-4: validateBook gaps |
| +- **Where:** `internal/plans/pricebook.go:201` and `:233`. |
| +- The countries in `country_currency` are format-checked, not checked against `CountryBands()`. A typo such as `"UK"` loads, and GB buyers then default to USD silently. |
| +- `base[c]*t.BandFactorsBps[band]` is an unchecked int64 multiplication, while everything else uses `mulDivFloor` and its relatives. |
| +- Duplicate JSON keys (two `"eur"` lists) are merged by the decoder without error. |
| +- **Fix:** |
| + - Require every listed country to be in `CountryBands()`. |
| + - Use `mulDivFloor` with half-up rounding, or `big.Int`. |
| + - Optionally pin in a test that every book a stored row may name stays embedded. Removing a book makes `Validate` fail for those rows. |
| + |
| +### P3-5: The public export's documented allowance rule is wrong for the free plan |
| +- **Where:** `pricebook.go:630-633` and the design's export section. |
| +- **Scenario:** the docs say "another band's allowance is that figure × band factor". A pricing page following that would show the free plan in band D with 35% of its allowance. Free is `band_scaled:false`, and `Quote` gives it 100% in every band. |
| +- **Fix:** say "for a plan with `band_scaled: true`; otherwise the figure × quantity". |
| +- Nothing in the export discloses more than the checkout already uses: bands are per-country inputs, and the floors and rates are the book's. Minor units and rounding are right: GBP Community B is 338, and the annual price is unit × quantity with no further rounding. |
| + |
| +### P3-6: The pairs of currency checks are each masked by the other in tests |
| +- **Where:** the fake `internal/console/billingservice_test.go:194` reports the item's price currency equal to the subscription's currency. |
| +- **Effect:** removing either `billingCurrencyAgrees(…, sub.Currency)` or `billingCurrencyAgrees(…, Items[0].PriceCurrency)` on its own, in reconcile or in `quantityProviderItem`, passes every test. In `TestQuantityWorkerChecksTheStoredCurrency` the second case is caught by the price-id check, not by currency. |
| +- **Fix:** let the fake set the two independently and add one case for each. |
| + |
| +### P3-7: Legacy subscriptions bought with Adaptive Pricing are now refused |
| +- **Where:** `billingCurrencyAgrees`, used at `billingservice.go` reconcile, the quantity worker and invoice preflight. |
| +- **Effect:** the old flow set `adaptive_pricing[enabled]=true`. If Stripe creates such subscriptions in the buyer's local currency, any legacy (test-mode) subscription from outside the eurozone is now refused everywhere. |
| +- This is correct behaviour, because the old engine would have sent EUR invoices to it. It is a change for existing rows, though, and none should exist (no catalog). Stripe item 8 below confirms it. |
| + |
| +## Answers to the specific questions |
| + |
| +- **C (legacy bytes and hashes):** these hold. |
| + - A V1 calculation marshals byte-identically. The struct order is unchanged, the new fields are omitempty and empty for legacy, and `MonthAllowance` and `Assignment` gain only omitempty fields. |
| + - Legacy draft and line hashes are unchanged: `requestHash` blanks `Currency`, the field is `json:",omitempty"` and last, and finalize and pay hash only the invoice id. |
| + - No stored legacy row is read as non-EUR. A legacy intent left open at upgrade is expired and replaced when a book is configured. |
| +- **F (arithmetic):** `overageLines`, `OverageFor` and `PriceFor` match the legacy arithmetic for EUR. The only gap is the unchecked multiplication in P3-4. |
| +- **G (configuration):** a live console with no book is not possible in practice, apart from the contrived case in claim 5. A draft book cannot be live, prices are distinct per key, and the counts are exact. |
| +- **E (currency changes nothing else):** band, quantity, tax code, coupon and allowances are identical across currencies. |
| + |
| +## Claims the code contradicts |
| + |
| +1. **Design, "A currency already held… checkout in another currency is refused":** there is no such check (P2-2). |
| +2. **Design rule 6, "held for review, never priced":** the month is held quietly with no review (P2-3). |
| +3. **Design rule 7, "a new row with no currency is refused":** `storedPriceTerms` accepts legacy terms for new rows. Only the cmd/console configuration prevents them, and tests and the demo write them. |
| +4. **`startBillingCheckout` comment, "refused… before anything is read or created" (`billingservice.go:204`):** the entitlement-authority preflight, the band lookup and the `Members` read run first. An unknown country with a bad currency returns the band error as a redirect, not 400. |
| +5. **Dispositions, "the configuration's explicit refusal of an empty book name" is redundant:** this is wrong. With `CONSOLE_BILLING_PRICE_BOOK` empty, `BookCurrencies("")` gives `[eur]`, so the required keys are `"/<plan>/<band>/eur"`, which a map can contain. |
| + - A 24-entry map (12 such keys plus the 12 legacy keys) passes. |
| + - `BookIsApproved("")` is true, so live mode starts with no book and records currency-less rows. |
| + - The first two surviving mutations (comparing the intent's currency when resumed and when begun) **are** redundant: a different currency or book always means a different price id, because price metadata must name the book and currency and config ids are unique. |
| +6. **Design, the sign-in entry points accept `?currency`:** `rememberCurrency` sets the cookie on every GET to any path. This is harmless but broader than written. |
| +7. **`TestCheckoutRefusesASessionInAnotherCurrency`, "nothing is offered to the buyer":** true for one click only (P2-1). |
| + |
| +## What must be verified in Stripe test mode before enabling, and what each run should show |
| + |
| +1. **Subscription Checkout with `currency` and `adaptive_pricing[enabled]=false`, at the pinned API version (2025-02-24.acacia):** use billing addresses in IE, GB, US and one Adaptive-eligible country (for example JP or CH). The response should show `currency` equal to the price's, `adaptive_pricing.enabled=false` and `currency_conversion` null. The hosted page should show the fixed amount. |
| +2. **A `currency` that differs from the price's currency:** Stripe should refuse it with a 400 and a stable error code. |
| +3. **The completed subscription:** `currency` and `items[0].price.currency` equal the chosen one, and the subscription metadata carries `price_book` and `currency`. |
| +4. **Customer reuse:** |
| + - With an active GBP subscription, a USD checkout should be refused, and the run shows at which call. |
| + - After a GBP subscription is cancelled, a USD checkout on the same customer: allowed or refused? |
| + - Then a GBP draft invoice and invoice item for the old month on that customer: allowed or refused? This decides P2-2 and P3-1. |
| +5. **Overage invoice in gbp and usd** (draft linked to the subscription, items with tax codes, `automatic_tax`, `pending_invoice_items_behavior=exclude`) for UK and US customers with only the IE registrations. Expect `automatic_tax.status=complete`, every line in the currency, and the subtotal equal to the calculation. |
| +6. **Quantity increase with proration on a GBP subscription:** the proration items and invoice should be in GBP, and the update response's `price.currency` should be GBP. |
| +7. **Promotion codes on a non-EUR session:** an EUR `amount_off` code should be refused, and the 50% percent code accepted. |
| +8. **One old-style session with `adaptive_pricing[enabled]=true` for a US buyer:** shows the resulting subscription's currency, which settles whether P3-7 can affect existing test rows. |
| +9. **`GET /v1/prices/{id}?expand[]=currency_options` on a single-currency price:** `currency_options` should be absent or empty, so the existing check does not refuse a valid price. |
| + |
| +## Verdict |
| + |
| +Accept with fixes: P2-1, P2-2 and P2-3 must be fixed (with tests) before billing is enabled anywhere. The P3s and the corrected claims can follow. |
diff --git a/internal/billingstripe/client.go b/internal/billingstripe/client.go| index c0c9824ec..fd21b642d 100644 |
| --- a/internal/billingstripe/client.go |
| +++ b/internal/billingstripe/client.go |
| @@ -72,6 +72,10 @@ 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 string |
| } |
| // RefusedParam reports whether Stripe refused the request outright as |
| @@ -552,10 +556,20 @@ type CreateCheckoutParams struct { |
| HouseCoupon string |
| // Currency is the currency the session must be in: the price's own. |
| - // Required. It is sent to Stripe, which refuses a session whose price |
| - // is in another currency, and the session that comes back is checked |
| - // against it. |
| + // It is sent to Stripe, which refuses a session whose price is in |
| + // another currency, and the session that comes back is checked |
| + // against it. Required, unless LegacyAdaptive. |
| Currency string |
| + |
| + // LegacyAdaptive asks for the session exactly as it was asked for |
| + // before prices were fixed per currency: no currency parameter and |
| + // Adaptive Pricing enabled, EUR prices presented in the buyer's own |
| + // currency. It is for a purchase that is not priced from a price |
| + // book, and exists so that such a request, replayed under its |
| + // original idempotency key, is still the same request. It must be |
| + // set on purpose: a missing Currency without it is refused, so that |
| + // forgetting the currency never switches Adaptive Pricing on. |
| + LegacyAdaptive bool |
| } |
| // CodeCurrencyMismatch: Stripe answered with a session that is not in the |
| @@ -589,14 +603,22 @@ func (c *Client) CreateCheckout(ctx context.Context, p CreateCheckoutParams, ide |
| if !validID(p.CustomerID) { |
| return nil, &Error{Code: CodeInvalidID} |
| } |
| - if !validCurrency(p.Currency) || (p.Metadata.Currency != "" && p.Metadata.Currency != p.Currency) { |
| + if p.LegacyAdaptive { |
| + // Not a purchase from a price book, so it names neither a currency |
| + // nor a book. |
| + if p.Currency != "" || p.Metadata.PriceBook != "" || p.Metadata.Currency != "" { |
| + return nil, &Error{Code: CodeInvalidParams} |
| + } |
| + } else if !validCurrency(p.Currency) || (p.Metadata.Currency != "" && p.Metadata.Currency != p.Currency) { |
| return nil, &Error{Code: CodeInvalidParams} |
| } |
| form := url.Values{} |
| form.Set("mode", "subscription") |
| - form.Set("currency", p.Currency) |
| - form.Set("adaptive_pricing[enabled]", "false") |
| + if !p.LegacyAdaptive { |
| + form.Set("currency", p.Currency) |
| + form.Set("adaptive_pricing[enabled]", "false") |
| + } |
| form.Set("line_items[0][price]", p.PriceID) |
| form.Set("line_items[0][quantity]", strconv.FormatInt(p.Quantity, 10)) |
| form.Set("success_url", p.SuccessURL) |
| @@ -613,6 +635,9 @@ func (c *Client) CreateCheckout(ctx context.Context, p CreateCheckoutParams, ide |
| } else { |
| form.Set("allow_promotion_codes", "true") |
| } |
| + if p.LegacyAdaptive { |
| + form.Set("adaptive_pricing[enabled]", "true") |
| + } |
| p.Metadata.set(form, "metadata") |
| p.Metadata.set(form, "subscription_data[metadata]") |
| @@ -623,8 +648,8 @@ func (c *Client) CreateCheckout(ctx context.Context, p CreateCheckoutParams, ide |
| if err := validateRedirectURL(sess.URL, "checkout.stripe.com"); err != nil { |
| return nil, err |
| } |
| - if !sess.FixedCurrency(p.Currency) { |
| - return nil, &Error{Code: CodeCurrencyMismatch} |
| + if !p.LegacyAdaptive && !sess.FixedCurrency(p.Currency) { |
| + return nil, &Error{Code: CodeCurrencyMismatch, SessionID: sess.ID} |
| } |
| return &sess, nil |
| } |
diff --git a/internal/billingstripe/client_test.go b/internal/billingstripe/client_test.go| index 52d76c681..5f7530273 100644 |
| --- a/internal/billingstripe/client_test.go |
| +++ b/internal/billingstripe/client_test.go |
| @@ -219,13 +219,63 @@ func TestCreateCheckout_FixedCurrency(t *testing.T) { |
| } |
| return |
| } |
| - if e, isErr := err.(*Error); !isErr || e.Code != tc.wantCode || sess != nil { |
| - t.Fatalf("CreateCheckout: %+v %v, want %s and no session", sess, err, tc.wantCode) |
| + // The refusal names the session the provider did create, so |
| + // the caller can expire it. |
| + if e, isErr := err.(*Error); !isErr || e.Code != tc.wantCode || sess != nil || e.SessionID != "cs_1" { |
| + t.Fatalf("CreateCheckout: %+v %v, want %s naming cs_1 and no session", sess, err, tc.wantCode) |
| } |
| }) |
| } |
| } |
| +// A purchase that is not priced from a price book is asked for as it was |
| +// before prices were fixed per currency, byte for byte: no currency, and |
| +// Adaptive Pricing on. That has to be asked for by name. It cannot be |
| +// combined with a currency or a book, and a request with neither a |
| +// currency nor the flag is refused, so a forgotten currency can never |
| +// switch Adaptive Pricing on. |
| +func TestCreateCheckout_LegacyAdaptiveIsTheOldRequest(t *testing.T) { |
| + var gotForm map[string][]string |
| + calls := 0 |
| + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| + calls++ |
| + _ = r.ParseForm() |
| + gotForm = map[string][]string(r.PostForm) |
| + // The provider answers in whatever it converted to; the old |
| + // request never checked. |
| + _ = json.NewEncoder(w).Encode(map[string]any{"id": "cs_1", "url": "https://checkout.stripe.com/c/pay/cs_1", "livemode": false, "currency": "eur", "adaptive_pricing": map[string]any{"enabled": true}}) |
| + })) |
| + defer srv.Close() |
| + c := testClient(t, srv.URL) |
| + base := CreateCheckoutParams{CustomerID: "cus_abc", PriceID: "price_123", Quantity: 1, SuccessURL: "https://example.com/s", CancelURL: "https://example.com/c", Metadata: validMetadata(), LegacyAdaptive: true} |
| + if _, err := c.CreateCheckout(context.Background(), base, "idem"); err != nil { |
| + t.Fatalf("CreateCheckout: %v", err) |
| + } |
| + if got := gotForm["adaptive_pricing[enabled]"]; len(got) != 1 || got[0] != "true" { |
| + t.Errorf("adaptive_pricing[enabled] = %v, want true as before", got) |
| + } |
| + for _, absent := range []string{"currency", "metadata[currency]", "metadata[price_book]", "subscription_data[metadata][currency]"} { |
| + if _, ok := gotForm[absent]; ok { |
| + t.Errorf("the old request gained %q", absent) |
| + } |
| + } |
| + calls = 0 |
| + for name, edit := range map[string]func(p *CreateCheckoutParams){ |
| + "with a currency": func(p *CreateCheckoutParams) { p.Currency = "eur" }, |
| + "with a book in the metadata": func(p *CreateCheckoutParams) { p.Metadata.PriceBook, p.Metadata.Currency = "b1", "eur" }, |
| + "neither the flag nor a currency": func(p *CreateCheckoutParams) { p.LegacyAdaptive = false }, |
| + } { |
| + p := base |
| + edit(&p) |
| + if _, err := c.CreateCheckout(context.Background(), p, "idem"); err == nil { |
| + t.Errorf("%s: accepted", name) |
| + } |
| + } |
| + if calls != 0 { |
| + t.Fatalf("%d requests were sent", calls) |
| + } |
| +} |
| + |
| // A currency that is not a lower-case three-letter code, or metadata that |
| // disagrees with it or names a book and no currency, never reaches Stripe. |
| func TestCreateCheckout_RefusesAMalformedCurrency(t *testing.T) { |
diff --git a/internal/console/billingcurrency_test.go b/internal/console/billingcurrency_test.go| index 23db4538d..bd80699ff 100644 |
| --- a/internal/console/billingcurrency_test.go |
| +++ b/internal/console/billingcurrency_test.go |
| @@ -2,6 +2,8 @@ package console |
| import ( |
| "context" |
| + "crypto/sha256" |
| + "encoding/hex" |
| "encoding/json" |
| "errors" |
| "net/http" |
| @@ -11,7 +13,6 @@ import ( |
| "testing" |
| "time" |
| - "github.com/carlosframework/platform/internal/billingstripe" |
| "github.com/carlosframework/platform/internal/plans" |
| "github.com/carlosframework/platform/internal/usage" |
| ) |
| @@ -149,6 +150,20 @@ func TestCheckoutRefusesACurrencyThatIsNotSold(t *testing.T) { |
| if intent, err := fx.st.billingCheckoutIntent(acct.ID); err != nil || intent.Currency != "" || intent.PriceBook != "" { |
| t.Fatalf("a purchase with no book is stored with no currency: %+v %v", intent, err) |
| } |
| + // And it is asked of the provider exactly as it was before books: no |
| + // currency parameter, Adaptive Pricing on, no book in the metadata. |
| + form := f.forms[len(f.forms)-1] |
| + if form.Get("adaptive_pricing[enabled]") != "true" || form.Has("currency") || form.Has("metadata[currency]") || form.Has("metadata[price_book]") { |
| + t.Fatalf("a purchase with no book was asked for as %v", form) |
| + } |
| + // A currency that is not sold is what the buyer is told about, whatever |
| + // else is wrong with the request: an unknown country, an unknown plan. |
| + fx, f, acct = newCurrencyFixture(t) |
| + for _, tc := range [][2]string{{"ZZ", plans.PlanPersonal}, {"IE", "no-such-plan"}, {"US", plans.PlanPersonal}} { |
| + if _, err := fx.srv.startBillingCheckout(context.Background(), acct, tc[1], tc[0], "", "jpy", f.now); !errors.Is(err, errBillingCurrency) { |
| + t.Errorf("country %s, plan %s, currency jpy: %v, want errBillingCurrency", tc[0], tc[1], err) |
| + } |
| + } |
| } |
| // Through the door: the form's currency field, posted by an owner who has |
| @@ -234,19 +249,102 @@ func TestCheckoutRefusesAProviderPriceThatIsNotTheBooks(t *testing.T) { |
| } |
| } |
| -// A session that comes back in another currency than it was asked for is |
| -// not kept: the intent has no session, and nothing is offered to the buyer. |
| -func TestCheckoutRefusesASessionInAnotherCurrency(t *testing.T) { |
| - fx, f, acct := newCurrencyFixture(t) |
| - f.customerCountry, f.sessionCurrency = "GB", "eur" |
| - dest, err := fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now) |
| - var stripeErr *billingstripe.Error |
| - if dest != "" || !errors.As(err, &stripeErr) || stripeErr.Code != billingstripe.CodeCurrencyMismatch { |
| - t.Fatalf("dest %q err %v, want a currency mismatch and no destination", dest, err) |
| - } |
| - if intent, err := fx.st.billingCheckoutIntent(acct.ID); err != nil || intent.SessionID != "" || intent.URL != "" { |
| - t.Fatalf("the session was kept: %+v %v", intent, err) |
| +// A session the provider creates in another currency than it was asked for |
| +// is never offered to the buyer: not on the click that created it, and not |
| +// on a later one that finds it again. It is expired at the provider and |
| +// its intent retired, so the next attempt starts clean. |
| +func TestCheckoutNeverOffersASessionInAnotherCurrency(t *testing.T) { |
| + click := func(fx *consoleFixture, f *fakeBillingStripe, acct *Account, after time.Duration) (string, error) { |
| + return fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now.Add(after)) |
| + } |
| + noIntent := func(t *testing.T, fx *consoleFixture, acct *Account) { |
| + t.Helper() |
| + if intent, err := fx.st.billingCheckoutIntent(acct.ID); err == nil { |
| + t.Fatalf("the intent was not retired: %+v", intent) |
| + } |
| } |
| + t.Run("refused, expired and retired on the click that created it", func(t *testing.T) { |
| + fx, f, acct := newCurrencyFixture(t) |
| + f.customerCountry, f.sessionCurrency = "GB", "eur" |
| + if dest, err := click(fx, f, acct, 0); dest != "" || !errors.Is(err, errBillingIdentity) { |
| + t.Fatalf("dest %q err %v, want no destination and an identity error", dest, err) |
| + } |
| + if f.expireCalls != 1 { |
| + t.Fatalf("the session was expired %d time(s), want once", f.expireCalls) |
| + } |
| + noIntent(t, fx, acct) |
| + // The buyer tries again while the provider still answers in euros: |
| + // a new intent, refused the same way, and still no destination. |
| + f.mu.Lock() |
| + f.sessionStatus = "open" |
| + f.mu.Unlock() |
| + if dest, err := click(fx, f, acct, time.Minute); dest != "" || err == nil { |
| + t.Fatalf("second click: dest %q err %v", dest, err) |
| + } |
| + if f.checkoutCalls != 2 || f.expireCalls != 2 { |
| + t.Fatalf("second click: %d sessions asked for, %d expired; want 2 and 2", f.checkoutCalls, f.expireCalls) |
| + } |
| + noIntent(t, fx, acct) |
| + // The provider answers as asked: the purchase goes ahead. |
| + f.mu.Lock() |
| + f.sessionStatus, f.sessionCurrency = "open", "" |
| + f.mu.Unlock() |
| + dest, err := click(fx, f, acct, 2*time.Minute) |
| + intent, ierr := fx.st.billingCheckoutIntent(acct.ID) |
| + if err != nil || dest == "" || ierr != nil || intent.Currency != "gbp" || intent.SessionID == "" { |
| + t.Fatalf("third click: dest %q err %v intent %+v %v", dest, err, intent, ierr) |
| + } |
| + }) |
| + t.Run("the expiry fails, and the second click finds the session again", func(t *testing.T) { |
| + fx, f, acct := newCurrencyFixture(t) |
| + f.customerCountry, f.sessionCurrency, f.failExpire = "GB", "eur", 1 |
| + if dest, err := click(fx, f, acct, 0); dest != "" || err == nil { |
| + t.Fatalf("first click: dest %q err %v", dest, err) |
| + } |
| + // The session is open at the provider, in euros, and the intent is |
| + // still there with no session bound to it. |
| + intent, err := fx.st.billingCheckoutIntent(acct.ID) |
| + if err != nil || intent.SessionID != "" || !intent.CustomerReady || f.sessionStatus != "open" { |
| + t.Fatalf("after a failed expiry: intent %+v %v, session %s", intent, err, f.sessionStatus) |
| + } |
| + // The second click replays the create under the same key, is |
| + // answered with the same session, and must not hand it over. |
| + if dest, err := click(fx, f, acct, time.Minute); dest != "" || !errors.Is(err, errBillingIdentity) { |
| + t.Fatalf("second click: dest %q err %v, want no destination", dest, err) |
| + } |
| + if f.sessionStatus != "expired" { |
| + t.Fatalf("the session is %s at the provider", f.sessionStatus) |
| + } |
| + noIntent(t, fx, acct) |
| + }) |
| + t.Run("found by the background pass", func(t *testing.T) { |
| + fx, f, acct := newCurrencyFixture(t) |
| + f.customerCountry, f.sessionCurrency, f.failExpire = "GB", "eur", 1 |
| + if dest, err := click(fx, f, acct, 0); dest != "" || err == nil { |
| + t.Fatalf("first click: dest %q err %v", dest, err) |
| + } |
| + _ = fx.srv.billingPass(context.Background(), f.now.Add(time.Minute)) |
| + if intent, err := fx.st.billingCheckoutIntent(acct.ID); err == nil && intent.SessionID != "" { |
| + t.Fatalf("the pass bound the session to the intent: %+v", intent) |
| + } |
| + if f.sessionStatus != "expired" { |
| + t.Fatalf("the pass left the session %s at the provider", f.sessionStatus) |
| + } |
| + noIntent(t, fx, acct) |
| + }) |
| + t.Run("a bound session that later reads back in another currency is not resumed", func(t *testing.T) { |
| + fx, f, acct := newCurrencyFixture(t) |
| + f.customerCountry = "GB" |
| + if dest, err := click(fx, f, acct, 0); err != nil || dest == "" { |
| + t.Fatalf("first click: dest %q err %v", dest, err) |
| + } |
| + f.mu.Lock() |
| + f.sessionCurrency = "eur" |
| + f.mu.Unlock() |
| + if dest, err := click(fx, f, acct, time.Minute); dest != "" || !errors.Is(err, errBillingIdentity) { |
| + t.Fatalf("second click: dest %q err %v, want no destination", dest, err) |
| + } |
| + }) |
| } |
| // Changing currency while a checkout is open is a different purchase: the |
| @@ -530,7 +628,7 @@ func TestCurrencyChangesOnlyWithANewSubscriptionAndNeverInsideAMonth(t *testing. |
| for i, row := range rows { |
| history[i] = row.Assignment |
| } |
| - if _, err := plans.AllowanceForMonth("2026-09", history); !errors.Is(err, plans.ErrAssignmentHistory) { |
| + if _, err := plans.AllowanceForMonth("2026-09", history); !errors.Is(err, plans.ErrPriceTerms) { |
| t.Fatalf("a month paid for in pounds and then in dollars: %v, want it refused", err) |
| } |
| statement := usage.Statement{V: 1, Account: fx.sqid, Month: "2026-09", Meters: map[string]float64{usage.MeterInstanceHours: 1000}, DaysInMonth: 30, Closed: true} |
| @@ -697,11 +795,7 @@ func TestInvoiceEngineInvoicesInTheSubscriptionsCurrency(t *testing.T) { |
| inPounds := func(t *testing.T) *invoiceFixture { |
| ix := newInvoiceFixture(t) |
| // The fixture's account bought in pounds from the current book. |
| - for _, table := range []string{"billing_accounts", "billing_assignments"} { |
| - if _, err := ix.st.db.Exec(`UPDATE `+table+` SET currency='gbp', price_book=? WHERE account_id=?`, plans.CurrentPriceBook, ix.acct.ID); err != nil { |
| - t.Fatal(err) |
| - } |
| - } |
| + inCurrency(t, ix, "gbp") |
| ix.fake.subscriptionCurrency = "gbp" |
| return ix |
| } |
| @@ -786,7 +880,7 @@ func TestInvoiceEngineInvoicesInTheSubscriptionsCurrency(t *testing.T) { |
| // Only the recorded assignment changes. The provider still reports |
| // the subscription in pounds, so nothing but the comparison of the |
| // frozen month with the record can notice. |
| - if _, err := ix.st.db.Exec(`UPDATE billing_assignments SET currency='usd' WHERE account_id=?`, ix.acct.ID); err != nil { |
| + if _, err := ix.st.db.Exec(`UPDATE billing_book_assignments SET currency='usd' WHERE account_id=?`, ix.acct.ID); err != nil { |
| t.Fatal(err) |
| } |
| ix.now = ix.now.Add(time.Minute) |
| @@ -800,6 +894,23 @@ func TestInvoiceEngineInvoicesInTheSubscriptionsCurrency(t *testing.T) { |
| }) |
| } |
| +// inCurrency rewrites an invoice fixture's recorded purchase as one bought |
| +// from the current book in currency: the account row, and its assignment |
| +// history moved to the table book-priced assignments are kept in. |
| +func inCurrency(t *testing.T, ix *invoiceFixture, currency string) { |
| + t.Helper() |
| + for _, stmt := range []string{ |
| + `UPDATE billing_accounts SET currency=?1, price_book=?2 WHERE account_id=?3`, |
| + `INSERT INTO billing_book_assignments(account_id,revision,customer,subscription,effective_at,plan,terms,band,quantity,house,status,period_start,period_end,live_mode,currency,price_book) |
| + SELECT account_id,revision,customer,subscription,effective_at,plan,terms,band,quantity,house,status,period_start,period_end,live_mode,?1,?2 FROM billing_assignments WHERE account_id=?3`, |
| + `DELETE FROM billing_assignments WHERE account_id=?3 AND ?1<>'' AND ?2<>''`, |
| + } { |
| + if _, err := ix.st.db.Exec(stmt, currency, plans.CurrentPriceBook, ix.acct.ID); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| +} |
| + |
| // A request's currency is part of its hash. A legacy month's requests |
| // hash exactly as they did before a currency was a parameter, so an |
| // operation journaled by an earlier build is recognised; they are still |
| @@ -939,13 +1050,16 @@ func TestCurrencyColumnsOnAnExistingDatabase(t *testing.T) { |
| if err := st.applyBillingState(billingState{AccountID: acct.ID, Customer: "cus_one", Subscription: "sub_a", Status: "active", Plan: plans.PlanPersonal, Terms: plans.CurrentTermsVersion, Band: plans.BandA, Quantity: 1, PeriodStart: at.Unix(), PeriodEnd: at.AddDate(1, 0, 0).Unix()}, 0, "e1", at); err != nil { |
| t.Fatal(err) |
| } |
| - for _, table := range []string{"billing_accounts", "billing_checkout_intents", "billing_assignments"} { |
| + for _, table := range []string{"billing_accounts", "billing_checkout_intents"} { |
| for _, column := range []string{"currency", "price_book"} { |
| if _, err := st.db.Exec(`ALTER TABLE ` + table + ` DROP COLUMN ` + column); err != nil { |
| t.Fatalf("fixture: drop %s.%s: %v", table, column, err) |
| } |
| } |
| } |
| + if _, err := st.db.Exec(`DROP TABLE billing_book_assignments`); err != nil { |
| + t.Fatal(err) |
| + } |
| if err := st.migrateBilling(); err != nil { |
| t.Fatal(err) |
| } |
| @@ -975,3 +1089,355 @@ func TestCurrencyColumnsOnAnExistingDatabase(t *testing.T) { |
| t.Fatalf("stored as %+v", after) |
| } |
| } |
| + |
| +// A checkout whose create response was lost is found again by listing the |
| +// customer's sessions. The one found is bound to the intent only if it is |
| +// the purchase the intent describes, in the intent's currency: its own |
| +// response was never checked. One that is not is expired and the intent |
| +// retired, unless it has already been paid, which the provider will not |
| +// expire: then nothing is bound or recorded, and a person has to look. |
| +func TestRecoveredSessionMustBeInTheIntentsCurrency(t *testing.T) { |
| + for name, tc := range map[string]struct { |
| + status string |
| + spoil func(f *fakeBillingStripe) |
| + want string // recovered, retired, or untouched |
| + }{ |
| + "complete, in the intent's currency": {"complete", func(*fakeBillingStripe) {}, "recovered"}, |
| + "open, in another currency": {"open", func(f *fakeBillingStripe) { f.sessionCurrency = "eur" }, "retired"}, |
| + "open, its metadata says another currency": {"open", func(f *fakeBillingStripe) { f.meta["currency"] = "usd" }, "retired"}, |
| + "open, its metadata names no book": {"open", func(f *fakeBillingStripe) { delete(f.meta, "price_book"); delete(f.meta, "currency") }, "retired"}, |
| + "complete and paid, in another currency": {"complete", func(f *fakeBillingStripe) { f.sessionCurrency = "eur" }, "untouched"}, |
| + "complete and paid, metadata says another": {"complete", func(f *fakeBillingStripe) { f.meta["currency"] = "usd" }, "untouched"}, |
| + } { |
| + t.Run(name, func(t *testing.T) { |
| + fx, f, acct := newCurrencyFixture(t) |
| + f.customerCountry, f.failCreate = "GB", true |
| + if _, err := fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now); err == nil { |
| + t.Fatal("fixture: the create response was not lost") |
| + } |
| + 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.subscriptionPrice = tc.status, intent.Price |
| + tc.spoil(f) |
| + f.mu.Unlock() |
| + _ = fx.srv.billingPass(context.Background(), f.now.Add(time.Minute)) |
| + state, _ := fx.st.billingAccount(acct.ID) |
| + after, ierr := fx.st.billingCheckoutIntent(acct.ID) |
| + switch tc.want { |
| + case "recovered": |
| + if state.Subscription != "sub_paid" || state.Currency != "gbp" || f.checkoutCalls != 1 { |
| + t.Fatalf("not recovered: %+v, %d checkout calls", state, f.checkoutCalls) |
| + } |
| + case "retired": |
| + if state.Subscription != "" || ierr == nil || f.sessionStatus != "expired" { |
| + t.Fatalf("state %+v, intent %+v (%v), session %s; want nothing recorded, the intent retired and the session expired", state, after, ierr, f.sessionStatus) |
| + } |
| + case "untouched": |
| + if state.Subscription != "" || ierr != nil || after.SessionID != "" || f.sessionStatus != "complete" { |
| + t.Fatalf("state %+v, intent %+v (%v), session %s; want nothing recorded and the intent left unbound", state, after, ierr, f.sessionStatus) |
| + } |
| + } |
| + }) |
| + } |
| +} |
| + |
| +// legacyCalculationGolden is a legacy month's calculation exactly as the |
| +// build before price books wrote it: these bytes and this digest were |
| +// produced by that build's own code (commit 12e70d95) from the inputs in |
| +// the test below. They are a fixture, not a reconstruction: none of this |
| +// build's types went into them. |
| +const legacyCalculationGolden = `{"statement_object":{"key":""},"v":1,"arithmetic":"base_units_floor_v1","currency":"eur","statement":{"v":1,"account":"bab","month":"2026-09","meters":{"instance_hours":1000,"storage_gb_month":40},"covered_days":0,"days_in_month":30,"updated_at":"0001-01-01T00:00:00Z","closed":true},"statement_sha256":"83d2a01200b4377ced8c93f2fdd398de7ae238c02074b2ed2642cd466a35b260","allowance":{"month":"2026-09","terms_version":"2026-09-07-v1","house":false,"resources":{"awake_seconds":306000,"storage_byte_months":2000000000,"transfer_bytes":2000000000,"writes":0},"assignments":[{"effective_at":"2026-08-10T00:00:00Z","revision":1,"terms_version":"2026-09-07-v1","plan":"personal","band":"A","quantity":1,"house":false,"status":"active","paid_through":"2027-08-10T00:00:00Z"}]},"usage":{"awake_seconds":3600000,"storage_byte_months":40000000000,"transfer_bytes":0,"writes":0},"overage":{"AwakeMicroEUR":9150000,"StorageMicroEUR":4028000,"TransferMicroEUR":0,"WritesMicroEUR":0,"TotalMicroEUR":13178000},"invoice":{"SubscriptionMicroEUR":0,"OverageMicroEUR":13178000,"TotalMicroEUR":13178000,"TotalCents":1318,"Waived":false},"lines":[{"Meter":"awake_seconds","AmountCents":915},{"Meter":"storage_byte_months","AmountCents":403}],"tax_code":"txcd_test"}` |
| + |
| +const legacyCalculationGoldenDigest = "dfac0bfde9cb1226054ba957aa9ae6fab043d80a87aafa16bc02b21e7515f171" |
| + |
| +// A month from before price books is frozen, by this build, to the very |
| +// bytes the previous build would have frozen, and so to the same digest: |
| +// the digest names the invoice's idempotency keys and is written into the |
| +// provider's metadata. And a calculation the previous build froze reads |
| +// back whole. |
| +func TestLegacyCalculationBytesAreUnchanged(t *testing.T) { |
| + sum := sha256.Sum256([]byte(legacyCalculationGolden)) |
| + if hex.EncodeToString(sum[:]) != legacyCalculationGoldenDigest { |
| + t.Fatal("fixture: the golden bytes and digest disagree") |
| + } |
| + statement := usage.Statement{V: 1, Account: "bab", Month: "2026-09", Meters: map[string]float64{usage.MeterInstanceHours: 1000, usage.MeterStorageGBMonth: 40}, DaysInMonth: 30, Closed: true} |
| + calc, err := priceBillingCalculation(statement, monthHistory(plans.LegacyPriceTerms(plans.CurrentTermsVersion), false)) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + calc.TaxCode = "txcd_test" |
| + got, err := json.Marshal(calc) |
| + if err != nil || string(got) != legacyCalculationGolden { |
| + t.Fatalf("a legacy month marshals as\n%s\nand the previous build wrote\n%s", got, legacyCalculationGolden) |
| + } |
| + var stored billingCalculation |
| + if err := json.Unmarshal([]byte(legacyCalculationGolden), &stored); err != nil || !stored.wellFormed() || stored.totalMinor() != 1318 || stored.waived() || stored.Currency != "eur" || !stored.legacy() { |
| + t.Fatalf("a calculation the previous build froze: %+v %v", stored, err) |
| + } |
| + again, _ := json.Marshal(stored) |
| + if string(again) != legacyCalculationGolden { |
| + t.Fatalf("read and written again it is\n%s", again) |
| + } |
| + // With an explicit "eur" on the assignment, as a legacy row may be |
| + // written, the bytes are still these: one spelling is stored. |
| + history := monthHistory(plans.LegacyPriceTerms(plans.CurrentTermsVersion), false) |
| + history[0].Currency = plans.EUR |
| + spelled, err := priceBillingCalculation(statement, history) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + spelled.TaxCode = "txcd_test" |
| + if raw, _ := json.Marshal(spelled); string(raw) == legacyCalculationGolden { |
| + t.Log("an assignment spelled with eur marshals as the golden") |
| + } else if !strings.Contains(string(raw), `"currency":"eur"`) { |
| + t.Fatalf("unexpected bytes for a legacy assignment spelled with eur: %s", raw) |
| + } |
| +} |
| + |
| +// 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 |
| +// 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) { |
| + fx, f, acct, _ := buyIn(t, "GB", "") |
| + ctx := context.Background() |
| + requests := func() int { f.mu.Lock(); defer f.mu.Unlock(); return f.checkoutCalls } |
| + before := requests() |
| + for _, currency := range []string{"", "gbp", "usd", "eur"} { |
| + if dest, err := fx.srv.startBillingCheckout(ctx, acct, plans.PlanPersonal, "GB", "", currency, f.now.Add(time.Minute)); dest != "" || !errors.Is(err, errBillingConflict) { |
| + t.Errorf("with a live subscription, currency %q: dest %q err %v, want errBillingConflict", currency, dest, err) |
| + } |
| + } |
| + if requests() != before { |
| + t.Fatalf("a checkout session was asked for beside a live subscription") |
| + } |
| + f.mu.Lock() |
| + f.subscriptionStatus = "canceled" |
| + f.mu.Unlock() |
| + if err := fx.srv.reconcileBillingSubscription(ctx, "sub_paid", "", f.now.Add(2*time.Minute)); err != nil { |
| + t.Fatal(err) |
| + } |
| + if state, _ := fx.st.billingAccount(acct.ID); state.Status != "canceled" || state.Currency != "gbp" { |
| + t.Fatalf("after cancelling: %+v", state) |
| + } |
| + for _, currency := range []string{"usd", "eur"} { |
| + if dest, err := fx.srv.startBillingCheckout(ctx, acct, plans.PlanPersonal, "GB", "", currency, f.now.Add(3*time.Minute)); dest != "" || !errors.Is(err, errBillingConflict) { |
| + t.Errorf("after the subscription ended, in %s: dest %q err %v, want errBillingConflict", currency, dest, err) |
| + } |
| + } |
| + // A buyer now billed to another country, whose default is another |
| + // currency, is refused the default too, not quietly moved to it. |
| + f.mu.Lock() |
| + f.customerCountry = "US" |
| + f.mu.Unlock() |
| + if dest, err := fx.srv.startBillingCheckout(ctx, acct, plans.PlanPersonal, "US", "94107", "", f.now.Add(3*time.Minute)); dest != "" || !errors.Is(err, errBillingConflict) { |
| + t.Errorf("after the subscription ended, a country whose default is dollars: dest %q err %v", dest, err) |
| + } |
| + if requests() != before { |
| + t.Fatalf("a checkout session was asked for in another currency") |
| + } |
| + if _, err := fx.st.billingCheckoutIntent(acct.ID); err == nil { |
| + t.Fatal("an intent was opened in another currency") |
| + } |
| + // In pounds again, it goes ahead (and from the United States, with the |
| + // currency chosen). |
| + f.mu.Lock() |
| + f.sessionStatus = "open" |
| + f.mu.Unlock() |
| + if dest, err := fx.srv.startBillingCheckout(ctx, acct, plans.PlanPersonal, "US", "94107", "gbp", f.now.Add(4*time.Minute)); err != nil || dest == "" { |
| + t.Fatalf("in pounds again: dest %q err %v", dest, err) |
| + } |
| +} |
| + |
| +// A month whose paid spans were bought in two currencies is not left to |
| +// retry quietly for ever: the engine files a review, freezes nothing and |
| +// asks nothing of the provider. |
| +func TestInvoiceEngineReviewsAMonthBoughtInTwoCurrencies(t *testing.T) { |
| + ix := newInvoiceFixture(t) |
| + inCurrency(t, ix, "gbp") |
| + ix.fake.subscriptionCurrency = "gbp" |
| + state, err := ix.st.billingAccount(ix.acct.ID) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + // The pound subscription ends on the 10th and a dollar one starts on |
| + // the 12th, both inside the fixture's month. |
| + ended := state |
| + ended.Status = "canceled" |
| + if err := ix.st.applyBillingState(ended, state.Revision, "evt_end", time.Date(2026, 9, 10, 0, 0, 0, 0, time.UTC)); err != nil { |
| + t.Fatal(err) |
| + } |
| + start := time.Date(2026, 9, 12, 0, 0, 0, 0, time.UTC) |
| + next := billingState{AccountID: ix.acct.ID, Customer: state.Customer, Subscription: "sub_usd", Status: "active", Plan: plans.PlanPersonal, Terms: plans.CurrentTermsVersion, PriceBook: plans.CurrentPriceBook, Currency: "usd", Band: plans.BandA, Quantity: 1, PeriodStart: start.Unix(), PeriodEnd: start.AddDate(1, 0, 0).Unix()} |
| + if err := ix.st.applyBillingState(next, state.Revision+1, "evt_usd", start); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := ix.run(t); !errors.Is(err, errBillingInvoiceHeld) { |
| + t.Fatalf("run: %v, want the month held for review", err) |
| + } |
| + found := false |
| + for _, reason := range ix.reviews(t) { |
| + if reason == "billing_invoice_price_terms:"+invoiceFixtureMonth { |
| + found = true |
| + } |
| + } |
| + if !found { |
| + t.Fatalf("no review was filed for a month bought in two currencies: %v", ix.reviews(t)) |
| + } |
| + if _, err := ix.st.billingMonth(ix.acct.ID, invoiceFixtureMonth); err == nil { |
| + t.Fatal("the month was frozen") |
| + } |
| + if _, mutations, _ := ix.fake.counts(); len(mutations) != 0 { |
| + t.Fatalf("something was written: %v", mutations) |
| + } |
| +} |
| + |
| +// A build from before price books reads assignments from one table and |
| +// knows nothing of currencies. A purchase priced from a book is recorded |
| +// in another, so that build, run against this database, finds no history |
| +// for it and has nothing to price in EUR. A purchase with no book is where |
| +// it always was. |
| +func TestBookPricedHistoryIsInvisibleToAnOlderBuild(t *testing.T) { |
| + // The older build's own history query, word for word. |
| + const olderBuildHistory = `SELECT revision,effective_at,terms,plan,band,quantity,house,status,period_end,customer,subscription,live_mode FROM billing_assignments WHERE account_id=? ORDER BY revision` |
| + const olderBuildAccounts = `SELECT count(*) FROM accounts a JOIN billing_assignments b ON b.account_id=a.id WHERE a.deleted_at=0 AND a.id=? AND b.plan<>?` |
| + count := func(t *testing.T, st *Store, query string, args ...any) int { |
| + t.Helper() |
| + rows, err := st.db.Query(query, args...) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer rows.Close() |
| + n := 0 |
| + for rows.Next() { |
| + n++ |
| + } |
| + return n |
| + } |
| + fx, _, acct, _ := buyIn(t, "GB", "") |
| + if n := count(t, fx.st, olderBuildHistory, acct.ID); n != 0 { |
| + t.Fatalf("an older build would read %d assignment(s) of a purchase in pounds", n) |
| + } |
| + var listed int |
| + if err := fx.st.db.QueryRow(olderBuildAccounts, acct.ID, plans.PlanFree).Scan(&listed); err != nil || listed != 0 { |
| + t.Fatalf("an older build would list the account for invoicing (%d, %v)", listed, err) |
| + } |
| + rows, err := fx.st.billingInvoiceHistory(acct.ID) |
| + if err != nil || len(rows) != 1 || rows[0].Currency != plans.GBP { |
| + t.Fatalf("this build's history: %+v %v", rows, err) |
| + } |
| + if accounts, err := fx.st.billingInvoiceAccounts(0, 100); err != nil || len(accounts) != 1 { |
| + t.Fatalf("this build's invoice accounts: %+v %v", accounts, err) |
| + } |
| + // A purchase with no book, by a console with none configured. |
| + old, f, oldAcct := newBillingFixture(t) |
| + if _, err := old.srv.startBillingCheckout(context.Background(), oldAcct, plans.PlanPersonal, "IE", "", "", f.now); err != nil { |
| + t.Fatal(err) |
| + } |
| + f.mu.Lock() |
| + f.sessionStatus = "complete" |
| + f.mu.Unlock() |
| + if err := old.srv.reconcileBillingSubscription(context.Background(), "sub_paid", "", f.now); err != nil { |
| + t.Fatal(err) |
| + } |
| + if n := count(t, old.st, olderBuildHistory, oldAcct.ID); n != 1 { |
| + t.Fatalf("a purchase with no book has %d assignment(s) where an older build reads", n) |
| + } |
| + var inBook int |
| + if err := old.st.db.QueryRow(`SELECT count(*) FROM billing_book_assignments`).Scan(&inBook); err != nil || inBook != 0 { |
| + t.Fatalf("a purchase with no book was recorded among the book-priced: %d %v", inBook, err) |
| + } |
| + // The table itself refuses a row with no currency or no book. |
| + if _, err := old.st.db.Exec(`INSERT INTO billing_book_assignments(account_id,revision,customer,subscription,effective_at,plan,terms,band,quantity,house,status,period_start,period_end,currency,price_book) VALUES(1,99,'c','s',1,'personal','t','A',1,0,'active',1,2,'','b')`); err == nil { |
| + t.Fatal("a book-priced assignment with no currency was stored") |
| + } |
| +} |
| + |
| +// The subscription and the item it is a subscription to each report a |
| +// currency, and each is checked on its own: at reconcile, in the quantity |
| +// worker, and before an overage invoice is written. |
| +func TestSubscriptionAndItemCurrencyAreEachChecked(t *testing.T) { |
| + t.Run("reconcile and the quantity worker", func(t *testing.T) { |
| + fx, f, acct, _ := buyIn(t, "GB", "") |
| + state, _ := fx.st.billingAccount(acct.ID) |
| + for name, set := range map[string]func(){ |
| + "the subscription alone is in euros": func() { f.subscriptionCurrency, f.itemCurrency = "eur", "gbp" }, |
| + "the item alone is in euros": func() { f.subscriptionCurrency, f.itemCurrency = "gbp", "eur" }, |
| + } { |
| + f.mu.Lock() |
| + set() |
| + f.subscriptionStatus = "past_due" |
| + f.mu.Unlock() |
| + if err := fx.srv.reconcileBillingSubscription(context.Background(), "sub_paid", "", f.now.Add(time.Minute)); !errors.Is(err, errBillingIdentity) { |
| + t.Errorf("reconcile, %s: %v", name, err) |
| + } |
| + if _, _, err := fx.srv.quantityProviderItem(context.Background(), acct, state); !errors.Is(err, errBillingIdentity) { |
| + t.Errorf("quantity worker, %s: %v", name, err) |
| + } |
| + if after, _ := fx.st.billingAccount(acct.ID); after != state { |
| + t.Errorf("%s: the account changed: %+v", name, after) |
| + } |
| + } |
| + }) |
| + for name, set := range map[string]func(ix *invoiceFixture){ |
| + "the item alone is in euros": func(ix *invoiceFixture) { ix.fake.itemCurrency = "eur" }, |
| + "the subscription has two items": func(ix *invoiceFixture) { ix.fake.itemCount = 2 }, |
| + "the subscription has no item at all": func(ix *invoiceFixture) { ix.fake.itemCount = -1 }, |
| + } { |
| + t.Run("invoice preflight, "+name, func(t *testing.T) { |
| + ix := newInvoiceFixture(t) |
| + inCurrency(t, ix, "gbp") |
| + ix.fake.subscriptionCurrency = "gbp" |
| + set(ix) |
| + if err := ix.run(t); !errors.Is(err, errBillingIdentity) { |
| + t.Fatalf("run: %v, want errBillingIdentity", err) |
| + } |
| + if _, mutations, _ := ix.fake.counts(); len(mutations) != 0 { |
| + t.Fatalf("something was written: %v", mutations) |
| + } |
| + }) |
| + } |
| +} |
| + |
| +// A draft or a line whose response was lost is found again by a read. What |
| +// is found was never checked when it was made, so it is checked then: one |
| +// in another currency is not acknowledged as this operation's. |
| +func TestInvoiceRecoveryChecksTheCurrencyOfWhatItFinds(t *testing.T) { |
| + for _, kind := range []string{"draft", "line"} { |
| + t.Run(kind, func(t *testing.T) { |
| + ix := newInvoiceFixture(t) |
| + inCurrency(t, ix, "gbp") |
| + ix.fake.subscriptionCurrency = "gbp" |
| + calc := ix.expectedCalc(t) |
| + lost := kind |
| + if kind == "line" { |
| + lost = "line-" + calc.Lines[0].Meter |
| + } |
| + ix.fake.loseResponse[lost] = true |
| + if err := ix.run(t); !errors.Is(err, errBillingPending) { |
| + t.Fatalf("lost response not pending: %v", err) |
| + } |
| + // Past the window the object is looked up, and it reads back in |
| + // euros. |
| + if kind == "draft" { |
| + ix.fake.reportInvoiceCurrency = "eur" |
| + } else { |
| + ix.fake.reportLineCurrency = "eur" |
| + } |
| + ix.now = ix.now.Add(25 * time.Hour) |
| + _ = ix.run(t) |
| + if rec := ix.month(t); rec.State == "complete" || rec.State == "open" || rec.State == "paying" { |
| + t.Fatalf("the month reached %q on a recovered %s in another currency", rec.State, kind) |
| + } |
| + if _, mutations, _ := ix.fake.counts(); mutations["finalize"] != 0 || mutations["pay"] != 0 { |
| + t.Fatalf("the invoice was finalized or paid: %v", mutations) |
| + } |
| + if op := ix.ops(t)[lost]; op.State == billingOpAcked && op.ErrorCode == "" { |
| + t.Fatalf("the recovered %s was acknowledged as sound: %+v", kind, op) |
| + } |
| + }) |
| + } |
| +} |
diff --git a/internal/console/billinginvoice.go b/internal/console/billinginvoice.go| index 4cfa80b81..3e9282876 100644 |
| --- a/internal/console/billinginvoice.go |
| +++ b/internal/console/billinginvoice.go |
| @@ -314,6 +314,12 @@ func (s *Server) invoiceBillingMonth(ctx context.Context, acct *Account, month s |
| if errors.Is(err, errBillingTaxCodeMixed) { |
| return r.review("billing_invoice_tax_code_mixed") |
| } |
| + // A month paid for in two currencies or from two price books, or from |
| + // one this build does not have, is priced in neither. It will not mend |
| + // by waiting, so it is not left to retry quietly: a person is told. |
| + if errors.Is(err, plans.ErrPriceTerms) { |
| + return r.review("billing_invoice_price_terms") |
| + } |
| if err != nil { |
| return err |
| } |
| @@ -559,9 +565,22 @@ func (r *billingInvoiceRun) preflight(ctx context.Context, id *billingInvoiceIde |
| // The usage is invoiced in the currency the subscription was bought |
| // in. A subscription the provider now reports in another currency is |
| // not the one this month was metered under. |
| - if !billingCurrencyAgrees(r.calc.Allowance.PriceTerms(), sub.Currency) { |
| + terms := r.calc.Allowance.PriceTerms() |
| + if !billingCurrencyAgrees(terms, sub.Currency) { |
| + return errBillingIdentity |
| + } |
| + // And so is what it is a subscription to. A purchase from a price book |
| + // has exactly one item, in the purchase's currency; anything else is |
| + // not something to invoice in that currency. A subscription from |
| + // before books is checked as far as it says. |
| + if !terms.Legacy() && (len(sub.Items) != 1 || sub.ItemsHasMore) { |
| return errBillingIdentity |
| } |
| + for _, item := range sub.Items { |
| + if !billingCurrencyAgrees(terms, item.PriceCurrency) { |
| + return errBillingIdentity |
| + } |
| + } |
| if !persisted { |
| id.LinkSubscription = sub.Status != "canceled" && sub.Status != "incomplete_expired" |
| } |
| @@ -914,7 +933,11 @@ func (r *billingInvoiceRun) stepCreating(ctx context.Context) error { |
| case 0: |
| return "", "", nil |
| case 1: |
| - return found[0].ID, found[0].Status, nil |
| + // A draft found by lookup was not checked when it was made: |
| + // its response was lost. It is this operation's only if it |
| + // is the invoice this month calls for, its currency |
| + // included. |
| + return found[0].ID, found[0].Status, r.verifyInvoice(&found[0], "draft") |
| } |
| return "", "", errBillingDuplicate |
| }, |
| @@ -998,7 +1021,7 @@ func (r *billingInvoiceRun) stepDraft(ctx context.Context) error { |
| case 0: |
| return "", "", nil |
| case 1: |
| - if matches[0].Amount != line.AmountCents { |
| + if matches[0].Amount != line.AmountCents || matches[0].Currency != r.calc.Currency { |
| return matches[0].InvoiceItem, "attached", errBillingInvoiceMismatch |
| } |
| return matches[0].InvoiceItem, "attached", nil |
diff --git a/internal/console/billinginvoice_test.go b/internal/console/billinginvoice_test.go| index 1f5f9fe35..2581f52db 100644 |
| --- a/internal/console/billinginvoice_test.go |
| +++ b/internal/console/billinginvoice_test.go |
| @@ -55,6 +55,11 @@ type fakeInvoiceStripe struct { |
| currencies map[string]string |
| reportInvoiceCurrency, reportItemCurrency, reportLineCurrency string |
| subscriptionCurrency string |
| + // itemCurrency is the currency of the subscription's item's price when |
| + // it differs from the subscription's; itemCount is how many items it |
| + // reports (0 means one, a negative number none). |
| + itemCurrency string |
| + itemCount int |
| } |
| type fakeKeyed struct { |
| @@ -81,6 +86,25 @@ func newFakeInvoiceStripe(sqid string, now func() time.Time) *fakeInvoiceStripe |
| invoices: map[string]*fakeInvoice{}, keyed: map[string]fakeKeyed{}, arrivals: map[string]int{}, mutations: map[string]int{}, loseResponse: map[string]bool{}, dropRequest: map[string]bool{}, refuse: map[string]int{}, currencies: map[string]string{}} |
| } |
| +// subscriptionItems is what the subscription reports as its items: none |
| +// when it reports no currency (a subscription from before currencies, as |
| +// the fixtures had it), otherwise one, on a price in the subscription's |
| +// currency unless itemCurrency says another, or as many as itemCount says. |
| +func (f *fakeInvoiceStripe) subscriptionItems() []any { |
| + if f.subscriptionCurrency == "" { |
| + return []any{} |
| + } |
| + n := 1 |
| + if f.itemCount != 0 { |
| + n = f.itemCount |
| + } |
| + items := []any{} |
| + for i := 0; i < n; i++ { |
| + items = append(items, map[string]any{"id": "si_hist_" + strconv.Itoa(i), "quantity": 1, "price": map[string]string{"id": "price_hist", "currency": either(f.itemCurrency, f.subscriptionCurrency)}}) |
| + } |
| + return items |
| +} |
| + |
| func either(override, own string) string { |
| if override != "" { |
| return override |
| @@ -246,7 +270,7 @@ func (f *fakeInvoiceStripe) serve(w http.ResponseWriter, r *http.Request) { |
| out = map[string]any{"id": id, "livemode": false, "address": map[string]string{"country": "IE"}, "shipping": nil, "metadata": map[string]string{"account_sqid": f.customerSqid}, "tax": map[string]any{"automatic_tax": f.customerTax, "location": map[string]string{"country": "IE"}}} |
| case strings.HasPrefix(path, "/v1/subscriptions/"): |
| id := strings.TrimPrefix(path, "/v1/subscriptions/") |
| - out = map[string]any{"id": id, "customer": f.subscriptionCustomer, "status": f.subscriptionStatus, "livemode": false, "created": 1, "current_period_start": 1, "current_period_end": 2, "metadata": map[string]string{"account_sqid": f.sqid}, "currency": f.subscriptionCurrency, "items": map[string]any{"data": []any{}, "has_more": false}} |
| + out = map[string]any{"id": id, "customer": f.subscriptionCustomer, "status": f.subscriptionStatus, "livemode": false, "created": 1, "current_period_start": 1, "current_period_end": 2, "metadata": map[string]string{"account_sqid": f.sqid}, "currency": f.subscriptionCurrency, "items": map[string]any{"data": f.subscriptionItems(), "has_more": false}} |
| case path == "/v1/invoices": |
| q := r.URL.Query() |
| gte, _ := strconv.ParseInt(q.Get("created[gte]"), 10, 64) |
diff --git a/internal/console/billinginvoicestore.go b/internal/console/billinginvoicestore.go| index c481e305f..7484e861a 100644 |
| --- a/internal/console/billinginvoicestore.go |
| +++ b/internal/console/billinginvoicestore.go |
| @@ -495,7 +495,7 @@ type billingInvoiceAccount struct { |
| // accounts an overage invoice can exist for. Which of their months are due is |
| // the pass's question, answered from history, never from current state. |
| func (s *Store) billingInvoiceAccounts(after int64, limit int) ([]billingInvoiceAccount, error) { |
| - rows, err := s.db.Query(`SELECT a.id,a.sqid,MIN(b.effective_at) FROM accounts a JOIN billing_assignments b ON b.account_id=a.id |
| + rows, err := s.db.Query(`SELECT a.id,a.sqid,MIN(b.effective_at) FROM accounts a JOIN `+billingAllAssignments+` b ON b.account_id=a.id |
| WHERE a.deleted_at=0 AND a.id>? AND b.plan<>? GROUP BY a.id ORDER BY a.id LIMIT ?`, after, plans.PlanFree, limit) |
| if err != nil { |
| return nil, err |
| @@ -527,7 +527,7 @@ type billingAssignmentRow struct { |
| // because AllowanceForMonth validates revision order against time order over |
| // everything it is given. |
| func (s *Store) billingInvoiceHistory(id int64) ([]billingAssignmentRow, error) { |
| - rows, err := s.db.Query(`SELECT revision,effective_at,terms,plan,band,quantity,house,status,period_end,customer,subscription,live_mode,price_book,currency FROM billing_assignments WHERE account_id=? ORDER BY revision`, id) |
| + rows, err := s.db.Query(`SELECT revision,effective_at,terms,plan,band,quantity,house,status,period_end,customer,subscription,live_mode,price_book,currency FROM `+billingAllAssignments+` WHERE account_id=? ORDER BY revision`, id) |
| if err != nil { |
| return nil, err |
| } |
diff --git a/internal/console/billingquantity.go b/internal/console/billingquantity.go| index c85037d1f..1ee18385e 100644 |
| --- a/internal/console/billingquantity.go |
| +++ b/internal/console/billingquantity.go |
| @@ -1099,6 +1099,12 @@ func (s *Server) sendQuantityOperation(ctx context.Context, state billingState, |
| if updated.ID != op.Item || updated.Quantity != op.Target { |
| return quantityStep{}, errBillingIdentity |
| } |
| + // The item that came back is still on the account's own price, in its |
| + // own currency. An update that answered with another price is not |
| + // confirmed as this operation's. |
| + if updated.PriceID != b.cfg.Prices[billingPriceKey(state.priceTerms(), state.Plan, state.Band)] || !billingCurrencyAgrees(state.priceTerms(), updated.PriceCurrency) { |
| + return quantityStep{}, errBillingIdentity |
| + } |
| // The provider answered this operation's request with success. That is |
| // what lets the reconcile date the assignment at the operation's |
| // instant, so it is recorded before anything else can observe the new |
diff --git a/internal/console/billingquantity_test.go b/internal/console/billingquantity_test.go| index 06bd44ef7..77440f933 100644 |
| --- a/internal/console/billingquantity_test.go |
| +++ b/internal/console/billingquantity_test.go |
| @@ -21,15 +21,16 @@ import ( |
| // quantityStripe is a fake provider holding one team subscription whose |
| // quantity the worker may raise. |
| type quantityStripe struct { |
| - mu sync.Mutex |
| - sqid string |
| - quantity int64 |
| - status string |
| - gets int |
| - posts []map[string]string // quantity, proration_behavior, proration_date, key |
| - failPost bool |
| - start time.Time |
| - failAfter bool // run the POST, then answer 500, and remember the 500 for the key |
| + updatePrice map[string]string |
| + mu sync.Mutex |
| + sqid string |
| + quantity int64 |
| + status string |
| + gets int |
| + posts []map[string]string // quantity, proration_behavior, proration_date, key |
| + failPost bool |
| + start time.Time |
| + failAfter bool // run the POST, then answer 500, and remember the 500 for the key |
| // failIndeterminate answers 500 and remembers it for the key WITHOUT |
| // changing the quantity: the provider's indeterminate error, whose |
| // effect may arrive later (a test sets the quantity when it does). |
| @@ -171,7 +172,13 @@ func (q *quantityStripe) serve(w http.ResponseWriter, r *http.Request) { |
| w.WriteHeader(http.StatusInternalServerError) |
| return |
| } |
| - _ = json.NewEncoder(w).Encode(item()) |
| + answer := item() |
| + if q.updatePrice != nil { |
| + // A provider that answers the update with the item on some |
| + // other price. |
| + answer["price"] = q.updatePrice |
| + } |
| + _ = json.NewEncoder(w).Encode(answer) |
| default: |
| w.WriteHeader(http.StatusNotFound) |
| } |
| @@ -2548,3 +2555,29 @@ func TestQuantityYoungOperationWithAnOpenReviewIsNotSentAgain(t *testing.T) { |
| t.Fatalf("billed %d, want 11", state.Quantity) |
| } |
| } |
| + |
| +// The provider's answer to the quantity update is the item as it now is. An |
| +// answer with the item on another price, or in another currency, is not |
| +// this operation's success: the operation is not confirmed. |
| +func TestQuantityWorkerDoesNotConfirmAnUpdateOnAnotherPrice(t *testing.T) { |
| + for name, price := range map[string]map[string]string{ |
| + "another price": {"id": "price_community_B"}, |
| + "the same price in another currency": {"id": "price_community_A", "currency": "usd"}, |
| + } { |
| + t.Run(name, func(t *testing.T) { |
| + fx, q, now := newQuantityFixture(t, billingProrationImmediate) |
| + q.updatePrice = price |
| + if err := fx.st.AddMember(OpsAccountID, "eleventh@example.test", RoleMember, now.Add(time.Minute)); err != nil { |
| + t.Fatal(err) |
| + } |
| + err := fx.srv.billingQuantityPass(context.Background(), now.Add(2*time.Minute)) |
| + op, ok, oerr := fx.st.quantityOperationFor(OpsAccountID) |
| + if oerr != nil || !ok || op.ConfirmedAt != 0 { |
| + t.Fatalf("the operation was confirmed on an answer naming %v (pass: %v): %+v ok=%v %v", price, err, op, ok, oerr) |
| + } |
| + if events := pendingAdditions(t, fx); len(events) != 1 { |
| + t.Fatalf("the addition was marked processed: %d pending", len(events)) |
| + } |
| + }) |
| + } |
| +} |
diff --git a/internal/console/billingservice.go b/internal/console/billingservice.go| index 50238d005..21fe890fe 100644 |
| --- a/internal/console/billingservice.go |
| +++ b/internal/console/billingservice.go |
| @@ -177,6 +177,88 @@ func billingCurrencyAgrees(t plans.PriceTerms, reported string) bool { |
| return reported == string(t.Currency) |
| } |
| +// checkoutParams is the request for intent's checkout session. It is built |
| +// in one place because the same request is sent twice: when the session is |
| +// first created, and again under the same idempotency key when its |
| +// response was lost, and the two must be the same request. |
| +// |
| +// A purchase from a price book is asked for in its currency with Adaptive |
| +// Pricing off. A purchase with no book is asked for exactly as it was |
| +// before books (billingstripe.CreateCheckoutParams.LegacyAdaptive): only a |
| +// console with no book configured opens one, and an intent a build from |
| +// before books opened is replayed as that build sent it. |
| +func (s *Server) checkoutParams(acct *Account, intent billingIntent) billingstripe.CreateCheckoutParams { |
| + back := s.cfg.Origin + "/a/" + acct.Sqid + "/billing" |
| + p := billingstripe.CreateCheckoutParams{PriceID: intent.Price, Quantity: intent.Quantity, SuccessURL: back + "?pending=1", CancelURL: back, CustomerID: intent.Customer, ClientReferenceID: acct.Sqid, |
| + Metadata: billingstripe.CheckoutMetadata{AccountSqid: acct.Sqid, TermsVersion: intent.Terms, Plan: intent.Plan, Band: intent.Band, IntentID: intent.ID, PriceBook: intent.PriceBook, Currency: intent.Currency}, |
| + HouseCoupon: intent.Coupon} |
| + if terms := intent.priceTerms(); terms.Legacy() { |
| + p.LegacyAdaptive = true |
| + } else { |
| + p.Currency = string(terms.Currency) |
| + } |
| + return p |
| +} |
| + |
| +// intentSession reports whether a checkout session read from the provider |
| +// is the purchase the intent describes: this account's, in this mode, for |
| +// this intent, on the intent's own price terms, and, for a purchase from a |
| +// price book, in the intent's currency with no Adaptive Pricing and no |
| +// conversion on it. Every session the console binds to an intent, offers |
| +// to a buyer or records a purchase from passes through here, however it |
| +// was come by: created, replayed, listed, or read back by its id. |
| +func (b *billingRuntime) intentSession(session *billingstripe.Session, intent billingIntent, acct *Account) bool { |
| + if session == nil || session.ID == "" || session.LiveMode != b.cfg.LiveMode || session.Metadata["checkout_intent"] != intent.ID || session.ClientReferenceID != acct.Sqid { |
| + return false |
| + } |
| + terms, err := priceTermsFromMetadata(session.Metadata) |
| + if err != nil || terms != intent.priceTerms() { |
| + return false |
| + } |
| + return terms.Legacy() || session.FixedCurrency(string(terms.Currency)) |
| +} |
| + |
| +// ownSession reports whether a session is this intent's at all: this |
| +// account's, in this mode, carrying the intent's id. Only such a session is |
| +// ever expired on the intent's behalf. |
| +func (b *billingRuntime) ownSession(session *billingstripe.Session, intent billingIntent, acct *Account) bool { |
| + return session != nil && session.ID != "" && session.LiveMode == b.cfg.LiveMode && session.Metadata["checkout_intent"] == intent.ID && session.ClientReferenceID == acct.Sqid |
| +} |
| + |
| +// refuseSession deals with a session the provider created for intent that |
| +// is not the purchase the intent describes: in another currency, converted, |
| +// or with Adaptive Pricing on it. Such a session is never offered to the |
| +// buyer and never bound to the intent. It is expired at the provider, and |
| +// once the provider says it is expired the intent is retired, so the next |
| +// attempt starts clean under a new key. The attempt that met it fails. |
| +// |
| +// If the session cannot be shown expired (it was already completed, or the |
| +// provider will not say) the intent stays as it is and the answer is an |
| +// identity error: a person has to look at what was paid for. |
| +func (s *Server) refuseSession(ctx context.Context, intent billingIntent, sessionID string, now time.Time) error { |
| + expired, err := s.billing.client.ExpireCheckout(ctx, sessionID, "carloku-expire-"+intent.ID) |
| + if err != nil { |
| + return err |
| + } |
| + if expired.ID != sessionID || expired.Status != "expired" { |
| + return errBillingIdentity |
| + } |
| + if err := s.store.retireRefusedCheckout(intent, expired, now); err != nil && !errors.Is(err, errBillingConflict) { |
| + return err |
| + } |
| + return errBillingIdentity |
| +} |
| + |
| +// 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) { |
| + var stripeErr *billingstripe.Error |
| + if errors.As(err, &stripeErr) && stripeErr.Code == billingstripe.CodeCurrencyMismatch && stripeErr.SessionID != "" { |
| + return stripeErr.SessionID, true |
| + } |
| + return "", false |
| +} |
| + |
| // errBillingCurrency: the buyer named a currency that is not one of the |
| // currencies sold. Nothing is created and no default is substituted. |
| var errBillingCurrency = errors.New("billing_currency_invalid") |
| @@ -201,14 +283,25 @@ func priceTermsFromMetadata(md map[string]string) (plans.PriceTerms, error) { |
| // |
| // currency is what the buyer chose, or empty for the billing country's |
| // default. One that is not a currency sold is refused with |
| -// errBillingCurrency before anything is read or created: it is never |
| -// replaced by a default. The band comes from the country alone, so the |
| +// errBillingCurrency before anything else is looked at, read or created: |
| +// it is never replaced by a default. The band comes from the country alone, so the |
| // choice of currency changes the money and nothing else. |
| func (s *Server) startBillingCheckout(ctx context.Context, acct *Account, plan, country, postal, currency string, now time.Time) (string, error) { |
| b := s.billing |
| if b == nil { |
| return "", errBillingUnavailable |
| } |
| + // An explicit currency is checked before anything is read: whatever |
| + // else is wrong with the request, a currency that is not sold is what |
| + // the buyer is told. |
| + if currency != "" { |
| + if _, err := plans.ChooseCurrency(b.cfg.PriceBook, currency, ""); err != nil { |
| + if errors.Is(err, plans.ErrUnknownCurrency) { |
| + return "", errBillingCurrency |
| + } |
| + return "", err |
| + } |
| + } |
| defer b.locks.lock(acct.Sqid, "billing")() |
| // A live purchase must be publishable: confirm this database owns the |
| // bucket's entitlement authority before any Stripe call. |
| @@ -315,6 +408,22 @@ func (s *Server) startBillingCheckout(ctx context.Context, acct *Account, plan, |
| if err != nil { |
| return "", err |
| } |
| + // An account with a subscription that has not ended buys nothing more: |
| + // the store refuses the intent in any case, and saying so here spares |
| + // the provider calls in between. |
| + if state.Subscription != "" && state.Status != "canceled" && state.Status != "incomplete_expired" { |
| + return "", errBillingConflict |
| + } |
| + // An account that has bought in one currency does not buy in another |
| + // here, even after its subscription has ended. Its provider customer |
| + // may still hold it to the old currency, and months of the old |
| + // subscription may still be owed an invoice in that currency on that |
| + // customer. Until it is known what the provider permits |
| + // (2026-10-01-multi-currency-price-book-design.md, the test-mode |
| + // list), a change of currency is an operator's to make, not a form's. |
| + if state.Plan != "" && state.priceTerms().Money() != terms.Money() { |
| + return "", errBillingConflict |
| + } |
| coupon := "" |
| if state.House { |
| coupon = b.cfg.HouseCoupon |
| @@ -355,7 +464,7 @@ func (s *Server) startBillingCheckout(ctx context.Context, acct *Account, plan, |
| if err != nil { |
| return "", err |
| } |
| - if session.ID != previous.SessionID || session.Metadata["checkout_intent"] != previous.ID || session.ClientReferenceID != acct.Sqid || session.LiveMode != b.cfg.LiveMode { |
| + if session.ID != previous.SessionID || !b.intentSession(session, previous, acct) { |
| return "", errBillingIdentity |
| } |
| if session.Status == "open" && (previous.Currency != request.Currency || previous.PriceBook != request.PriceBook || previous.Plan != request.Plan || previous.Band != request.Band || previous.Quantity != request.Quantity || previous.Country != request.Country || previous.Price != request.Price || previous.Coupon != request.Coupon || previous.LocDigest != request.LocDigest || previous.LiveMode != request.LiveMode || previous.Members != request.Members) { |
| @@ -402,7 +511,7 @@ func (s *Server) startBillingCheckout(ctx context.Context, acct *Account, plan, |
| if err != nil { |
| return "", err |
| } |
| - if session.Metadata["checkout_intent"] != intent.ID || session.ClientReferenceID != acct.Sqid || session.LiveMode != b.cfg.LiveMode { |
| + if session.ID != intent.SessionID || !b.intentSession(session, intent, acct) { |
| return "", errBillingIdentity |
| } |
| switch session.Status { |
| @@ -429,12 +538,20 @@ func (s *Server) startBillingCheckout(ctx context.Context, acct *Account, plan, |
| if err != nil { |
| return "", err |
| } |
| - back := s.cfg.Origin + "/a/" + acct.Sqid + "/billing" |
| - session, err := b.client.CreateCheckout(ctx, billingstripe.CreateCheckoutParams{PriceID: intent.Price, Quantity: intent.Quantity, SuccessURL: back + "?pending=1", CancelURL: back, CustomerID: intent.Customer, ClientReferenceID: acct.Sqid, Metadata: billingstripe.CheckoutMetadata{AccountSqid: acct.Sqid, TermsVersion: intent.Terms, Plan: intent.Plan, Band: intent.Band, IntentID: intent.ID, PriceBook: intent.PriceBook, Currency: intent.Currency}, HouseCoupon: intent.Coupon, Currency: string(intent.priceTerms().Money())}, "carloku-checkout-"+intent.ID) |
| + 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, now) |
| + } |
| if err != nil { |
| return "", err |
| } |
| - if session.ID == "" || session.LiveMode != b.cfg.LiveMode || session.ExpiresAt <= now.Unix() { |
| + if session.ExpiresAt <= now.Unix() { |
| + return "", errBillingUnavailable |
| + } |
| + if !b.intentSession(session, intent, acct) { |
| + if b.ownSession(session, intent, acct) { |
| + return "", s.refuseSession(ctx, intent, session.ID, now) |
| + } |
| return "", errBillingUnavailable |
| } |
| if err := s.store.saveBillingCheckout(intent, session.ID, session.URL, session.ExpiresAt); err != nil { |
| @@ -899,10 +1016,15 @@ func (s *Server) recoverReadyIntentSession(ctx context.Context, acct *Account, i |
| // never open a second purchase. Background passes pass allowCreate=false so |
| // they never POST; they rely on the read-only listing below instead. |
| if allowCreate && within() { |
| - back := s.cfg.Origin + "/a/" + acct.Sqid + "/billing" |
| - session, err := b.client.CreateCheckout(ctx, billingstripe.CreateCheckoutParams{PriceID: intent.Price, Quantity: intent.Quantity, SuccessURL: back + "?pending=1", CancelURL: back, CustomerID: intent.Customer, ClientReferenceID: acct.Sqid, Metadata: billingstripe.CheckoutMetadata{AccountSqid: acct.Sqid, TermsVersion: intent.Terms, Plan: intent.Plan, Band: intent.Band, IntentID: intent.ID, PriceBook: intent.PriceBook, Currency: intent.Currency}, HouseCoupon: intent.Coupon, Currency: string(intent.priceTerms().Money())}, "carloku-checkout-"+intent.ID) |
| + 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, now) |
| + } |
| if err == nil { |
| - if session.ID == "" || session.LiveMode != b.cfg.LiveMode || session.Metadata["checkout_intent"] != intent.ID || session.ClientReferenceID != acct.Sqid { |
| + if !b.intentSession(session, intent, acct) { |
| + if b.ownSession(session, intent, acct) { |
| + return nil, s.refuseSession(ctx, intent, session.ID, now) |
| + } |
| return nil, errBillingIdentity |
| } |
| return session, nil |
| @@ -925,6 +1047,14 @@ func (s *Server) recoverReadyIntentSession(ctx context.Context, acct *Account, i |
| if session.LiveMode != b.cfg.LiveMode || session.ClientReferenceID != acct.Sqid { |
| return nil, errBillingIdentity |
| } |
| + // A session found by listing was not checked when it was created: |
| + // its response was lost, or it was created and refused. It is |
| + // bound to the intent only if it is the purchase the intent |
| + // describes. One that is this intent's and is not that purchase |
| + // is expired and the intent retired. |
| + if !b.intentSession(&session, intent, acct) { |
| + return nil, s.refuseSession(ctx, intent, session.ID, now) |
| + } |
| return &session, nil |
| } |
| // No proof a session exists and no safe way to create one: stay pending. |
diff --git a/internal/console/billingservice_test.go b/internal/console/billingservice_test.go| index b60fcc7bc..12c8f9438 100644 |
| --- a/internal/console/billingservice_test.go |
| +++ b/internal/console/billingservice_test.go |
| @@ -47,6 +47,18 @@ type fakeBillingStripe struct { |
| prices map[string]map[string]any |
| subscriptionPrice, subscriptionCurrency string |
| subscriptionQuantity int64 |
| + // itemCurrency overrides the currency of the subscription's item's |
| + // price alone, apart from the subscription's own. |
| + itemCurrency string |
| + // failExpire makes that many expire requests fail without effect. |
| + failExpire int |
| +} |
| + |
| +func (f *fakeBillingStripe) itemMoney() string { |
| + if f.itemCurrency != "" { |
| + return f.itemCurrency |
| + } |
| + return f.subscriptionMoney() |
| } |
| // subscriptionPrice, subscriptionCurrency and subscriptionQuantity override |
| @@ -181,6 +193,18 @@ func (f *fakeBillingStripe) serve(w http.ResponseWriter, r *http.Request) { |
| out = f.session() |
| case r.URL.Path == "/v1/checkout/sessions/cs_test_paid/expire" && r.Method == http.MethodPost: |
| f.expireCalls++ |
| + if f.failExpire > 0 { |
| + f.failExpire-- |
| + w.WriteHeader(http.StatusBadGateway) |
| + return |
| + } |
| + // The provider does not expire a session that has been completed. |
| + if f.sessionStatus == "complete" { |
| + w.WriteHeader(http.StatusBadRequest) |
| + _, _ = w.Write([]byte(`{"error":{"type":"invalid_request_error","message":"Only Checkout Sessions with a status in [\"open\"] can be expired."}}`)) |
| + return |
| + } |
| + f.sessionStatus = "expired" |
| out = map[string]any{"id": "cs_test_paid", "url": "https://checkout.stripe.com/c/paid", "status": "expired", "customer": "cus_paid", "livemode": false, "client_reference_id": f.sqid, "metadata": f.meta} |
| case r.URL.Path == "/v1/checkout/sessions/cs_test_paid": |
| out = f.session() |
| @@ -191,7 +215,7 @@ func (f *fakeBillingStripe) serve(w http.ResponseWriter, r *http.Request) { |
| out = map[string]any{"id": "coupon_house", "valid": true, "livemode": false, "percent_off": 100, "duration": "forever"} |
| } |
| case r.URL.Path == "/v1/subscriptions/sub_paid": |
| - out = map[string]any{"id": "sub_paid", "customer": "cus_paid", "status": f.subscriptionStatus, "livemode": false, "created": f.now.Unix(), "current_period_start": f.now.Unix(), "current_period_end": f.now.AddDate(1, 0, 0).Unix(), "metadata": f.meta, "currency": f.subscriptionMoney(), "items": map[string]any{"data": []map[string]any{{"id": "si_paid", "price": map[string]string{"id": f.subscriptionPriceID(), "currency": f.subscriptionMoney()}, "quantity": f.subscriptionQty()}}, "has_more": false}} |
| + out = map[string]any{"id": "sub_paid", "customer": "cus_paid", "status": f.subscriptionStatus, "livemode": false, "created": f.now.Unix(), "current_period_start": f.now.Unix(), "current_period_end": f.now.AddDate(1, 0, 0).Unix(), "metadata": f.meta, "currency": f.subscriptionMoney(), "items": map[string]any{"data": []map[string]any{{"id": "si_paid", "price": map[string]string{"id": f.subscriptionPriceID(), "currency": f.itemMoney()}, "quantity": f.subscriptionQty()}}, "has_more": false}} |
| default: |
| w.WriteHeader(http.StatusNotFound) |
| return |
diff --git a/internal/console/billingstore.go b/internal/console/billingstore.go| index b07c3270c..17a5c2afc 100644 |
| --- a/internal/console/billingstore.go |
| +++ b/internal/console/billingstore.go |
| @@ -136,6 +136,20 @@ func (s *Store) migrateBilling() error { |
| PRIMARY KEY(account_id, revision) |
| )`, |
| `CREATE INDEX IF NOT EXISTS billing_assignments_time ON billing_assignments(account_id, effective_at)`, |
| + // Assignments of a purchase priced from a price book, with the book |
| + // and the currency. They are kept apart from billing_assignments on |
| + // purpose (billingAllAssignments). |
| + `CREATE TABLE IF NOT EXISTS billing_book_assignments ( |
| + account_id INTEGER NOT NULL, revision INTEGER NOT NULL, |
| + customer TEXT NOT NULL, subscription TEXT NOT NULL, |
| + effective_at INTEGER NOT NULL, plan TEXT NOT NULL, terms TEXT NOT NULL, |
| + band TEXT NOT NULL, quantity INTEGER NOT NULL, house INTEGER NOT NULL, |
| + status TEXT NOT NULL, period_start INTEGER NOT NULL, period_end INTEGER NOT NULL, |
| + live_mode INTEGER NOT NULL DEFAULT 0, |
| + currency TEXT NOT NULL CHECK (currency <> ''), price_book TEXT NOT NULL CHECK (price_book <> ''), |
| + PRIMARY KEY(account_id, revision) |
| + )`, |
| + `CREATE INDEX IF NOT EXISTS billing_book_assignments_time ON billing_book_assignments(account_id, effective_at)`, |
| `CREATE TABLE IF NOT EXISTS billing_reviews (account_id INTEGER NOT NULL, subscription TEXT NOT NULL, reason TEXT NOT NULL, created_at INTEGER NOT NULL, PRIMARY KEY(account_id,subscription,reason))`, |
| `CREATE TABLE IF NOT EXISTS billing_events (id TEXT PRIMARY KEY, received_at INTEGER NOT NULL)`, |
| `CREATE TABLE IF NOT EXISTS billing_rejected_events (id TEXT PRIMARY KEY, event_type TEXT NOT NULL, reason TEXT NOT NULL, object_id TEXT NOT NULL DEFAULT '', subscription_id TEXT NOT NULL DEFAULT '', account_id INTEGER NOT NULL DEFAULT 0, received_at INTEGER NOT NULL)`, |
| @@ -188,7 +202,7 @@ func (s *Store) migrateBilling() error { |
| // a row from before price books existed: EUR, under the terms' own |
| // figures. Nothing is backfilled, because that is exactly what such a |
| // row is. |
| - for _, table := range []string{"billing_accounts", "billing_checkout_intents", "billing_assignments"} { |
| + for _, table := range []string{"billing_accounts", "billing_checkout_intents"} { |
| for _, column := range []string{"currency", "price_book"} { |
| if err := s.addColumnIfMissing(table, column, `TEXT NOT NULL DEFAULT ''`); err != nil { |
| return err |
| @@ -207,6 +221,22 @@ func (s *Store) migrateBilling() error { |
| return s.migrateEntitlements() |
| } |
| +// billingAllAssignments is every recorded assignment: those of purchases |
| +// from before price books (billing_assignments, EUR, no currency stored) |
| +// and those of purchases priced from a book (billing_book_assignments). |
| +// |
| +// They are two tables so that a build from before price books cannot read |
| +// a book-priced assignment at all. Such a build knows nothing of |
| +// currencies: given a GBP assignment in the table it reads, it would price |
| +// the month in EUR and freeze that. Run against this database (a |
| +// rollback), it finds no assignment for a book-priced purchase, so it has |
| +// no history to price and bills nothing for it. Its other readers of a |
| +// book-priced account refuse on their own: reconcile and the quantity |
| +// worker look the provider's price up under a key that has no currency in |
| +// it, and find another price. |
| +const billingAllAssignments = `(SELECT account_id,revision,customer,subscription,effective_at,plan,terms,band,quantity,house,status,period_start,period_end,live_mode,'' AS currency,'' AS price_book FROM billing_assignments |
| + UNION ALL SELECT account_id,revision,customer,subscription,effective_at,plan,terms,band,quantity,house,status,period_start,period_end,live_mode,currency,price_book FROM billing_book_assignments)` |
| + |
| func scanBilling(row interface{ Scan(...any) error }, accountID int64) (billingState, error) { |
| x := billingState{AccountID: accountID} |
| err := row.Scan(&x.Revision, &x.House, &x.Customer, &x.Subscription, &x.Status, |
| @@ -328,7 +358,31 @@ func (s *Store) saveBillingCheckout(intent billingIntent, sessionID, dest string |
| } |
| func (s *Store) expireBillingCheckout(intent billingIntent, proof *billingstripe.Session, now time.Time) error { |
| - if proof == nil || intent.SessionID == "" || proof.ID != intent.SessionID || proof.Status != "expired" || proof.LiveMode != intent.LiveMode || proof.Metadata["checkout_intent"] != intent.ID || proof.SubscriptionID != "" || proof.PaymentStatus == "paid" { |
| + if intent.SessionID == "" || proof == nil || proof.ID != intent.SessionID { |
| + return errBillingIdentity |
| + } |
| + return s.retireIntent(intent, proof, now) |
| +} |
| + |
| +// retireRefusedCheckout removes a ready intent that was never bound to a |
| +// session, on proof that the one session the provider created for it has |
| +// been expired unpaid. That session was not the purchase the intent |
| +// describes (another currency, a conversion), so it was never bound and |
| +// never offered; with it expired nothing can be paid for on this intent, |
| +// and the next attempt starts clean. |
| +func (s *Store) retireRefusedCheckout(intent billingIntent, proof *billingstripe.Session, now time.Time) error { |
| + if intent.SessionID != "" || !intent.CustomerReady { |
| + return errBillingIdentity |
| + } |
| + return s.retireIntent(intent, proof, now) |
| +} |
| + |
| +// retireIntent deletes an intent on proof that its session is expired, |
| +// unpaid and without a subscription. The delete names the intent's own |
| +// session id, which is empty for an intent that was never bound, so an |
| +// intent that has since been bound to a session is not this one. |
| +func (s *Store) retireIntent(intent billingIntent, proof *billingstripe.Session, now time.Time) error { |
| + if proof == nil || proof.ID == "" || proof.Status != "expired" || proof.LiveMode != intent.LiveMode || proof.Metadata["checkout_intent"] != intent.ID || proof.SubscriptionID != "" || proof.PaymentStatus == "paid" { |
| return errBillingIdentity |
| } |
| tx, err := s.db.Begin() |
| @@ -493,7 +547,7 @@ func (s *Store) applyBillingStateDated(next billingState, expectedRevision int64 |
| return err |
| } |
| var latest int64 |
| - if err := tx.QueryRow(`SELECT COALESCE(MAX(effective_at),0) FROM billing_assignments WHERE account_id=?`, next.AccountID).Scan(&latest); err != nil { |
| + if err := tx.QueryRow(`SELECT COALESCE(MAX(effective_at),0) FROM `+billingAllAssignments+` WHERE account_id=?`, next.AccountID).Scan(&latest); err != nil { |
| return err |
| } |
| assignedAt := effective |
| @@ -605,11 +659,24 @@ func (s *Store) applyBillingStateDated(next billingState, expectedRevision int64 |
| } |
| } |
| } |
| - _, err = tx.Exec(`INSERT INTO billing_assignments(account_id,revision,customer,subscription,effective_at,plan,terms,band,quantity,house,status,period_start,period_end,live_mode,currency,price_book) |
| - SELECT account_id,revision,customer,subscription,?,plan,terms,band,quantity,house,status,period_start,period_end,live_mode,currency,price_book FROM billing_accounts WHERE account_id=?`, assignedAt.Unix(), next.AccountID) |
| + // The assignment goes in the table for what it is (billingAllAssignments). |
| + var inserted sql.Result |
| + if next.PriceBook == "" { |
| + inserted, err = tx.Exec(`INSERT INTO billing_assignments(account_id,revision,customer,subscription,effective_at,plan,terms,band,quantity,house,status,period_start,period_end,live_mode) |
| + SELECT account_id,revision,customer,subscription,?,plan,terms,band,quantity,house,status,period_start,period_end,live_mode FROM billing_accounts WHERE account_id=? AND price_book='' AND currency=''`, assignedAt.Unix(), next.AccountID) |
| + } else { |
| + inserted, err = tx.Exec(`INSERT INTO billing_book_assignments(account_id,revision,customer,subscription,effective_at,plan,terms,band,quantity,house,status,period_start,period_end,live_mode,currency,price_book) |
| + SELECT account_id,revision,customer,subscription,?,plan,terms,band,quantity,house,status,period_start,period_end,live_mode,currency,price_book FROM billing_accounts WHERE account_id=? AND price_book<>'' AND currency<>''`, assignedAt.Unix(), next.AccountID) |
| + } |
| if err != nil { |
| return err |
| } |
| + if n, err := inserted.RowsAffected(); err != nil || n != 1 { |
| + if err != nil { |
| + return err |
| + } |
| + return errBillingConflict |
| + } |
| if err := queueEntitlementTx(tx, next.AccountID, current.Revision+1, queuedAt); err != nil { |
| return err |
| } |
diff --git a/internal/plans/month.go b/internal/plans/month.go| index 696df50d2..ee5b516c9 100644 |
| --- a/internal/plans/month.go |
| +++ b/internal/plans/month.go |
| @@ -9,6 +9,14 @@ import ( |
| var ErrAssignmentHistory = errors.New("plans: assignment_history_incomplete") |
| +// ErrPriceTerms: the month's paid spans do not share one usable set of price |
| +// terms. They were bought in different currencies or from different price |
| +// books, or one names a book or a currency this build does not have. The |
| +// month cannot be priced on one basis and is never priced on two. Unlike an |
| +// incomplete history, this does not mend itself with time: a person has to |
| +// look at it. |
| +var ErrPriceTerms = errors.New("plans: price_terms_unusable") |
| + |
| type Assignment struct { |
| EffectiveAt time.Time `json:"effective_at"` |
| Revision int64 `json:"revision"` |
| @@ -81,12 +89,14 @@ func (m MonthAllowance) PriceTerms() PriceTerms { |
| // ends in, a lapse, a span not paid through. What is refused |
| // (ErrAssignmentHistory) is a history that cannot be read at all (empty, |
| // unordered, zero times or revisions), a month with no paid span, and paid |
| -// spans that disagree on the terms version, the house marker, the price |
| -// book or the currency, whose usage could not be priced on one basis. A |
| -// month paid for partly in one currency and partly in another is never |
| -// priced in either, and nothing is converted. A paid span whose price |
| -// terms this build does not know (a book it does not have, a currency the |
| -// book does not sell in) is refused the same way. |
| +// spans that disagree on the terms version or the house marker, whose |
| +// usage could not be priced on one basis. |
| +// |
| +// Paid spans that disagree on the price book or the currency, and a paid |
| +// span whose price terms this build does not know (a book it does not |
| +// have, a currency the book does not sell in), are refused with |
| +// ErrPriceTerms. A month paid for partly in one currency and partly in |
| +// another is never priced in either, and nothing is converted. |
| func AllowanceForMonth(month string, history []Assignment) (*MonthAllowance, error) { |
| start, err := time.Parse("2006-01", month) |
| if err != nil || start.Format("2006-01") != month || len(history) == 0 { |
| @@ -163,14 +173,16 @@ func AllowanceForMonth(month string, history []Assignment) (*MonthAllowance, err |
| if !paid { |
| out.TermsVersion, out.House, paid = a.TermsVersion, a.House, true |
| out.PriceBook, out.Currency = a.PriceBook, currency |
| - } else if a.TermsVersion != out.TermsVersion || a.House != out.House || a.PriceBook != out.PriceBook || currency != out.Currency { |
| + } else if a.TermsVersion != out.TermsVersion || a.House != out.House { |
| return nil, ErrAssignmentHistory |
| + } else if a.PriceBook != out.PriceBook || currency != out.Currency { |
| + return nil, ErrPriceTerms |
| } |
| if err := a.PriceTerms().Validate(); err != nil { |
| if errors.Is(err, ErrUnknownTermsVersion) { |
| return nil, err |
| } |
| - return nil, ErrAssignmentHistory |
| + return nil, ErrPriceTerms |
| } |
| quote, err := Quote(a.TermsVersion, a.Plan, a.Band, a.Quantity) |
| if err != nil { |
diff --git a/internal/plans/pricebook.go b/internal/plans/pricebook.go| index 0f3dcd35f..6d13a96fc 100644 |
| --- a/internal/plans/pricebook.go |
| +++ b/internal/plans/pricebook.go |
| @@ -7,6 +7,7 @@ import ( |
| "errors" |
| "fmt" |
| "io" |
| + "math" |
| "sort" |
| ) |
| @@ -116,7 +117,16 @@ func init() { |
| } |
| t = *decoded |
| } |
| - books, err := decodePriceBooks(priceBookJSON, &t) |
| + // Likewise the countries a book may name. |
| + known := countryBandsByISO2 |
| + if known == nil { |
| + _, decoded, err := decodeCountryTerms(countriesJSON) |
| + if err != nil { |
| + panic("plans: invalid embedded countries.json: " + err.Error()) |
| + } |
| + known = decoded |
| + } |
| + books, err := decodePriceBooks(priceBookJSON, &t, known) |
| if err != nil { |
| panic("plans: invalid embedded pricebook.json: " + err.Error()) |
| } |
| @@ -130,7 +140,17 @@ func init() { |
| // against the terms they price: an unknown field, trailing content, a |
| // missing price, or a price that is not what the book's own rule gives, |
| // fails here and not at a checkout. |
| -func decodePriceBooks(data []byte, t *termsDoc) (map[string]*bookDoc, error) { |
| +// |
| +// knownCountries is every country the country terms know, by ISO code. A book |
| +// may give a currency only to a country that has a band: a code that is |
| +// not one (a typo, "UK" for "GB") would otherwise load, and that |
| +// country's buyers would silently get the default. |
| +func decodePriceBooks(data []byte, t *termsDoc, knownCountries map[string]string) (map[string]*bookDoc, error) { |
| + // encoding/json keeps the last of two equal keys and says nothing. In |
| + // a price list that is one price silently replacing another. |
| + if err := rejectDuplicateKeys(data); err != nil { |
| + return nil, err |
| + } |
| dec := json.NewDecoder(bytes.NewReader(data)) |
| dec.DisallowUnknownFields() |
| var doc booksDoc |
| @@ -152,7 +172,7 @@ func decodePriceBooks(data []byte, t *termsDoc) (map[string]*bookDoc, error) { |
| if out[b.Version] != nil { |
| return nil, fmt.Errorf("book %q appears twice", b.Version) |
| } |
| - if err := validateBook(b, t); err != nil { |
| + if err := validateBook(b, t, knownCountries); err != nil { |
| return nil, fmt.Errorf("book %q: %w", b.Version, err) |
| } |
| out[b.Version] = b |
| @@ -160,7 +180,51 @@ func decodePriceBooks(data []byte, t *termsDoc) (map[string]*bookDoc, error) { |
| return out, nil |
| } |
| -func validateBook(b *bookDoc, t *termsDoc) error { |
| +// rejectDuplicateKeys walks a JSON document and refuses one in which an |
| +// object has the same key twice, at any depth. |
| +func rejectDuplicateKeys(data []byte) error { |
| + dec := json.NewDecoder(bytes.NewReader(data)) |
| + var walk func() error |
| + walk = func() error { |
| + tok, err := dec.Token() |
| + if err != nil { |
| + return err |
| + } |
| + delim, ok := tok.(json.Delim) |
| + if !ok { |
| + return nil |
| + } |
| + switch delim { |
| + case '{': |
| + seen := map[string]bool{} |
| + for dec.More() { |
| + keyTok, err := dec.Token() |
| + if err != nil { |
| + return err |
| + } |
| + key, _ := keyTok.(string) |
| + if seen[key] { |
| + return fmt.Errorf("the key %q appears twice in one object", key) |
| + } |
| + seen[key] = true |
| + if err := walk(); err != nil { |
| + return err |
| + } |
| + } |
| + case '[': |
| + for dec.More() { |
| + if err := walk(); err != nil { |
| + return err |
| + } |
| + } |
| + } |
| + _, err = dec.Token() // the closing delimiter |
| + return err |
| + } |
| + return walk() |
| +} |
| + |
| +func validateBook(b *bookDoc, t *termsDoc, knownCountries map[string]string) error { |
| if b.TermsVersion != t.Version { |
| return fmt.Errorf("terms_version %q is not the embedded terms %q", b.TermsVersion, t.Version) |
| } |
| @@ -195,6 +259,9 @@ func validateBook(b *bookDoc, t *termsDoc) error { |
| if len(country) != 2 || country[0] < 'A' || country[0] > 'Z' || country[1] < 'A' || country[1] > 'Z' { |
| return fmt.Errorf("country_currency[%q] has %q, which is not an ISO 3166-1 alpha-2 code", c, country) |
| } |
| + if _, known := knownCountries[country]; !known { |
| + return fmt.Errorf("country_currency[%q] has %s, which the country terms do not know", c, country) |
| + } |
| if other, dup := b.byCountry[country]; dup { |
| return fmt.Errorf("country %s is given both %q and %q", country, other, c) |
| } |
| @@ -230,7 +297,14 @@ func validateBook(b *bookDoc, t *termsDoc) error { |
| // any quantity. |
| want := base[c] |
| if pt.BandScaled { |
| - want = roundHalfUpDiv(base[c]*t.BandFactorsBps[band], bpsScale) |
| + // Checked: a base large enough to wrap the product |
| + // would otherwise validate against the wrapped figure. |
| + scaled, err := mulDivFloor(base[c], 2*t.BandFactorsBps[band], 1) |
| + if err != nil || scaled > math.MaxInt64-bpsScale { |
| + return fmt.Errorf("annual_base_minor[%q][%q] is too large to price", plan, c) |
| + } |
| + // floor((2ab + s) / 2s) is a*b/s rounded half up. |
| + want = (scaled + bpsScale) / (2 * bpsScale) |
| } |
| if got != want { |
| return fmt.Errorf("annual_unit_minor[%q][%q][%q] = %d, and the rounding rule gives %d", plan, c, band, got, want) |
| @@ -369,6 +443,18 @@ func BookIsApproved(book string) (bool, error) { |
| return b.Status == BookApproved, nil |
| } |
| +// PriceBooks lists every price book this build has, sorted. A purchase |
| +// recorded under a book stays priced from it for as long as it lasts, so a |
| +// book that has ever been sold from must stay in the build. |
| +func PriceBooks() []string { |
| + out := make([]string, 0, len(priceBooks)) |
| + for version := range priceBooks { |
| + out = append(out, version) |
| + } |
| + sort.Strings(out) |
| + return out |
| +} |
| + |
| // BookCurrencies lists the currencies a book sells in, in the book's order. |
| func BookCurrencies(book string) ([]Currency, error) { |
| if book == LegacyPriceBook { |
| @@ -628,9 +714,12 @@ type publicOverage struct { |
| // fixed order. |
| // |
| // Prices are for one member for one year, in minor units; an annual price |
| -// is that times the quantity. resources_per_member is band A's; another |
| -// band's pooled allowance is the figure times the quantity times the band |
| -// factor, divided by 10000 and rounded down. |
| +// is that times the quantity. resources_per_member is the plan's own |
| +// figure. For a plan with band_scaled true, a band's pooled allowance is |
| +// the figure times the quantity times the band factor, divided by 10000 |
| +// and rounded down. For a plan with band_scaled false (the free plan) the |
| +// band changes nothing: the allowance is the figure times the quantity in |
| +// every band. |
| func PublicPriceBook(book string) ([]byte, error) { |
| b, err := lookupBook(book) |
| if err != nil { |
diff --git a/internal/plans/pricebook_test.go b/internal/plans/pricebook_test.go| index 27c42fc15..d73720a8f 100644 |
| --- a/internal/plans/pricebook_test.go |
| +++ b/internal/plans/pricebook_test.go |
| @@ -273,6 +273,64 @@ func TestOverageInEachCurrency(t *testing.T) { |
| } |
| } |
| +// The four meters in each currency, against figures worked out by hand |
| +// from the rates in the design, not read from the book. Usage over |
| +// allowance: 10 awake hours, 7.5 GB-months of storage, 12.25 GB of |
| +// transfer and 5,500 writes. |
| +func TestOverageRatesInEachCurrencyByHand(t *testing.T) { |
| + usage := Resources{AwakeSeconds: 36000, StorageByteMonths: 7_500_000_000, TransferBytes: 12_250_000_000, Writes: 5500} |
| + for cur, want := range map[Currency][5]int64{ |
| + USD: {120000, 954000, 6085800, 151800, 7311600}, |
| + EUR: {100000, 795000, 5071500, 126500, 6093000}, |
| + GBP: {90000, 715500, 4564350, 113850, 5483700}, |
| + } { |
| + c, err := OverageFor(bookTerms(cur), PlanBusiness, usage, Resources{}) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if got := [5]int64{c.AwakeMicro, c.StorageMicro, c.TransferMicro, c.WritesMicro, c.TotalMicro}; got != want || c.Currency != cur { |
| + t.Errorf("%s: %v, want %v", cur, got, want) |
| + } |
| + inv, lines, err := OverageInvoiceFor(bookTerms(cur), *c, false) |
| + if err != nil || inv.Waived { |
| + t.Fatal(inv, err) |
| + } |
| + // Rounded once, half up, to minor units; the lines sum to it. |
| + wantMinor := map[Currency]int64{USD: 731, EUR: 609, GBP: 548}[cur] |
| + var sum int64 |
| + for _, l := range lines { |
| + sum += l.AmountCents |
| + } |
| + if inv.TotalMinor != wantMinor || sum != wantMinor || len(lines) != 4 { |
| + t.Errorf("%s: %d minor in %d lines summing to %d, want %d", cur, inv.TotalMinor, len(lines), sum, wantMinor) |
| + } |
| + } |
| + // A cost too large for the arithmetic is an error in every currency, |
| + // never a wrapped number. |
| + for _, cur := range allCurrencies { |
| + huge := Resources{TransferBytes: 1<<63 - 1, StorageByteMonths: 1<<63 - 1, AwakeSeconds: 1<<63 - 1, Writes: 1<<63 - 1} |
| + if c, err := OverageFor(bookTerms(cur), PlanBusiness, huge, Resources{}); err == nil { |
| + t.Errorf("%s: an overflowing total was priced: %+v", cur, c) |
| + } |
| + } |
| +} |
| + |
| +// Every book a stored purchase may name has to stay in the build: a |
| +// purchase recorded under one is priced from it for as long as it lasts, |
| +// and without the book its months cannot be priced at all. Add to this |
| +// list when a book is published; never remove from it. |
| +func TestPublishedBooksStayEmbedded(t *testing.T) { |
| + have := map[string]bool{} |
| + for _, b := range PriceBooks() { |
| + have[b] = true |
| + } |
| + for _, b := range []string{"2026-10-01-draft1"} { |
| + if !have[b] { |
| + t.Errorf("price book %s is no longer embedded", b) |
| + } |
| + } |
| +} |
| + |
| // The floor under which an overage invoice is waived is the currency's own: |
| // 1.20 USD, 1.00 EUR, 0.90 GBP. One micro below it is waived, at it is |
| // billed. |
| @@ -348,10 +406,24 @@ func TestAllowanceForMonthKeepsOneCurrency(t *testing.T) { |
| "a book and no currency": span(2, 10, CurrentPriceBook, ""), |
| "legacy with a currency": span(2, 10, "", GBP), |
| } { |
| - if _, err := AllowanceForMonth("2026-09", []Assignment{first, second}); !errors.Is(err, ErrAssignmentHistory) { |
| - t.Errorf("%s: %v, want ErrAssignmentHistory", name, err) |
| + // Its own error, not the one for a history that is merely |
| + // incomplete: this one does not mend with time. |
| + if _, err := AllowanceForMonth("2026-09", []Assignment{first, second}); !errors.Is(err, ErrPriceTerms) || errors.Is(err, ErrAssignmentHistory) { |
| + t.Errorf("%s: %v, want ErrPriceTerms", name, err) |
| } |
| } |
| + // One span alone, on terms this build does not have. |
| + alone := span(1, 1, "2031-01-01-v9", GBP) |
| + alone.EffectiveAt = first.EffectiveAt |
| + if _, err := AllowanceForMonth("2026-09", []Assignment{alone}); !errors.Is(err, ErrPriceTerms) { |
| + t.Errorf("a month bought from a book this build lacks: %v, want ErrPriceTerms", err) |
| + } |
| + // Spans that disagree on the house marker are still the older refusal. |
| + house := span(2, 10, CurrentPriceBook, GBP) |
| + house.House = true |
| + if _, err := AllowanceForMonth("2026-09", []Assignment{first, house}); !errors.Is(err, ErrAssignmentHistory) || errors.Is(err, ErrPriceTerms) { |
| + t.Errorf("a house span beside one that is not: %v, want ErrAssignmentHistory", err) |
| + } |
| } |
| // An assignment recorded before books existed marshals exactly as it did: |
| @@ -381,7 +453,8 @@ func TestPriceBookValidation(t *testing.T) { |
| if err != nil { |
| t.Fatal(err) |
| } |
| - if _, err := decodePriceBooks(priceBookJSON, tm); err != nil { |
| + countries := CountryBands() |
| + if _, err := decodePriceBooks(priceBookJSON, tm, countries); err != nil { |
| t.Fatalf("the embedded book: %v", err) |
| } |
| for name, edit := range map[string]func(s string) string{ |
| @@ -418,13 +491,31 @@ func TestPriceBookValidation(t *testing.T) { |
| }, |
| "no version": func(s string) string { return strings.Replace(s, `"version": "2026-10-01-draft1"`, `"version": ""`, 1) }, |
| "trailing content": func(s string) string { return s + "{}" }, |
| + // A code that is well formed and is no country the terms know: |
| + // the United Kingdom is GB. |
| + "a country the terms do not know": func(s string) string { return strings.Replace(s, `"gbp": ["GB"]`, `"gbp": ["UK"]`, 1) }, |
| + // 2800 plus 2^62: times any band factor (each divisible by four) |
| + // the product wraps to the real one, so unchecked arithmetic |
| + // would accept the listed prices. |
| + "a base that wraps the rounding rule": func(s string) string { |
| + return strings.Replace(s, `"personal": {"usd": 2800,`, `"personal": {"usd": 4611686018427390704,`, 1) |
| + }, |
| + // The same key twice in one object: the decoder would keep the |
| + // last and say nothing. |
| + "a currency's country list given twice": func(s string) string { |
| + return strings.Replace(s, `"gbp": ["GB"]`, `"gbp": ["GB"], "gbp": ["GB"]`, 1) |
| + }, |
| + "a price given twice": func(s string) string { |
| + return strings.Replace(s, `"personal": {"usd": 2800, "eur": 2400, "gbp": 2200}`, `"personal": {"usd": 2800, "eur": 2400, "gbp": 2200, "gbp": 2200}`, 1) |
| + }, |
| + "a band given twice": func(s string) string { return strings.Replace(s, `"D": 980}`, `"D": 980, "D": 980}`, 1) }, |
| } { |
| edited := edit(string(priceBookJSON)) |
| if edited == string(priceBookJSON) { |
| t.Errorf("%s: fixture: the edit changed nothing", name) |
| continue |
| } |
| - if _, err := decodePriceBooks([]byte(edited), tm); err == nil { |
| + if _, err := decodePriceBooks([]byte(edited), tm, countries); err == nil { |
| t.Errorf("%s: accepted", name) |
| } |
| } |
| @@ -435,7 +526,7 @@ func TestPriceBookValidation(t *testing.T) { |
| } |
| doc["books"] = append(doc["books"], doc["books"][0]) |
| twice, _ := json.Marshal(doc) |
| - if _, err := decodePriceBooks(twice, tm); err == nil { |
| + if _, err := decodePriceBooks(twice, tm, countries); err == nil { |
| t.Error("one version twice: accepted") |
| } |
| } |