From 5060edae922275bbf52665126674a2dc62233b57 Mon Sep 17 00:00:00 2001 From: EOEboh Date: Mon, 3 Aug 2026 15:20:06 +0100 Subject: [PATCH] fix(billing): stop the repair reporting rows it has already reconciled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After marking the live subscription non-recurring, re-running the repair still reported one row outstanding — "auto_renews false -> false" — and would have done so on every run for ever. The selection is provider_sub_id NOT LIKE 'SUB_%', which stays true permanently for a bank-transfer payment: there is no subscription at Paystack, so a SUB_ code will never appear. Harmless in effect, since re-marking is idempotent, but it destroys the "nothing to do" signal that says the system is clean. A row already marked non-recurring has been reconciled, so it is now excluded. Also makes two zero-value hazards explicit. models.Subscription.AutoRenews defaults to false, meaning "does not renew" — the test fixture was seeding rows that way without intending to, which is what surfaced this. Both production write paths set it explicitly, so nothing was affected, but the synthetic free subscription returned for users without a row now sets it too, rather than reporting a free plan as something that will not renew. --- internal/store/store.go | 17 ++++++++++----- internal/store/store_test.go | 40 +++++++++++++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/internal/store/store.go b/internal/store/store.go index c12b407..3bfd7ab 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -722,12 +722,15 @@ func (s *Store) GetSubscription(userID string) (*models.Subscription, error) { &sub.CreatedAt, &sub.UpdatedAt, ) if err == sql.ErrNoRows { - // No subscription row: return a default free subscription + // No subscription row: return a default free subscription. + // AutoRenews is true rather than the zero value so the payload does + // not describe a free plan as something that will not renew. return &models.Subscription{ - UserID: userID, - Plan: "free", - Status: "active", - Currency: "usd", + UserID: userID, + Plan: "free", + Status: "active", + Currency: "usd", + AutoRenews: true, }, nil } if err != nil { @@ -925,6 +928,10 @@ func (s *Store) ListPaystackSubscriptionsNeedingRepair() ([]*models.Subscription FROM subscriptions WHERE provider = 'paystack' AND COALESCE(provider_sub_id,'') NOT LIKE 'SUB\_%' ESCAPE '\' + -- Already reconciled. A row marked non-recurring will never gain a + -- SUB_ code, because there is no subscription to find — without this + -- it is reported as outstanding on every run, for ever. + AND COALESCE(auto_renews,1) = 1 ORDER BY created_at`) if err != nil { return nil, err diff --git a/internal/store/store_test.go b/internal/store/store_test.go index f6bc1ee..7de5975 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -290,7 +290,10 @@ func seedSub(t *testing.T, s *Store, email, provider, subID string) *models.Subs Status: "active", Currency: "ngn", Interval: "month", - CreatedAt: time.Now().UTC(), + // Explicit: the zero value is false, which would mean "does not + // renew" and quietly exclude the row from the repair selection. + AutoRenews: true, + CreatedAt: time.Now().UTC(), } if err := s.UpsertSubscription(sub); err != nil { t.Fatalf("seed subscription: %v", err) @@ -363,3 +366,38 @@ func TestRepairPaystackSubscription(t *testing.T) { t.Errorf("%d rows still need repair after repairing them all", len(rows)) } } + +// A row already marked non-recurring has been reconciled. It will never gain a +// SUB_ code, so leaving it selected means the repair reports outstanding work +// for ever and the "nothing to do" signal becomes meaningless. +func TestListPaystackSubscriptionsSkipsReconciledRows(t *testing.T) { + s := newTestStore(t) + + sub := seedSub(t, s, "transfer@example.com", "paystack", "T981099203072787") + + rows, err := s.ListPaystackSubscriptionsNeedingRepair() + if err != nil { + t.Fatalf("list: %v", err) + } + if len(rows) != 1 { + t.Fatalf("selected %d rows before repair, want 1", len(rows)) + } + + if err := s.MarkSubscriptionNonRecurring(sub.ID); err != nil { + t.Fatalf("mark: %v", err) + } + + rows, err = s.ListPaystackSubscriptionsNeedingRepair() + if err != nil { + t.Fatalf("list after repair: %v", err) + } + if len(rows) != 0 { + t.Errorf("still selected after being marked non-recurring: %d rows", len(rows)) + } + + // The flag itself must have stuck. + got, _ := s.GetSubscription(sub.UserID) + if got.AutoRenews { + t.Error("auto_renews reverted to true") + } +}