abono: transaction-bound reads for OnChange, bounded past_due, and a SKILL.md that is true
Summary
Add HoldingsIn and HasPlanIn so Holder and OnChange can read through their own transaction without waiting on it. Bound past_due: with no period start the 30 days count from the period end, with neither it does not grant, and a cancelling one stops at the earlier of start+30d and the period end. Drop the unreviewed failing_nodate status. Correct doc.go and make SKILL.md match the code.
Agent notes
Ruling R30. Describe follows the shared grants rule. The regression test for the OnChange pattern fails without the In forms (stale read here; a deadlock on a single writer connection). SKILL.md is 7.9 KB, budget unchanged.
9 files changed,
+214
−51
SKILL.md+19 −9describe.go+9 −11describe_test.go+9 −2doc.go+2 −1example/app_test.go+1 −1holdings.go+73 −17holdings_test.go+95 −2skillmd_test.go+1 −1templates_test.go+5 −7
CI passed — run details
CI log
=== vet === go vet ./... === fmt === === test === go test ./... -count=1 ok amadan.net/rastrillo/abono 1.213s ok amadan.net/rastrillo/abono/abonotest 0.117s ok amadan.net/rastrillo/abono/cmd/abono 1.418s ok amadan.net/rastrillo/abono/internal/fakestripe 0.031s ok amadan.net/rastrillo/abono/internal/stripeapi 0.019s ok example 0.260s === race === CGO_ENABLED=1 go test ./... -race -count=1 ok amadan.net/rastrillo/abono 8.527s ok amadan.net/rastrillo/abono/abonotest 1.715s ok amadan.net/rastrillo/abono/cmd/abono 3.247s ok amadan.net/rastrillo/abono/internal/fakestripe 1.140s ok amadan.net/rastrillo/abono/internal/stripeapi 1.056s ok example 2.116s === browser === ok example 1.327s
Intent
diff --git a/SKILL.md b/SKILL.md| index cf39301..e65cb5b 100644 |
| --- a/SKILL.md |
| +++ b/SKILL.md |
| @@ -20,11 +20,19 @@ svc, err := abono.New(abono.Config{ |
| Key: key, WebhookSecrets: []string{whsec}, // both or neither |
| Holder: holder, OnChange: onChange, Sandbox: dev, |
| }) |
| -specs, _ := abono.ParsePlans("annual=price_A|price_old,monthly=price_B") |
| -err = svc.SeedPlans(ctx, specs) // idempotent; never deletes |
| +if err != nil { |
| + return fmt.Errorf("abono: %w", err) // svc is nil here |
| +} |
| +specs, err := abono.ParsePlans("annual=price_A|price_old,monthly=price_B") |
| +if err != nil { |
| + return fmt.Errorf("abono plans: %w", err) |
| +} |
| +if err := svc.SeedPlans(ctx, specs); err != nil { // idempotent; never deletes |
| + return fmt.Errorf("abono plans: %w", err) |
| +} |
| ``` |
| -In `ParsePlans`' format the first price after `=` is sold; the rest are legacy prices whose subscriptions are the same plan. With no Key, `svc.On()` is false and nothing is for sale. A Key without secrets, or the reverse, refuses to boot. |
| +Stop startup on any of these. In `ParsePlans`' format the first price after `=` is sold; the rest are legacy prices whose subscriptions are the same plan. With no Key, `svc.On()` is false and nothing is for sale, unless `Sandbox` is set: the sandbox stands in for Stripe and `On()` is true. A Key without secrets, or the reverse, refuses to boot. |
| 3. `csrf.Protect(origin, csrf.Exempt(abono.WebhookPath, abono.SyncPath))`. Neither carries a browser Origin: the webhook checks Stripe's signature, the sync checks carlos's bearer. |
| 4. Mount `svc.ServeWebhook` (POST `WebhookPath`) and `svc.ServeSync` (POST `SyncPath`). In development also mount `svc.SandboxHandler()` at `abono.SandboxPath`; without `Sandbox` it answers 404, so mounting it always is fine. |
| @@ -34,21 +42,23 @@ In `ParsePlans`' format the first price after `=` is sold; the rest are legacy p |
| Restricted key (`rk_...`) scopes: Checkout Sessions and Customer portal write; Subscriptions, Customers, Prices and Promotion codes read. Nothing else. abono sends Stripe-Version `2025-03-31.basil` itself, so the account's default version does not matter. |
| -Add a webhook endpoint at `svc.Origin()` plus the path you mounted, with these events: `checkout.session.completed`, `checkout.session.async_payment_succeeded`, `customer.subscription.created`, `.updated`, `.deleted`, `.paused`, `.resumed`, `invoice.paid`, `invoice.payment_succeeded`, `invoice.payment_failed`. Events are read for ids only and the subscription is fetched again, so ordering and replays are harmless. During secret rotation pass both secrets. `svc.TestMode()` reads the key prefix (`rk_test_`) and is for a warning banner. |
| +Add a webhook endpoint at `svc.Origin()` plus the path you mounted, with these events: `checkout.session.completed`, `checkout.session.async_payment_succeeded`, `customer.subscription.created`, `.updated`, `.deleted`, `.paused`, `.resumed`, `invoice.paid`, `invoice.payment_succeeded`, `invoice.payment_failed`. Each event is read for ids and for who it belongs to: Checkout events carry the `client_reference_id` and the address typed there, subscription and invoice events the `abono_subject` metadata and the customer's address. The subscription itself is fetched again, so ordering and replays are harmless. During secret rotation pass both secrets. `svc.TestMode()` reads the key prefix (`rk_test_`) and is for a warning banner. |
| ## Your two callbacks |
| -`Holder(ctx, tx, email, name) (subject, error)` says whose a payment is. Make a user if paying should be a way in, or return `""` to leave the payment unclaimed until you call `svc.Claim(ctx, subject, email)` at sign-in, once you know they own the address. It runs inside the write transaction and is asked again on every sync of an unclaimed row that has an address, so keep it cheap and idempotent. Use `tx`, never your own handle, or it waits forever on the writer. |
| +`Holder(ctx, tx, email, name) (subject, error)` says whose a payment is. Make a user if paying should be a way in, or return `""` to leave the payment unclaimed until you call `svc.Claim(ctx, subject, email)` at sign-in, once you know they own the address. It runs inside the write transaction and is asked again on every sync of an unclaimed row that has an address, so keep it cheap and idempotent. It is skipped for a price no offer recognises and for a customer already mapped to a subject. |
| -`OnChange(ctx, tx, subject)` runs in the same transaction as every write (a webhook, Finish, Comp, EndComp, Claim, Adopt) and again on every reconcile for every subject holding anything. Make it idempotent: recompute the app's state from `HasPlan`, do not apply a delta. It is never called with `""`. Its error rolls the write back. |
| +`OnChange(ctx, tx, subject)` runs in the same transaction as the write when a subscription's subject, plan, status, period or cancel flag changes, and for Comp, EndComp, Claim and Adopt. Reconcile also runs it for every subject holding anything, so it must be idempotent: recompute the app's state, do not apply a delta. It is never called with `""`, and an error rolls the write back. |
| + |
| +Both callbacks get `tx`. Use it for everything, including abono's own reads: `svc.HasPlanIn(ctx, tx, subject, plan)` and `svc.HoldingsIn(ctx, tx, subject)`. `HasPlan` and `Holdings` use the service's own handle, which does not join `tx`; with one writer connection that waits on the transaction that is calling you, and at best it reads the state from before the write. The `In` forms see the write and use the service clock. |
| ## Asking and selling |
| `svc.HasPlan(ctx, subject, plan)` is the gate. `svc.Holdings(ctx, subject)` lists everything, ended included, newest first. `abono.Describe(h, now)` (also `h.Describe(now)`) turns a holding into a `Status`: a message key, a tone and a date. Pass `svc.Now()` as now so the words agree with `Grants`. |
| -Only subscriptions on a price that belongs to an offer grant anything. Others appear under `svc.Unrecognised` for the owner to `Adopt` onto a plan. `past_due` still grants until 30 days after the start of the unpaid period (`Backstop` marks that case); a cancelling membership stops at its period end. |
| +Only subscriptions on a price that belongs to an offer grant anything. Others appear under `svc.Unrecognised` for the owner to `Adopt` onto a plan. A subscription grants as follows. `active` and `trialing` grant, but a cancelling one stops at its period end. `past_due` grants until 30 days after the start of the unpaid period (30 days after the period end if the start is unknown; with neither, not at all), or until the period end if that is sooner and it is cancelling. `Backstop` marks a `past_due` holding past its paid period. Every other status (`canceled`, `unpaid`, `incomplete`, `paused`) does not grant. A comp grants until its end, or for good. |
| -Selling: `svc.Offers(ctx)` for the pricing page. `svc.Checkout(ctx, abono.CheckoutRequest{Plan, Interval, Subject, Email, SuccessPath, CancelPath})` returns a URL to redirect to; the reader types a promotion code on Stripe's page, or you pass one. `svc.Finish(ctx, sessionID)` on the success page syncs at once rather than waiting for the webhook; it signs nobody in. `svc.Portal(ctx, subject, returnPath)` is where they change card or plan or cancel. Owner tools: `svc.Comp(ctx, subject, plan, endsAt, note)` and `EndComp`, `Members`, `Adopt`, `AddPrice`. A comp needs no Stripe. |
| +Selling: `svc.Offers(ctx)` for the pricing page. `url, err := svc.Checkout(ctx, abono.CheckoutRequest{Plan: "annual", Interval: "year", Subject: subject, Email: email, SuccessPath: "/subscribe/done", CancelPath: "/subscribe"})` returns a URL to redirect to; the reader types a promotion code on Stripe's page, or you pass `PromotionCode`. `svc.Finish(ctx, sessionID)` on the success page syncs at once rather than waiting for the webhook; it signs nobody in. `svc.Portal(ctx, subject, returnPath)` is where they change card or plan or cancel. Owner tools: `svc.Comp(ctx, subject, plan, endsAt, note)` and `EndComp`, `Members`, `Adopt`, `AddPrice`. A comp needs no Stripe. |
| ## Pages and words |
| @@ -58,7 +68,7 @@ Partials: parse `abono.Templates()` with `ParseFS(..., "*.html")` after `ui.Temp |
| ## Testing |
| -`abonotest.New(t)` is a fake Stripe on 127.0.0.1: `AddPrice`, `Pay`, `Decline`, `Cancel`, `Advance`, `FailNextRenewal`, `AddSubscription`, `SendEvent`. `stripe.Config(base)` points a Config at it and shares its clock; `SetWebhookURL(app.URL + abono.WebhookPath)` sends it your events. In development, `Sandbox: true` does the same inside the app with click-through Checkout and Portal pages. It needs a loopback Origin and no Key. |
| +`abonotest.New(t)` is a fake Stripe on 127.0.0.1: `AddPrice`, `Pay`, `Decline`, `Cancel`, `Advance`, `FailNextRenewal`, `AddSubscription`, `SendEvent`. `stripe.Config(base)` points a Config at it, and installs the fake's clock when `base.Now` is nil (so `Advance` moves abono too); `SetWebhookURL(app.URL + abono.WebhookPath)` sends it your events. In development, `Sandbox: true` does the same inside the app with click-through Checkout and Portal pages. It needs a loopback Origin and no Key. |
| ## Not in v0.1 |
diff --git a/describe.go b/describe.go| index 67354c4..45ff3a4 100644 |
| --- a/describe.go |
| +++ b/describe.go |
| @@ -12,12 +12,9 @@ const ( |
| StatusRenews = "abono.status.renews" |
| StatusCancelling = "abono.status.cancelling" |
| StatusFailing = "abono.status.failing" |
| - // StatusFailingNoDate is failing when the day access stops is unknown; |
| - // its sentence has no {date}. |
| - StatusFailingNoDate = "abono.status.failing_nodate" |
| - StatusEnded = "abono.status.ended" |
| - StatusComp = "abono.status.comp" |
| - StatusCompUntil = "abono.status.comp_until" |
| + StatusEnded = "abono.status.ended" |
| + StatusComp = "abono.status.comp" |
| + StatusCompUntil = "abono.status.comp_until" |
| ) |
| // Status is the one line a holding reads as: a message key, a tone, and |
| @@ -36,8 +33,8 @@ type Status struct { |
| // renews active or trialing, with a period end, not cancelling: Date is the period end |
| // cancelling cancel_at_period_end and granting: Date is the period end |
| // failing past_due and granting: Date is the day access stops, 30 days after the |
| -// unpaid period started |
| -// failing_nodate the same, when the start is unknown so there is no date |
| +// unpaid period started (after the period end when the start is |
| +// unknown), or the period end if that is sooner and it is cancelling |
| // ended does not grant |
| // comp a comp with no end |
| // comp_until a comp with an end: Date is ends_at |
| @@ -58,10 +55,11 @@ func Describe(h Holding, now time.Time) Status { |
| } |
| switch { |
| case h.Status == "past_due": |
| - if h.PeriodStart == nil { |
| - return Status{Key: StatusFailingNoDate, Tone: "warning"} |
| + // Granting means there is a stop date to give (ruling R30). |
| + stops, ok := pastDueStops(h.PeriodStart, h.Until, h.CancelAtPeriodEnd) |
| + if !ok { |
| + return Status{Key: StatusEnded, Tone: "neutral"} |
| } |
| - stops := h.PeriodStart.Add(backstopWindow) |
| return Status{Key: StatusFailing, Tone: "warning", Date: &stops} |
| case h.CancelAtPeriodEnd: |
| return Status{Key: StatusCancelling, Tone: "warning", Date: copyTime(h.Until)} |
diff --git a/describe_test.go b/describe_test.go| index 18e0afd..ed99528 100644 |
| --- a/describe_test.go |
| +++ b/describe_test.go |
| @@ -43,9 +43,16 @@ func TestDescribe(t *testing.T) { |
| {"failing and cancelling reads as failing", |
| abono.Holding{Source: abono.SourceSubscription, Status: "past_due", PeriodStart: &past, Until: ptr(past.AddDate(1, 0, 0)), CancelAtPeriodEnd: true}, |
| "abono.status.failing", "warning", ptr(past.Add(30 * day))}, |
| - {"failing with no period start has no date", |
| + // R30: with no period start the 30 days count from the period end. |
| + {"failing with no period start counts from the period end", |
| abono.Holding{Source: abono.SourceSubscription, Status: "past_due", Until: &end}, |
| - "abono.status.failing_nodate", "warning", nil}, |
| + "abono.status.failing", "warning", ptr(end.Add(30 * day))}, |
| + {"failing and cancelling stops at the period end when that is sooner", |
| + abono.Holding{Source: abono.SourceSubscription, Status: "past_due", PeriodStart: ptr(end.Add(-20 * day)), Until: &end, CancelAtPeriodEnd: true}, |
| + "abono.status.failing", "warning", &end}, |
| + {"ended, past_due with no period at all", |
| + abono.Holding{Source: abono.SourceSubscription, Status: "past_due"}, |
| + "abono.status.ended", "neutral", nil}, |
| {"failing one second before start+30d", |
| abono.Holding{Source: abono.SourceSubscription, Status: "past_due", PeriodStart: ptr(now.Add(-30*day + time.Second)), Until: ptr(now.AddDate(1, 0, 0))}, |
| "abono.status.failing", "warning", ptr(now.Add(time.Second))}, |
diff --git a/doc.go b/doc.go| index 63dc56a..e1dd925 100644 |
| --- a/doc.go |
| +++ b/doc.go |
| @@ -1,5 +1,6 @@ |
| // Package abono is rastrillo's payments addon: Stripe subscriptions |
| -// and one-off payments for an app, and the memberships they grant. |
| +// for an app, and the memberships they grant. One-off payments are not |
| +// covered. |
| // |
| // Two rulings bind everything here, both from rastrillo's addons |
| // doctrine. The arrow points one way: abono depends on rastrillo, |
diff --git a/example/app_test.go b/example/app_test.go| index 005b089..6af6e86 100644 |
| --- a/example/app_test.go |
| +++ b/example/app_test.go |
| @@ -55,7 +55,7 @@ var testCatalog = sync.OnceValue(func() rastrillo.Catalog { |
| } |
| } |
| for _, k := range []string{abono.StatusActive, abono.StatusRenews, abono.StatusCancelling, abono.StatusFailing, |
| - abono.StatusFailingNoDate, abono.StatusEnded, abono.StatusComp, abono.StatusCompUntil} { |
| + abono.StatusEnded, abono.StatusComp, abono.StatusCompUntil} { |
| cat[k] = say(k) |
| } |
| return cat |
diff --git a/holdings.go b/holdings.go| index 2a9e2f9..72c9873 100644 |
| --- a/holdings.go |
| +++ b/holdings.go |
| @@ -4,6 +4,8 @@ import ( |
| "context" |
| "sort" |
| "time" |
| + |
| + "gorm.io/gorm" |
| ) |
| // backstopWindow is how long past a paid period's end a past_due subscription |
| @@ -11,7 +13,8 @@ import ( |
| // good once its retries end, so abono stops holding it 30 days after the |
| // period it was paid to, whatever the account's setting. The paid period's |
| // end is the unpaid one's current_period_start: a failed renewal has |
| -// already moved current_period_end a whole period on (ruling R21). |
| +// already moved current_period_end a whole period on (ruling R21). With no |
| +// start the count is from current_period_end instead (ruling R30). |
| const backstopWindow = 30 * 24 * time.Hour |
| // Source says where a holding came from. |
| @@ -53,18 +56,52 @@ type Holding struct { |
| CompID int64 // set for SourceComp |
| } |
| +// pastDueAnchor is where the 30 days of a past_due subscription count from: |
| +// the start of the unpaid period, or, when that is unknown, the period end |
| +// (ruling R30). ok is false when neither is known. |
| +func pastDueAnchor(periodStart, periodEnd *time.Time) (anchor time.Time, ok bool) { |
| + switch { |
| + case periodStart != nil: |
| + return *periodStart, true |
| + case periodEnd != nil: |
| + return *periodEnd, true |
| + } |
| + return time.Time{}, false |
| +} |
| + |
| +// pastDueStops is the moment a past_due subscription stops granting: 30 days |
| +// after its anchor, or the period end if that is sooner and the subscription |
| +// is set to cancel with its period (ruling R30). ok is false when there is |
| +// nothing to count from, and then it does not grant. |
| +func pastDueStops(periodStart, periodEnd *time.Time, cancelAtPeriodEnd bool) (stops time.Time, ok bool) { |
| + anchor, ok := pastDueAnchor(periodStart, periodEnd) |
| + if !ok { |
| + return time.Time{}, false |
| + } |
| + stops = anchor.Add(backstopWindow) |
| + if cancelAtPeriodEnd && periodEnd != nil && periodEnd.Before(stops) { |
| + stops = *periodEnd |
| + } |
| + return stops, true |
| +} |
| + |
| // grants is the one holding rule for a Stripe subscription, used by |
| // Holdings and by Describe, so a screen can never say what access does not. |
| // |
| // active, trialing grant, unless cancelling: then only |
| // while now < periodEnd |
| -// past_due grants while now < periodStart + 30 days |
| +// past_due grants while now < the earlier of |
| +// periodStart + 30 days and, if cancelling, |
| +// periodEnd. With no periodStart the 30 |
| +// days count from periodEnd; with neither |
| +// it does not grant |
| // canceled, unpaid, incomplete, |
| // incomplete_expired, paused, anything |
| // Stripe adds later does not |
| // |
| -// backstop is true for a past_due subscription held only by the 30-day |
| -// window, that is once now has reached periodStart. |
| +// backstop is true for a past_due subscription that grants only because of |
| +// the 30-day window, that is once now has reached the anchor (periodStart, |
| +// or periodEnd when there is none). |
| func grants(status string, periodStart, periodEnd *time.Time, cancelAtPeriodEnd bool, now time.Time) (grants, backstop bool) { |
| switch status { |
| case "active", "trialing": |
| @@ -77,19 +114,15 @@ func grants(status string, periodStart, periodEnd *time.Time, cancelAtPeriodEnd |
| } |
| return true, false |
| case "past_due": |
| - // Counted from the unpaid period's start, never its end: Stripe |
| - // moved the end a whole period on when the renewal failed. |
| - if periodStart == nil { |
| - // Stripe is retrying the card and we have no period start to count |
| - // the 30 days from. Nothing says the access should have stopped, so |
| - // it grants, and it is not flagged as held by the backstop because |
| - // there is no backstop to hold it. |
| - return true, false |
| - } |
| - if !now.Before(periodStart.Add(backstopWindow)) { |
| + // Counted from the unpaid period's start, never its end, when the |
| + // start is known: Stripe moved the end a whole period on when the |
| + // renewal failed. |
| + stops, ok := pastDueStops(periodStart, periodEnd, cancelAtPeriodEnd) |
| + if !ok || !now.Before(stops) { |
| return false, false |
| } |
| - return true, !now.Before(*periodStart) |
| + anchor, _ := pastDueAnchor(periodStart, periodEnd) |
| + return true, !now.Before(anchor) |
| } |
| return false, false |
| } |
| @@ -129,10 +162,23 @@ func (subscriptionRow) TableName() string { return "abono_subscriptions" } |
| // Subscriptions whose price no offer recognises (plan "") and payments not |
| // yet tied to a subject are left out: neither can grant. |
| func (s *Service) Holdings(ctx context.Context, subject string) ([]Holding, error) { |
| + return s.holdings(ctx, s.cfg.DB, subject) |
| +} |
| + |
| +// HoldingsIn is Holdings read through tx, for a Holder or OnChange callback, |
| +// which runs inside abono's write transaction: Holdings uses the service's |
| +// own handle, which does not join that transaction and so waits on it, on the |
| +// single writer connection, forever. HoldingsIn sees the transaction's own |
| +// writes and judges them by the service's clock. |
| +func (s *Service) HoldingsIn(ctx context.Context, tx *gorm.DB, subject string) ([]Holding, error) { |
| + return s.holdings(ctx, tx, subject) |
| +} |
| + |
| +func (s *Service) holdings(ctx context.Context, db *gorm.DB, subject string) ([]Holding, error) { |
| if subject == "" { |
| return nil, nil |
| } |
| - g := s.cfg.DB.WithContext(ctx) |
| + g := db.WithContext(ctx) |
| var subs []subscriptionRow |
| if err := g.Where("subject = ? AND plan <> ''", subject).Find(&subs).Error; err != nil { |
| return nil, err |
| @@ -180,10 +226,20 @@ func (s *Service) Holdings(ctx context.Context, subject string) ([]Holding, erro |
| // or a comp. (The design calls this Grants; it is renamed so it cannot be |
| // confused with a comp.) |
| func (s *Service) HasPlan(ctx context.Context, subject, plan string) (bool, error) { |
| + return s.hasPlan(ctx, s.cfg.DB, subject, plan) |
| +} |
| + |
| +// HasPlanIn is HasPlan read through tx; see HoldingsIn. It is what OnChange |
| +// calls to learn the subject's state after the write that triggered it. |
| +func (s *Service) HasPlanIn(ctx context.Context, tx *gorm.DB, subject, plan string) (bool, error) { |
| + return s.hasPlan(ctx, tx, subject, plan) |
| +} |
| + |
| +func (s *Service) hasPlan(ctx context.Context, db *gorm.DB, subject, plan string) (bool, error) { |
| if plan == "" { |
| return false, nil |
| } |
| - hs, err := s.Holdings(ctx, subject) |
| + hs, err := s.holdings(ctx, db, subject) |
| if err != nil { |
| return false, err |
| } |
diff --git a/holdings_test.go b/holdings_test.go| index 8a92188..87a7c51 100644 |
| --- a/holdings_test.go |
| +++ b/holdings_test.go |
| @@ -6,6 +6,8 @@ import ( |
| "testing" |
| "time" |
| + "gorm.io/gorm" |
| + |
| "amadan.net/rastrillo/abono" |
| ) |
| @@ -55,8 +57,22 @@ func TestHoldingTable(t *testing.T) { |
| {name: "past_due, one second before start+30d", status: "past_due", start: ptr(end), end: ptr(end.AddDate(1, 0, 0)), now: at(30*day - time.Second), grants: true, backstop: true}, |
| {name: "past_due at exactly start+30d", status: "past_due", start: ptr(end), end: ptr(end.AddDate(1, 0, 0)), now: at(30 * day), grants: false}, |
| {name: "past_due past start+30d, the unpaid period's end still months off", status: "past_due", start: ptr(end), end: ptr(end.AddDate(1, 0, 0)), now: at(31 * day), grants: false}, |
| - {name: "past_due with no period start", status: "past_due", end: ptr(end), now: at(400 * day), grants: true}, |
| - {name: "past_due with no period at all", status: "past_due", now: at(400 * day), grants: true}, |
| + // R30: with no period start the 30 days count from the period end; |
| + // with neither, nothing says when to stop, so it does not grant. |
| + {name: "past_due, no period start, before the period end", status: "past_due", end: ptr(end), now: at(-time.Second), grants: true}, |
| + {name: "past_due, no period start, at the period end", status: "past_due", end: ptr(end), now: at(0), grants: true, backstop: true}, |
| + {name: "past_due, no period start, one second before end+30d", status: "past_due", end: ptr(end), now: at(30*day - time.Second), grants: true, backstop: true}, |
| + {name: "past_due, no period start, at end+30d", status: "past_due", end: ptr(end), now: at(30 * day), grants: false}, |
| + {name: "past_due, no period start, long after", status: "past_due", end: ptr(end), now: at(400 * day), grants: false}, |
| + {name: "past_due with no period at all", status: "past_due", now: at(0), grants: false}, |
| + // R30: cancelling stops at the earlier of start+30d and the period |
| + // end. The period end is 10 days past the start here, so it is |
| + // sooner; in the case above (end a year off) start+30d is. |
| + {name: "past_due, cancelling, one second before the period end", status: "past_due", start: ptr(end.Add(-10 * day)), end: ptr(end), cancel: true, now: at(-time.Second), grants: true, backstop: true}, |
| + {name: "past_due, cancelling, at the period end", status: "past_due", start: ptr(end.Add(-10 * day)), end: ptr(end), cancel: true, now: at(0), grants: false}, |
| + {name: "past_due, same period, not cancelling, past the period end", status: "past_due", start: ptr(end.Add(-10 * day)), end: ptr(end), now: at(day), grants: true, backstop: true}, |
| + {name: "past_due, cancelling, start+30d sooner than the period end", status: "past_due", start: ptr(end), end: ptr(end.AddDate(1, 0, 0)), cancel: true, now: at(30 * day), grants: false}, |
| + {name: "past_due, cancelling, no period start, at the period end", status: "past_due", end: ptr(end), cancel: true, now: at(0), grants: false}, |
| {name: "canceled", status: "canceled", end: ptr(end), now: at(-day), grants: false}, |
| {name: "unpaid", status: "unpaid", end: ptr(end), now: at(-day), grants: false}, |
| {name: "incomplete", status: "incomplete", end: ptr(end), now: at(-day), grants: false}, |
| @@ -181,3 +197,80 @@ func TestHoldingsNewestFirstAndOnlyThisSubjects(t *testing.T) { |
| t.Errorf("plans newest first = %v", got) |
| } |
| } |
| + |
| +// TestOnChangeReadsItsOwnTransactionWithHasPlanIn is the pattern SKILL.md |
| +// gives OnChange: Holdings and HasPlan use the service's own handle, which |
| +// does not join the transaction OnChange runs in and so waits on it forever |
| +// (one writer connection); HoldingsIn and HasPlanIn read through the tx |
| +// OnChange was handed, see the write that triggered it, and judge it by the |
| +// service's clock. |
| +func TestOnChangeReadsItsOwnTransactionWithHasPlanIn(t *testing.T) { |
| + now := t0 |
| + d := newStripeDouble(t) |
| + type seen struct { |
| + has bool |
| + holding []abono.Holding |
| + } |
| + var ( |
| + s *abono.Service |
| + outcomes []seen |
| + ) |
| + onChange := func(ctx context.Context, tx *gorm.DB, subject string) error { |
| + has, err := s.HasPlanIn(ctx, tx, subject, "annual") |
| + if err != nil { |
| + return err |
| + } |
| + hs, err := s.HoldingsIn(ctx, tx, subject) |
| + if err != nil { |
| + return err |
| + } |
| + outcomes = append(outcomes, seen{has, hs}) |
| + return nil |
| + } |
| + s, _ = d.service(t, &now, abono.Config{OnChange: onChange}) |
| + seedPlans(t, s) |
| + |
| + sub := d.put(t, subSpec{ID: "sub_1", Status: "active", Price: "price_A", Customer: "cus_1", Email: "ana@example.com", |
| + Start: t0.Add(-day), End: t0.Add(300 * day)}) |
| + done := make(chan error, 1) |
| + go func() { done <- s.Sync(context.Background(), sub, abono.Hint{Subject: "ana"}) }() |
| + select { |
| + case err := <-done: |
| + if err != nil { |
| + t.Fatalf("Sync: %v", err) |
| + } |
| + case <-time.After(10 * time.Second): |
| + t.Fatal("Sync did not finish: OnChange waits on its own transaction") |
| + } |
| + |
| + if len(outcomes) == 0 { |
| + t.Fatal("OnChange was never called") |
| + } |
| + last := outcomes[len(outcomes)-1] |
| + if !last.has || len(last.holding) != 1 || !last.holding[0].Grants { |
| + t.Errorf("OnChange saw has=%v holdings=%+v after the sync; want the new subscription granting", last.has, last.holding) |
| + } |
| + |
| + // The service's clock decides, inside the transaction too: once the |
| + // period has long run out, a cancelling subscription synced then is |
| + // seen as not granting. |
| + now = t0.Add(400 * day) |
| + outcomes = nil |
| + ended := d.put(t, subSpec{ID: "sub_1", Status: "active", Price: "price_A", Customer: "cus_1", Email: "ana@example.com", |
| + Start: t0.Add(-day), End: t0.Add(300 * day), Cancel: true}) |
| + go func() { done <- s.Sync(context.Background(), ended, abono.Hint{}) }() |
| + select { |
| + case err := <-done: |
| + if err != nil { |
| + t.Fatalf("Sync (cancelling): %v", err) |
| + } |
| + case <-time.After(10 * time.Second): |
| + t.Fatal("Sync did not finish the second time") |
| + } |
| + if len(outcomes) == 0 { |
| + t.Fatal("OnChange was not called for the cancelling subscription") |
| + } |
| + if last := outcomes[len(outcomes)-1]; last.has { |
| + t.Errorf("OnChange saw has=true for a cancelling subscription 100 days past its period end") |
| + } |
| +} |
diff --git a/skillmd_test.go b/skillmd_test.go| index c34f050..b293c29 100644 |
| --- a/skillmd_test.go |
| +++ b/skillmd_test.go |
| @@ -25,7 +25,7 @@ func TestSkillMDIsWithinBudgetAndNamesTheSurface(t *testing.T) { |
| "abono.New", "abono.Schema", "BootSchema", "csrf.Exempt", |
| "WebhookPath", "SyncPath", "carlos schedule", |
| "Holder", "Claim", "OnChange", "idempotent", |
| - "HasPlan", "Holdings", "Describe", "Checkout", "Finish", "Portal", |
| + "HasPlan", "HasPlanIn", "HoldingsIn", "Holdings", "Describe", "Checkout", "Finish", "Portal", |
| "Adopt", "Comp", "SeedPlans", "Templates()", "Catalog()", |
| "abono pages", "abonotest", "Sandbox", "2025-03-31.basil", |
| "Checkout Sessions", "Customer portal", "Subscriptions", "Customers", |
diff --git a/templates_test.go b/templates_test.go| index 548e7bb..f6c0fac 100644 |
| --- a/templates_test.go |
| +++ b/templates_test.go |
| @@ -28,7 +28,6 @@ var testCatalog = rastrillo.Catalog{ |
| "abono.status.renews": "T-RENEWS {date}", |
| "abono.status.cancelling": "T-CANCELLING {date}", |
| "abono.status.failing": "T-FAILING {date}", |
| - "abono.status.failing_nodate": "T-FAILING-NODATE", |
| "abono.status.ended": "T-ENDED", |
| "abono.status.comp": "T-COMP", |
| "abono.status.comp_until": "T-COMPUNTIL {date}", |
| @@ -186,14 +185,14 @@ func holdingCases() []holdingCase { |
| failing := sub("past_due", false) |
| failing.PeriodStart = &started |
| active := abono.Holding{Plan: "supporter", Source: abono.SourceSubscription, Status: "active", Grants: true} |
| - noStart := sub("past_due", false) |
| + noStart := sub("past_due", false) // no period start: counted from the end |
| compUntil := abono.Holding{Plan: "supporter", Source: abono.SourceComp, Status: "active", Until: &soon, Grants: true} |
| return []holdingCase{ |
| {"active", active, abono.StatusActive, "positive", "T-ACTIVE", false}, |
| {"renews", sub("active", false), abono.StatusRenews, "positive", "T-RENEWS", true}, |
| {"cancelling", sub("active", true), abono.StatusCancelling, "warning", "T-CANCELLING", true}, |
| {"failing", failing, abono.StatusFailing, "warning", "T-FAILING", true}, |
| - {"failing_nodate", noStart, abono.StatusFailingNoDate, "warning", "T-FAILING-NODATE", false}, |
| + {"failing, no period start", noStart, abono.StatusFailing, "warning", "T-FAILING", true}, |
| {"ended", sub("canceled", false), abono.StatusEnded, "neutral", "T-ENDED", false}, |
| {"comp", abono.Holding{Plan: "supporter", Source: abono.SourceComp, Status: "active", Grants: true}, abono.StatusComp, "positive", "T-COMP", false}, |
| {"comp_until", compUntil, abono.StatusCompUntil, "positive", "T-COMPUNTIL", true}, |
| @@ -233,7 +232,7 @@ func TestStatusRendersEveryKey(t *testing.T) { |
| } |
| }) |
| } |
| - for _, key := range []string{abono.StatusActive, abono.StatusRenews, abono.StatusCancelling, abono.StatusFailing, abono.StatusFailingNoDate, abono.StatusEnded, abono.StatusComp, abono.StatusCompUntil} { |
| + for _, key := range []string{abono.StatusActive, abono.StatusRenews, abono.StatusCancelling, abono.StatusFailing, abono.StatusEnded, abono.StatusComp, abono.StatusCompUntil} { |
| if !seen[key] { |
| t.Errorf("no case renders %s", key) |
| } |
| @@ -549,7 +548,7 @@ func partialKeys(t *testing.T) map[string]bool { |
| // The keys of the agreed copy. A key outside this list is copy nobody |
| // reviewed, so a partial that reaches for one fails here. |
| var agreedKeys = []string{ |
| - "abono.status.active", "abono.status.renews", "abono.status.cancelling", "abono.status.failing", "abono.status.failing_nodate", |
| + "abono.status.active", "abono.status.renews", "abono.status.cancelling", "abono.status.failing", |
| "abono.status.ended", "abono.status.comp", "abono.status.comp_until", |
| "abono.price.per_month", "abono.price.per_year", "abono.price.monthly", "abono.price.yearly", "abono.price.switch_label", |
| "abono.plan.interval_unavailable", "abono.plan.code_label", "abono.plan.code_hint", |
| @@ -648,7 +647,7 @@ func usedKeys(t *testing.T) map[string]bool { |
| func TestUsedKeysSeesEveryConsumer(t *testing.T) { |
| used := usedKeys(t) |
| for _, k := range []string{ |
| - abono.StatusFailingNoDate, "abono.paywall.title", // Go constant, partial |
| + abono.StatusFailing, "abono.paywall.title", // Go constant, partial |
| "abono.sandbox.pay", "abono.sandbox.portal_return", // the sandbox pages |
| "abono.pages.closed_body", "abono.error.free_plan", // a starter page, the starter pages' Go |
| } { |
| @@ -667,7 +666,6 @@ func TestUsedKeysSeesEveryConsumer(t *testing.T) { |
| // reviewed text lands (Task 8b's follow-up), and a key left on it after |
| // that fails. |
| var pendingReview = []string{ |
| - "abono.status.failing_nodate", |
| "abono.owner.free_no_member", |
| } |