Skip to content

Commit 5cdcfa4

Browse files
fix(subscriptions): make cancel_at a real schedule signal and harden the plan-change state machine (#545)
`subscriptions.cancel_at` shipped as NOT NULL DEFAULT now() with no writer ever advancing it, so every row was born with a cancel date permanently in the past. Three bugs fell out of that: 1. The Solana crank ORed `cancel_at <= now()` into its cancel decision, so it cancelled EVERY subscription at its first rollover. 2. `change_subscription_plan` writes `cancel_at = NULL` on the reactivate branch, which the NOT NULL column rejected with 23502 — every A -> B -> A plan switch failed. 3. Nothing gated price: a student on a free plan could switch to a paid one, and a self-managed (non-Stripe/LS) subscriber could upgrade, with no payment taken. Schema (20260726120000): `cancel_at` becomes nullable with no default, `cancel_at_period_end` becomes NOT NULL DEFAULT false, and a CHECK (`subscriptions_cancel_at_requires_schedule`) pins the contract — a cancel date may exist only while the flag is set. `cancel_at_period_end` is now the ONLY signal that a cancel is scheduled; `cancel_at` is informational. `canceled_at`/`ended_at` lose their now() defaults too, and live rows are backfilled clean. handle_new_subscription (20260726120100): the parallel-subscription backstop now counts `renewed` as live, and a resubscribe clears the stale cancel fields instead of inheriting them. change_subscription_plan (20260726120200): takes a per-user advisory lock so a double-submit cannot build two subscriptions, writes `cancel_at = NULL` alongside the flag, refuses an upgrade that would not settle natively (`upgrade_requires_payment`), and carries a pending cancel across the swap instead of silently dropping or resurrecting it. Application side: - solana-pull-decision reads only `cancel_at_period_end`; a stale date no longer cancels a healthy subscription. - `renewed` added to BLOCKING_SUBSCRIPTION_STATUSES (it bills and grants access, so it must block a parallel checkout). - The `subscription.past_due` webhook branch now actually writes the status instead of dropping the event. (Refund handling is deliberately untouched.) - Cancelling never IMPROVES a status: a past_due student who cancels stays past_due rather than being rewritten as healthy. Reactivating clears `cancel_at` with the flag. - Billing UI treats past_due as live (it was falling through to the "no subscription" empty state mid-dunning), shows a dunning notice, and only offers same-or-cheaper plans when the provider cannot settle the swap. Tests: new `subscription-plan-change-rpc.spec.ts` drives the RPC against a real DB as an authenticated student (A -> B, A -> B -> A, cancel-then-switch, same_plan, cross-tenant, the price gate, the advisory lock), asserting the entitlement delta per course via `has_course_access`. plan-change.spec.ts previously CLAIMED that coverage while nothing exercised the RPC at all — which is how bugs 2 and 3 shipped; its header now points at the real spec. Each fix was proven load-bearing by restoring the old schema/function and watching the new tests go red (A -> B -> A reproduced the exact 23502). Unit: 427/427 (411 on master). Playwright plan-change + entitlement batch 19/19. Closes #545 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WiXe7s6snHsso7q9LE9gCq
1 parent 138e68f commit 5cdcfa4

26 files changed

Lines changed: 1704 additions & 84 deletions

CLAUDE.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ Also supported (`products.payment_provider`): `paypal`, `lemonsqueezy`, `solana`
105105

106106
**Key invariants:**
107107
- Transaction `status`: `pending`, `successful`, `failed`, `archived`, `canceled`, `refunded`
108+
- **`subscriptions.cancel_at_period_end` is the ONLY signal that a cancel is scheduled** (since `20260726120000`, issue #545). `cancel_at` is nullable and purely informational — the CHECK `subscriptions_cancel_at_requires_schedule` allows it to be non-NULL only while the flag is set, so clear both together (both reactivate actions do). It shipped as `NOT NULL DEFAULT now()` with no writer setting it, which made the Solana crank cancel every subscription at its first rollover. `subscription_status` is `active`/`canceled`/`expired`/`renewed`/`past_due`; `renewed` and `past_due` both count as LIVE (parallel-subscription guards, billing UI, plan change), and cancelling must never *improve* a status
108109
- **Access control lives in `entitlements`, not `enrollments`** (since migration `20260516150000`): `entitlements` (`user_id`, `course_id`, `tenant_id`, `source_type`, `source_id`, `status`, `expires_at`) is the polymorphic source of truth for product/subscription access. `enrollments.product_id`/`subscription_id` and their old CHECK constraint were dropped — `enrollments` is now a learning-progress record only (`user_id`, `course_id`, `status`, `tenant_id`, `enrollment_date`)
109110
- `enroll_user()` RPC loops through ALL courses for a product (a product can map to multiple courses via `product_courses`) and writes to `entitlements`
110111
- **Subscriptions grant access, not auto-enrollment** — students self-enroll via `/dashboard/student/browse` (`useEnrollment()` hook); `plan_courses` defines which courses a plan covers

app/[locale]/(public)/checkout/actions.ts

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -249,9 +249,33 @@ function mapPlanChangeError(raw: string): string {
249249
if (raw.includes('no_active_subscription')) return "You don't have an active subscription to change.";
250250
if (raw.includes('invalid_plan') || raw.includes('cross_tenant_plan')) return "Plan not found.";
251251
if (raw.includes('plan_deleted')) return "That plan is no longer available.";
252+
if (raw.includes('upgrade_requires_payment')) {
253+
return "This plan costs more than your current one. Contact your school to arrange the payment for the upgrade.";
254+
}
255+
// 23502 (not-null) / 23514 (check) on the subscriptions cancel columns. The
256+
// #545 schema change should make these unreachable; before it, the
257+
// reactivate branch of the RPC wrote NULL into a NOT NULL `cancel_at` and
258+
// every switch back to a previously-held plan died here behind the generic
259+
// message below, with no clue for the student or the logs.
260+
if (raw.includes('23502') || raw.includes('23514') || raw.includes('cancel_at')) {
261+
return "We couldn't switch your plan because of a problem on our side. Please contact your school.";
262+
}
252263
return "Could not change your plan. Please try again.";
253264
}
254265

266+
/**
267+
* RPC errors raised BEFORE the function writes anything, where our DB is
268+
* already in the state the caller wanted (or never had a subscription to
269+
* change). Compensating a native provider swap on one of these actively breaks
270+
* the pairing: on a double-click, call A swaps the provider to plan B and
271+
* commits the DB; call B swaps to B again, gets `same_plan`, and a blind revert
272+
* would put the provider back on plan A's price with `proration_behavior:
273+
* 'none'` — DB on B, billing A, no error the student ever sees (#545).
274+
*/
275+
function isNoOpPlanChangeError(raw: string): boolean {
276+
return raw.includes('same_plan') || raw.includes('no_active_subscription');
277+
}
278+
255279
/**
256280
* Switch the student's current subscription to a different plan of the SAME
257281
* school + payment provider (supersession — never two live subs; issue #463).
@@ -360,8 +384,12 @@ export async function changePlan(newPlanId?: string) {
360384
});
361385
if (rpcError) {
362386
// Compensate: put the provider subscription back on the old price
363-
// so billing and our DB stay consistent.
364-
if (currentPlan?.provider_price_id) {
387+
// so billing and our DB stay consistent — but only when the RPC
388+
// genuinely failed to apply the switch. `same_plan` /
389+
// `no_active_subscription` mean our DB is already where the
390+
// student wanted it (or never had a subscription to move), so a
391+
// revert would DESYNC the provider rather than restore it.
392+
if (currentPlan?.provider_price_id && !isNoOpPlanChangeError(rpcError.message)) {
365393
try {
366394
await provider.updateSubscription(current.provider_subscription_id, {
367395
newProviderPriceId: currentPlan.provider_price_id,

app/[locale]/dashboard/student/billing/page.tsx

Lines changed: 60 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,20 @@ interface Subscription {
5151
cancel_at_period_end: boolean | null
5252
plan_id: number
5353
payment_provider: string
54+
provider_subscription_id: string | null
5455
}
5556

57+
/**
58+
* Statuses whose subscription is the student's LIVE one — the row this page
59+
* renders and offers cancel / switch controls for.
60+
*
61+
* `past_due` belongs here (#545). It used to be excluded, so a student mid
62+
* dunning got the "no subscription" empty state: neither ChangePlanDialog nor
63+
* ManageSubscription mounted and they could neither cancel nor switch while
64+
* still being billed, even though both server actions accept the status.
65+
*/
66+
const LIVE_SUBSCRIPTION_STATUSES: SubscriptionStatus[] = ['active', 'renewed', 'past_due']
67+
5668
interface PaymentRequest {
5769
request_id: number
5870
status: string
@@ -197,7 +209,7 @@ export default async function StudentBillingPage() {
197209
// ── 2. Subscriptions ────────────────────────────────────────────────────────
198210
const { data: rawSubs } = await supabase
199211
.from('subscriptions')
200-
.select('subscription_id, subscription_status, current_period_end, end_date, cancel_at_period_end, plan_id, payment_provider')
212+
.select('subscription_id, subscription_status, current_period_end, end_date, cancel_at_period_end, plan_id, payment_provider, provider_subscription_id')
201213
.eq('user_id', userId)
202214
.eq('tenant_id', tenantId)
203215
.order('created', { ascending: false })
@@ -235,9 +247,10 @@ export default async function StudentBillingPage() {
235247
}
236248

237249
// Derived
238-
const activeSubscription = subscriptions.find(
239-
(s) => s.subscription_status === 'active' || s.subscription_status === 'renewed'
250+
const activeSubscription = subscriptions.find((s) =>
251+
LIVE_SUBSCRIPTION_STATUSES.includes(s.subscription_status)
240252
)
253+
const isPastDue = activeSubscription?.subscription_status === 'past_due'
241254

242255
// ── Plan-change (switch) support ────────────────────────────────────────────
243256
// Switchable when the provider does an in-place swap (Stripe/LS) OR is
@@ -250,14 +263,38 @@ export default async function StudentBillingPage() {
250263
!!activeCaps &&
251264
(activeCaps.supportsPlanChange || !activeCaps.supportsNativeSubscriptions)
252265

266+
// Does this subscription settle a price difference by itself? Only an
267+
// in-place provider swap with proration does (Stripe / Lemon Squeezy, and
268+
// only when we actually hold a provider subscription to swap). Mirrors
269+
// `_settles_natively` in change_subscription_plan.
270+
const settlesNatively =
271+
!!activeCaps?.supportsPlanChange && !!activeSubscription?.provider_subscription_id
272+
253273
let switchablePlans: SwitchablePlan[] = []
254-
if (canSwitchPlan) {
255-
const { data: rawPlans } = await supabase
256-
.from('plans')
257-
.select('plan_id, plan_name, price, payment_provider')
258-
.eq('tenant_id', tenantId)
259-
.is('deleted_at', null)
260-
switchablePlans = (rawPlans ?? []) as SwitchablePlan[]
274+
if (canSwitchPlan && activeSubscription) {
275+
const [{ data: rawPlans }, { data: currentPlanRow }] = await Promise.all([
276+
supabase
277+
.from('plans')
278+
.select('plan_id, plan_name, price, payment_provider')
279+
.eq('tenant_id', tenantId)
280+
.is('deleted_at', null),
281+
supabase.from('plans').select('price').eq('plan_id', activeSubscription.plan_id).maybeSingle(),
282+
])
283+
const allPlans = (rawPlans ?? []) as SwitchablePlan[]
284+
285+
// Price gate, server-side (#545). This list used to be every non-deleted
286+
// tenant plan, so a student on a one-click FREE plan could pick the
287+
// school's paid bank-transfer plan and walk away with its entitlements —
288+
// no transaction, no payment_requests row, nothing for an admin to
289+
// reconcile. Anything that does not settle natively may only move to a
290+
// plan costing the same or less; the RPC refuses the rest with
291+
// `upgrade_requires_payment` regardless of what the UI offers.
292+
// `currentPlanRow` is null for a soft-deleted current plan, which fails
293+
// closed to free-or-nothing rather than opening the gate.
294+
const currentPrice = Number(currentPlanRow?.price ?? 0)
295+
switchablePlans = settlesNatively
296+
? allPlans
297+
: allPlans.filter((p) => Number(p.price) <= currentPrice)
261298
}
262299

263300
return (
@@ -298,13 +335,24 @@ export default async function StudentBillingPage() {
298335
</div>
299336
</CardHeader>
300337
<CardContent className="pt-0 space-y-2">
338+
{/* Dunning notice (#545): a past_due subscription still bills and
339+
still grants access during the provider's retry window, so say
340+
so instead of rendering it as if nothing were wrong. */}
341+
{isPastDue && (
342+
<div className="flex items-start gap-2 rounded-md bg-amber-50 dark:bg-amber-900/20 border border-amber-200 dark:border-amber-800 px-3 py-2 text-xs text-amber-800 dark:text-amber-400">
343+
<IconAlertTriangle className="w-3.5 h-3.5 shrink-0 mt-0.5" />
344+
<span>{t('subscription.pastDueNotice')}</span>
345+
</div>
346+
)}
301347
{/* Renewal / expiry date */}
302348
{(activeSubscription.current_period_end ?? activeSubscription.end_date) && (
303349
<div className="flex items-center justify-between text-sm">
304350
<span className="text-muted-foreground">
305351
{activeSubscription.cancel_at_period_end
306352
? t('subscription.cancelsOn')
307-
: t('subscription.renewsOn')}
353+
: isPastDue
354+
? t('subscription.paymentDueSince')
355+
: t('subscription.renewsOn')}
308356
</span>
309357
<span className="font-medium tabular-nums">
310358
{formatDate(activeSubscription.current_period_end ?? activeSubscription.end_date)}
@@ -340,7 +388,7 @@ export default async function StudentBillingPage() {
340388
<ChangePlanDialog
341389
currentPlanId={activeSubscription.plan_id}
342390
currentProvider={activeProvider}
343-
isNativeSwap={!!activeCaps?.supportsPlanChange}
391+
isNativeSwap={settlesNatively}
344392
plans={switchablePlans}
345393
/>
346394
)}

app/actions/admin/subscriptions.ts

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,17 @@ export async function cancelSubscription(
6262
}
6363
}
6464

65+
// A period-end cancel keeps whatever live status the row already had —
66+
// cancelling must never IMPROVE it (#545). Writing 'active' unconditionally
67+
// erased a `past_due` delinquency from billing health and the admin views
68+
// the moment anyone scheduled a cancel. Only the legacy `renewed` status is
69+
// normalized to 'active'; nothing writes it any more.
70+
const liveStatus =
71+
subscription.subscription_status === 'renewed' ? 'active' : subscription.subscription_status
72+
6573
const updates: any = {
66-
// Use 'canceled' (correct enum value) for immediate; keep 'active' for period-end
67-
subscription_status: immediate ? 'canceled' : 'active',
74+
// Use 'canceled' (correct enum value) for immediate; keep the live status for period-end
75+
subscription_status: immediate ? 'canceled' : liveStatus,
6876
canceled_at: new Date().toISOString(),
6977
}
7078

@@ -160,18 +168,16 @@ export async function reactivateSubscription(subscriptionId: number) {
160168
}
161169
}
162170

163-
// Reactivate the subscription. NB: `cancel_at` is NOT NULL in the schema —
164-
// setting it to null here would fail the update (a latent bug that silently
165-
// broke admin reactivate). Clearing `cancel_at_period_end` un-schedules the
166-
// subscription-expiry crons, but the Solana auto-pull crank
167-
// (lib/payments/solana-pull-decision.ts) reads `cancel_at` on its own, so we
168-
// push it forward to the live period end instead of leaving a past value that
169-
// would re-cancel the row. `canceled_at` is cleared.
171+
// Reactivate the subscription. `cancel_at` is cleared WITH the flag: since
172+
// #545 it is nullable and the CHECK
173+
// `subscriptions_cancel_at_requires_schedule` rejects a cancel date that
174+
// outlives its schedule, so a leftover date can no longer be read as a live
175+
// cancel by the Solana auto-pull crank. `canceled_at` is cleared too.
170176
const { error: updateError } = await supabase
171177
.from('subscriptions')
172178
.update({
173179
cancel_at_period_end: false,
174-
cancel_at: subscription.current_period_end || subscription.end_date,
180+
cancel_at: null,
175181
canceled_at: null,
176182
})
177183
.eq('subscription_id', subscriptionId)

app/actions/subscriptions.ts

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -75,15 +75,26 @@ export async function cancelMySubscription(subscriptionId: number): Promise<Acti
7575
}
7676
}
7777

78-
// Schedule the cancel in our DB. Status stays 'active' so access continues to
79-
// period end; the cron / webhook flips it to expired when the period lapses.
78+
// Schedule the cancel in our DB. The status stays live so access continues
79+
// to period end; the cron / webhook flips it to expired when the period
80+
// lapses.
81+
//
82+
// Cancelling must never IMPROVE a status (#545). This used to write
83+
// 'active' unconditionally, so a student mid-dunning who cancelled had
84+
// their `past_due` row rewritten as healthy and the delinquency vanished
85+
// from billing health and the admin views. Only the legacy `renewed`
86+
// status is normalized — it means the same thing as `active` and nothing
87+
// else writes it any more.
88+
const nextStatus =
89+
subscription.subscription_status === 'renewed' ? 'active' : subscription.subscription_status
90+
8091
const { error: updateError } = await supabase
8192
.from('subscriptions')
8293
.update({
8394
cancel_at_period_end: true,
8495
cancel_at: subscription.current_period_end || subscription.end_date,
8596
canceled_at: new Date().toISOString(),
86-
subscription_status: 'active',
97+
subscription_status: nextStatus,
8798
})
8899
.eq('subscription_id', subscriptionId)
89100
.eq('tenant_id', tenantId)
@@ -155,17 +166,15 @@ export async function reactivateMySubscription(subscriptionId: number): Promise<
155166
}
156167
}
157168

158-
// NB: `cancel_at` is NOT NULL in the schema — never set it to null here or the
159-
// update is rejected. Clearing `cancel_at_period_end` un-schedules the
160-
// subscription-expiry crons, but the Solana auto-pull crank
161-
// (lib/payments/solana-pull-decision.ts) reads `cancel_at` on its own, so we
162-
// push it forward to the live period end instead of leaving a past value that
163-
// could re-cancel the row. `canceled_at` is cleared.
169+
// Un-schedule the cancel. `cancel_at` is cleared WITH the flag: since #545
170+
// it is nullable and the CHECK `subscriptions_cancel_at_requires_schedule`
171+
// rejects a cancel date that outlives its schedule, so a leftover date can
172+
// no longer be read as a live cancel by the Solana auto-pull crank.
164173
const { error: updateError } = await supabase
165174
.from('subscriptions')
166175
.update({
167176
cancel_at_period_end: false,
168-
cancel_at: subscription.current_period_end || subscription.end_date,
177+
cancel_at: null,
169178
canceled_at: null,
170179
})
171180
.eq('subscription_id', subscriptionId)

components/student/change-plan-dialog.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,12 @@ export function ChangePlanDialog({
5555

5656
// Only plans on the same provider (a cross-provider switch needs a new
5757
// checkout), excluding the plan the student is already on.
58+
//
59+
// `plans` is ALREADY price-gated by the billing page (#545) — a subscription
60+
// whose provider does not settle the difference itself is handed only
61+
// same-or-cheaper plans, so an unpaid upgrade is never on offer. This filter
62+
// is presentation only; `change_subscription_plan` enforces the price rule
63+
// server-side no matter what reaches this list.
5864
const switchable = plans.filter(
5965
(p) => p.plan_id !== currentPlanId && (p.payment_provider ?? 'manual') === currentProvider,
6066
)

components/student/manage-subscription.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ export function ManageSubscription({
7777
variant="outline"
7878
size="sm"
7979
className="gap-1.5"
80+
data-testid="resume-subscription-button"
8081
onClick={handleReactivate}
8182
disabled={pending}
8283
>
@@ -92,6 +93,7 @@ export function ManageSubscription({
9293
variant="outline"
9394
size="sm"
9495
className="gap-1.5 text-destructive"
96+
data-testid="cancel-subscription-button"
9597
onClick={() => setOpen(true)}
9698
>
9799
<IconX className="h-3.5 w-3.5" />

docs/DATABASE_SCHEMA.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -525,8 +525,8 @@ Student subscriptions to a school's plans. Tenant-scoped.
525525
| `start_date` / `end_date` | TIMESTAMPTZ | |
526526
| `current_period_start` / `current_period_end` | TIMESTAMPTZ | |
527527
| `trial_start` / `trial_end` | TIMESTAMPTZ | |
528-
| `cancel_at` | TIMESTAMPTZ | NOT NULL — must be advanced on renewal, or reactivation breaks |
529-
| `cancel_at_period_end` | BOOLEAN | Student self-cancel flow |
528+
| `cancel_at` | TIMESTAMPTZ | NULLABLE. When the subscription terminates, or NULL when no cancel is scheduled — informational, never a signal. CHECK `subscriptions_cancel_at_requires_schedule`: non-NULL only while `cancel_at_period_end` (#545) |
529+
| `cancel_at_period_end` | BOOLEAN | NOT NULL DEFAULT false. **The single source of truth for "a cancel is scheduled"** — read by the Solana crank, the expiry crons and the billing UI. It used to be OR'd with `cancel_at <= now()`, and since `cancel_at` defaulted to `now()` on every INSERT, that canceled every `solana_subs` subscription at its first rollover (#545) |
530530
| `canceled_at` / `ended_at` | TIMESTAMPTZ | |
531531
| `superseded_by` | INTEGER | Set on plan change — points at the replacement subscription |
532532
| `payment_provider` | VARCHAR | |

lib/database.types.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5466,8 +5466,8 @@ export type Database = {
54665466
}
54675467
subscriptions: {
54685468
Row: {
5469-
cancel_at: string
5470-
cancel_at_period_end: boolean | null
5469+
cancel_at: string | null
5470+
cancel_at_period_end: boolean
54715471
canceled_at: string | null
54725472
created: string
54735473
current_period_end: string
@@ -5489,8 +5489,8 @@ export type Database = {
54895489
user_id: string
54905490
}
54915491
Insert: {
5492-
cancel_at?: string
5493-
cancel_at_period_end?: boolean | null
5492+
cancel_at?: string | null
5493+
cancel_at_period_end?: boolean
54945494
canceled_at?: string | null
54955495
created?: string
54965496
current_period_end?: string
@@ -5512,8 +5512,8 @@ export type Database = {
55125512
user_id: string
55135513
}
55145514
Update: {
5515-
cancel_at?: string
5516-
cancel_at_period_end?: boolean | null
5515+
cancel_at?: string | null
5516+
cancel_at_period_end?: boolean
55175517
canceled_at?: string | null
55185518
created?: string
55195519
current_period_end?: string
@@ -6323,8 +6323,8 @@ export type Database = {
63236323
change_subscription_plan: {
63246324
Args: { _new_plan_id: number }
63256325
Returns: {
6326-
cancel_at: string
6327-
cancel_at_period_end: boolean | null
6326+
cancel_at: string | null
6327+
cancel_at_period_end: boolean
63286328
canceled_at: string | null
63296329
created: string
63306330
current_period_end: string

0 commit comments

Comments
 (0)