rastrillo / abono Public

abono: keep a price's interval_count, and never sell one billed other than monthly or yearly

Summary

stripeapi reads recurring.interval_count (Price.Recurring and Subscription.Recurring). abono_prices gains interval_count, written by every price refresh and by Adopt. abono.Price gains IntervalCount and an Unsupported method: anything but every one month or every one year. Checkout never picks an unsupported price, the plan-card and plan-picker partials and the generated pricing cards never show one, and the owner's offers page shows it without a per-month or per-year sentence and with a new badge, abono.owner.price_unsupported. The fake reports a count set with SetIntervalCount.

Agent notes

Final review I7, ruling R39. migrations/0001_init.sql edited before v0.1.0 a third time: frozenAbonoChecksum re-recorded from 5701b242... to 906331b7...; development databases need a reset. New key abono.owner.price_unsupported is rendered as a key and added to pendingReview and newKeys for the third mini copy review. An interval_count of 0 (not recorded yet) reads as 1. Example pages regenerated. Regressions: TestGetPrice and TestSubscriptionWire read the count; TestAnUnsupportedPriceIsNeverSold; TestUnsupportedPricesAreNeverOffered (partials); example TestUnsupportedPriceIsShownNotSold (pricing page, owner page, abonoCards).

claude 2 weeks ago 04fe354be12dd7ce34a83692406fbbb55c3be0d7 parent d6921b9

18 files changed, +230 −48

adopt.go +4 −3
checkout.go +8 −4
cmd/abono/main_test.go +1 −1
cmd/abono/pages/abono_pages.go.tmpl +3 −2
cmd/abono/pages/owner-offers.html.tmpl +4 −2
example/abono_pages.go +3 −2
example/app_test.go +29 −0
example/templates/abono/owner-offers.html +4 −2
internal/fakestripe/fake.go +23 −2
internal/stripeapi/client.go +23 −10
internal/stripeapi/client_test.go +9 −4
migrations/0001_init.sql +4 −1
offers.go +32 −10
offers_test.go +35 −0
schema_test.go +1 −1
templates/abono-plan-card.html +4 −2
templates/abono-plan-picker.html +3 −2
templates_test.go +40 −0

CI passed — run details

CI log

=== vet ===
go vet ./...
=== fmt ===
=== test ===
go test ./... -count=1
ok  	amadan.net/rastrillo/abono	2.668s
ok  	amadan.net/rastrillo/abono/abonotest	0.068s
ok  	amadan.net/rastrillo/abono/cmd/abono	0.799s
ok  	amadan.net/rastrillo/abono/internal/fakestripe	0.021s
ok  	amadan.net/rastrillo/abono/internal/stripeapi	0.008s
ok  	example	0.172s
=== race ===
CGO_ENABLED=1 go test ./... -race -count=1
ok  	amadan.net/rastrillo/abono	8.482s
ok  	amadan.net/rastrillo/abono/abonotest	1.457s
ok  	amadan.net/rastrillo/abono/cmd/abono	1.959s
ok  	amadan.net/rastrillo/abono/internal/fakestripe	1.080s
ok  	amadan.net/rastrillo/abono/internal/stripeapi	1.022s
ok  	example	1.972s
=== browser ===
ok  	example	0.725s

Intent

full ledger: v0.1

diff --git a/adopt.go b/adopt.go

index 775e477..a9a4671 100644

--- a/adopt.go

+++ b/adopt.go

@@ -38,7 +38,8 @@ func (s *Service) Adopt(ctx context.Context, subscriptionID, subject, plan strin

if err != nil {

return err

}

- price, interval := sub.PriceID()

+ price, _ := sub.PriceID()

+ interval, count := sub.Recurring()

if price == "" {

return fmt.Errorf("abono: subscription %q has no price", subscriptionID)

}

@@ -55,8 +56,8 @@ func (s *Service) Adopt(ctx context.Context, subscriptionID, subject, plan strin

case err != nil:

return err

case on == "":

- if err := tx.Exec(`INSERT INTO abono_prices (stripe_price_id, plan, sold, interval) VALUES (?, ?, 0, ?)`,

- price, plan, interval).Error; err != nil {

+ if err := tx.Exec(`INSERT INTO abono_prices (stripe_price_id, plan, sold, interval, interval_count) VALUES (?, ?, 0, ?, ?)`,

+ price, plan, interval, count).Error; err != nil {

return err

}

case on != plan:

diff --git a/checkout.go b/checkout.go

index 63e5fa5..69ae22d 100644

--- a/checkout.go

+++ b/checkout.go

@@ -138,15 +138,16 @@ func (s *Service) refreshPrice(ctx context.Context, id string) error {

return fmt.Errorf("abono: reading price %q: %w", id, err)

}

return s.cfg.DB.WithContext(ctx).Exec(`UPDATE abono_prices

- SET interval = ?, amount = ?, currency = ?, refreshed_at = ? WHERE stripe_price_id = ?`,

- p.Recurring.Interval, p.UnitAmount, p.Currency, s.now(), id).Error

+ SET interval = ?, interval_count = ?, amount = ?, currency = ?, refreshed_at = ? WHERE stripe_price_id = ?`,

+ p.Recurring.Interval, p.Recurring.IntervalCount, p.UnitAmount, p.Currency, s.now(), id).Error

}

// soldPrice is the price to sell for plan at interval: the sold price with

-// that interval, or with no interval the plan's only sold price.

+// that interval, or with no interval the plan's only sold price. A price

+// v0.1 cannot sell (Price.Unsupported) is never one of them.

func soldPrice(g *gorm.DB, plan, interval string) (string, error) {

var prices []priceRow

- if err := g.Raw(`SELECT p.stripe_price_id, p.interval FROM abono_prices p

+ if err := g.Raw(`SELECT p.stripe_price_id, p.interval, p.interval_count FROM abono_prices p

JOIN abono_offers o ON o.plan = p.plan

WHERE p.plan = ? AND p.sold = 1 AND o.archived_at IS NULL

ORDER BY p.stripe_price_id`, plan).Scan(&prices).Error; err != nil {

@@ -154,6 +155,9 @@ func soldPrice(g *gorm.DB, plan, interval string) (string, error) {

}

var match []string

for _, p := range prices {

+ if p.price().Unsupported() {

+ continue

+ }

if interval == "" || p.Interval == interval {

match = append(match, p.StripePriceID)

}

diff --git a/cmd/abono/main_test.go b/cmd/abono/main_test.go

index ef9466d..37e58ca 100644

--- a/cmd/abono/main_test.go

+++ b/cmd/abono/main_test.go

@@ -284,7 +284,7 @@ var partialKeys = []string{"abono.price.per_month", "abono.price.per_year"}

// newKeys are keys the pages need that the draft lacks (ruling R28),

// going through their own copy review. The pages must use each.

-var newKeys = []string{"abono.owner.free_no_member", "abono.pages.done_processing"}

+var newKeys = []string{"abono.owner.free_no_member", "abono.pages.done_processing", "abono.owner.price_unsupported"}

var keyLiteral = regexp.MustCompile(`"(abono\.[a-z][a-z_.]*[a-z])"`)

diff --git a/cmd/abono/pages/abono_pages.go.tmpl b/cmd/abono/pages/abono_pages.go.tmpl

index bdfeded..beddd43 100644

--- a/cmd/abono/pages/abono_pages.go.tmpl

+++ b/cmd/abono/pages/abono_pages.go.tmpl

@@ -123,14 +123,15 @@ type abonoCard struct {

// abonoCards is a card per offer and sold interval, monthly before

// yearly. An offer sold at one price, or whose prices' intervals are not

// known yet, gets one card with no interval; an offer with nothing sold

-// gets none.

+// gets none. A price billed other than every month or every year

+// (Unsupported) is not sold, so it makes no card.

func abonoCards(offers []abono.Offer) []abonoCard {

var cards []abonoCard

for _, o := range offers {

var intervals []string

sold, unknown := 0, false

for _, pr := range o.Prices {

- if !pr.Sold {

+ if !pr.Sold || pr.Unsupported() {

continue

}

sold++

diff --git a/cmd/abono/pages/owner-offers.html.tmpl b/cmd/abono/pages/owner-offers.html.tmpl

index 1ef14b1..e88c160 100644

--- a/cmd/abono/pages/owner-offers.html.tmpl

+++ b/cmd/abono/pages/owner-offers.html.tmpl

@@ -1,5 +1,7 @@

{{/* abono/owner-offers: GET /admin/offers, each plan with its prices

- (the one on sale, and old ones whose members keep the plan), and the

+ (the one on sale, and old ones whose members keep the plan; a price

+ billed other than every month or every year is marked unsupported,

+ since it is never sold), and the

paying subscriptions on a price no plan has, each with a form that

puts it, and everyone else on its price, onto a plan.

@@ -24,7 +26,7 @@

{{range .Offers}}<section rst-box>

<ul>

-{{range .Prices}}<li><code>{{.ID}}</code>{{if .Currency}}{{$m := money $L .Amount .Currency}} {{if eq .Interval "month"}}{{Tf $L "abono.price.per_month" "amount" $m}}{{else if eq .Interval "year"}}{{Tf $L "abono.price.per_year" "amount" $m}}{{else}}{{$m}}{{end}}{{end}} {{if .Sold}}{{template "badge" dict "L" $L "Label" (T $L "abono.owner.price_sold") "Tone" "positive"}}{{else}}{{template "badge" dict "L" $L "Label" (T $L "abono.owner.price_legacy")}}{{end}}</li>

+{{range .Prices}}<li><code>{{.ID}}</code>{{if .Currency}}{{$m := money $L .Amount .Currency}} {{if .Unsupported}}{{$m}}{{else if eq .Interval "month"}}{{Tf $L "abono.price.per_month" "amount" $m}}{{else if eq .Interval "year"}}{{Tf $L "abono.price.per_year" "amount" $m}}{{else}}{{$m}}{{end}}{{end}} {{if .Sold}}{{template "badge" dict "L" $L "Label" (T $L "abono.owner.price_sold") "Tone" "positive"}}{{else}}{{template "badge" dict "L" $L "Label" (T $L "abono.owner.price_legacy")}}{{end}}{{if .Unsupported}} {{template "badge" dict "L" $L "Label" (T $L "abono.owner.price_unsupported") "Tone" "warning"}}{{end}}</li>

{{end}}</ul>

</section>

{{else}}{{template "empty-state" dict "L" $L "Body" (T $L "abono.owner.offers_empty")}}

diff --git a/example/abono_pages.go b/example/abono_pages.go

index c6ccfd7..4ffe650 100644

--- a/example/abono_pages.go

+++ b/example/abono_pages.go

@@ -123,14 +123,15 @@ type abonoCard struct {

// abonoCards is a card per offer and sold interval, monthly before

// yearly. An offer sold at one price, or whose prices' intervals are not

// known yet, gets one card with no interval; an offer with nothing sold

-// gets none.

+// gets none. A price billed other than every month or every year

+// (Unsupported) is not sold, so it makes no card.

func abonoCards(offers []abono.Offer) []abonoCard {

var cards []abonoCard

for _, o := range offers {

var intervals []string

sold, unknown := 0, false

for _, pr := range o.Prices {

- if !pr.Sold {

+ if !pr.Sold || pr.Unsupported() {

continue

}

sold++

diff --git a/example/app_test.go b/example/app_test.go

index 1432a67..0cb83aa 100644

--- a/example/app_test.go

+++ b/example/app_test.go

@@ -653,3 +653,32 @@ func TestBillingStaysAfterSwitchingToAnUnknownPrice(t *testing.T) {

t.Errorf("billing for a customer not the reader's: %d to %q", st, loc)

}

+// TestUnsupportedPriceIsShownNotSold is final review I7 (ruling R39) on

+// the generated pages: a sold price billed every twelve months is listed

+// for the owner, marked unsupported, and is not on the pricing page.

+func TestUnsupportedPriceIsShownNotSold(t *testing.T) {

+ var d *db.DB

+ app := newApp(t, func(c *Config) { d = c.DB })

+ // The sandbox read price_monthly at start-up as every month; make it

+ // every twelve, as Stripe would report such a price.

+ if err := d.G.Exec(`UPDATE abono_prices SET interval_count = 12 WHERE stripe_price_id = 'price_monthly'`).Error; err != nil {

+ t.Fatal(err)

+ }

+ reader := newClient(t, app)

+ status, body := reader.get("/subscribe")

+ want(t, "the plans", status, http.StatusOK, body, `action="/subscribe/annual"`)

+ if strings.Contains(body, `action="/subscribe/monthly"`) {

+ t.Error("the pricing page sells a price billed every twelve months")

+ }

+ boss := newClient(t, app)

+ boss.signin(owner)

+ status, body = boss.get("/admin/offers")

+ want(t, "the offers page", status, http.StatusOK, body, "price_monthly", say("abono.owner.price_unsupported"))

+ if strings.Contains(body, say("abono.price.per_month")) {

+ t.Error("the offers page calls a price billed every twelve months monthly")

+ }

+ if cards := abonoCards([]abono.Offer{{Plan: "dozen", Prices: []abono.Price{{ID: "p", Sold: true, Interval: "month", IntervalCount: 12, Currency: "eur"}}}}); len(cards) != 0 {

+ t.Errorf("abonoCards made %d cards for an unsupported price", len(cards))

+ }

diff --git a/example/templates/abono/owner-offers.html b/example/templates/abono/owner-offers.html

index 1ef14b1..e88c160 100644

--- a/example/templates/abono/owner-offers.html

+++ b/example/templates/abono/owner-offers.html

@@ -1,5 +1,7 @@

{{/* abono/owner-offers: GET /admin/offers, each plan with its prices

- (the one on sale, and old ones whose members keep the plan), and the

+ (the one on sale, and old ones whose members keep the plan; a price

+ billed other than every month or every year is marked unsupported,

+ since it is never sold), and the

paying subscriptions on a price no plan has, each with a form that

puts it, and everyone else on its price, onto a plan.

@@ -24,7 +26,7 @@

{{range .Offers}}<section rst-box>

<ul>

{{end}}</ul>

</section>

{{else}}{{template "empty-state" dict "L" $L "Body" (T $L "abono.owner.offers_empty")}}

diff --git a/internal/fakestripe/fake.go b/internal/fakestripe/fake.go

index adb11c1..fbd0904 100644

--- a/internal/fakestripe/fake.go

+++ b/internal/fakestripe/fake.go

@@ -114,6 +114,7 @@ type Fake struct {

type price struct {

id, plan, currency, interval string

amount int64

+ count int // interval_count; 0 reads as 1

}

type customer struct{ id, email, name string }

@@ -177,6 +178,17 @@ func (f *Fake) AddPrice(id, plan string, amount int64, currency, interval string

f.prices[id] = &price{id: id, plan: plan, amount: amount, currency: currency, interval: interval}

}

+// SetIntervalCount makes price id bill every n of its intervals (a price

+// of "month" with n 12 bills once a year). AddPrice's prices bill every

+// one.

+func (f *Fake) SetIntervalCount(id string, n int) {

+ f.mu.Lock()

+ defer f.mu.Unlock()

+ if p := f.prices[id]; p != nil {

+ p.count = n

+ }

// AddPromotionCode makes code a live promotion code with id.

func (f *Fake) AddPromotionCode(code, id string) {

f.mu.Lock()

@@ -822,7 +834,7 @@ func (f *Fake) subObject(s *subscription, expand bool) map[string]any {

}

var interval any

if s.interval != "" {

- interval = map[string]any{"interval": s.interval, "interval_count": 1}

+ interval = map[string]any{"interval": s.interval, "interval_count": intervalCount(f.prices[s.price])}

}

metadata := map[string]string{}

for k, v := range s.metadata {

@@ -870,7 +882,7 @@ func (f *Fake) invoice(s *subscription, status string) map[string]any {

func priceObject(p *price) map[string]any {

var recurring any

if p.interval != "" {

- recurring = map[string]any{"interval": p.interval, "interval_count": 1}

+ recurring = map[string]any{"interval": p.interval, "interval_count": intervalCount(p)}

}

typ := "recurring"

if recurring == nil {

@@ -880,6 +892,15 @@ func priceObject(p *price) map[string]any {

"recurring": recurring, "type": typ, "active": true}

}

+// intervalCount is p's interval_count, 1 unless SetIntervalCount said

+// otherwise.

+func intervalCount(p *price) int {

+ if p == nil || p.count < 1 {

+ return 1

+ }

+ return p.count

// periodAfter is the end of the n-th period of interval counted from

// anchor. Months and years keep the anchor's day, clamped to the last day

// of a shorter month (31 January, 28 February, 31 March), which is how

diff --git a/internal/stripeapi/client.go b/internal/stripeapi/client.go

index f92f5d9..a7e73ff 100644

--- a/internal/stripeapi/client.go

+++ b/internal/stripeapi/client.go

@@ -54,10 +54,8 @@ type Subscription struct {

CurrentPeriodStart int64 `json:"current_period_start"`

CurrentPeriodEnd int64 `json:"current_period_end"`

Price struct {

- ID string `json:"id"`

- Recurring struct {

- Interval string `json:"interval"`

- } `json:"recurring"`

+ ID string `json:"id"`

+ Recurring Recurring `json:"recurring"`

} `json:"price"`

} `json:"data"`

} `json:"items"`

@@ -111,6 +109,16 @@ func (s Subscription) PriceID() (id, interval string) {

return p.ID, p.Recurring.Interval

}

+// Recurring is how often the first item's price bills: every count

+// intervals ("month", 12 is yearly in twelve monthly steps, not monthly).

+func (s Subscription) Recurring() (interval string, count int) {

+ if len(s.Items.Data) == 0 {

+ return "", 0

+ }

+ r := s.Items.Data[0].Price.Recurring

+ return r.Interval, r.IntervalCount

// subscriptionWire is a subscription as the API sends it, whose

// customer is an id unless expanded.

type subscriptionWire struct {

@@ -172,12 +180,17 @@ func (c *Client) ListSubscriptions(ctx context.Context, each func(Subscription)

// Price is a recurring price: what a plan costs and how often.

type Price struct {

- ID string `json:"id"`

- UnitAmount int64 `json:"unit_amount"`

- Currency string `json:"currency"`

- Recurring struct {

- Interval string `json:"interval"`

- } `json:"recurring"`

+ ID string `json:"id"`

+ UnitAmount int64 `json:"unit_amount"`

+ Currency string `json:"currency"`

+ Recurring Recurring `json:"recurring"`

+// Recurring is how often a price bills: every IntervalCount Intervals.

+// Both are empty for a one-off price.

+type Recurring struct {

+ Interval string `json:"interval"`

+ IntervalCount int `json:"interval_count"`

}

// GetPrice fetches one price.

diff --git a/internal/stripeapi/client_test.go b/internal/stripeapi/client_test.go

index 56b027c..3c9fd27 100644

--- a/internal/stripeapi/client_test.go

+++ b/internal/stripeapi/client_test.go

@@ -17,8 +17,8 @@ import (

func TestSubscriptionWire(t *testing.T) {

t.Parallel()

for name, raw := range map[string]string{

- "basil, expanded": `{"id":"sub_1","status":"active","customer":{"id":"cus_1","email":"a@example.com"},"items":{"data":[{"current_period_start":1700000000,"current_period_end":1800000000,"price":{"id":"price_A","recurring":{"interval":"year"}}}]}}`,

- "older, id only": `{"id":"sub_1","status":"active","customer":"cus_1","current_period_start":1700000000,"current_period_end":1800000000,"items":{"data":[{"price":{"id":"price_A","recurring":{"interval":"year"}}}]}}`,

+ "basil, expanded": `{"id":"sub_1","status":"active","customer":{"id":"cus_1","email":"a@example.com"},"items":{"data":[{"current_period_start":1700000000,"current_period_end":1800000000,"price":{"id":"price_A","recurring":{"interval":"year","interval_count":2}}}]}}`,

+ "older, id only": `{"id":"sub_1","status":"active","customer":"cus_1","current_period_start":1700000000,"current_period_end":1800000000,"items":{"data":[{"price":{"id":"price_A","recurring":{"interval":"year","interval_count":2}}}]}}`,

} {

var w subscriptionWire

if err := json.Unmarshal([]byte(raw), &w); err != nil {

@@ -29,6 +29,9 @@ func TestSubscriptionWire(t *testing.T) {

t.Fatal(err)

}

price, interval := s.PriceID()

+ if _, count := s.Recurring(); count != 2 {

+ t.Errorf("%s: interval_count %d, want 2", name, count)

+ }

if s.Customer.ID != "cus_1" || price != "price_A" || interval != "year" || !s.PeriodEnd().Equal(time.Unix(1_800_000_000, 0)) ||

!s.PeriodStart().Equal(time.Unix(1_700_000_000, 0)) {

t.Errorf("%s: %+v", name, s)

@@ -174,13 +177,15 @@ func TestGetPrice(t *testing.T) {

var path string

c := fake(t, func(w http.ResponseWriter, r *http.Request) {

path = r.URL.Path

- w.Write([]byte(`{"id":"price_A","unit_amount":4900,"currency":"eur","recurring":{"interval":"year"}}`))

+ w.Write([]byte(`{"id":"price_A","unit_amount":4900,"currency":"eur","recurring":{"interval":"month","interval_count":12}}`))

})

p, err := c.GetPrice(context.Background(), "price_A")

if err != nil {

t.Fatal(err)

}

- if path != "/v1/prices/price_A" || p.ID != "price_A" || p.UnitAmount != 4900 || p.Currency != "eur" || p.Recurring.Interval != "year" {

+ // Every twelve months is not monthly (final review I7): the count is kept.

+ if path != "/v1/prices/price_A" || p.ID != "price_A" || p.UnitAmount != 4900 || p.Currency != "eur" ||

+ p.Recurring.Interval != "month" || p.Recurring.IntervalCount != 12 {

t.Errorf("%s %+v", path, p)

}

diff --git a/migrations/0001_init.sql b/migrations/0001_init.sql

index 06f4948..47df524 100644

--- a/migrations/0001_init.sql

+++ b/migrations/0001_init.sql

@@ -15,12 +15,15 @@ CREATE TABLE abono_offers (

-- The Stripe prices that belong to a plan. `sold` marks a price the

-- app offers to new buyers; a price no longer sold stays, so that

--- existing subscriptions on it still resolve to their plan.

+-- existing subscriptions on it still resolve to their plan. A price

+-- bills every `interval_count` `interval`s, as Stripe last said (0 until

+-- read); v0.1 sells only every one month or one year.

CREATE TABLE abono_prices (

stripe_price_id TEXT NOT NULL PRIMARY KEY,

plan TEXT NOT NULL REFERENCES abono_offers(plan),

sold INTEGER NOT NULL DEFAULT 0,

interval TEXT NOT NULL DEFAULT '',

+ interval_count INTEGER NOT NULL DEFAULT 0,

amount INTEGER NOT NULL DEFAULT 0,

currency TEXT NOT NULL DEFAULT '',

refreshed_at DATETIME

diff --git a/offers.go b/offers.go

index 7069157..11f7cfc 100644

--- a/offers.go

+++ b/offers.go

@@ -91,14 +91,32 @@ type Offer struct {

Prices []Price

}

-// Price is one Stripe price of an offer. Interval, Amount and Currency are

-// what Stripe last said, and empty until a refresh has read them.

+// Price is one Stripe price of an offer. Interval, IntervalCount, Amount

+// and Currency are what Stripe last said, and empty until a refresh has

+// read them. The price bills every IntervalCount Intervals.

type Price struct {

- ID string

- Sold bool

- Interval string

- Amount int64

- Currency string

+ ID string

+ Sold bool

+ Interval string

+ IntervalCount int

+ Amount int64

+ Currency string

+// Unsupported reports whether the price recurs in a way v0.1 does not

+// sell (ruling R39): anything but every one month or every one year, such

+// as every twelve months or every week. Checkout refuses it and the

+// partials never show it for sale; the owner's offers page marks it. A

+// price not read from Stripe yet, or a one-off one, has no interval and is

+// not unsupported. An IntervalCount of 0 (not recorded) reads as 1.

+func (p Price) Unsupported() bool {

+ switch p.Interval {

+ case "":

+ return false

+ case "month", "year":

+ return p.IntervalCount > 1

+ }

+ return true

}

type offerRow struct {

@@ -115,11 +133,17 @@ type priceRow struct {

Plan string

Sold bool

Interval string

+ IntervalCount int

Amount int64

Currency string

RefreshedAt *time.Time

}

+func (p priceRow) price() Price {

+ return Price{ID: p.StripePriceID, Sold: p.Sold, Interval: p.Interval, IntervalCount: p.IntervalCount,

+ Amount: p.Amount, Currency: p.Currency}

func (priceRow) TableName() string { return "abono_prices" }

// SeedPlans makes the database match specs: each plan gets an offer and

@@ -209,9 +233,7 @@ func (s *Service) Offers(ctx context.Context) ([]Offer, error) {

}

byPlan := map[string][]Price{}

for _, p := range prices {

- byPlan[p.Plan] = append(byPlan[p.Plan], Price{

- ID: p.StripePriceID, Sold: p.Sold, Interval: p.Interval, Amount: p.Amount, Currency: p.Currency,

- })

+ byPlan[p.Plan] = append(byPlan[p.Plan], p.price())

}

offers := make([]Offer, 0, len(rows))

for _, r := range rows {

diff --git a/offers_test.go b/offers_test.go

index 9302631..b10c544 100644

--- a/offers_test.go

+++ b/offers_test.go

@@ -2,6 +2,7 @@ package abono_test

import (

"context"

+ "errors"

"testing"

"amadan.net/rastrillo/abono"

@@ -156,3 +157,37 @@ func TestSeedPlansDemotesTheOldSoldPrice(t *testing.T) {

t.Errorf("annual prices =\n%s\nwant\n%s", got, want)

}

+// TestAnUnsupportedPriceIsNeverSold is final review I7 (ruling R39): a

+// price of €120 every twelve months has Stripe's interval "month". abono

+// keeps its interval_count, never sells it, and says so on Offers.

+func TestAnUnsupportedPriceIsNeverSold(t *testing.T) {

+ now := t0

+ d := newStripeDouble(t)

+ s, db := d.service(t, &now, abono.Config{})

+ ctx := context.Background()

+ d.price("price_12m", "month", 12000, "eur")

+ d.SetIntervalCount("price_12m", 12)

+ if err := s.SeedPlans(ctx, []abono.PlanSpec{{Plan: "dozen", Sold: "price_12m"}}); err != nil {

+ t.Fatalf("SeedPlans: %v", err)

+ }

+ for _, iv := range []string{"month", ""} {

+ _, err := s.Checkout(ctx, abono.CheckoutRequest{Plan: "dozen", Interval: iv, SuccessPath: "/done"})

+ if !errors.Is(err, abono.ErrUnknownPlan) {

+ t.Errorf("Checkout(dozen, %q): err = %v, want ErrUnknownPlan", iv, err)

+ }

+ if len(d.CheckoutForms()) != 0 {

+ t.Error("a Checkout session was made for an unsupported price")

+ }

+ if got := db.dump(`SELECT interval, interval_count FROM abono_prices WHERE stripe_price_id = 'price_12m'`); got != "month|12\n" {

+ t.Errorf("price_12m = %q, want month|12", got)

+ }

+ offers, err := s.Offers(ctx)

+ if err != nil || len(offers) != 1 || len(offers[0].Prices) != 1 {

+ t.Fatalf("Offers = %+v, %v", offers, err)

+ }

+ if p := offers[0].Prices[0]; p.IntervalCount != 12 || !p.Unsupported() {

+ t.Errorf("Offers price = %+v, want every 12 months and unsupported", p)

+ }

diff --git a/schema_test.go b/schema_test.go

index 5bca8c8..395eb13 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 = "5701b2420d58c9f91d9c59038e8a7f4f242f98441468f6ba1f660ff707d37112"

+const frozenAbonoChecksum = "906331b740701f831c91bb51cc414f1f43b66502b24599ac6c01da6eaef52858"

func TestSchemaFrozenChecksum(t *testing.T) {

for _, m := range abono.Schema.All() {

diff --git a/templates/abono-plan-card.html b/templates/abono-plan-card.html

index 931f625..f2bd8f0 100644

--- a/templates/abono-plan-card.html

+++ b/templates/abono-plan-card.html

@@ -3,7 +3,9 @@

A plan with nothing sold at Interval says so ("abono.plan.interval_unavailable")

and offers no button, rather than quietly selling another interval.

- Prices that are no longer sold (legacy prices) are never shown.

+ Prices that are no longer sold (legacy prices) are never shown, nor

+ is a price billed other than every month or every year

+ (Price.Unsupported).

Keys:

Offer abono.Offer, required

@@ -13,4 +15,4 @@

Action string, required — where the buy form posts

Code bool, optional — a promotion code field in the buy form

L *ui.Localizer */}}

-{{define "abono-plan-card"}}{{$L := localizer $}}{{$iv := or (opt . "Interval") ""}}{{$found := false}}<div rst-box><h3>{{or .Offer.Name .Offer.Plan}}</h3>{{range .Offer.Prices}}{{if and .Sold (or (eq $iv "") (eq .Interval $iv))}}{{$found = true}}{{if .Currency}}<p>{{template "abono-price" dict "L" $L "Price" .}}</p>{{end}}{{end}}{{end}}{{if $found}}{{template "abono-subscribe-button" dict "L" $L "Action" .Action "Plan" .Offer.Plan "Interval" $iv "Code" (opt . "Code")}}{{else}}<p>{{T $L "abono.plan.interval_unavailable"}}</p>{{end}}</div>{{end}}

+{{define "abono-plan-card"}}{{$L := localizer $}}{{$iv := or (opt . "Interval") ""}}{{$found := false}}<div rst-box><h3>{{or .Offer.Name .Offer.Plan}}</h3>{{range .Offer.Prices}}{{if and .Sold (not .Unsupported) (or (eq $iv "") (eq .Interval $iv))}}{{$found = true}}{{if .Currency}}<p>{{template "abono-price" dict "L" $L "Price" .}}</p>{{end}}{{end}}{{end}}{{if $found}}{{template "abono-subscribe-button" dict "L" $L "Action" .Action "Plan" .Offer.Plan "Interval" $iv "Code" (opt . "Code")}}{{else}}<p>{{T $L "abono.plan.interval_unavailable"}}</p>{{end}}</div>{{end}}

diff --git a/templates/abono-plan-picker.html b/templates/abono-plan-picker.html

index 2d56157..ef63430 100644

--- a/templates/abono-plan-picker.html

+++ b/templates/abono-plan-picker.html

@@ -1,6 +1,7 @@

{{/* abono-plan-picker — the plans on sale, as cards grouped by interval:

the monthly plans, then the yearly ones. A plan appears in a group

- only when it has a price sold at that interval. With only one

+ only when it has a price sold at that interval (every month or every

+ year: an unsupported price is never shown). With only one

interval on sale there is one group and no switch label; with nothing

on sale the picker renders nothing.

@@ -11,4 +12,4 @@

Offers []abono.Offer, required — from Service.Offers

Action string, required — where each card's buy form posts

L *ui.Localizer */}}

-{{define "abono-plan-picker"}}{{$L := localizer $}}{{$action := .Action}}{{$month := false}}{{$year := false}}{{range .Offers}}{{range .Prices}}{{if .Sold}}{{if eq .Interval "month"}}{{$month = true}}{{else if eq .Interval "year"}}{{$year = true}}{{end}}{{end}}{{end}}{{end}}{{if or $month $year}}<div{{if and $month $year}} role="group" aria-label="{{T $L "abono.price.switch_label"}}"{{end}}>{{range $iv := list "month" "year"}}{{if or (and (eq $iv "month") $month) (and (eq $iv "year") $year)}}<section>{{if and $month $year}}<p><strong>{{if eq $iv "month"}}{{T $L "abono.price.monthly"}}{{else}}{{T $L "abono.price.yearly"}}{{end}}</strong></p>{{end}}{{range $.Offers}}{{$has := false}}{{range .Prices}}{{if and .Sold (eq .Interval $iv)}}{{$has = true}}{{end}}{{end}}{{if $has}}{{template "abono-plan-card" dict "L" $L "Offer" . "Interval" $iv "Action" $action}}{{end}}{{end}}</section>{{end}}{{end}}</div>{{end}}{{end}}

+{{define "abono-plan-picker"}}{{$L := localizer $}}{{$action := .Action}}{{$month := false}}{{$year := false}}{{range .Offers}}{{range .Prices}}{{if and .Sold (not .Unsupported)}}{{if eq .Interval "month"}}{{$month = true}}{{else if eq .Interval "year"}}{{$year = true}}{{end}}{{end}}{{end}}{{end}}{{if or $month $year}}<div{{if and $month $year}} role="group" aria-label="{{T $L "abono.price.switch_label"}}"{{end}}>{{range $iv := list "month" "year"}}{{if or (and (eq $iv "month") $month) (and (eq $iv "year") $year)}}<section>{{if and $month $year}}<p><strong>{{if eq $iv "month"}}{{T $L "abono.price.monthly"}}{{else}}{{T $L "abono.price.yearly"}}{{end}}</strong></p>{{end}}{{range $.Offers}}{{$has := false}}{{range .Prices}}{{if and .Sold (not .Unsupported) (eq .Interval $iv)}}{{$has = true}}{{end}}{{end}}{{if $has}}{{template "abono-plan-card" dict "L" $L "Offer" . "Interval" $iv "Action" $action}}{{end}}{{end}}</section>{{end}}{{end}}</div>{{end}}{{end}}

diff --git a/templates_test.go b/templates_test.go

index fda5fd1..c0cec61 100644

--- a/templates_test.go

+++ b/templates_test.go

@@ -100,6 +100,9 @@ var (

yearly = abono.Price{ID: "price_y", Sold: true, Interval: "year", Amount: 5000, Currency: "EUR"}

monthly = abono.Price{ID: "price_m", Sold: true, Interval: "month", Amount: 500, Currency: "EUR"}

oldYr = abono.Price{ID: "price_old", Sold: false, Interval: "year", Amount: 4000, Currency: "EUR"}

+ // dozen is sold, monthly by Stripe's interval, but every twelve months:

+ // v0.1 cannot sell it and must never show it as €120 a month.

+ dozen = abono.Price{ID: "price_12m", Sold: true, Interval: "month", IntervalCount: 12, Amount: 12000, Currency: "EUR"}

)

func supporter() abono.Offer {

@@ -342,6 +345,42 @@ func TestPlanCard(t *testing.T) {

}

+// TestUnsupportedPricesAreNeverOffered is final review I7 (ruling R39):

+// a price billed other than every month or every year is not shown or sold

+// by the card or the picker, whatever its interval says.

+func TestUnsupportedPricesAreNeverOffered(t *testing.T) {

+ l := testLocalizer(t, testCatalog)

+ odd := abono.Offer{Plan: "dozen", Name: "Dozen", Prices: []abono.Price{dozen}}

+ got := render(t, "abono-plan-card", map[string]any{"L": l, "Offer": odd, "Interval": "month", "Action": "/subscribe"})

+ if strings.Contains(got, "€120.00") || strings.Contains(got, "T-PERMONTH") || strings.Contains(got, "<form") {

+ t.Errorf("a card sells a price billed every twelve months as monthly:\n%s", got)

+ }

+ if !strings.Contains(got, "T-NOINTERVAL") {

+ t.Errorf("the card does not say the plan is not sold that way:\n%s", got)

+ }

+ got = render(t, "abono-plan-picker", map[string]any{"L": l, "Offers": []abono.Offer{odd}, "Action": "/subscribe"})

+ if strings.TrimSpace(got) != "" {

+ t.Errorf("the picker offers an unsupported price:\n%s", got)

+ }

+ got = render(t, "abono-plan-picker", map[string]any{"L": l, "Offers": []abono.Offer{odd, yearlyOnly()}, "Action": "/subscribe"})

+ if strings.Contains(got, "Dozen") || strings.Contains(got, "T-MONTHLY") {

+ t.Errorf("the picker shows the unsupported plan or a monthly group:\n%s", got)

+ }

+ for _, c := range []struct {

+ p abono.Price

+ want bool

+ }{

+ {dozen, true}, {monthly, false}, {yearly, false},

+ {abono.Price{Interval: "week", IntervalCount: 1}, true},

+ {abono.Price{Interval: "year", IntervalCount: 2}, true},

+ {abono.Price{}, false}, // not read from Stripe yet, or a one-off

+ } {

+ if got := c.p.Unsupported(); got != c.want {

+ t.Errorf("%+v.Unsupported() = %v, want %v", c.p, got, c.want)

+ }

// A key documented as optional may be left out, in either shape of data

// a partial takes: a map without it, or a struct without the field.

func TestOptionalKeysMayBeOmitted(t *testing.T) {

@@ -671,6 +710,7 @@ var pendingReview = []string{

"abono.owner.free_no_member",

// The final-review fix wave's keys, for the third mini copy review.

"abono.pages.done_processing",

+ "abono.owner.price_unsupported",

}

// Every key a consumer uses is in en.toml (bar the pendingReview ones), and