amadan
abono: record unrecognised subscriptions found without a hint; refresh unknown prices once
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Summary
R16a: a sync with no hint records any subscription that grants; on a price no offer has it is an unrecognised row (plan ''), subject from the customer mapping or '', Holder never called, nobody told, and listed by Unrecognised for adoption. Ended subscriptions first seen now are still never recorded. R19: Checkout reads from Stripe only sold prices never refreshed (refreshed_at NULL), so a one-off price with no interval is read once.
Agent notes
The hint no longer gates creation at all: creation is grants or adopt. It still supplies the subject (beating the address) and the fallback address for Holder. Task 6 may create rows from updated events for granting subscriptions, per the controller.
4 files changed,
+69
−27
checkout.go+7 −5checkout_test.go+15 −0sync.go+12 −13sync_test.go+35 −9
CI passed — run details
CI log
=== vet === go vet ./... === fmt === === test === go test ./... -count=1 ok amadan.net/rastrillo/abono 0.432s ok amadan.net/rastrillo/abono/internal/stripeapi 0.006s === race === CGO_ENABLED=1 go test ./... -race -count=1 ok amadan.net/rastrillo/abono 3.529s ok amadan.net/rastrillo/abono/internal/stripeapi 1.013s === browser === browser: no browser test file yet, nothing to run
Intent
diff --git a/checkout.go b/checkout.go| index 74c9b90..5d6c7a4 100644 |
| --- a/checkout.go |
| +++ b/checkout.go |
| @@ -107,14 +107,16 @@ func (s *Service) Checkout(ctx context.Context, req CheckoutRequest) (string, er |
| return cs.URL, nil |
| } |
| -// learnIntervals reads from Stripe every sold price of plan whose interval |
| -// is not known yet (a price seeded from the environment, or added by id), |
| -// and records its interval, amount and currency, so the price can be chosen |
| -// by interval (ruling R17). A price Stripe cannot read stops the sale. |
| +// learnIntervals reads from Stripe every sold price of plan never read |
| +// before (refreshed_at NULL: a price seeded from the environment, or added |
| +// by id), and records its interval, amount and currency, so the price can |
| +// be chosen by interval (rulings R17 and R19). A price read once is known, |
| +// even with no interval (a one-off). A price Stripe cannot read stops the |
| +// sale. |
| func (s *Service) learnIntervals(ctx context.Context, plan string) error { |
| var ids []string |
| if err := s.cfg.DB.WithContext(ctx).Raw(`SELECT stripe_price_id FROM abono_prices |
| - WHERE plan = ? AND sold = 1 AND interval = '' ORDER BY stripe_price_id`, plan).Scan(&ids).Error; err != nil { |
| + WHERE plan = ? AND sold = 1 AND refreshed_at IS NULL ORDER BY stripe_price_id`, plan).Scan(&ids).Error; err != nil { |
| return err |
| } |
| for _, id := range ids { |
diff --git a/checkout_test.go b/checkout_test.go| index 6de18f1..1c8edb6 100644 |
| --- a/checkout_test.go |
| +++ b/checkout_test.go |
| @@ -158,6 +158,21 @@ func TestCheckoutChoosesThePriceByInterval(t *testing.T) { |
| t.Errorf("a plan with no sold price: err = %v, want ErrUnknownPlan", err) |
| } |
| + // A price with no interval at Stripe (a one-off) is read once, not on |
| + // every sale: refreshed_at, not the interval, says it is known (R19). |
| + d.price("price_once", "", 500, "eur") |
| + if err := s.SeedPlans(ctx, []abono.PlanSpec{{Plan: "once", Sold: "price_once"}}); err != nil { |
| + t.Fatalf("SeedPlans: %v", err) |
| + } |
| + for range 2 { |
| + if _, err := s.Checkout(ctx, req("once", "")); err != nil { |
| + t.Fatalf("one-off price: %v", err) |
| + } |
| + } |
| + if got := d.fetches()["price_once"]; got != 1 { |
| + t.Errorf("price_once read %d times, want once", got) |
| + } |
| + |
| // A sold price Stripe no longer has stops the sale rather than guessing. |
| if err := s.AddPrice(ctx, "annual", "price_gone", true); err != nil { |
| t.Fatalf("AddPrice: %v", err) |
diff --git a/sync.go b/sync.go| index 6e7733d..1f7ac43 100644 |
| --- a/sync.go |
| +++ b/sync.go |
| @@ -15,10 +15,9 @@ import ( |
| // customer: Checkout's client_reference_id (or the subscription's |
| // abono_subject metadata) and the address typed there. |
| // |
| -// A hint is also what lets a subscription on a price no offer has be |
| -// recorded as unrecognised: with the zero Hint only a subscription that |
| -// grants on a known price is recorded (ruling R16), so a reconcile recovers |
| -// a webhook Stripe gave up on and the account's other sales stay out. |
| +// A hint does not decide whether a row is made: any sync of a subscription |
| +// that grants records it (rulings R16 and R16a), so a reconcile recovers a |
| +// webhook Stripe gave up on. |
| type Hint struct { |
| Subject, Email string |
| } |
| @@ -58,12 +57,12 @@ func (s *Service) sync(ctx context.Context, sub stripeapi.Subscription, hint Hin |
| // first's row and updates it, and Holder is asked at most once. Holder is |
| // only ever called from here, inside tx. |
| // |
| -// With no row yet, one is made when adopt is set (an owner said so), or when |
| -// the subscription grants and either its price belongs to a plan or there |
| -// is a hint. 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 and never grants. |
| +// 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 |
| +// 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 |
| +// by Unrecognised for an owner to adopt; Holder is not asked about it. |
| // |
| // OnChange runs for the subject before and after whenever the subject, |
| // plan, status, period end or cancel flag changed, but only for a side that |
| @@ -102,10 +101,10 @@ func (s *Service) syncTx(ctx context.Context, tx *gorm.DB, sub stripeapi.Subscri |
| if len(have) == 0 { |
| ok, _ := grants(row.Status, row.CurrentPeriodEnd, row.CancelAtPeriodEnd, now) |
| - if !adopt && (!ok || (plan == "" && hint == (Hint{}))) { |
| + if !ok && !adopt { |
| // An ended subscription first seen now (a reconcile reading the |
| - // account's history), or another sale on the account that nobody |
| - // tied to this site: not ours to record. |
| + // account's history): recording it could only take away access |
| + // someone was given another way. |
| return "", nil |
| } |
| // Holder is never asked about a price no offer recognises. |
diff --git a/sync_test.go b/sync_test.go| index 88cfee9..c3b3845 100644 |
| --- a/sync_test.go |
| +++ b/sync_test.go |
| @@ -45,10 +45,11 @@ func eq(t *testing.T, what string, got, want []string) { |
| } |
| } |
| -// TestSyncCreatesOnlyWithAHint is the creation rule (spec 4.1, ruling R16): |
| -// with no hint, only a subscription that grants on a known price is |
| -// recorded, so a reconcile recovers a webhook Stripe gave up on; an ended |
| -// one, or one on a price no offer has, needs a hint. |
| +// TestSyncCreatesOnlyWithAHint is the creation rule (spec 4.1, rulings R16 |
| +// and R16a): with no hint, a subscription that grants is recorded, so a |
| +// reconcile recovers a webhook Stripe gave up on. On a price no offer has |
| +// it is recorded unrecognised, for the owner to adopt, and Holder is never |
| +// asked. An ended subscription first seen now is never recorded. |
| func TestSyncCreatesOnlyWithAHint(t *testing.T) { |
| now := t0 |
| d := newStripeDouble(t) |
| @@ -68,18 +69,29 @@ func TestSyncCreatesOnlyWithAHint(t *testing.T) { |
| t.Fatalf("an ended subscription first seen now was recorded:\n%s", got) |
| } |
| - // Granting, on a price no offer has, no hint: the account's other |
| - // sales stay out, and Holder is not asked. |
| + // Granting, on a price no offer has, no hint: recorded unrecognised and |
| + // tied to nobody, without asking Holder or telling anyone. |
| other := d.put(t, subSpec{ID: "sub_z", Status: "active", Price: "price_Z", Customer: "cus_9", Email: "zed@example.com", End: t0.Add(30 * day)}) |
| if err := s.Sync(ctx, other, abono.Hint{}); err != nil { |
| t.Fatalf("Sync of an unknown price with no hint: %v", err) |
| } |
| - if got := db.dump(rowsQuery); got != "" { |
| - t.Fatalf("an unknown price with no hint was recorded:\n%s", got) |
| + if got, want := db.dump(rowsQuery), "sub_z|cus_9|||price_Z|active|0|zed@example.com\n"; got != want { |
| + t.Fatalf("rows =\n%s\nwant\n%s", got, want) |
| } |
| asked, changes := rec.take() |
| eq(t, "Holder asked", asked, nil) |
| eq(t, "OnChange", changes, nil) |
| + if un, err := s.Unrecognised(ctx); err != nil || len(un) != 1 || un[0].SubscriptionID != "sub_z" { |
| + t.Errorf("Unrecognised = %+v, %v; want sub_z", un, err) |
| + } |
| + // Ended, on a price no offer has, no hint: nothing. |
| + gone := d.put(t, subSpec{ID: "sub_y", Status: "canceled", Price: "price_Z", Customer: "cus_9", Email: "zed@example.com", End: t0.Add(-day)}) |
| + if err := s.Sync(ctx, gone, abono.Hint{}); err != nil { |
| + t.Fatalf("Sync of an ended unknown price: %v", err) |
| + } |
| + if got := db.dump("SELECT * FROM abono_subscriptions WHERE stripe_subscription_id = 'sub_y'"); got != "" { |
| + t.Errorf("an ended unknown-price subscription was recorded: %q", got) |
| + } |
| // Granting, on a known price, no hint: recovered, its holder found by |
| // the customer's address. |
| @@ -87,7 +99,8 @@ func TestSyncCreatesOnlyWithAHint(t *testing.T) { |
| if err := s.Sync(ctx, sub, abono.Hint{}); err != nil { |
| t.Fatalf("Sync with no hint: %v", err) |
| } |
| - if got, want := db.dump(rowsQuery), "sub_1|cus_1|42|annual|price_A|active|0|ana@example.com\n"; got != want { |
| + if got, want := db.dump(rowsQuery), "sub_1|cus_1|42|annual|price_A|active|0|ana@example.com\n"+ |
| + "sub_z|cus_9|||price_Z|active|0|zed@example.com\n"; got != want { |
| t.Fatalf("rows =\n%s\nwant\n%s", got, want) |
| } |
| if got := db.dump("SELECT stripe_customer_id, subject FROM abono_customers"); got != "cus_1|42\n" { |
| @@ -109,6 +122,19 @@ func TestSyncCreatesOnlyWithAHint(t *testing.T) { |
| asked, changes = rec.take() |
| eq(t, "Holder asked for a mapped customer", asked, nil) |
| eq(t, "OnChange for the legacy row", changes, []string{"42"}) |
| + |
| + // An unknown price of a mapped customer is tied to them, still |
| + // unrecognised, and tells nobody. |
| + mapped := d.put(t, subSpec{ID: "sub_z2", Status: "active", Price: "price_Z", Customer: "cus_1", Email: "ana@example.com", End: t0.Add(30 * day)}) |
| + if err := s.Sync(ctx, mapped, abono.Hint{}); err != nil { |
| + t.Fatalf("Sync of a mapped customer's unknown price: %v", err) |
| + } |
| + if got := db.dump("SELECT subject, plan FROM abono_subscriptions WHERE stripe_subscription_id = 'sub_z2'"); got != "42|\n" { |
| + t.Errorf("sub_z2 = %q, want 42 and no plan", got) |
| + } |
| + asked, changes = rec.take() |
| + eq(t, "Holder asked for an unknown price", asked, nil) |
| + eq(t, "OnChange for an unknown price", changes, nil) |
| if !hasPlan(t, s, "42", "annual") { |
| t.Error("HasPlan(42, annual) is false after the recovered sync") |
| } |