Compare commits
No commits in common. "78b2c2be288278b6d37f53e613f2b36fc9118f85" and "83ffa7dbf8b8875c6c82d4761fa22c26c864e5f9" have entirely different histories.
78b2c2be28
...
83ffa7dbf8
4 changed files with 110 additions and 494 deletions
6
.gitignore
vendored
6
.gitignore
vendored
|
|
@ -9,10 +9,8 @@ __pycache__/
|
||||||
.ruff_cache/
|
.ruff_cache/
|
||||||
.venv/
|
.venv/
|
||||||
venv/
|
venv/
|
||||||
# Everything under backup/ is operational data, never source: DB dumps and
|
backup/*.sql
|
||||||
# pre-change .env copies (which hold live Stripe/SMTP secrets). The earlier
|
backup/*.sql.gz
|
||||||
# backup/*.sql* patterns missed the .env copies — ignore the whole directory.
|
|
||||||
backup/
|
|
||||||
*.egg-info/
|
*.egg-info/
|
||||||
build/
|
build/
|
||||||
dist/
|
dist/
|
||||||
|
|
|
||||||
|
|
@ -19,7 +19,7 @@ from __future__ import annotations
|
||||||
|
|
||||||
import asyncio
|
import asyncio
|
||||||
import json
|
import json
|
||||||
from typing import Any, Literal
|
from typing import Any, Literal, Optional
|
||||||
|
|
||||||
import stripe
|
import stripe
|
||||||
from fastapi import APIRouter, Body, Depends, HTTPException, Request
|
from fastapi import APIRouter, Body, Depends, HTTPException, Request
|
||||||
|
|
@ -75,21 +75,51 @@ def _price_for(cadence: str) -> str:
|
||||||
raise HTTPException(status_code=400, detail="cadence must be 'monthly' or 'annual'")
|
raise HTTPException(status_code=400, detail="cadence must be 'monthly' or 'annual'")
|
||||||
|
|
||||||
|
|
||||||
# NOTE: we deliberately never pass `currency` to Stripe, so every
|
# Rough country → currency mapping. Covers the markets we have a stated
|
||||||
# checkout bills the Price's base currency — GBP. An earlier version
|
# rate for; everything else falls back to GBP (the home currency) and
|
||||||
# sniffed CF-IPCountry / Accept-Language and selected a matching
|
# Stripe handles the FX at checkout. Configure the per-currency
|
||||||
# `currency_options` entry, but /pricing renders £7 and £70 as static
|
# unit_amount on each Price's `currency_options` in the Stripe Dashboard
|
||||||
# copy: a US visitor was shown £7 and charged $9.99. Showing one price
|
# — we just signal which option to use here.
|
||||||
# and billing another is exactly what the UK CPRs and the EU
|
_COUNTRY_CURRENCY: dict[str, str] = {
|
||||||
# price-indication rules prohibit, so the sniffing was removed rather
|
"US": "usd", "CA": "usd",
|
||||||
# than the disclosure patched. The `currency_options` still configured
|
"GB": "gbp", "IM": "gbp", "JE": "gbp", "GG": "gbp",
|
||||||
# on the Prices in the Dashboard are simply unused.
|
**dict.fromkeys((
|
||||||
#
|
"DE", "FR", "IT", "ES", "PT", "NL", "BE", "IE", "AT", "FI",
|
||||||
# To reinstate geo-pricing, /pricing must render the matching currency
|
"GR", "LU", "MT", "CY", "EE", "LV", "LT", "SI", "SK", "HR",
|
||||||
# in its copy, its buttons AND its annual-saving claim first (the claim
|
), "eur"),
|
||||||
# is currency-specific: "two months free" is true at £70/£84 and
|
}
|
||||||
# $94.99/$119.88, but not at €80/€84). See git history for the removed
|
|
||||||
# _sniff_currency helper and its country/locale tables.
|
# Accept-Language locale → currency, used when CF-IPCountry is absent.
|
||||||
|
# Ambiguous locales (e.g. plain "fr" without region) get EUR because
|
||||||
|
# that's the majority outcome.
|
||||||
|
_LOCALE_CURRENCY: dict[str, str] = {
|
||||||
|
"en-gb": "gbp", "en": "gbp",
|
||||||
|
"en-us": "usd", "en-ca": "usd",
|
||||||
|
"fr": "eur", "de": "eur", "it": "eur", "es": "eur",
|
||||||
|
"pt": "eur", "nl": "eur",
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def _sniff_currency(request: Request) -> str:
|
||||||
|
"""Best-effort currency detection for new-customer checkouts.
|
||||||
|
|
||||||
|
Order: explicit Cloudflare country header, then Accept-Language
|
||||||
|
(exact match then language-only). GBP as the final fallback. Only
|
||||||
|
consulted when the user has no Stripe customer record yet — Stripe
|
||||||
|
locks currency at customer creation, so an existing customer's
|
||||||
|
currency wins regardless of the request locale.
|
||||||
|
"""
|
||||||
|
cc = (request.headers.get("cf-ipcountry") or "").upper()
|
||||||
|
if cc in _COUNTRY_CURRENCY:
|
||||||
|
return _COUNTRY_CURRENCY[cc]
|
||||||
|
al = (request.headers.get("accept-language") or "").lower()
|
||||||
|
first = al.split(",", 1)[0].split(";", 1)[0].strip()
|
||||||
|
if first in _LOCALE_CURRENCY:
|
||||||
|
return _LOCALE_CURRENCY[first]
|
||||||
|
short = first.split("-", 1)[0]
|
||||||
|
if short in _LOCALE_CURRENCY:
|
||||||
|
return _LOCALE_CURRENCY[short]
|
||||||
|
return "gbp"
|
||||||
|
|
||||||
|
|
||||||
def _stripe_client() -> stripe.StripeClient:
|
def _stripe_client() -> stripe.StripeClient:
|
||||||
|
|
@ -106,6 +136,10 @@ def _stripe_client() -> stripe.StripeClient:
|
||||||
|
|
||||||
class CheckoutRequest(BaseModel):
|
class CheckoutRequest(BaseModel):
|
||||||
cadence: Literal["monthly", "annual"]
|
cadence: Literal["monthly", "annual"]
|
||||||
|
# Optional override; when omitted we sniff from request headers.
|
||||||
|
# Honoured only for first-time checkouts (Stripe locks currency
|
||||||
|
# to the customer at creation).
|
||||||
|
currency: Optional[Literal["gbp", "usd", "eur"]] = None
|
||||||
|
|
||||||
|
|
||||||
class CheckoutResponse(BaseModel):
|
class CheckoutResponse(BaseModel):
|
||||||
|
|
@ -115,6 +149,7 @@ class CheckoutResponse(BaseModel):
|
||||||
@router.post("/api/stripe/checkout", response_model=CheckoutResponse)
|
@router.post("/api/stripe/checkout", response_model=CheckoutResponse)
|
||||||
async def create_checkout(
|
async def create_checkout(
|
||||||
body: CheckoutRequest,
|
body: CheckoutRequest,
|
||||||
|
request: Request,
|
||||||
session: AsyncSession = Depends(get_session),
|
session: AsyncSession = Depends(get_session),
|
||||||
cu: CurrentUser = Depends(require_auth),
|
cu: CurrentUser = Depends(require_auth),
|
||||||
) -> CheckoutResponse:
|
) -> CheckoutResponse:
|
||||||
|
|
@ -142,19 +177,14 @@ async def create_checkout(
|
||||||
# Lets us paste in a referral coupon at checkout once the
|
# Lets us paste in a referral coupon at checkout once the
|
||||||
# referral redemption flow ships.
|
# referral redemption flow ships.
|
||||||
"allow_promotion_codes": True,
|
"allow_promotion_codes": True,
|
||||||
# Collect a billing address on every checkout so each Stripe
|
|
||||||
# Customer carries a country. Two reasons: card-fraud checks get
|
|
||||||
# materially better with AVS data, and EU B2C digital-services
|
|
||||||
# VAT is charged at the *consumer's* place of supply — we can't
|
|
||||||
# answer the OSS question at all without knowing where buyers
|
|
||||||
# are. Note this is the address on the card, not an IP guess,
|
|
||||||
# which is the evidence a tax authority actually accepts.
|
|
||||||
"billing_address_collection": "required",
|
|
||||||
}
|
}
|
||||||
# No `currency` kwarg — every checkout bills the Price's base
|
# Multi-currency: for first-time buyers (no stripe_customer_id yet)
|
||||||
# currency (GBP), matching the static £7 / £70 copy on /pricing.
|
# we pass the detected/requested currency. Stripe picks the matching
|
||||||
# See the note above _stripe_client() before reintroducing one.
|
# `currency_options` rate configured on the Price in the Dashboard,
|
||||||
#
|
# then locks that currency to the new customer record. Existing
|
||||||
|
# customers keep their original currency regardless.
|
||||||
|
if not user.stripe_customer_id:
|
||||||
|
create_kwargs["currency"] = body.currency or _sniff_currency(request)
|
||||||
# Per-cadence cooling-off treatment:
|
# Per-cadence cooling-off treatment:
|
||||||
#
|
#
|
||||||
# - Annual gets a 14-day free trial. No money moves during the
|
# - Annual gets a 14-day free trial. No money moves during the
|
||||||
|
|
@ -173,12 +203,6 @@ async def create_checkout(
|
||||||
create_kwargs["subscription_data"] = {"trial_period_days": 14}
|
create_kwargs["subscription_data"] = {"trial_period_days": 14}
|
||||||
if user.stripe_customer_id:
|
if user.stripe_customer_id:
|
||||||
create_kwargs["customer"] = user.stripe_customer_id
|
create_kwargs["customer"] = user.stripe_customer_id
|
||||||
# Required for billing_address_collection to actually persist:
|
|
||||||
# when `customer` is supplied, Stripe collects the address for
|
|
||||||
# the payment but leaves the Customer record untouched unless
|
|
||||||
# customer_update.address is "auto". Without this the country
|
|
||||||
# lands on the PaymentIntent and nowhere durable.
|
|
||||||
create_kwargs["customer_update"] = {"address": "auto"}
|
|
||||||
else:
|
else:
|
||||||
create_kwargs["customer_email"] = user.email
|
create_kwargs["customer_email"] = user.email
|
||||||
|
|
||||||
|
|
@ -308,15 +332,11 @@ async def _grant_paid(
|
||||||
await convert_referral(session, user)
|
await convert_referral(session, user)
|
||||||
|
|
||||||
|
|
||||||
async def _revoke_paid(user: User, *, keep_subscription: bool = False) -> None:
|
async def _revoke_paid(user: User) -> None:
|
||||||
user.tier = "free"
|
user.tier = "free"
|
||||||
if not keep_subscription:
|
user.stripe_subscription_id = None
|
||||||
user.stripe_subscription_id = None
|
|
||||||
user.stripe_trial_end_at = None
|
user.stripe_trial_end_at = None
|
||||||
# Keep stripe_customer_id so a re-subscription matches this row.
|
# Keep stripe_customer_id so a re-subscription matches this row.
|
||||||
# `keep_subscription` is for a pause: the subscription still exists
|
|
||||||
# at Stripe and will resume under the same id, so nulling our copy
|
|
||||||
# would lose the link while access is merely suspended.
|
|
||||||
|
|
||||||
|
|
||||||
async def _handle_checkout_completed(
|
async def _handle_checkout_completed(
|
||||||
|
|
@ -354,16 +374,6 @@ async def _handle_subscription_event(
|
||||||
customer_id=obj.get("customer"))
|
customer_id=obj.get("customer"))
|
||||||
return
|
return
|
||||||
status = obj.get("status")
|
status = obj.get("status")
|
||||||
# `pause_collection` is a *different* mechanism from status="paused":
|
|
||||||
# the subscription stays `active` while Stripe simply stops invoicing.
|
|
||||||
# Unhandled, that leaves the customer on paid features indefinitely
|
|
||||||
# without paying, so treat any live pause as not-paid regardless of
|
|
||||||
# status. Pause is disabled in our live portal configuration, so in
|
|
||||||
# practice this only fires if someone re-enables it there or pauses
|
|
||||||
# from the Dashboard — which is exactly when we'd want it to work.
|
|
||||||
if obj.get("pause_collection"):
|
|
||||||
await _revoke_paid(user, keep_subscription=True)
|
|
||||||
return
|
|
||||||
# Stripe statuses: trialing, active, past_due, canceled, unpaid,
|
# Stripe statuses: trialing, active, past_due, canceled, unpaid,
|
||||||
# incomplete, incomplete_expired, paused. Treat trialing/active as
|
# incomplete, incomplete_expired, paused. Treat trialing/active as
|
||||||
# paid; everything else holds tier the same until we get an explicit
|
# paid; everything else holds tier the same until we get an explicit
|
||||||
|
|
@ -390,22 +400,6 @@ async def _handle_subscription_deleted(
|
||||||
await _revoke_paid(user)
|
await _revoke_paid(user)
|
||||||
|
|
||||||
|
|
||||||
async def _handle_subscription_paused(
|
|
||||||
session: AsyncSession, event_type: str, obj: dict[str, Any],
|
|
||||||
) -> None:
|
|
||||||
"""customer.subscription.paused — status flips to `paused` when a
|
|
||||||
trial ends with no usable payment method (trial_settings.end_behavior
|
|
||||||
.missing_payment_method = pause). No money is being collected, so
|
|
||||||
paid features come off. `.resumed` routes to the normal subscription
|
|
||||||
handler, which grants again on active/trialing."""
|
|
||||||
user = await _find_user(session, customer_id=obj.get("customer"))
|
|
||||||
if user is None:
|
|
||||||
log.warning("stripe.user_not_found", event_type=event_type,
|
|
||||||
customer_id=obj.get("customer"))
|
|
||||||
return
|
|
||||||
await _revoke_paid(user, keep_subscription=True)
|
|
||||||
|
|
||||||
|
|
||||||
async def _handle_audit_only(
|
async def _handle_audit_only(
|
||||||
session: AsyncSession, event_type: str, obj: dict[str, Any],
|
session: AsyncSession, event_type: str, obj: dict[str, Any],
|
||||||
) -> None:
|
) -> None:
|
||||||
|
|
@ -420,8 +414,6 @@ _HANDLERS = {
|
||||||
"customer.subscription.created": _handle_subscription_event,
|
"customer.subscription.created": _handle_subscription_event,
|
||||||
"customer.subscription.updated": _handle_subscription_event,
|
"customer.subscription.updated": _handle_subscription_event,
|
||||||
"customer.subscription.deleted": _handle_subscription_deleted,
|
"customer.subscription.deleted": _handle_subscription_deleted,
|
||||||
"customer.subscription.paused": _handle_subscription_paused,
|
|
||||||
"customer.subscription.resumed": _handle_subscription_event,
|
|
||||||
"invoice.paid": _handle_audit_only,
|
"invoice.paid": _handle_audit_only,
|
||||||
"invoice.payment_failed": _handle_audit_only,
|
"invoice.payment_failed": _handle_audit_only,
|
||||||
"charge.refunded": _handle_audit_only,
|
"charge.refunded": _handle_audit_only,
|
||||||
|
|
|
||||||
|
|
@ -1,222 +0,0 @@
|
||||||
# Currency-localised pricing — Design Spec
|
|
||||||
|
|
||||||
**Date:** 2026-07-29
|
|
||||||
**Status:** Draft — pending implementation plan
|
|
||||||
|
|
||||||
## Context
|
|
||||||
|
|
||||||
`/pricing` hardcodes `£7` and `£70` in its copy and its buttons. Until
|
|
||||||
commit `4169a67`, checkout sniffed `CF-IPCountry` / `Accept-Language`
|
|
||||||
and passed a matching `currency` to Stripe, which then selected a
|
|
||||||
`currency_options` rate off the Price. A US visitor was shown £7 and
|
|
||||||
billed $9.99; a German visitor was billed €7. The page's "Prices in GBP"
|
|
||||||
line was untrue for two of the three currencies.
|
|
||||||
|
|
||||||
`4169a67` fixed that by forcing GBP for everyone — correct, but it
|
|
||||||
gives up genuine multi-currency pricing that is already configured and
|
|
||||||
paid for on the Stripe side. This spec restores it properly: the page
|
|
||||||
displays the currency the customer will actually be charged.
|
|
||||||
|
|
||||||
Live Prices today (both `livemode: true`, base currency GBP):
|
|
||||||
|
|
||||||
| Price | Interval | GBP | EUR | USD |
|
|
||||||
|---|---|---|---|---|
|
|
||||||
| `price_1TbNshDLpLwvRJpKnuWjdU1x` | month | 700 | 700 | 999 |
|
|
||||||
| `price_1TbNtWDLpLwvRJpKze87qOJ4` | year | 7000 | 7000 | 9499 |
|
|
||||||
|
|
||||||
The EUR annual was corrected from 8000 to 7000 on 2026-07-29, so the
|
|
||||||
"two months free" claim now holds in GBP and EUR (16.7%) and understates
|
|
||||||
USD (20.8%).
|
|
||||||
|
|
||||||
## Goals
|
|
||||||
|
|
||||||
- A visitor sees prices in the currency they will be charged, in the
|
|
||||||
page copy, the buttons, and the saving claim.
|
|
||||||
- The visitor can override the detected currency, and the choice sticks.
|
|
||||||
- The displayed amounts are structurally incapable of disagreeing with
|
|
||||||
what Stripe charges.
|
|
||||||
- The monthly cooling-off waiver is effective for non-UK customers.
|
|
||||||
- `/it/pricing` renders in Italian, matching how the landing page
|
|
||||||
already works.
|
|
||||||
|
|
||||||
## Non-goals
|
|
||||||
|
|
||||||
- Adding currencies beyond GBP/EUR/USD. Each would need
|
|
||||||
`currency_options` on both live Prices first.
|
|
||||||
- VAT calculation or Stripe Tax. `automatic_tax` is currently `false`;
|
|
||||||
see Open Questions.
|
|
||||||
- Changing the monthly/annual plan structure. Annual keeps its 14-day
|
|
||||||
trial, monthly keeps immediate billing with a waiver.
|
|
||||||
- Localising any public page other than `/pricing`.
|
|
||||||
|
|
||||||
## Design
|
|
||||||
|
|
||||||
### Two axes, both user-switchable
|
|
||||||
|
|
||||||
| Axis | Values | Detection order | Cookie |
|
|
||||||
|---|---|---|---|
|
|
||||||
| Language | `en`, `it` | existing `detect_public_lang` | `rtm.lang` |
|
|
||||||
| Currency | `gbp`, `eur`, `usd` | cookie → country → Accept-Language → `gbp` | `rtm.ccy` |
|
|
||||||
|
|
||||||
An earlier draft added a third, non-switchable `jurisdiction` axis to
|
|
||||||
select between UK Reg-36 and Italian art. 59 consent wording. It was
|
|
||||||
dropped: see "Consent wording" below. Nothing legally operative is
|
|
||||||
derived from IP geolocation.
|
|
||||||
|
|
||||||
### Data flow
|
|
||||||
|
|
||||||
```
|
|
||||||
GET /pricing (or /it/pricing)
|
|
||||||
├─ lang = detect_public_lang(cookie, accept-language, cf-country, user)
|
|
||||||
├─ currency = detect_currency(cookie, cf-country, accept-language)
|
|
||||||
│ overridden by users.stripe_currency when set
|
|
||||||
└─ amounts = pricing_catalog.get(currency)
|
|
||||||
↓
|
|
||||||
render symbol + amounts + computed saving %, in `lang`
|
|
||||||
↓
|
|
||||||
POST /api/stripe/checkout {cadence, currency}
|
|
||||||
currency honoured only when the user has no stripe_customer_id
|
|
||||||
```
|
|
||||||
|
|
||||||
### Components
|
|
||||||
|
|
||||||
**`app/services/pricing_catalog.py`** (new)
|
|
||||||
|
|
||||||
Reads both Prices with `expand[]=currency_options`, caches the result in
|
|
||||||
memory for 1 hour, and exposes:
|
|
||||||
|
|
||||||
```python
|
|
||||||
get(currency: str) -> PriceSet # monthly, annual, symbol, saving_pct
|
|
||||||
available() -> list[str] # currencies present on BOTH prices
|
|
||||||
```
|
|
||||||
|
|
||||||
`saving_pct` is computed as `1 - annual / (12 * monthly)` and rounded
|
|
||||||
down to a whole percent. It is never written by hand — this is what
|
|
||||||
structurally prevents a repeat of the €80-vs-€84 drift.
|
|
||||||
|
|
||||||
Knows nothing about HTTP, requests, or templates. Takes a Stripe client
|
|
||||||
as a constructor argument so tests inject a fake.
|
|
||||||
|
|
||||||
**Currency detection** — added to `app/services/locales.py` next to
|
|
||||||
`detect_public_lang`, reusing its country tables rather than starting a
|
|
||||||
parallel module. Pure function, no I/O:
|
|
||||||
|
|
||||||
```python
|
|
||||||
detect_currency(cookie_ccy, cf_country, accept_language, allowed) -> str
|
|
||||||
```
|
|
||||||
|
|
||||||
Priority: an explicit cookie beats everything; then `CF-IPCountry`;
|
|
||||||
then the first `Accept-Language` tag; then `gbp`. `allowed` is passed in
|
|
||||||
by the caller from `pricing_catalog.available()` — the function stays
|
|
||||||
pure and does no I/O of its own; anything not in `allowed` falls through
|
|
||||||
to the next rule.
|
|
||||||
|
|
||||||
The country table is carried over unchanged from the removed version,
|
|
||||||
including `CA -> usd`. No CAD price exists, so every choice for Canada is
|
|
||||||
a proxy; USD is the closest familiar one. Adding a real CAD
|
|
||||||
`currency_options` entry would be the actual fix, and is out of scope.
|
|
||||||
|
|
||||||
**`/pricing` route** (`app/routers/public.py`) gains the currency in its
|
|
||||||
context and a sibling `/it/pricing` route. Copy moves into the existing
|
|
||||||
`app/locales/{en,it}.yaml` under a `pricing.` key, matching the landing
|
|
||||||
page. A `?ccy=` query parameter sets the cookie and redirects, so the
|
|
||||||
switcher works without JavaScript.
|
|
||||||
|
|
||||||
**`/api/stripe/checkout`** restores the `currency` field on
|
|
||||||
`CheckoutRequest`, validated against `pricing_catalog.available()`, and
|
|
||||||
passes it only when `user.stripe_customer_id` is unset. This reverts the
|
|
||||||
mechanical part of `4169a67` while keeping its guarantee: the page and
|
|
||||||
the charge always agree, because both now read the same catalog.
|
|
||||||
|
|
||||||
### Consent wording
|
|
||||||
|
|
||||||
The monthly waiver currently cites *Regulation 36 of the Consumer
|
|
||||||
Contracts Regulations 2013*. That is UK law; for a customer resident
|
|
||||||
elsewhere the citation does not apply, and an ineffective waiver means a
|
|
||||||
monthly subscriber retains the 14-day refund right the checkbox was
|
|
||||||
meant to remove.
|
|
||||||
|
|
||||||
UK Reg 36 and Italian `Codice del Consumo` art. 59 both implement
|
|
||||||
Directive 2011/83/EU art. 16(m). The waiver takes effect from its
|
|
||||||
substance — an express request for immediate performance plus an
|
|
||||||
acknowledgement that the cancellation right is lost — not from the
|
|
||||||
citation. Wording that states the substance and cites no statute is
|
|
||||||
therefore effective under both regimes, whereas citing the wrong one is
|
|
||||||
worse than citing none.
|
|
||||||
|
|
||||||
New wording, in place of the current sentence:
|
|
||||||
|
|
||||||
> I request that the service starts immediately, and I understand that
|
|
||||||
> once it has started I lose my right to cancel and get a refund.
|
|
||||||
|
|
||||||
The Terms-of-Service agreement in the same checkbox is unchanged. The
|
|
||||||
Italian rendering of this sentence is a translation of substance, not of
|
|
||||||
a statutory reference, so it carries the same weight as the existing
|
|
||||||
`auth.ack` translations.
|
|
||||||
|
|
||||||
This wording is subject to the legal sign-off already tracked on the
|
|
||||||
launch blocker list. It is not a lawyer-authored sentence.
|
|
||||||
|
|
||||||
### Locked currency
|
|
||||||
|
|
||||||
Stripe locks currency to the Customer at creation. A returning customer
|
|
||||||
whose subscription lapsed could otherwise be shown €7 and billed £7.
|
|
||||||
|
|
||||||
Add `users.stripe_currency` (`String(3)`, nullable), populated in
|
|
||||||
`_grant_paid` from the subscription object. When set, `/pricing` renders
|
|
||||||
that currency and disables the switcher with a one-line explanation.
|
|
||||||
Requires a small Alembic migration.
|
|
||||||
|
|
||||||
The simpler alternative — disable the switcher for anyone with a
|
|
||||||
`stripe_customer_id`, without storing the currency — is rejected because
|
|
||||||
it still shows a possibly-wrong currency; it only stops the user
|
|
||||||
changing it.
|
|
||||||
|
|
||||||
### Failure modes
|
|
||||||
|
|
||||||
| Condition | Behaviour |
|
|
||||||
|---|---|
|
|
||||||
| Stripe unreachable, warm cache | Serve stale cache indefinitely; log a warning |
|
|
||||||
| Stripe unreachable, cold cache | Static GBP amounts, switcher hidden — i.e. exactly today's page |
|
|
||||||
| Requested currency absent from a Price | Excluded from `available()`, so unreachable |
|
|
||||||
| `?ccy=` with an unknown value | Ignored, cookie untouched |
|
|
||||||
|
|
||||||
The page never returns an error because of a pricing lookup.
|
|
||||||
|
|
||||||
## Testing
|
|
||||||
|
|
||||||
- `detect_currency` — table-driven unit tests over the priority chain,
|
|
||||||
including values outside `allowed` falling through to the next rule.
|
|
||||||
- `pricing_catalog` — fake Stripe client: happy path, `saving_pct`
|
|
||||||
arithmetic, currency missing from one Price but not the other, cold-cache
|
|
||||||
failure, stale-cache-on-failure.
|
|
||||||
- Route tests — `/pricing` and `/it/pricing` render expected symbols and
|
|
||||||
amounts per cookie/header combination; `?ccy=` sets the cookie.
|
|
||||||
- **Cross-check test:** for each currency, assert the amount rendered in
|
|
||||||
the page equals the amount Stripe would charge for the currency
|
|
||||||
checkout sends. This is the regression guard for the original bug and
|
|
||||||
is the most important test in the set.
|
|
||||||
- Locked-currency test — a user with `stripe_currency` set sees that
|
|
||||||
currency regardless of headers or cookie.
|
|
||||||
|
|
||||||
## Open questions
|
|
||||||
|
|
||||||
1. **EU VAT.** `automatic_tax` is `false`, so no VAT is charged. B2C
|
|
||||||
digital services sold into the EU have no VAT threshold — VAT is due
|
|
||||||
in the customer's member state from the first sale, normally via a
|
|
||||||
non-Union OSS registration. Displaying EUR does not create this
|
|
||||||
obligation, but selling to EU consumers does. Resolve before taking
|
|
||||||
EUR money. Registration decision, not a code change.
|
|
||||||
2. **`billing_address_collection`.** Currently unset, so Stripe defaults
|
|
||||||
to `auto` and may capture only a postal code. Setting it to
|
|
||||||
`required` puts a country on every Customer record — useful for the
|
|
||||||
VAT question above and for knowing where customers are. Recommended,
|
|
||||||
independent of this feature.
|
|
||||||
|
|
||||||
## Out of scope / follow-ups
|
|
||||||
|
|
||||||
- `customer.subscription.paused` and `.resumed` are subscribed at Stripe
|
|
||||||
but absent from `_HANDLERS`. Harmless while pause is disabled in the
|
|
||||||
portal configuration, but a live trap if it is ever enabled.
|
|
||||||
- Localising `/terms` and `/privacy`, which the Italian pricing page
|
|
||||||
will link to in English.
|
|
||||||
|
|
@ -341,131 +341,6 @@ def test_subscription_active_grants_paid(tmp_path):
|
||||||
assert asyncio.run(_check_tier()) == "paid"
|
assert asyncio.run(_check_tier()) == "paid"
|
||||||
|
|
||||||
|
|
||||||
# --- pause / resume --------------------------------------------------------
|
|
||||||
|
|
||||||
|
|
||||||
def _activate(client, *, customer="cus_p", subscription="sub_p", evt="evt_a"):
|
|
||||||
"""Link user 1 to a Stripe customer and put them on paid."""
|
|
||||||
return _post_webhook(client, body={
|
|
||||||
"id": evt,
|
|
||||||
"type": "checkout.session.completed",
|
|
||||||
"data": {"object": {
|
|
||||||
"client_reference_id": "1",
|
|
||||||
"customer": customer,
|
|
||||||
"subscription": subscription,
|
|
||||||
}},
|
|
||||||
})
|
|
||||||
|
|
||||||
|
|
||||||
def _tier_and_sub(factory):
|
|
||||||
async def _check():
|
|
||||||
from sqlalchemy import select
|
|
||||||
from app.models import User
|
|
||||||
async with factory() as session:
|
|
||||||
u = (await session.execute(
|
|
||||||
select(User).where(User.id == 1)
|
|
||||||
)).scalar_one()
|
|
||||||
return u.tier, u.stripe_subscription_id
|
|
||||||
return asyncio.run(_check())
|
|
||||||
|
|
||||||
|
|
||||||
def test_subscription_paused_drops_tier(tmp_path):
|
|
||||||
"""status=paused means Stripe has stopped collecting (trial ended
|
|
||||||
with no usable card). Paid features must come off — otherwise the
|
|
||||||
customer keeps everything for free."""
|
|
||||||
client, factory, _ = _build_app(tmp_path)
|
|
||||||
_activate(client)
|
|
||||||
assert _tier_and_sub(factory)[0] == "paid"
|
|
||||||
|
|
||||||
r = _post_webhook(client, body={
|
|
||||||
"id": "evt_paused",
|
|
||||||
"type": "customer.subscription.paused",
|
|
||||||
"data": {"object": {
|
|
||||||
"id": "sub_p", "customer": "cus_p", "status": "paused",
|
|
||||||
}},
|
|
||||||
})
|
|
||||||
assert r.status_code == 200, r.text
|
|
||||||
assert r.json()["status"] == "ok", "must not fall through to 'ignored'"
|
|
||||||
|
|
||||||
tier, sub = _tier_and_sub(factory)
|
|
||||||
assert tier == "free"
|
|
||||||
# The subscription still exists at Stripe and resumes under the same
|
|
||||||
# id, so we keep our link to it.
|
|
||||||
assert sub == "sub_p"
|
|
||||||
|
|
||||||
|
|
||||||
def test_subscription_resumed_regrants_tier(tmp_path):
|
|
||||||
client, factory, _ = _build_app(tmp_path)
|
|
||||||
_activate(client)
|
|
||||||
_post_webhook(client, body={
|
|
||||||
"id": "evt_paused2",
|
|
||||||
"type": "customer.subscription.paused",
|
|
||||||
"data": {"object": {
|
|
||||||
"id": "sub_p", "customer": "cus_p", "status": "paused",
|
|
||||||
}},
|
|
||||||
})
|
|
||||||
assert _tier_and_sub(factory)[0] == "free"
|
|
||||||
|
|
||||||
r = _post_webhook(client, body={
|
|
||||||
"id": "evt_resumed",
|
|
||||||
"type": "customer.subscription.resumed",
|
|
||||||
"data": {"object": {
|
|
||||||
"id": "sub_p", "customer": "cus_p", "status": "active",
|
|
||||||
}},
|
|
||||||
})
|
|
||||||
assert r.status_code == 200, r.text
|
|
||||||
assert r.json()["status"] == "ok"
|
|
||||||
assert _tier_and_sub(factory) == ("paid", "sub_p")
|
|
||||||
|
|
||||||
|
|
||||||
def test_pause_collection_drops_tier_despite_active_status(tmp_path):
|
|
||||||
"""The portal's pause uses `pause_collection` and leaves status as
|
|
||||||
`active`, so the status check alone would keep the user on paid while
|
|
||||||
Stripe bills them nothing."""
|
|
||||||
client, factory, _ = _build_app(tmp_path)
|
|
||||||
_activate(client)
|
|
||||||
assert _tier_and_sub(factory)[0] == "paid"
|
|
||||||
|
|
||||||
r = _post_webhook(client, body={
|
|
||||||
"id": "evt_pause_coll",
|
|
||||||
"type": "customer.subscription.updated",
|
|
||||||
"data": {"object": {
|
|
||||||
"id": "sub_p",
|
|
||||||
"customer": "cus_p",
|
|
||||||
"status": "active",
|
|
||||||
"pause_collection": {"behavior": "void"},
|
|
||||||
}},
|
|
||||||
})
|
|
||||||
assert r.status_code == 200, r.text
|
|
||||||
assert _tier_and_sub(factory) == ("free", "sub_p")
|
|
||||||
|
|
||||||
|
|
||||||
def test_unpause_collection_regrants_tier(tmp_path):
|
|
||||||
"""Resuming collection sends subscription.updated with
|
|
||||||
pause_collection cleared to null — that must grant paid back."""
|
|
||||||
client, factory, _ = _build_app(tmp_path)
|
|
||||||
_activate(client)
|
|
||||||
_post_webhook(client, body={
|
|
||||||
"id": "evt_pc_on",
|
|
||||||
"type": "customer.subscription.updated",
|
|
||||||
"data": {"object": {
|
|
||||||
"id": "sub_p", "customer": "cus_p", "status": "active",
|
|
||||||
"pause_collection": {"behavior": "void"},
|
|
||||||
}},
|
|
||||||
})
|
|
||||||
assert _tier_and_sub(factory)[0] == "free"
|
|
||||||
|
|
||||||
_post_webhook(client, body={
|
|
||||||
"id": "evt_pc_off",
|
|
||||||
"type": "customer.subscription.updated",
|
|
||||||
"data": {"object": {
|
|
||||||
"id": "sub_p", "customer": "cus_p", "status": "active",
|
|
||||||
"pause_collection": None,
|
|
||||||
}},
|
|
||||||
})
|
|
||||||
assert _tier_and_sub(factory) == ("paid", "sub_p")
|
|
||||||
|
|
||||||
|
|
||||||
# --- idempotency + unknown ------------------------------------------------
|
# --- idempotency + unknown ------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
|
@ -590,93 +465,50 @@ def test_checkout_endpoint_requires_login(tmp_path):
|
||||||
assert r.status_code == 401, r.text
|
assert r.status_code == 401, r.text
|
||||||
|
|
||||||
|
|
||||||
def test_checkout_never_passes_currency(tmp_path):
|
def test_checkout_passes_sniffed_currency_for_new_customer(tmp_path):
|
||||||
"""Every checkout bills the Price's base currency (GBP), whatever the
|
"""First-time buyer (no stripe_customer_id yet) gets the currency
|
||||||
visitor's geo headers say.
|
sniffed from the request. CF-IPCountry=US → 'usd', and Stripe will
|
||||||
|
look up the USD currency_option on the Price."""
|
||||||
/pricing renders "£7" and "£70" as static copy, so selecting a
|
|
||||||
`currency_options` rate would show one price and charge another —
|
|
||||||
a US visitor saw £7 and was billed $9.99. Regression guard: if
|
|
||||||
geo-pricing is ever reinstated, the pricing page must become
|
|
||||||
currency-aware in the same change.
|
|
||||||
"""
|
|
||||||
client, _, session_cookie = _build_app(tmp_path)
|
|
||||||
|
|
||||||
seen = []
|
|
||||||
|
|
||||||
def asserter(params):
|
|
||||||
seen.append(params)
|
|
||||||
assert "currency" not in params, (
|
|
||||||
"no currency may be sent — /pricing advertises GBP only"
|
|
||||||
)
|
|
||||||
|
|
||||||
for headers in (
|
|
||||||
{"cf-ipcountry": "US"},
|
|
||||||
{"cf-ipcountry": "DE"},
|
|
||||||
{"accept-language": "en-US,en;q=0.5"},
|
|
||||||
{},
|
|
||||||
):
|
|
||||||
with patch("app.routers.stripe_billing._stripe_client",
|
|
||||||
return_value=_fake_checkout_client(asserter)):
|
|
||||||
r = client.post(
|
|
||||||
"/api/stripe/checkout",
|
|
||||||
json={"cadence": "monthly"},
|
|
||||||
cookies={"cassandra_session": session_cookie},
|
|
||||||
headers=headers,
|
|
||||||
)
|
|
||||||
assert r.status_code == 200, r.text
|
|
||||||
|
|
||||||
assert len(seen) == 4
|
|
||||||
|
|
||||||
|
|
||||||
def test_checkout_rejects_currency_in_body(tmp_path):
|
|
||||||
"""The `currency` field is gone from CheckoutRequest. A client that
|
|
||||||
still sends one must not silently get GBP under a USD label — the
|
|
||||||
extra key is simply ignored by pydantic, so assert the call is still
|
|
||||||
currency-free rather than trusting the caller."""
|
|
||||||
client, _, session_cookie = _build_app(tmp_path)
|
client, _, session_cookie = _build_app(tmp_path)
|
||||||
|
|
||||||
def asserter(params):
|
def asserter(params):
|
||||||
assert "currency" not in params
|
assert params["currency"] == "usd"
|
||||||
|
|
||||||
with patch("app.routers.stripe_billing._stripe_client",
|
with patch("app.routers.stripe_billing._stripe_client",
|
||||||
return_value=_fake_checkout_client(asserter)):
|
return_value=_fake_checkout_client(asserter)):
|
||||||
r = client.post(
|
r = client.post(
|
||||||
"/api/stripe/checkout",
|
"/api/stripe/checkout",
|
||||||
json={"cadence": "monthly", "currency": "usd"},
|
json={"cadence": "monthly"},
|
||||||
cookies={"cassandra_session": session_cookie},
|
cookies={"cassandra_session": session_cookie},
|
||||||
|
headers={"cf-ipcountry": "US"},
|
||||||
)
|
)
|
||||||
assert r.status_code == 200, r.text
|
assert r.status_code == 200, r.text
|
||||||
|
|
||||||
|
|
||||||
def test_checkout_requires_billing_address(tmp_path):
|
def test_checkout_body_currency_overrides_sniff(tmp_path):
|
||||||
"""Every checkout must collect a billing address, so each Stripe
|
"""Explicit `currency` in the request body beats header sniffing —
|
||||||
Customer ends up with a country. EU B2C digital-services VAT is due
|
lets a UK-based buyer choose EUR if they want to."""
|
||||||
at the consumer's place of supply, and the card's billing country is
|
|
||||||
the evidence for that — an IP guess is not."""
|
|
||||||
client, _, session_cookie = _build_app(tmp_path)
|
client, _, session_cookie = _build_app(tmp_path)
|
||||||
|
|
||||||
def asserter(params):
|
def asserter(params):
|
||||||
assert params["billing_address_collection"] == "required"
|
assert params["currency"] == "eur"
|
||||||
# New customer (no stored customer id): customer_update is only
|
|
||||||
# valid alongside `customer`, so it must be absent here.
|
|
||||||
assert "customer_update" not in params
|
|
||||||
|
|
||||||
for cadence in ("monthly", "annual"):
|
with patch("app.routers.stripe_billing._stripe_client",
|
||||||
with patch("app.routers.stripe_billing._stripe_client",
|
return_value=_fake_checkout_client(asserter)):
|
||||||
return_value=_fake_checkout_client(asserter)):
|
r = client.post(
|
||||||
r = client.post(
|
"/api/stripe/checkout",
|
||||||
"/api/stripe/checkout",
|
json={"cadence": "monthly", "currency": "eur"},
|
||||||
json={"cadence": cadence},
|
cookies={"cassandra_session": session_cookie},
|
||||||
cookies={"cassandra_session": session_cookie},
|
headers={"cf-ipcountry": "GB"},
|
||||||
)
|
)
|
||||||
assert r.status_code == 200, r.text
|
assert r.status_code == 200, r.text
|
||||||
|
|
||||||
|
|
||||||
def test_checkout_uses_existing_customer_ref(tmp_path):
|
def test_checkout_omits_currency_for_existing_customer(tmp_path):
|
||||||
"""Existing customer: use the stored `customer` ref rather than
|
"""Existing customer: Stripe locked their currency at first
|
||||||
`customer_email`, so repeat checkouts don't mint duplicate Stripe
|
checkout, so passing `currency` again would error. Verify we omit
|
||||||
customers."""
|
it (and also use the existing `customer` ref instead of
|
||||||
|
customer_email)."""
|
||||||
import asyncio
|
import asyncio
|
||||||
|
|
||||||
from app.models import User
|
from app.models import User
|
||||||
|
|
@ -692,20 +524,36 @@ def test_checkout_uses_existing_customer_ref(tmp_path):
|
||||||
asyncio.run(_link())
|
asyncio.run(_link())
|
||||||
|
|
||||||
def asserter(params):
|
def asserter(params):
|
||||||
assert "currency" not in params
|
assert "currency" not in params, (
|
||||||
|
"currency must not be passed once a customer exists — "
|
||||||
|
"Stripe rejects mismatches against the locked customer currency"
|
||||||
|
)
|
||||||
assert params["customer"] == "cus_existing_xxxxxxxxxxxxxx"
|
assert params["customer"] == "cus_existing_xxxxxxxxxxxxxx"
|
||||||
assert "customer_email" not in params
|
|
||||||
# Without customer_update.address the collected address is
|
|
||||||
# attached to the payment only and the Customer record keeps a
|
|
||||||
# null address — i.e. still no country for the VAT question.
|
|
||||||
assert params["customer_update"] == {"address": "auto"}
|
|
||||||
|
|
||||||
with patch("app.routers.stripe_billing._stripe_client",
|
with patch("app.routers.stripe_billing._stripe_client",
|
||||||
return_value=_fake_checkout_client(asserter)):
|
return_value=_fake_checkout_client(asserter)):
|
||||||
r = client.post(
|
r = client.post(
|
||||||
"/api/stripe/checkout",
|
"/api/stripe/checkout",
|
||||||
json={"cadence": "monthly"},
|
json={"cadence": "monthly", "currency": "usd"},
|
||||||
cookies={"cassandra_session": session_cookie},
|
cookies={"cassandra_session": session_cookie},
|
||||||
headers={"cf-ipcountry": "US"},
|
headers={"cf-ipcountry": "US"},
|
||||||
)
|
)
|
||||||
assert r.status_code == 200, r.text
|
assert r.status_code == 200, r.text
|
||||||
|
|
||||||
|
|
||||||
|
def test_sniff_currency_fallback_chain():
|
||||||
|
"""Unit-test the header-sniffing helper: CF country wins, then
|
||||||
|
Accept-Language exact, then language-only, then GBP default."""
|
||||||
|
from types import SimpleNamespace
|
||||||
|
|
||||||
|
from app.routers.stripe_billing import _sniff_currency
|
||||||
|
|
||||||
|
def _req(headers):
|
||||||
|
return SimpleNamespace(headers=headers)
|
||||||
|
|
||||||
|
assert _sniff_currency(_req({"cf-ipcountry": "DE"})) == "eur"
|
||||||
|
assert _sniff_currency(_req({"cf-ipcountry": "us"})) == "usd" # case-insensitive
|
||||||
|
assert _sniff_currency(_req({"accept-language": "fr-FR,fr;q=0.9"})) == "eur"
|
||||||
|
assert _sniff_currency(_req({"accept-language": "en-US,en;q=0.5"})) == "usd"
|
||||||
|
assert _sniff_currency(_req({"accept-language": "ja,ja-JP;q=0.5"})) == "gbp"
|
||||||
|
assert _sniff_currency(_req({})) == "gbp"
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue