amadan
stripeapi: period start reads the items first
Summary
PeriodStart takes the earliest item current_period_start when any item has one, and the top-level current_period_start only when none does (ruling R21), so a later top-level start can no longer lengthen the past_due window. TestSubscriptionWire gains conflicting cases: top-level later and earlier than the item, items that disagree, an item without a start, and top-level only.
2 files changed,
+35
−2
internal/stripeapi/client.go+10 −2internal/stripeapi/client_test.go+25 −0
CI passed — run details
CI log
=== vet === go vet ./... === fmt === === test === go test ./... -count=1 ok amadan.net/rastrillo/abono 0.589s ok amadan.net/rastrillo/abono/abonotest 0.052s ok amadan.net/rastrillo/abono/internal/fakestripe 0.019s ok amadan.net/rastrillo/abono/internal/stripeapi 0.007s === race === CGO_ENABLED=1 go test ./... -race -count=1 ok amadan.net/rastrillo/abono 4.889s ok amadan.net/rastrillo/abono/abonotest 1.433s ok amadan.net/rastrillo/abono/internal/fakestripe 1.125s ok amadan.net/rastrillo/abono/internal/stripeapi 1.020s === browser === browser: no browser test file yet, nothing to run
Intent
diff --git a/internal/stripeapi/client.go b/internal/stripeapi/client.go| index cbf91b1..f92f5d9 100644 |
| --- a/internal/stripeapi/client.go |
| +++ b/internal/stripeapi/client.go |
| @@ -81,13 +81,21 @@ func (s Subscription) PeriodEnd() time.Time { |
| // PeriodStart is when the current period began, or the zero time. After |
| // a failed renewal Stripe has already moved the period on, so this is |
| // where the unpaid period starts: the end of the one last paid for. |
| +// |
| +// The items are read first (ruling R21): the earliest item start, since |
| +// the past_due window counts from it and a later one would lengthen it. |
| +// The top-level start, an older API version's, is read only when no item |
| +// has one. |
| func (s Subscription) PeriodStart() time.Time { |
| - start := s.CurrentPeriodStart |
| + var start int64 |
| for _, it := range s.Items.Data { |
| - if it.CurrentPeriodStart > start { |
| + if it.CurrentPeriodStart != 0 && (start == 0 || it.CurrentPeriodStart < start) { |
| start = it.CurrentPeriodStart |
| } |
| } |
| + if start == 0 { |
| + start = s.CurrentPeriodStart |
| + } |
| if start == 0 { |
| return time.Time{} |
| } |
diff --git a/internal/stripeapi/client_test.go b/internal/stripeapi/client_test.go| index f5be0ae..56b027c 100644 |
| --- a/internal/stripeapi/client_test.go |
| +++ b/internal/stripeapi/client_test.go |
| @@ -34,6 +34,31 @@ func TestSubscriptionWire(t *testing.T) { |
| t.Errorf("%s: %+v", name, s) |
| } |
| } |
| + // Items first (ruling R21): a top-level start, later or not, is read |
| + // only when no item has one, and of several items the earliest wins. |
| + for name, c := range map[string]struct { |
| + raw string |
| + want int64 |
| + }{ |
| + "top-level later than the item": {`{"id":"sub_1","current_period_start":1750000000,"items":{"data":[{"current_period_start":1700000000}]}}`, 1_700_000_000}, |
| + "top-level earlier than the item": {`{"id":"sub_1","current_period_start":1650000000,"items":{"data":[{"current_period_start":1700000000}]}}`, 1_700_000_000}, |
| + "items disagree": {`{"id":"sub_1","items":{"data":[{"current_period_start":1710000000},{"current_period_start":1700000000}]}}`, 1_700_000_000}, |
| + "an item without one": {`{"id":"sub_1","current_period_start":1750000000,"items":{"data":[{},{"current_period_start":1700000000}]}}`, 1_700_000_000}, |
| + "only top-level": {`{"id":"sub_1","current_period_start":1750000000,"items":{"data":[{}]}}`, 1_750_000_000}, |
| + } { |
| + var w subscriptionWire |
| + if err := json.Unmarshal([]byte(c.raw), &w); err != nil { |
| + t.Fatal(err) |
| + } |
| + s, err := w.decode() |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if got := s.PeriodStart(); !got.Equal(time.Unix(c.want, 0)) { |
| + t.Errorf("%s: PeriodStart = %v, want %v", name, got, time.Unix(c.want, 0).UTC()) |
| + } |
| + } |
| + |
| var none Subscription |
| if !none.PeriodStart().IsZero() || !none.PeriodEnd().IsZero() { |
| t.Error("a subscription with no period has a start or an end") |