amadan
abono: an owner's adoption and a reader's claim pin the subscription's subject
Summary
abono_subscriptions gains subject_pinned. Adopt with a subject and Claim set it. While it is set, a hint (Checkout's client_reference_id, the subscription's abono_subject metadata) never changes the row's subject or remaps its customer; only another Adopt with a subject does. Before, the next invoice or subscription event, a revisit of Checkout's return, or the reconcile gave an adopted subscription back to whoever bought it.
Agent notes
Final review I3, ruling R36. migrations/0001_init.sql edited before v0.1.0 again: frozenAbonoChecksum re-recorded from 871e1b39... to 5701b242...; development databases need a reset. Regression: TestAnOwnersAdoptionOutlastsLaterHints (Adopt for B after A bought it; a subscription event, a Checkout event, Finish again and Reconcile all leave it with B; a claimed row ignores a later hint).
5 files changed,
+107
−13
adopt.go+5 −3adopt_test.go+67 −0migrations/0001_init.sql+4 −0schema_test.go+1 −1sync.go+30 −9
CI passed — run details
CI log
=== vet === go vet ./... === fmt === === test === go test ./... -count=1 ok amadan.net/rastrillo/abono 2.555s ok amadan.net/rastrillo/abono/abonotest 0.053s ok amadan.net/rastrillo/abono/cmd/abono 0.618s ok amadan.net/rastrillo/abono/internal/fakestripe 0.019s ok amadan.net/rastrillo/abono/internal/stripeapi 0.007s ok example 0.116s === race === CGO_ENABLED=1 go test ./... -race -count=1 ok amadan.net/rastrillo/abono 7.936s ok amadan.net/rastrillo/abono/abonotest 1.440s ok amadan.net/rastrillo/abono/cmd/abono 1.936s ok amadan.net/rastrillo/abono/internal/fakestripe 1.065s ok amadan.net/rastrillo/abono/internal/stripeapi 1.025s ok example 1.731s === browser === ok example 0.663s
Intent
diff --git a/adopt.go b/adopt.go| index 080d47a..775e477 100644 |
| --- a/adopt.go |
| +++ b/adopt.go |
| @@ -16,9 +16,11 @@ import ( |
| // every subscription on it means. |
| // |
| // With subject "", the holder is found as a sync finds it: the customer's |
| -// recorded subject, then Holder for the customer's address. The row is |
| -// recorded whatever the subscription's status, and OnChange runs in the |
| -// same transaction. |
| +// recorded subject, then Holder for the customer's address. With a |
| +// subject, that subject is pinned: Checkout's reference and the |
| +// subscription's abono_subject metadata, which may still name whoever |
| +// bought it, never move it back (ruling R36). The row is recorded whatever |
| +// the subscription's status, and OnChange runs in the same transaction. |
| func (s *Service) Adopt(ctx context.Context, subscriptionID, subject, plan string) error { |
| if !s.On() { |
| return errOff |
diff --git a/adopt_test.go b/adopt_test.go| index 70282f5..006c6ca 100644 |
| --- a/adopt_test.go |
| +++ b/adopt_test.go |
| @@ -3,6 +3,7 @@ package abono_test |
| import ( |
| "context" |
| "errors" |
| + "net/http" |
| "testing" |
| "amadan.net/rastrillo/abono" |
| @@ -106,3 +107,69 @@ func TestAdoptWithoutSubjectFindsTheHolder(t *testing.T) { |
| t.Errorf("Holder asked %q for a mapped customer", asked) |
| } |
| } |
| + |
| +// TestAnOwnersAdoptionOutlastsLaterHints is final review I3 (ruling R36): |
| +// A bought the subscription signed in, so its metadata and its Checkout |
| +// session both name A. The owner then adopts it for B. Later events, a |
| +// second visit to Checkout's return and the reconcile all carry A as a |
| +// hint; none of them gives it back to A. Claim pins a row the same way. |
| +func TestAnOwnersAdoptionOutlastsLaterHints(t *testing.T) { |
| + now := t0 |
| + d := newStripeDouble(t) |
| + rec := &recorder{} |
| + s, db := d.service(t, &now, rec.config()) |
| + seedPlans(t, s) |
| + ctx := context.Background() |
| + |
| + d.put(t, subSpec{ID: "sub_1", Status: "active", Price: "price_A", Customer: "cus_1", Email: "ana@example.com", |
| + End: t0.Add(30 * day), Metadata: map[string]string{"abono_subject": "A"}}) |
| + d.session("cs_1", "complete", "sub_1", "A", "ana@example.com") |
| + if _, err := s.Finish(ctx, "cs_1"); err != nil { |
| + t.Fatalf("Finish: %v", err) |
| + } |
| + if err := s.Adopt(ctx, "sub_1", "B", "annual"); err != nil { |
| + t.Fatalf("Adopt for B: %v", err) |
| + } |
| + holder := func(when string) { |
| + t.Helper() |
| + if got := db.dump(`SELECT subject FROM abono_subscriptions WHERE stripe_subscription_id = 'sub_1'`); got != "B\n" { |
| + t.Errorf("%s: subject = %q, want B, whom the owner adopted it for", when, got) |
| + } |
| + if got := db.dump(`SELECT subject FROM abono_customers WHERE stripe_customer_id = 'cus_1'`); got != "B\n" { |
| + t.Errorf("%s: cus_1 is mapped to %q, want B", when, got) |
| + } |
| + } |
| + holder("after Adopt") |
| + |
| + wantStatus(t, deliver(t, s, event("evt_1", "customer.subscription.updated", map[string]any{"id": "sub_1"})), http.StatusOK) |
| + holder("after a subscription event naming A") |
| + wantStatus(t, deliver(t, s, event("evt_2", "checkout.session.completed", |
| + map[string]any{"id": "cs_1", "subscription": "sub_1", "client_reference_id": "A"})), http.StatusOK) |
| + holder("after a Checkout event naming A") |
| + if f, err := s.Finish(ctx, "cs_1"); err != nil || f.Subject != "B" { |
| + t.Errorf("Finish again = %+v, %v; want B's", f, err) |
| + } |
| + holder("after Checkout's return again") |
| + if _, err := s.Reconcile(ctx); err != nil { |
| + t.Fatalf("Reconcile: %v", err) |
| + } |
| + holder("after Reconcile") |
| + if hasPlan(t, s, "A", "annual") || !hasPlan(t, s, "B", "annual") { |
| + t.Error("the plan went back to A") |
| + } |
| + |
| + // Claim is an explicit decision too. |
| + d.put(t, subSpec{ID: "sub_2", Status: "active", Price: "price_M", Customer: "cus_2", Email: "walkin@example.com", End: t0.Add(30 * day)}) |
| + if err := s.Sync(ctx, d.sub(t, "sub_2"), abono.Hint{}); err != nil { |
| + t.Fatalf("Sync: %v", err) |
| + } |
| + if n, err := s.Claim(ctx, "C", "walkin@example.com"); err != nil || n != 1 { |
| + t.Fatalf("Claim = %d, %v", n, err) |
| + } |
| + if err := s.Sync(ctx, d.sub(t, "sub_2"), abono.Hint{Subject: "D"}); err != nil { |
| + t.Fatalf("Sync with D's hint: %v", err) |
| + } |
| + if got := db.dump(`SELECT subject FROM abono_subscriptions WHERE stripe_subscription_id = 'sub_2'`); got != "C\n" { |
| + t.Errorf("a claimed subscription went to %q on a later hint, want C", got) |
| + } |
| +} |
diff --git a/migrations/0001_init.sql b/migrations/0001_init.sql| index 06a5566..06f4948 100644 |
| --- a/migrations/0001_init.sql |
| +++ b/migrations/0001_init.sql |
| @@ -40,10 +40,14 @@ CREATE INDEX abono_customers_subject ON abono_customers(subject); |
| -- first unpaid day abono saw (the unpaid period's start), kept while the |
| -- subscription stays past_due or unpaid and cleared when it is paid: the |
| -- 30 days of past_due count from it, never from a later period's start. |
| +-- subject_pinned is set when the subject was decided explicitly (an |
| +-- owner's Adopt for a subject, a reader's Claim): from then on a hint |
| +-- from Checkout or Stripe's metadata never changes it. |
| CREATE TABLE abono_subscriptions ( |
| stripe_subscription_id TEXT NOT NULL PRIMARY KEY, |
| stripe_customer_id TEXT NOT NULL, |
| subject TEXT NOT NULL DEFAULT '', |
| + subject_pinned INTEGER NOT NULL DEFAULT 0, |
| plan TEXT NOT NULL DEFAULT '', |
| stripe_price_id TEXT NOT NULL DEFAULT '', |
| status TEXT NOT NULL, |
diff --git a/schema_test.go b/schema_test.go| index 832a529..5bca8c8 100644 |
| --- a/schema_test.go |
| +++ b/schema_test.go |
| @@ -140,7 +140,7 @@ func TestSchemaTimestampsDecode(t *testing.T) { |
| // migration file beside it, with a new ID and a new entry here. See |
| // rastrillo's own migrate/frozen_checksums_test.go, which this test is |
| // deliberately shaped after. |
| -const frozenAbonoChecksum = "871e1b3937108565a3bc50789fa78461758cda939f5bce89a51de8ea5fde52bb" |
| +const frozenAbonoChecksum = "5701b2420d58c9f91d9c59038e8a7f4f242f98441468f6ba1f660ff707d37112" |
| func TestSchemaFrozenChecksum(t *testing.T) { |
| for _, m := range abono.Schema.All() { |
diff --git a/sync.go b/sync.go| index 5777780..6576ecc 100644 |
| --- a/sync.go |
| +++ b/sync.go |
| @@ -27,6 +27,7 @@ type subRecord struct { |
| StripeSubscriptionID string |
| StripeCustomerID string |
| Subject string |
| + SubjectPinned bool |
| Plan string |
| StripePriceID string |
| Status string |
| @@ -60,7 +61,10 @@ func (s *Service) sync(ctx context.Context, sub stripeapi.Subscription, hint Hin |
| // only ever called from here, inside tx. |
| // |
| // With no row yet, one is made when the subscription grants, or when adopt |
| -// is set (an owner said so). An ended subscription first seen now is never |
| +// is set (an owner said so). adopt with a hint subject is an explicit |
| +// decision of whose the subscription is: it pins the row's subject, and a |
| +// pinned subject is never changed by a hint after (ruling R36), only by |
| +// another adoption. Claim pins too. An ended subscription first seen now is never |
| // recorded (the spec's "no resurrection" rule). The plan is the one whose |
| // prices include the subscription's first price, or "" when none does: such |
| // a subscription is recorded as unrecognised, never grants, and is listed |
| @@ -109,6 +113,8 @@ func (s *Service) syncTx(ctx context.Context, tx *gorm.DB, sub stripeapi.Subscri |
| } |
| row.UnpaidSince = unpaidSince(row.Status, oldUnpaid, row.CurrentPeriodStart, row.CurrentPeriodEnd) |
| + explicit := adopt && hint.Subject != "" |
| + |
| if len(have) == 0 { |
| ok, _ := grants(row.Status, row.UnpaidSince, row.CurrentPeriodStart, row.CurrentPeriodEnd, row.CancelAtPeriodEnd, now) |
| if !ok && !adopt { |
| @@ -121,11 +127,12 @@ func (s *Service) syncTx(ctx context.Context, tx *gorm.DB, sub stripeapi.Subscri |
| if row.Subject, err = s.holder(ctx, tx, sub, hint, plan != ""); err != nil { |
| return "", err |
| } |
| + row.SubjectPinned = explicit |
| if err := tx.Exec(`INSERT INTO abono_subscriptions |
| - (stripe_subscription_id, stripe_customer_id, subject, plan, stripe_price_id, status, |
| + (stripe_subscription_id, stripe_customer_id, subject, subject_pinned, plan, stripe_price_id, status, |
| current_period_start, current_period_end, cancel_at_period_end, unpaid_since, email, created_at, updated_at) |
| - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, |
| - row.StripeSubscriptionID, row.StripeCustomerID, row.Subject, row.Plan, row.StripePriceID, row.Status, |
| + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, |
| + row.StripeSubscriptionID, row.StripeCustomerID, row.Subject, row.SubjectPinned, row.Plan, row.StripePriceID, row.Status, |
| row.CurrentPeriodStart, row.CurrentPeriodEnd, row.CancelAtPeriodEnd, row.UnpaidSince, row.Email, row.CreatedAt, row.UpdatedAt).Error; err != nil { |
| return "", err |
| } |
| @@ -136,7 +143,19 @@ func (s *Service) syncTx(ctx context.Context, tx *gorm.DB, sub stripeapi.Subscri |
| } |
| old := have[0] |
| + row.SubjectPinned = old.SubjectPinned || explicit |
| switch { |
| + case explicit: |
| + // An owner adopting it for someone: this outranks everything, |
| + // including an earlier adoption. |
| + row.Subject = hint.Subject |
| + if err := mapCustomer(tx, sub.Customer.ID, hint.Subject); err != nil { |
| + return "", err |
| + } |
| + case old.SubjectPinned: |
| + // Decided explicitly before (Adopt, Claim): a Checkout reference or |
| + // metadata hint from before that decision does not undo it. |
| + row.Subject = old.Subject |
| case hint.Subject != "": |
| // The signed-in buyer, or an owner adopting: this beats whoever the |
| // row was first tied to. |
| @@ -167,14 +186,15 @@ func (s *Service) syncTx(ctx context.Context, tx *gorm.DB, sub stripeapi.Subscri |
| moved := row.Subject != old.Subject || row.Plan != old.Plan || row.Status != old.Status || |
| row.CancelAtPeriodEnd != old.CancelAtPeriodEnd || !sameTime(row.CurrentPeriodEnd, old.CurrentPeriodEnd) || |
| !sameTime(row.CurrentPeriodStart, old.CurrentPeriodStart) || !sameTime(row.UnpaidSince, old.UnpaidSince) |
| - if !moved && row.StripePriceID == old.StripePriceID && row.StripeCustomerID == old.StripeCustomerID && row.Email == old.Email { |
| + if !moved && row.StripePriceID == old.StripePriceID && row.StripeCustomerID == old.StripeCustomerID && row.Email == old.Email && |
| + row.SubjectPinned == old.SubjectPinned { |
| return row.Subject, nil |
| } |
| if err := tx.Exec(`UPDATE abono_subscriptions SET |
| - stripe_customer_id = ?, subject = ?, plan = ?, stripe_price_id = ?, status = ?, |
| + stripe_customer_id = ?, subject = ?, subject_pinned = ?, plan = ?, stripe_price_id = ?, status = ?, |
| current_period_start = ?, current_period_end = ?, cancel_at_period_end = ?, unpaid_since = ?, email = ?, updated_at = ? |
| WHERE stripe_subscription_id = ?`, |
| - row.StripeCustomerID, row.Subject, row.Plan, row.StripePriceID, row.Status, |
| + row.StripeCustomerID, row.Subject, row.SubjectPinned, row.Plan, row.StripePriceID, row.Status, |
| row.CurrentPeriodStart, row.CurrentPeriodEnd, row.CancelAtPeriodEnd, row.UnpaidSince, row.Email, now, row.StripeSubscriptionID).Error; err != nil { |
| return "", err |
| } |
| @@ -332,7 +352,8 @@ func (s *Service) Unrecognised(ctx context.Context) ([]Unrecognised, error) { |
| // anyone to subject, records their customers as subject's, tells the app |
| // (OnChange, in the same transaction), and returns how many it attached. |
| // The app calls it once it knows subject owns email, at sign-in say: abono |
| -// takes its word for that. |
| +// takes its word for that. A claimed subscription's subject is pinned: no |
| +// later hint changes it (ruling R36). |
| func (s *Service) Claim(ctx context.Context, subject, email string) (int, error) { |
| subject, email = strings.TrimSpace(subject), normalEmail(email) |
| if subject == "" || email == "" { |
| @@ -346,7 +367,7 @@ func (s *Service) Claim(ctx context.Context, subject, email string) (int, error) |
| } |
| tell := false |
| for _, r := range rows { |
| - if err := tx.Exec(`UPDATE abono_subscriptions SET subject = ?, updated_at = ? WHERE stripe_subscription_id = ?`, |
| + if err := tx.Exec(`UPDATE abono_subscriptions SET subject = ?, subject_pinned = 1, updated_at = ? WHERE stripe_subscription_id = ?`, |
| subject, s.now(), r.StripeSubscriptionID).Error; err != nil { |
| return err |
| } |