amadan
abono: reconcile ties a recovered subscription to its buyer, from Checkout's metadata
Summary
Reconcile syncs each listed subscription with the hint a subscription event carries: the abono_subject metadata Checkout set for a signed-in buyer, then the customer's address. Before, it passed no hint, so a subscription whose return and webhooks were lost went to whoever the app knew by the payment address, or stayed unclaimed. The webhook and reconcile now share subscriptionHint.
Agent notes
Final review I1, ruling R34. Regression: TestReconcileRestoresTheSignedInBuyer (A buys with B's address, every event lost; reconcile gives it to A and maps the customer to A).
3 files changed,
+42
−2
reconcile.go+4 −2reconcile_test.go+28 −0webhook.go+10 −0
CI passed — run details
CI log
=== vet === go vet ./... === fmt === === test === go test ./... -count=1 ok amadan.net/rastrillo/abono 0.729s ok amadan.net/rastrillo/abono/abonotest 0.051s ok amadan.net/rastrillo/abono/cmd/abono 0.626s ok amadan.net/rastrillo/abono/internal/fakestripe 0.018s ok amadan.net/rastrillo/abono/internal/stripeapi 0.006s ok example 0.119s === race === CGO_ENABLED=1 go test ./... -race -count=1 ok amadan.net/rastrillo/abono 5.628s ok amadan.net/rastrillo/abono/abonotest 1.390s ok amadan.net/rastrillo/abono/cmd/abono 1.803s ok amadan.net/rastrillo/abono/internal/fakestripe 1.056s ok amadan.net/rastrillo/abono/internal/stripeapi 1.017s ok example 1.720s === browser === ok example 0.709s
Intent
diff --git a/reconcile.go b/reconcile.go| index 45b974d..3e5ec92 100644 |
| --- a/reconcile.go |
| +++ b/reconcile.go |
| @@ -19,7 +19,9 @@ type Report struct { |
| } |
| // Reconcile is the daily tick's work. It lists every subscription on the |
| -// Stripe account and syncs each with no hint, which makes up for a webhook |
| +// Stripe account and syncs each with what the subscription itself says of |
| +// its buyer (Checkout's abono_subject metadata, then the customer's |
| +// address, as a subscription event does), which makes up for a webhook |
| // Stripe gave up on (a subscription that fails to sync is reported, by id, |
| // and the rest carry on; only the listing itself failing stops the walk); it refreshes every price's interval, amount and |
| // currency; and it runs OnChange for every distinct subject that holds any |
| @@ -41,7 +43,7 @@ func (s *Service) Reconcile(ctx context.Context) (Report, error) { |
| rep.Subscriptions++ |
| // One subscription failing (the app's Holder or OnChange, say) |
| // must not stop the rest being refreshed on every tick after. |
| - if err := s.sync(ctx, sub, Hint{}); err != nil { |
| + if err := s.sync(ctx, sub, subscriptionHint(sub)); err != nil { |
| errs = append(errs, fmt.Errorf("abono: reconcile: subscription %s: %w", sub.ID, err)) |
| } |
| return nil |
diff --git a/reconcile_test.go b/reconcile_test.go| index d93b9d5..1d78a0a 100644 |
| --- a/reconcile_test.go |
| +++ b/reconcile_test.go |
| @@ -196,3 +196,31 @@ func TestReconcileCarriesOnPastOneBadSubscription(t *testing.T) { |
| t.Errorf("price_M amount = %q", got) |
| } |
| } |
| + |
| +// TestReconcileRestoresTheSignedInBuyer is final review I1 (ruling R34): |
| +// subject A, signed in, pays with an address that belongs to B, and both |
| +// Checkout's return and the webhooks are lost. The reconcile that recovers |
| +// the subscription reads Checkout's abono_subject metadata, so it is A's, |
| +// not B's by address, and not left unclaimed. |
| +func TestReconcileRestoresTheSignedInBuyer(t *testing.T) { |
| + now := t0 |
| + d := newStripeDouble(t) |
| + rec := &recorder{subject: "B"} // the app knows the address as B's |
| + s, db := d.service(t, &now, rec.config()) |
| + seedPlans(t, s) |
| + d.put(t, subSpec{ID: "sub_1", Status: "active", Price: "price_A", Customer: "cus_1", Email: "b@example.com", |
| + End: t0.Add(30 * day), Metadata: map[string]string{"abono_subject": "A", "abono_plan": "annual"}}) |
| + |
| + if _, err := s.Reconcile(context.Background()); err != nil { |
| + t.Fatalf("Reconcile: %v", err) |
| + } |
| + if got := db.dump(subsQuery); got != "sub_1|A|annual|active|b@example.com\n" { |
| + t.Errorf("subscriptions:\n%s\nwant sub_1 held by A, who bought it", got) |
| + } |
| + if got := db.dump(`SELECT stripe_customer_id, subject FROM abono_customers`); got != "cus_1|A\n" { |
| + t.Errorf("customers = %q, want cus_1 mapped to A", got) |
| + } |
| + if !hasPlan(t, s, "A", "annual") || hasPlan(t, s, "B", "annual") { |
| + t.Error("the plan went to the address's owner, not the buyer") |
| + } |
| +} |
diff --git a/webhook.go b/webhook.go| index 017f899..7b14508 100644 |
| --- a/webhook.go |
| +++ b/webhook.go |
| @@ -95,6 +95,16 @@ func (e event) hint(sub stripeapi.Subscription) Hint { |
| if e.isCheckout() { |
| return Hint{Subject: e.Data.Object.ClientReferenceID, Email: e.Data.Object.CustomerDetails.Email} |
| } |
| + return subscriptionHint(sub) |
| +} |
| + |
| +// subscriptionHint is what a subscription says of whose it is: the |
| +// abono_subject Checkout put in its metadata for a signed-in buyer, and its |
| +// customer's address. The webhook's subscription and invoice events and |
| +// the reconcile both use it, so a reconcile that recovers lost events ties |
| +// the subscription to whoever bought it, not to whoever owns the address |
| +// it was paid with (ruling R34). |
| +func subscriptionHint(sub stripeapi.Subscription) Hint { |
| return Hint{Subject: sub.Metadata["abono_subject"], Email: sub.Customer.Email} |
| } |