amadan
abono: Checkout takes cards only, and a payment still processing is not 'no payment was made'
Summary
Checkout sends payment_method_types[0]=card, so settlement is synchronous and a subscription Stripe calls active has been paid for. Finished gains Processing: a complete Checkout whose payment is still unpaid. The generated done page shows the new key abono.pages.done_processing for it, under the done title, with the account link and no second-purchase button. Also gofmt of holdings.go from the previous commit.
Agent notes
Final review C2, ruling R33. New key abono.pages.done_processing is rendered as a key and added to pendingReview (templates_test.go) and newKeys (cmd/abono/main_test.go) for the third mini copy review. The processing page reuses the approved title abono.pages.done_title. Example pages regenerated. Tests: TestCheckoutForm expects the card parameter; TestFinishCompleteButUnpaid expects Processing; example TestDoneWhilePaymentIsProcessing drives the page through a proxy that reports the session unpaid.
11 files changed,
+123
−18
checkout.go+17 −5checkout_test.go+3 −0cmd/abono/main_test.go+1 −1cmd/abono/pages/abono_pages.go.tmpl+7 −1cmd/abono/pages/done.html.tmpl+8 −2example/abono_pages.go+7 −1example/app_test.go+64 −0example/templates/abono/done.html+8 −2holdings.go+1 −1sync_test.go+5 −5templates_test.go+2 −0
CI passed — run details
CI log
=== vet === go vet ./... === fmt === === test === go test ./... -count=1 ok amadan.net/rastrillo/abono 0.710s ok amadan.net/rastrillo/abono/abonotest 0.051s ok amadan.net/rastrillo/abono/cmd/abono 0.601s ok amadan.net/rastrillo/abono/internal/fakestripe 0.018s ok amadan.net/rastrillo/abono/internal/stripeapi 0.007s ok example 0.118s === race === CGO_ENABLED=1 go test ./... -race -count=1 ok amadan.net/rastrillo/abono 5.591s ok amadan.net/rastrillo/abono/abonotest 1.389s ok amadan.net/rastrillo/abono/cmd/abono 1.798s ok amadan.net/rastrillo/abono/internal/fakestripe 1.062s ok amadan.net/rastrillo/abono/internal/stripeapi 1.017s ok example 1.731s === browser === ok example 0.726s
Intent
diff --git a/checkout.go b/checkout.go| index ec36f47..464865c 100644 |
| --- a/checkout.go |
| +++ b/checkout.go |
| @@ -42,7 +42,11 @@ type CheckoutRequest struct { |
| } |
| // Checkout starts a hosted Checkout for a subscription to req.Plan and |
| -// returns its address. |
| +// returns its address. It takes cards only (ruling R33): a card settles |
| +// while Checkout waits, so a subscription Stripe calls active has been |
| +// paid for. Bank debits (ACH, SEPA) can leave a subscription active while |
| +// the money is on its way, or after it failed, and v0.1 does not handle |
| +// that. |
| func (s *Service) Checkout(ctx context.Context, req CheckoutRequest) (string, error) { |
| if !s.On() { |
| return "", errOff |
| @@ -70,6 +74,7 @@ func (s *Service) Checkout(ctx context.Context, req CheckoutRequest) (string, er |
| "success_url": {success}, |
| "cancel_url": {cancel}, |
| "subscription_data[metadata][abono_plan]": {req.Plan}, |
| + "payment_method_types[0]": {"card"}, |
| } |
| customer := "" |
| if subject := strings.TrimSpace(req.Subject); subject != "" { |
| @@ -222,17 +227,20 @@ func customerOf(g *gorm.DB, subject string) (string, error) { |
| // Finished is what Checkout's return found: the address paid with, the |
| // subject the subscription is now tied to ("" when unclaimed), and whether |
| -// it was paid. |
| +// it was paid. Processing is a Checkout that completed while its payment |
| +// is still on its way: nothing is recorded yet, the webhook does that when |
| +// it lands, and the reader must not be asked to buy again. |
| type Finished struct { |
| Email, Subject string |
| Paid bool |
| + Processing bool |
| } |
| // Finish is Checkout's return. It syncs the session's subscription at once |
| // rather than waiting for the webhook, so the next page already reflects |
| // the payment. It signs nobody in. Paid is true only for a complete session |
| -// whose payment_status is paid or no_payment_required; any other session |
| -// records nothing. |
| +// whose payment_status is paid or no_payment_required; a complete session |
| +// still unpaid is Processing; any other session records nothing. |
| func (s *Service) Finish(ctx context.Context, sessionID string) (Finished, error) { |
| if !s.On() { |
| return Finished{}, errOff |
| @@ -247,7 +255,11 @@ func (s *Service) Finish(ctx context.Context, sessionID string) (Finished, error |
| // webhook syncs when the payment lands. A trial or a 100% code has |
| // nothing to pay, and is paid. |
| paid := cs.PaymentStatus == "paid" || cs.PaymentStatus == "no_payment_required" |
| - if cs.Status != "complete" || !paid || cs.Subscription == "" { |
| + if cs.Status == "complete" && !paid { |
| + f.Processing = true |
| + return f, nil |
| + } |
| + if cs.Status != "complete" || cs.Subscription == "" { |
| return f, nil |
| } |
| sub, err := s.stripe.GetSubscription(ctx, cs.Subscription) |
diff --git a/checkout_test.go b/checkout_test.go| index 09a15d3..2afae9c 100644 |
| --- a/checkout_test.go |
| +++ b/checkout_test.go |
| @@ -26,6 +26,9 @@ func TestCheckoutForm(t *testing.T) { |
| "success_url": {"https://site.example/join/done?session_id={CHECKOUT_SESSION_ID}"}, |
| "cancel_url": {"https://site.example/join/annual"}, |
| "subscription_data[metadata][abono_plan]": {"annual"}, |
| + // Card only (ruling R33): settlement is synchronous, so a |
| + // subscription Stripe calls active has been paid for. |
| + "payment_method_types[0]": {"card"}, |
| } |
| with := func(kv ...string) url.Values { |
| v := url.Values{} |
diff --git a/cmd/abono/main_test.go b/cmd/abono/main_test.go| index 2b6dcce..ef9466d 100644 |
| --- a/cmd/abono/main_test.go |
| +++ b/cmd/abono/main_test.go |
| @@ -284,7 +284,7 @@ var partialKeys = []string{"abono.price.per_month", "abono.price.per_year"} |
| // newKeys are keys the pages need that the draft lacks (ruling R28), |
| // going through their own copy review. The pages must use each. |
| -var newKeys = []string{"abono.owner.free_no_member"} |
| +var newKeys = []string{"abono.owner.free_no_member", "abono.pages.done_processing"} |
| var keyLiteral = regexp.MustCompile(`"(abono\.[a-z][a-z_.]*[a-z])"`) |
diff --git a/cmd/abono/pages/abono_pages.go.tmpl b/cmd/abono/pages/abono_pages.go.tmpl| index 7fc9adb..4f83995 100644 |
| --- a/cmd/abono/pages/abono_pages.go.tmpl |
| +++ b/cmd/abono/pages/abono_pages.go.tmpl |
| @@ -263,6 +263,7 @@ func (p *AbonoPages) done(w http.ResponseWriter, r *http.Request) { |
| data := map[string]any{ |
| "Title": "abono.pages.done_unpaid_title", |
| "Paid": false, |
| + "Processing": false, |
| "Email": "", |
| "SignedIn": signedIn, |
| "AccountURL": abonoAccountPath, |
| @@ -286,8 +287,13 @@ func (p *AbonoPages) done(w http.ResponseWriter, r *http.Request) { |
| p.render(w, r, http.StatusBadGateway, "abono/done", data) |
| return |
| } |
| - if f.Paid { |
| + switch { |
| + case f.Paid: |
| data["Title"], data["Paid"], data["Email"] = "abono.pages.done_title", true, f.Email |
| + case f.Processing: |
| + // Complete, the money still on its way: not "no payment was made", |
| + // and no second purchase. |
| + data["Title"], data["Processing"] = "abono.pages.done_title", true |
| } |
| p.render(w, r, http.StatusOK, "abono/done", data) |
| } |
diff --git a/cmd/abono/pages/done.html.tmpl b/cmd/abono/pages/done.html.tmpl| index 850198d..04fb506 100644 |
| --- a/cmd/abono/pages/done.html.tmpl |
| +++ b/cmd/abono/pages/done.html.tmpl |
| @@ -1,11 +1,14 @@ |
| {{/* abono/done: GET /subscribe/done, where Checkout sends the reader |
| back. Paid, it thanks them; a reader who paid signed out is asked to |
| - sign in with the address they paid with. Not paid, it says nothing |
| - was charged and offers the plans again. |
| + sign in with the address they paid with. Processing (Checkout |
| + completed, the payment still on its way), it says so and offers no |
| + second purchase. Not paid, it says nothing was charged and offers the |
| + plans again. |
| Data: |
| Title string, the page title's message key |
| Paid bool |
| + Processing bool, complete but the payment still processing |
| Email string, the address paid with |
| SignedIn bool |
| AccountURL string, the account page |
| @@ -19,6 +22,9 @@ |
| {{if .SignedIn}}<p><a rst-btn="primary" href="{{.AccountURL}}">{{T $L "abono.button.go_to_account"}}</a></p> |
| {{else}}<p>{{Tf $L "abono.pages.done_signin" "email" .Email}}</p> |
| {{end}} |
| +{{else if .Processing}} |
| +<p>{{T $L "abono.pages.done_processing"}}</p> |
| +{{if .SignedIn}}<p><a rst-btn="primary" href="{{.AccountURL}}">{{T $L "abono.button.go_to_account"}}</a></p>{{end}} |
| {{else if .Error}} |
| {{template "callout" dict "L" $L "Tone" "negative" "Focus" true "Body" (T $L .Error)}} |
| {{if .SignedIn}}<p><a rst-btn="primary" href="{{.AccountURL}}">{{T $L "abono.button.go_to_account"}}</a></p>{{end}} |
diff --git a/example/abono_pages.go b/example/abono_pages.go| index f975d86..c1db06e 100644 |
| --- a/example/abono_pages.go |
| +++ b/example/abono_pages.go |
| @@ -263,6 +263,7 @@ func (p *AbonoPages) done(w http.ResponseWriter, r *http.Request) { |
| data := map[string]any{ |
| "Title": "abono.pages.done_unpaid_title", |
| "Paid": false, |
| + "Processing": false, |
| "Email": "", |
| "SignedIn": signedIn, |
| "AccountURL": abonoAccountPath, |
| @@ -286,8 +287,13 @@ func (p *AbonoPages) done(w http.ResponseWriter, r *http.Request) { |
| p.render(w, r, http.StatusBadGateway, "abono/done", data) |
| return |
| } |
| - if f.Paid { |
| + switch { |
| + case f.Paid: |
| data["Title"], data["Paid"], data["Email"] = "abono.pages.done_title", true, f.Email |
| + case f.Processing: |
| + // Complete, the money still on its way: not "no payment was made", |
| + // and no second purchase. |
| + data["Title"], data["Processing"] = "abono.pages.done_title", true |
| } |
| p.render(w, r, http.StatusOK, "abono/done", data) |
| } |
diff --git a/example/app_test.go b/example/app_test.go| index 6af6e86..cfc7d2c 100644 |
| --- a/example/app_test.go |
| +++ b/example/app_test.go |
| @@ -1,18 +1,22 @@ |
| package main |
| import ( |
| + "bytes" |
| "context" |
| + "encoding/json" |
| "io" |
| "io/fs" |
| "log/slog" |
| "net/http" |
| "net/http/cookiejar" |
| "net/http/httptest" |
| + "net/http/httputil" |
| "net/url" |
| "os" |
| "path" |
| "path/filepath" |
| "regexp" |
| + "strconv" |
| "strings" |
| "sync" |
| "testing" |
| @@ -545,3 +549,63 @@ func TestMachinePathsSkipCSRF(t *testing.T) { |
| } |
| } |
| } |
| + |
| +// TestDoneWhilePaymentIsProcessing is final review C2 (ruling R33): a |
| +// Checkout that completed while its payment is still on its way is not |
| +// "no payment was made". The return page says it is processing and offers |
| +// no second purchase. abonotest's Checkout always pays at once, so a proxy |
| +// in front of it reports the session's payment as still unpaid. |
| +func TestDoneWhilePaymentIsProcessing(t *testing.T) { |
| + stripe := abonotest.New(t) |
| + stripe.AddPrice("price_annual", "annual", 6000, "eur", "year") |
| + stripe.AddPrice("price_monthly", "monthly", 600, "eur", "month") |
| + target, err := url.Parse(stripe.URL) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + proxy := httputil.NewSingleHostReverseProxy(target) |
| + proxy.ModifyResponse = func(resp *http.Response) error { |
| + if resp.Request.Method != http.MethodGet || !strings.HasPrefix(resp.Request.URL.Path, "/v1/checkout/sessions/") { |
| + return nil |
| + } |
| + var cs map[string]any |
| + if err := json.NewDecoder(resp.Body).Decode(&cs); err != nil { |
| + return err |
| + } |
| + resp.Body.Close() |
| + cs["payment_status"] = "unpaid" |
| + b, err := json.Marshal(cs) |
| + if err != nil { |
| + return err |
| + } |
| + resp.Body = io.NopCloser(bytes.NewReader(b)) |
| + resp.ContentLength = int64(len(b)) |
| + resp.Header.Set("Content-Length", strconv.Itoa(len(b))) |
| + return nil |
| + } |
| + front := httptest.NewServer(proxy) |
| + t.Cleanup(front.Close) |
| + |
| + app := newApp(t, func(c *Config) { |
| + c.Abono = func(ac abono.Config) abono.Config { |
| + ac = stripe.Config(ac) |
| + ac.StripeBase = front.URL |
| + return ac |
| + } |
| + }) |
| + stripe.SetWebhookURL(app.URL + abono.WebhookPath) |
| + reader := newClient(t, app) |
| + reader.signin("reader@example.com") |
| + status, loc, body := reader.post("/subscribe/annual", nil) |
| + if status != http.StatusSeeOther || !strings.HasPrefix(loc, stripe.URL+"/checkout/") { |
| + t.Fatalf("POST /subscribe/annual: %d to %q\n%s", status, loc, body) |
| + } |
| + status, body = reader.get(stripe.Pay(t, loc, "")) |
| + want(t, "Checkout's return while the payment processes", status, http.StatusOK, body, |
| + say("abono.pages.done_processing"), `href="/account"`) |
| + for _, wrong := range []string{say("abono.pages.done_unpaid_title"), say("abono.pages.done_unpaid_body"), say("abono.button.try_again")} { |
| + if strings.Contains(body, wrong) { |
| + t.Errorf("the processing page says %s", wrong) |
| + } |
| + } |
| +} |
diff --git a/example/templates/abono/done.html b/example/templates/abono/done.html| index 850198d..04fb506 100644 |
| --- a/example/templates/abono/done.html |
| +++ b/example/templates/abono/done.html |
| @@ -1,11 +1,14 @@ |
| {{/* abono/done: GET /subscribe/done, where Checkout sends the reader |
| back. Paid, it thanks them; a reader who paid signed out is asked to |
| - sign in with the address they paid with. Not paid, it says nothing |
| - was charged and offers the plans again. |
| + sign in with the address they paid with. Processing (Checkout |
| + completed, the payment still on its way), it says so and offers no |
| + second purchase. Not paid, it says nothing was charged and offers the |
| + plans again. |
| Data: |
| Title string, the page title's message key |
| Paid bool |
| + Processing bool, complete but the payment still processing |
| Email string, the address paid with |
| SignedIn bool |
| AccountURL string, the account page |
| @@ -19,6 +22,9 @@ |
| {{if .SignedIn}}<p><a rst-btn="primary" href="{{.AccountURL}}">{{T $L "abono.button.go_to_account"}}</a></p> |
| {{else}}<p>{{Tf $L "abono.pages.done_signin" "email" .Email}}</p> |
| {{end}} |
| +{{else if .Processing}} |
| +<p>{{T $L "abono.pages.done_processing"}}</p> |
| +{{if .SignedIn}}<p><a rst-btn="primary" href="{{.AccountURL}}">{{T $L "abono.button.go_to_account"}}</a></p>{{end}} |
| {{else if .Error}} |
| {{template "callout" dict "L" $L "Tone" "negative" "Focus" true "Body" (T $L .Error)}} |
| {{if .SignedIn}}<p><a rst-btn="primary" href="{{.AccountURL}}">{{T $L "abono.button.go_to_account"}}</a></p>{{end}} |
diff --git a/holdings.go b/holdings.go| index 3e0aa44..8e9e5c1 100644 |
| --- a/holdings.go |
| +++ b/holdings.go |
| @@ -211,7 +211,7 @@ func (s *Service) holdings(ctx context.Context, db *gorm.DB, subject string) ([] |
| Plan: r.Plan, Source: SourceSubscription, Status: r.Status, |
| Until: utc(r.CurrentPeriodEnd), PeriodStart: utc(r.CurrentPeriodStart), UnpaidSince: utc(r.UnpaidSince), |
| CancelAtPeriodEnd: r.CancelAtPeriodEnd, |
| - SubscriptionID: r.StripeSubscriptionID, |
| + SubscriptionID: r.StripeSubscriptionID, |
| } |
| h.Grants, h.Backstop = h.evaluate(now) |
| all = append(all, dated{h, r.CreatedAt}) |
diff --git a/sync_test.go b/sync_test.go| index 329134e..7ee8af5 100644 |
| --- a/sync_test.go |
| +++ b/sync_test.go |
| @@ -374,8 +374,8 @@ func TestFinishUnpaidIsNotPaid(t *testing.T) { |
| if err != nil { |
| t.Fatalf("Finish: %v", err) |
| } |
| - if f.Paid { |
| - t.Errorf("Finish of an open session = %+v, want not paid", f) |
| + if f.Paid || f.Processing { |
| + t.Errorf("Finish of an open session = %+v, want neither paid nor processing", f) |
| } |
| if got := db.dump(rowsQuery); got != "" { |
| t.Errorf("an unpaid session made rows:\n%s", got) |
| @@ -419,8 +419,8 @@ func TestFinishCompleteButUnpaid(t *testing.T) { |
| if err != nil { |
| t.Fatalf("Finish: %v", err) |
| } |
| - if f.Paid { |
| - t.Errorf("Finish of a complete but unpaid session = %+v, want not paid", f) |
| + if f.Paid || !f.Processing { |
| + t.Errorf("Finish of a complete but unpaid session = %+v, want processing, not paid", f) |
| } |
| if got := db.dump(rowsQuery); got != "" { |
| t.Errorf("an unpaid session made rows:\n%s", got) |
| @@ -434,7 +434,7 @@ func TestFinishCompleteButUnpaid(t *testing.T) { |
| if err != nil { |
| t.Fatalf("Finish with nothing to pay: %v", err) |
| } |
| - if !f.Paid || f.Subject != "42" { |
| + if !f.Paid || f.Processing || f.Subject != "42" { |
| t.Errorf("Finish with nothing to pay = %+v, want paid by 42", f) |
| } |
| if got := db.dump("SELECT subject FROM abono_subscriptions"); got != "42\n" { |
diff --git a/templates_test.go b/templates_test.go| index f6c0fac..674f2ad 100644 |
| --- a/templates_test.go |
| +++ b/templates_test.go |
| @@ -667,6 +667,8 @@ func TestUsedKeysSeesEveryConsumer(t *testing.T) { |
| // that fails. |
| var pendingReview = []string{ |
| "abono.owner.free_no_member", |
| + // The final-review fix wave's keys, for the third mini copy review. |
| + "abono.pages.done_processing", |
| } |
| // Every key a consumer uses is in en.toml (bar the pendingReview ones), and |