amadan
abono: Offers reads any price never read, so a fresh site's pricing shows its intervals
Summary
Offers, with Stripe or the sandbox on, first reads from Stripe every price whose refreshed_at is still NULL (seeded from the environment, or added as legacy), best-effort: a price Stripe cannot read is logged and listed as it is. A freshly seeded site that has not reconciled yet now renders its picker and cards with intervals and amounts instead of nothing, or one card with no interval that Checkout then refused as ambiguous.
Agent notes
Final review I12, ruling R42. One Stripe call per new price, once; a price Stripe keeps refusing is asked for again on each Offers. The example's start-up reconcile comment no longer claims the pricing page depends on it. Regression: TestOffersReadsPricesNeverRead (no reconcile; Offers gives every interval, the picker renders both groups; a second Offers reads nothing new; Stripe off reads nothing).
3 files changed,
+85
−2
example/app.go+3 −2offers.go+19 −0offers_test.go+63 −0
CI passed — run details
CI log
=== vet === go vet ./... === fmt === === test === go test ./... -count=1 ok amadan.net/rastrillo/abono 2.622s ok amadan.net/rastrillo/abono/abonotest 0.055s ok amadan.net/rastrillo/abono/cmd/abono 0.658s ok amadan.net/rastrillo/abono/internal/fakestripe 0.020s ok amadan.net/rastrillo/abono/internal/stripeapi 0.006s ok example 0.174s === race === CGO_ENABLED=1 go test ./... -race -count=1 ok amadan.net/rastrillo/abono 8.729s ok amadan.net/rastrillo/abono/abonotest 1.437s ok amadan.net/rastrillo/abono/cmd/abono 1.955s ok amadan.net/rastrillo/abono/internal/fakestripe 1.067s ok amadan.net/rastrillo/abono/internal/stripeapi 1.021s ok example 2.200s === browser === ok example 0.882s
Intent
diff --git a/example/app.go b/example/app.go| index 79adcc4..1d75c08 100644 |
| --- a/example/app.go |
| +++ b/example/app.go |
| @@ -166,8 +166,9 @@ func App(cfg Config) (*http.ServeMux, error) { |
| if err := svc.SeedPlans(ctx, specs); err != nil { |
| return nil, err |
| } |
| - // One reconcile reads the prices, so the plans page can show them |
| - // before anyone has bought. The daily tick keeps them fresh. |
| + // A first reconcile catches up on whatever happened while the app was |
| + // down. The plans page does not need it: Offers reads any price it has |
| + // never read. The daily tick keeps prices fresh. |
| if _, err := svc.Reconcile(ctx); err != nil { |
| logger.Warn("example: first reconcile", "err", err) |
| } |
diff --git a/offers.go b/offers.go| index 760d786..6045717 100644 |
| --- a/offers.go |
| +++ b/offers.go |
| @@ -261,8 +261,27 @@ func (s *Service) AddPrice(ctx context.Context, plan, priceID string, sold bool) |
| // Offers lists the plans on sale, each with its prices (sold ones first), |
| // in the order they were first seeded. An archived offer is left out. |
| +// |
| +// With Stripe (or the sandbox) on, it first reads from Stripe every price |
| +// never read before (a seeded one, or one added as legacy), so a page never |
| +// shows a price without its interval, amount and currency (ruling R42). |
| +// That is one call per new price, once. It is best-effort: a price Stripe |
| +// cannot read is logged and listed as it is, and asked for again next time. |
| func (s *Service) Offers(ctx context.Context) ([]Offer, error) { |
| g := s.cfg.DB.WithContext(ctx) |
| + if s.On() { |
| + var unread []string |
| + if err := g.Raw(`SELECT stripe_price_id FROM abono_prices WHERE refreshed_at IS NULL ORDER BY stripe_price_id`). |
| + Scan(&unread).Error; err != nil { |
| + return nil, err |
| + } |
| + for _, id := range unread { |
| + if err := s.refreshPrice(ctx, id); err != nil { |
| + s.noteRefusedKey(err) |
| + s.cfg.Logger.Warn("abono: offers: a price could not be read", "price", id, "err", err) |
| + } |
| + } |
| + } |
| var rows []offerRow |
| if err := g.Where("archived_at IS NULL").Order("created_at, plan").Find(&rows).Error; err != nil { |
| return nil, err |
diff --git a/offers_test.go b/offers_test.go| index 0a275b2..815235e 100644 |
| --- a/offers_test.go |
| +++ b/offers_test.go |
| @@ -3,6 +3,8 @@ package abono_test |
| import ( |
| "context" |
| "errors" |
| + "reflect" |
| + "strings" |
| "testing" |
| "amadan.net/rastrillo/abono" |
| @@ -246,3 +248,64 @@ func TestAddPriceKeepsOneSoldPricePerInterval(t *testing.T) { |
| t.Error("a price Stripe does not have was recorded") |
| } |
| } |
| + |
| +// TestOffersReadsPricesNeverRead is final review I12 (ruling R42): a |
| +// freshly seeded site has never read its prices from Stripe, and nothing |
| +// in the documented wiring makes it reconcile first. Offers reads, once |
| +// and best-effort, every price never read, so the pricing UI knows each |
| +// interval before anyone buys. A price Stripe cannot read is left unknown |
| +// and does not fail the page. |
| +func TestOffersReadsPricesNeverRead(t *testing.T) { |
| + now := t0 |
| + d := newStripeDouble(t) |
| + s, db := d.service(t, &now, abono.Config{}) |
| + seedPlans(t, s) // annual: price_A (year) sold, price_old legacy; monthly: price_M |
| + ctx := context.Background() |
| + // annual is sold monthly too, recorded by id without a read, and a |
| + // legacy price Stripe no longer has. |
| + d.price("price_A_month", "month", 600, "eur") |
| + db.exec(`INSERT INTO abono_prices (stripe_price_id, plan, sold) VALUES ('price_A_month', 'annual', 1), ('price_lost', 'annual', 0)`) |
| + |
| + offers, err := s.Offers(ctx) |
| + if err != nil { |
| + t.Fatalf("Offers: %v", err) |
| + } |
| + intervals := map[string]string{} |
| + for _, o := range offers { |
| + for _, p := range o.Prices { |
| + intervals[p.ID] = p.Interval |
| + } |
| + } |
| + if want := map[string]string{"price_A": "year", "price_A_month": "month", "price_old": "year", "price_M": "month", "price_lost": ""}; !reflect.DeepEqual(intervals, want) { |
| + t.Errorf("intervals = %v, want %v", intervals, want) |
| + } |
| + l := testLocalizer(t, testCatalog) |
| + picker := render(t, "abono-plan-picker", map[string]any{"L": l, "Offers": offers, "Action": "/subscribe"}) |
| + for _, want := range []string{"T-MONTHLY", "T-YEARLY", `value="month"`, `value="year"`} { |
| + if !strings.Contains(picker, want) { |
| + t.Errorf("the picker on a fresh site lacks %q:\n%s", want, picker) |
| + } |
| + } |
| + |
| + // Read once: a second Offers asks Stripe nothing more (the lost |
| + // price aside, which is tried again). |
| + before := d.fetches() |
| + if _, err := s.Offers(ctx); err != nil { |
| + t.Fatalf("Offers again: %v", err) |
| + } |
| + after := d.fetches() |
| + for id, n := range after { |
| + if id != "price_lost" && n != before[id] { |
| + t.Errorf("%s read again: %d times, was %d", id, n, before[id]) |
| + } |
| + } |
| + |
| + // With Stripe off nothing is read, and Offers still answers. |
| + off, _ := newService(t, &now, abono.Config{}) |
| + if err := off.SeedPlans(ctx, []abono.PlanSpec{{Plan: "annual", Sold: "price_A"}}); err != nil { |
| + t.Fatal(err) |
| + } |
| + if offers, err := off.Offers(ctx); err != nil || len(offers) != 1 || offers[0].Prices[0].Interval != "" { |
| + t.Errorf("Offers with Stripe off = %+v, %v", offers, err) |
| + } |
| +} |