From 281d5ea495f40f6f6c5b374975fc1806f444bda2 Mon Sep 17 00:00:00 2001 From: Guillermo Marin <52298929+guillermoscript@users.noreply.github.com> Date: Sun, 26 Jul 2026 04:50:11 +0200 Subject: [PATCH] fix(billing): make school billing lifecycle reversible, bounded and consistently counted (#546) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five independent defects in the school→platform billing lifecycle, none of which had any test coverage. 1. Cancel was irreversible. PostgREST's ON CONFLICT DO UPDATE only writes the columns you name, so confirmManualPayment's upsert left a stale cancel_at_period_end = true: a school could cancel, change its mind, pay for a full year and still be dropped to free at the end of it, with no reminder and no grace (both cron phases filter on cancel_at_period_end = false). The confirm upsert and changePlan now clear the flag (and pass cancel_at_period_end: false to Stripe), and a new reactivateSubscription() action is wired into the billing page — there was no reactivate action in the UI at all. 2. An unpaid renewal request paused the expiry downgrade forever. Requests now carry expires_at (created + 14 days); a new cron phase sweeps lapsed ones to 'expired' with an admin email, and the pause only honours open requests. Both duplicate guards moved off .single() — with two matching rows PostgREST returns PGRST116 / data: null, so the guards PASSED — and the renewal window guard is no longer bypassed by any non-active subscription status. 3. forceTenantPlanChange on a live Stripe subscriber billed the school for the comped plan: the next customer.subscription.updated read Stripe's real price as a downgrade, found the tenant over the real plan's limits (the reason it was comped) and repriced Stripe onto the comped plan. The override is now stamped on the subscription and applyPortalPlanChange ignores overridden tenants; clearTenantPlanOverride, a confirmed payment or an in-app plan change lift it. 4. The grace window collapsed to zero after a cron outage — graceEnd came from the subscription's period end, so one pass could send both "overdue" and "downgraded". It is now max(period_end, now) + GRACE_DAYS, and phase 3 skips tenants that entered grace in the same run. 5. Four definitions of "how many courses does this tenant have" collapsed to one: the billing page, the downgrade pre-flight and creation enforcement all route through countTenantUsage, and the hardcoded PLAN_LIMITS_FALLBACK map is gone in favour of platform_plans. Tests: 411 → 447. New cron, action, TTL and count-parity suites; each new assertion was verified to fail with the corresponding fix patched back out. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01U63Vu8MHB1MzE3e9oFk2kc --- .../billing/billing-dashboard-client.tsx | 23 +- app/actions/admin/billing.ts | 185 ++++++++-- app/actions/platform/plans.ts | 51 ++- app/actions/teacher/courses.ts | 62 +--- .../expire-platform-subscriptions/route.ts | 109 +++++- components/admin/billing-overview.tsx | 15 +- lib/billing/payment-request-ttl.ts | 53 +++ lib/billing/plan-limits.ts | 43 +++ .../templates/payment-request-expired.ts | 37 ++ lib/payments/platform-plan-change.ts | 18 +- messages/en.json | 5 +- messages/es.json | 5 +- ...130000_platform_payment_request_expiry.sql | 36 ++ ...26130100_platform_plan_override_marker.sql | 33 ++ .../expire-platform-subscriptions.test.ts | 327 ++++++++++++++++++ tests/unit/payment-request-ttl.test.ts | 46 +++ tests/unit/plan-limit-count-parity.test.ts | 179 ++++++++++ tests/unit/platform-billing-lifecycle.test.ts | 302 ++++++++++++++++ tests/unit/platform-plan-change.test.ts | 73 +++- tests/unit/support/fake-supabase.ts | 208 +++++++++++ 20 files changed, 1711 insertions(+), 99 deletions(-) create mode 100644 lib/billing/payment-request-ttl.ts create mode 100644 lib/email/templates/payment-request-expired.ts create mode 100644 supabase/migrations/20260726130000_platform_payment_request_expiry.sql create mode 100644 supabase/migrations/20260726130100_platform_plan_override_marker.sql create mode 100644 tests/unit/expire-platform-subscriptions.test.ts create mode 100644 tests/unit/payment-request-ttl.test.ts create mode 100644 tests/unit/plan-limit-count-parity.test.ts create mode 100644 tests/unit/platform-billing-lifecycle.test.ts create mode 100644 tests/unit/support/fake-supabase.ts diff --git a/app/[locale]/dashboard/admin/billing/billing-dashboard-client.tsx b/app/[locale]/dashboard/admin/billing/billing-dashboard-client.tsx index e41db888..d47c80ea 100644 --- a/app/[locale]/dashboard/admin/billing/billing-dashboard-client.tsx +++ b/app/[locale]/dashboard/admin/billing/billing-dashboard-client.tsx @@ -19,7 +19,12 @@ import { } from '@/components/ui/alert-dialog' import { IconExternalLink, IconRefresh, IconCreditCard, IconPhoto, IconClock, IconLoader2 } from '@tabler/icons-react' import { useTranslations, useLocale } from 'next-intl' -import { uploadPaymentProof, requestManualRenewal, cancelSubscription } from '@/app/actions/admin/billing' +import { + uploadPaymentProof, + requestManualRenewal, + cancelSubscription, + reactivateSubscription, +} from '@/app/actions/admin/billing' interface BillingDashboardClientProps { status: { @@ -75,6 +80,7 @@ export function BillingDashboardClient({ status, paymentRequests }: BillingDashb const [uploadingFor, setUploadingFor] = useState(null) const [cancelOpen, setCancelOpen] = useState(false) const [cancelLoading, setCancelLoading] = useState(false) + const [reactivateLoading, setReactivateLoading] = useState(false) const handleManageBilling = async () => { setPortalLoading(true) @@ -134,6 +140,19 @@ export function BillingDashboardClient({ status, paymentRequests }: BillingDashb } } + const handleReactivate = async () => { + setReactivateLoading(true) + try { + await reactivateSubscription() + toast.success(t('reactivateSuccess')) + router.refresh() + } catch (e) { + toast.error(e instanceof Error ? e.message : t('reactivateError')) + } finally { + setReactivateLoading(false) + } + } + const handleProofUpload = async (requestId: string, file: File) => { setUploadingFor(requestId) try { @@ -181,6 +200,8 @@ export function BillingDashboardClient({ status, paymentRequests }: BillingDashb accessCutoffAt={status.accessCutoffAt} onManageClick={status.hasStripeCustomer ? handleManageBilling : undefined} onCancelClick={() => setCancelOpen(true)} + onReactivateClick={handleReactivate} + reactivateLoading={reactivateLoading} /> {/* Manual Subscription Renewal Section */} diff --git a/app/actions/admin/billing.ts b/app/actions/admin/billing.ts index aca18bf9..b95a4109 100644 --- a/app/actions/admin/billing.ts +++ b/app/actions/admin/billing.ts @@ -3,9 +3,14 @@ import { createClient } from '@/lib/supabase/server' import { createAdminClient } from '@/lib/supabase/admin' import {getCurrentTenantId, getCurrentUserId } from '@/lib/supabase/tenant' -import { checkPlanLimits, formatPlanLimitError } from '@/lib/billing/plan-limits' +import { checkPlanLimits, countTenantUsage, formatPlanLimitError } from '@/lib/billing/plan-limits' import { classifyPlanChange } from '@/lib/billing/plan-change' import { reconcileAccessCutoff } from '@/lib/billing/access-cutoff' +import { + OPEN_REQUEST_STATUSES, + isRequestOpen, + requestExpiresAt, +} from '@/lib/billing/payment-request-ttl' import { revalidatePath } from 'next/cache' async function verifyAdminAccess() { @@ -29,6 +34,30 @@ async function verifyAdminAccess() { return { userId, tenantId, supabase } } +/** + * Is there an open (pending and not lapsed) platform payment request for this + * tenant? One helper for both duplicate guards so a renewal can never be + * created alongside a pending upgrade — the combination that used to disable + * both guards permanently, because each returned `PGRST116 / data: null` from + * `.single()` once two rows matched. + */ +async function hasOpenPaymentRequest( + adminClient: Awaited>, + tenantId: string, +): Promise { + const { data } = await adminClient + .from('platform_payment_requests') + .select('request_id, status, expires_at') + .eq('tenant_id', tenantId) + .in('status', OPEN_REQUEST_STATUSES as unknown as string[]) + .order('created_at', { ascending: false }) + .limit(20) + + return ((data as { status: string; expires_at: string | null }[] | null) || []).some((r) => + isRequestOpen(r) + ) +} + /** * Get current subscription status and usage statistics */ @@ -36,8 +65,12 @@ export async function getSubscriptionStatus() { const { tenantId } = await verifyAdminAccess() const adminClient = await createAdminClient() - // Fetch tenant, subscription, plan, and usage in parallel - const [tenantResult, subscriptionResult, coursesCount, studentsCount] = await Promise.all([ + // Fetch tenant, subscription, plan, and usage in parallel. + // Usage goes through countTenantUsage (#546 §5) so the number an admin reads + // here is the same number the downgrade pre-flight and course-creation + // enforcement compare against — it used to count archived courses that + // neither of those did. + const [tenantResult, subscriptionResult, usage] = await Promise.all([ adminClient .from('tenants') .select('plan, billing_status, billing_period_end, billing_email, stripe_customer_id, access_cutoff_at') @@ -48,16 +81,7 @@ export async function getSubscriptionStatus() { .select('*, platform_plans(*)') .eq('tenant_id', tenantId) .single(), - adminClient - .from('courses') - .select('*', { count: 'exact', head: true }) - .eq('tenant_id', tenantId), - adminClient - .from('tenant_users') - .select('*', { count: 'exact', head: true }) - .eq('tenant_id', tenantId) - .eq('role', 'student') - .eq('status', 'active'), + countTenantUsage(adminClient, tenantId), ]) const tenant = tenantResult.data @@ -102,11 +126,11 @@ export async function getSubscriptionStatus() { } : null, usage: { courses: { - current: coursesCount.count || 0, + current: usage.courses, limit: limits.max_courses, }, students: { - current: studentsCount.count || 0, + current: usage.students, limit: limits.max_students, }, }, @@ -182,15 +206,12 @@ export async function requestManualPlanUpgrade(planId: string, interval: 'monthl throw new Error(formatPlanLimitError(limitCheck) || 'Plan limits exceeded') } - // Check for existing pending request - const { data: existingRequest } = await adminClient - .from('platform_payment_requests') - .select('request_id') - .eq('tenant_id', tenantId) - .in('status', ['pending', 'instructions_sent', 'payment_received']) - .single() - - if (existingRequest) { + // Check for an existing open request. Bounded list + array check, never + // `.single()` (#546 §2): with two or more matching rows `.single()` returns + // PGRST116 and `data: null`, so the guard PASSED and the table grew unbounded + // per tenant. Lapsed requests are ignored so a stale row cannot lock a school + // out of paying. + if (await hasOpenPaymentRequest(adminClient, tenantId)) { throw new Error('You already have a pending plan change request. Please wait for it to be processed.') } @@ -207,6 +228,7 @@ export async function requestManualPlanUpgrade(planId: string, interval: 'monthl request_type: requestType, bank_reference: bankReference || null, notes: notes || null, + expires_at: requestExpiresAt(), }) .select('request_id') .single() @@ -334,6 +356,18 @@ export async function confirmManualPayment(requestId: string) { grace_period_end: null, // Reset the reminder stamp so the next cycle can remind again. renewal_reminder_sent_at: null, + // A confirmed payment is an un-cancel (#546 §1). PostgREST's ON CONFLICT + // DO UPDATE only touches the columns supplied here, so omitting these two + // left a stale `cancel_at_period_end = true` on the row: the school paid + // for a full period and was then silently dropped to free at the end of + // it by the cron's cancel phase, with no reminder and no grace window + // (phases 1 and 2 both filter on `cancel_at_period_end = false`). + cancel_at_period_end: false, + canceled_at: null, + // A real payment supersedes any super-admin comp (#546 §3) — this is one + // of the override's exits, so portal changes start syncing again. + plan_override_by: null, + plan_override_at: null, updated_at: now.toISOString(), }, { onConflict: 'tenant_id' }) @@ -503,6 +537,11 @@ export async function changePlan(planId: string, interval: 'monthly' | 'yearly' await ctx.stripe.subscriptions.update(ctx.subId, { items: [{ id: ctx.itemId, price: ctx.targetPriceId }], proration_behavior: 'create_prorations', + // Paying for a different plan un-cancels (#546 §1). `resolvePlatformPlanChange` + // accepts a still-`active` subscription, which a cancel-at-period-end sub is + // right up to its last day — without this the school changes plan, is billed, + // and is still dropped to free at period end. + cancel_at_period_end: false, metadata: { tenant_id: tenantId, plan_id: ctx.plan.plan_id, @@ -517,7 +556,18 @@ export async function changePlan(planId: string, interval: 'monthly' | 'yearly' const now = new Date().toISOString() await adminClient .from('platform_subscriptions') - .update({ plan_id: ctx.plan.plan_id, interval, updated_at: now }) + .update({ + plan_id: ctx.plan.plan_id, + interval, + cancel_at_period_end: false, + canceled_at: null, + // An admin-initiated, Stripe-backed change supersedes a super-admin comp + // (#546 §3): Stripe and the DB now agree again, so the reconciler should + // resume applying portal changes for this tenant. + plan_override_by: null, + plan_override_at: null, + updated_at: now, + }) .eq('tenant_id', tenantId) await adminClient .from('tenants') @@ -593,6 +643,65 @@ export async function cancelSubscription() { return { success: true } } +/** + * Undo a pending cancellation (#546 §1). + * + * Cancellation was one-way: the only writers of `cancel_at_period_end` were + * `cancelSubscription` (sets it) and the Stripe webhook (mirrors Stripe), the + * billing UI imported no reactivate action at all, and re-checkout is blocked + * while the subscription is still `active` — which a cancel-at-period-end + * subscription is until its last day. A school that changed its mind had no way + * back short of contacting support. + */ +export async function reactivateSubscription() { + const { tenantId } = await verifyAdminAccess() + const adminClient = await createAdminClient() + + const { data: subscription } = await adminClient + .from('platform_subscriptions') + .select('stripe_subscription_id, payment_method, status, cancel_at_period_end, current_period_end') + .eq('tenant_id', tenantId) + .single() + + if (!subscription) throw new Error('No subscription to reactivate') + if (!subscription.cancel_at_period_end) { + throw new Error('This subscription is not scheduled for cancellation') + } + // Once the period has lapsed the cron has already downgraded (or is about + // to); reactivating would revive a plan nobody is paying for. Those schools + // go through checkout / a renewal request instead. + if (subscription.status !== 'active') { + throw new Error('This subscription has already ended. Please start a new plan instead.') + } + if ( + subscription.current_period_end && + new Date(subscription.current_period_end) <= new Date() + ) { + throw new Error('Your billing period has already ended. Please start a new plan instead.') + } + + if (subscription.payment_method === 'stripe' && subscription.stripe_subscription_id) { + // Stripe is authoritative for Stripe subs — clear it there first so a + // failure leaves both sides still cancelling rather than disagreeing. + const { getStripe } = await import('@/lib/stripe') + await getStripe().subscriptions.update(subscription.stripe_subscription_id, { + cancel_at_period_end: false, + }) + } + + await adminClient + .from('platform_subscriptions') + .update({ + cancel_at_period_end: false, + canceled_at: null, + updated_at: new Date().toISOString(), + }) + .eq('tenant_id', tenantId) + + revalidatePath('/dashboard/admin/billing') + return { success: true } +} + /** * Upload payment proof for a platform payment request (school admin) */ @@ -681,21 +790,22 @@ export async function requestManualRenewal() { const now = new Date() const daysUntilEnd = Math.ceil((periodEnd.getTime() - now.getTime()) / (1000 * 60 * 60 * 24)) - if (daysUntilEnd > 30 && subscription.status === 'active') { + // The window guard used to be bypassed entirely for any non-`active` status + // (#546 §2), so a canceled or long-lapsed subscription could mint the row + // that pauses the downgrade at any time. A school in grace (`past_due`) must + // still be able to pay — that is the whole point of the pause — so allow it + // explicitly rather than by falling through the guard. + const lapsed = daysUntilEnd <= 0 || subscription.status === 'past_due' + if (daysUntilEnd > 30 && !lapsed) { throw new Error('Renewal can only be requested within 30 days of period end') } - // Check for existing pending renewal request - const { data: existingRequest } = await adminClient - .from('platform_payment_requests') - .select('request_id') - .eq('tenant_id', tenantId) - .eq('request_type', 'renewal') - .in('status', ['pending', 'instructions_sent', 'payment_received']) - .single() - - if (existingRequest) { - throw new Error('You already have a pending renewal request') + // One open request per tenant, whatever its type: the old guard filtered on + // `request_type = 'renewal'` while the upgrade guard did not, so creating a + // renewal alongside a pending upgrade was the ordinary way to get two rows — + // after which `.single()` returned PGRST116 and BOTH guards passed forever. + if (await hasOpenPaymentRequest(adminClient, tenantId)) { + throw new Error('You already have a pending payment request. Please wait for it to be processed.') } const plan = subscription.platform_plans as { plan_id: string; price_monthly: number; price_yearly: number } @@ -712,6 +822,7 @@ export async function requestManualRenewal() { currency: 'usd', status: 'pending', request_type: 'renewal', + expires_at: requestExpiresAt(), }) .select('request_id') .single() diff --git a/app/actions/platform/plans.ts b/app/actions/platform/plans.ts index 3c3f3a0a..524f04fb 100644 --- a/app/actions/platform/plans.ts +++ b/app/actions/platform/plans.ts @@ -190,8 +190,23 @@ export async function sendPaymentInstructions(requestId: string) { return { success: true } } +/** + * Super admin: force a tenant onto a plan without any payment. + * + * This never calls Stripe, so a tenant with a live Stripe subscription keeps + * paying for the plan it actually bought while enjoying the comped one. The + * override is stamped on the subscription row (#546 §3) so + * `applyPortalPlanChange` can recognise the deliberate divergence and leave the + * Stripe subscription alone — without the stamp the next + * `customer.subscription.updated` read Stripe's real (unchanged) price as a + * downgrade, found the tenant over the real plan's limits (the reason it was + * comped) and "reverted" Stripe onto the comped plan's price, billing the + * school for its own comp. + * + * Exits: `confirmManualPayment`, `changePlan`, or `clearTenantPlanOverride`. + */ export async function forceTenantPlanChange(tenantId: string, planSlug: string) { - await verifySuperAdmin() + const superAdminId = await verifySuperAdmin() const adminClient = createAdminClient() const { data: plan } = await adminClient @@ -239,6 +254,8 @@ export async function forceTenantPlanChange(tenantId: string, planSlug: string) plan_id: plan.plan_id, status: 'canceled', canceled_at: nowIso, + plan_override_by: superAdminId, + plan_override_at: nowIso, updated_at: nowIso, }) .eq('tenant_id', tenantId) @@ -260,6 +277,8 @@ export async function forceTenantPlanChange(tenantId: string, planSlug: string) .update({ plan_id: plan.plan_id, status: 'active', + plan_override_by: superAdminId, + plan_override_at: nowIso, updated_at: nowIso, ...(liveCycle ? {} @@ -292,6 +311,8 @@ export async function forceTenantPlanChange(tenantId: string, planSlug: string) interval: 'monthly', current_period_start: nowIso, current_period_end: null, + plan_override_by: superAdminId, + plan_override_at: nowIso, }) } @@ -299,6 +320,34 @@ export async function forceTenantPlanChange(tenantId: string, planSlug: string) return { success: true } } +/** + * Super admin: end a plan override so portal-driven Stripe changes sync again + * (#546 §3). The marker's explicit exit — without one, a comped tenant would be + * frozen out of `applyPortalPlanChange` forever, which is the failure mode the + * issue flags as the risk of adding the marker at all. + * + * The tenant keeps whatever plan it is on; only the "ignore portal changes" + * behaviour is lifted, so the next `customer.subscription.updated` reconciles + * the DB back onto whatever Stripe is actually charging. + */ +export async function clearTenantPlanOverride(tenantId: string) { + await verifySuperAdmin() + const adminClient = createAdminClient() + + const { error } = await adminClient + .from('platform_subscriptions') + .update({ + plan_override_by: null, + plan_override_at: null, + updated_at: new Date().toISOString(), + }) + .eq('tenant_id', tenantId) + + if (error) throw new Error(`Failed to clear plan override: ${error.message}`) + revalidatePath('/platform/tenants') + return { success: true } +} + export async function suspendTenant(tenantId: string, suspend: boolean) { await verifySuperAdmin() const adminClient = createAdminClient() diff --git a/app/actions/teacher/courses.ts b/app/actions/teacher/courses.ts index cc65e9f4..afe946a4 100644 --- a/app/actions/teacher/courses.ts +++ b/app/actions/teacher/courses.ts @@ -6,17 +6,7 @@ import { getUserRole } from '@/lib/supabase/get-user-role' import { revalidatePath } from 'next/cache' import { sendEmail } from '@/lib/email/send' import { createAdminClient } from '@/lib/supabase/admin' - -// Fallback plan limits (used if platform_plans table query fails) -const PLAN_LIMITS_FALLBACK: Record = { - free: 5, - starter: 15, - basic: 20, - pro: 100, - professional: 100, - business: -1, - enterprise: -1, -} +import { countTenantUsage, getTenantPlanLimits } from '@/lib/billing/plan-limits' export interface CourseFormData { title: string @@ -61,41 +51,27 @@ export async function checkCourseLimit(): Promise<{ nextPlan?: string nextPlanPrice?: number }> { - const supabase = await createClient() const tenantId = await getCurrentTenantId() - // Get tenant's plan and limits from platform_plans - const { data: tenant } = await supabase - .from('tenants') - .select('plan') - .eq('id', tenantId) - .single() - - const plan = tenant?.plan || 'free' - - // Try to get limit from platform_plans table - let limit: number - const { data: platformPlan } = await supabase - .from('platform_plans') - .select('limits') - .eq('slug', plan) - .eq('is_active', true) - .single() - - if (platformPlan?.limits && typeof platformPlan.limits === 'object') { - const limits = platformPlan.limits as { max_courses?: number } - limit = limits.max_courses ?? PLAN_LIMITS_FALLBACK[plan] ?? 5 - } else { - limit = PLAN_LIMITS_FALLBACK[plan] ?? 5 - } - - // Count existing courses for this tenant - const { count } = await supabase - .from('courses') - .select('*', { count: 'exact', head: true }) - .eq('tenant_id', tenantId) + // Issue #546 §5: creation enforcement used to count ALL courses (archived + // included) against a limit resolved through a hardcoded fallback map, while + // the downgrade pre-flight, the access-cutoff reconciler and the number shown + // on the billing page all counted non-archived courses from + // `platform_plans.limits`. A school could be approved for a downgrade and + // then be unable to create a single course on the plan it just moved to, + // with an error telling it to archive courses that provably did not help. + // + // Both the count and the limit now come from lib/billing/plan-limits, on the + // service-role client so the number does not depend on what the calling + // teacher can see through RLS. + const adminClient = createAdminClient() + const [{ planSlug: plan, limits }, usage] = await Promise.all([ + getTenantPlanLimits(adminClient, tenantId), + countTenantUsage(adminClient, tenantId), + ]) - const currentCount = count || 0 + const limit = limits?.max_courses ?? -1 + const currentCount = usage.courses // -1 means unlimited const canCreate = limit === -1 || currentCount < limit const approaching = limit !== -1 && currentCount >= limit * 0.8 diff --git a/app/api/cron/expire-platform-subscriptions/route.ts b/app/api/cron/expire-platform-subscriptions/route.ts index fa4431ff..ba62aa1b 100644 --- a/app/api/cron/expire-platform-subscriptions/route.ts +++ b/app/api/cron/expire-platform-subscriptions/route.ts @@ -5,6 +5,12 @@ import { renewalReminderTemplate } from '@/lib/email/templates/renewal-reminder' import { planDowngradedTemplate } from '@/lib/email/templates/plan-downgraded' import { downgradeTenantToFree } from '@/lib/billing/downgrade-tenant' import { getTenantAdminEmails } from '@/lib/billing/tenant-admins' +import { paymentRequestExpiredTemplate } from '@/lib/email/templates/payment-request-expired' +import { + OPEN_REQUEST_STATUSES, + REQUEST_TTL_DAYS, + isRequestOpen, +} from '@/lib/billing/payment-request-ttl' export const runtime = 'nodejs' @@ -20,10 +26,12 @@ export const runtime = 'nodejs' * webhook-driven; their expiry is handled by /api/stripe/platform-webhook. * * Phases (all status-gated, so re-running is idempotent): + * 0. Request TTL — open payment request past `expires_at` → `expired` + email. + * Runs first so a lapsed request cannot pause phase 3 below. * 1. Reminder — active sub, period end within GRACE_DAYS, reminder not yet sent → email + stamp. * 2. Grace start — active sub, not cancel_at_period_end, period end passed → past_due + grace window + overdue email. * 3. Downgrade — past_due sub, grace window passed → downgrade to free + email, - * UNLESS a renewal payment request is still pending (pauses the downgrade). + * UNLESS an OPEN renewal payment request pauses it. * 4. Cancel — cancel_at_period_end sub, period end passed → downgrade to free + email (no renewal pause). * * Secured by CRON_SECRET env var (set the same value in the cron scheduler). @@ -32,9 +40,6 @@ export const runtime = 'nodejs' const GRACE_DAYS = 7 const DAY_MS = 24 * 60 * 60 * 1000 -// A renewal in any of these states means the school is mid-renewal; don't downgrade. -const PENDING_RENEWAL_STATUSES = ['pending', 'instructions_sent', 'payment_received'] - function getSupabaseAdmin(): SupabaseClient { const url = process.env.NEXT_PUBLIC_SUPABASE_URL const serviceKey = process.env.SUPABASE_SERVICE_ROLE_KEY @@ -62,6 +67,23 @@ type SubRow = { const SUB_SELECT = 'tenant_id, current_period_end, tenants(name), platform_plans(name)' +type LapsedRequestRow = { + request_id: string + tenant_id: string + amount: number | string | null + currency: string | null + expires_at: string | null + status: string + tenants: { name: string | null } | null + platform_plans: { name: string | null } | null +} + +function formatAmount(amount: number | string | null, currency: string | null): string { + const value = Number(amount ?? 0) + const code = (currency || 'usd').toUpperCase() + return `${Number.isFinite(value) ? value.toFixed(2) : '0.00'} ${code}` +} + export async function GET(req: NextRequest) { const cronSecret = process.env.CRON_SECRET const provided = req.headers.get('authorization')?.replace('Bearer ', '') @@ -76,7 +98,50 @@ export async function GET(req: NextRequest) { const appUrl = process.env.NEXT_PUBLIC_APP_URL || 'https://app.example.com' const billingUrl = `${appUrl}/dashboard/admin/billing` - const result = { reminded: 0, graceStarted: 0, downgraded: 0, canceled: 0, skippedPendingRenewal: 0 } + const result = { + requestsExpired: 0, + reminded: 0, + graceStarted: 0, + downgraded: 0, + canceled: 0, + skippedPendingRenewal: 0, + } + + // ---- Phase 0: expire lapsed payment requests (#546 §2) ---- + // Nothing else in the codebase ever moved a request out of an open state, so + // an unpaid renewal paused the downgrade forever: click "request renewal", + // never pay, keep the paid plan, its limits and its reduced platform fee. + // Closing them here first means phase 3 below sees the swept state. + const { data: lapsedRequests } = await supabase + .from('platform_payment_requests') + .select('request_id, tenant_id, amount, currency, expires_at, status, tenants(name), platform_plans(name)') + .in('status', OPEN_REQUEST_STATUSES as unknown as string[]) + .not('expires_at', 'is', null) + .lt('expires_at', nowIso) + + for (const req of (lapsedRequests as LapsedRequestRow[] | null) || []) { + const { error } = await supabase + .from('platform_payment_requests') + .update({ status: 'expired', updated_at: nowIso }) + .eq('request_id', req.request_id) + // Status-gated so a super admin confirming in the same instant wins. + .in('status', OPEN_REQUEST_STATUSES as unknown as string[]) + + if (error) { + console.error('expire-platform-subscriptions: failed to expire request', req.request_id, error) + continue + } + + const emails = await getTenantAdminEmails(supabase, req.tenant_id) + await safeEmail(emails, paymentRequestExpiredTemplate({ + schoolName: req.tenants?.name || 'your school', + planName: req.platform_plans?.name || 'your plan', + amount: formatAmount(req.amount, req.currency), + billingUrl, + ttlDays: REQUEST_TTL_DAYS, + })) + result.requestsExpired++ + } // ---- Phase 1: pre-expiry renewal reminder ---- const { data: reminderSubs } = await supabase @@ -116,8 +181,21 @@ export async function GET(req: NextRequest) { .not('current_period_end', 'is', null) .lt('current_period_end', nowIso) + // Tenants that entered grace in THIS pass. Phase 3 re-queries the table and + // would otherwise see them; belt-and-braces alongside the graceEnd fix below, + // so "started grace" and "downgraded" can never both happen to one school in + // a single run no matter how the arithmetic drifts. + const graceStartedNow = new Set() + for (const sub of (lapsedSubs as SubRow[] | null) || []) { - const graceEnd = new Date(new Date(sub.current_period_end!).getTime() + GRACE_DAYS * DAY_MS).toISOString() + // Grace starts NOW, not at the (possibly long-past) period end (#546 §4). + // Computing it from current_period_end meant that after a cron outage of + // more than GRACE_DAYS the window was already closed the moment it opened: + // the same request sent "your payment is overdue" and "you have been + // downgraded", with no opportunity to pay. Both counters incremented, so + // the response looked healthy. + const periodEndMs = new Date(sub.current_period_end!).getTime() + const graceEnd = new Date(Math.max(periodEndMs, now.getTime()) + GRACE_DAYS * DAY_MS).toISOString() await supabase .from('platform_subscriptions') .update({ status: 'past_due', grace_period_end: graceEnd, updated_at: nowIso }) @@ -135,6 +213,7 @@ export async function GET(req: NextRequest) { periodEnd: new Date(sub.current_period_end!).toLocaleDateString('en-US', { dateStyle: 'long' }), overdue: true, })) + graceStartedNow.add(sub.tenant_id) result.graceStarted++ } @@ -148,16 +227,24 @@ export async function GET(req: NextRequest) { .lt('grace_period_end', nowIso) for (const sub of (expiredSubs as SubRow[] | null) || []) { - // Pause the downgrade if the school has a renewal request in flight. + // A school that only just entered grace gets the full window, never a + // same-pass downgrade. + if (graceStartedNow.has(sub.tenant_id)) continue + + // Pause the downgrade only for a renewal request that is still OPEN — an + // unpaid one lapses at its TTL (phase 0 above) and stops holding the plan. const { data: pendingRenewal } = await supabase .from('platform_payment_requests') - .select('request_id') + .select('request_id, status, expires_at') .eq('tenant_id', sub.tenant_id) .eq('request_type', 'renewal') - .in('status', PENDING_RENEWAL_STATUSES) - .limit(1) + .in('status', OPEN_REQUEST_STATUSES as unknown as string[]) + .limit(20) + + const stillOpen = ((pendingRenewal as { status: string; expires_at: string | null }[] | null) || []) + .some((r) => isRequestOpen(r, now)) - if (pendingRenewal && pendingRenewal.length > 0) { + if (stillOpen) { result.skippedPendingRenewal++ continue } diff --git a/components/admin/billing-overview.tsx b/components/admin/billing-overview.tsx index 06cd4861..f3c55840 100644 --- a/components/admin/billing-overview.tsx +++ b/components/admin/billing-overview.tsx @@ -5,7 +5,7 @@ import { Badge } from '@/components/ui/badge' import { Button } from '@/components/ui/button' import { UsageMeter } from './usage-meter' import { LimitReachedBanner } from '@/components/shared/limit-reached-banner' -import { IconCreditCard, IconCalendar, IconAlertTriangle, IconX } from '@tabler/icons-react' +import { IconCreditCard, IconCalendar, IconAlertTriangle, IconRefresh, IconX } from '@tabler/icons-react' import Link from 'next/link' import { useTranslations } from 'next-intl' @@ -37,6 +37,8 @@ interface BillingOverviewProps { accessCutoffAt: string | null onManageClick?: () => void onCancelClick?: () => void + onReactivateClick?: () => void + reactivateLoading?: boolean } export function BillingOverview({ @@ -51,6 +53,8 @@ export function BillingOverview({ accessCutoffAt, onManageClick, onCancelClick, + onReactivateClick, + reactivateLoading = false, }: BillingOverviewProps) { const t = useTranslations('dashboard.admin.billing.overview') const isFree = plan === 'free' @@ -108,6 +112,15 @@ export function BillingOverview({ {t('cancelPlan')} )} + {/* Cancelling used to be one-way — there was no reactivate action + anywhere in the UI, and re-checkout is blocked while the sub is + still active (#546 §1). */} + {!isFree && onReactivateClick && subscription?.status === 'active' && subscription?.cancelAtPeriodEnd && ( + + )} diff --git a/lib/billing/payment-request-ttl.ts b/lib/billing/payment-request-ttl.ts new file mode 100644 index 00000000..82714434 --- /dev/null +++ b/lib/billing/payment-request-ttl.ts @@ -0,0 +1,53 @@ +/** + * TTL rules for `platform_payment_requests` (issue #546 §2). + * + * A manual bank-transfer request is an open promise to pay. Until #546 nothing + * ever closed one: the only terminal writers were the super-admin confirm and + * reject actions, and `expire-platform-subscriptions` paused the downgrade for + * as long as a renewal request sat in any pending state. Requesting a renewal + * and never paying therefore kept the paid plan — its course/student limits and + * its reduced transaction fee — forever. + * + * Every request now carries `expires_at` (created + TTL). Three call sites read + * these rules and must agree, which is why they live here: + * - the two duplicate-request guards in `app/actions/admin/billing.ts` + * - the downgrade pause in `app/api/cron/expire-platform-subscriptions` + * - the sweep in that same cron that flips lapsed rows to `expired` + */ + +/** Statuses that mean "this request is still an open promise to pay". */ +export const OPEN_REQUEST_STATUSES = ['pending', 'instructions_sent', 'payment_received'] as const + +/** + * Days a request stays open before the cron expires it. Mirrors the + * `expires_at` column default in migration 20260726130000; changing one without + * the other only shifts which side wins for freshly inserted rows, never + * whether a request can hang forever. + */ +export const REQUEST_TTL_DAYS = 14 + +const DAY_MS = 24 * 60 * 60 * 1000 + +/** The `expires_at` an inserted request should carry, as an ISO string. */ +export function requestExpiresAt(now: Date = new Date()): string { + return new Date(now.getTime() + REQUEST_TTL_DAYS * DAY_MS).toISOString() +} + +/** + * Does this request still block a new one / still pause the downgrade? + * + * Note the deliberate asymmetry with the cron sweep: a row whose `expires_at` + * has passed stops counting the instant it lapses, whether or not the cron has + * run yet. Tying "does it still count" to the sweep would hand the outage + * window back to the leak this TTL exists to close. + */ +export function isRequestOpen( + request: { status: string; expires_at: string | null }, + now: Date = new Date() +): boolean { + if (!(OPEN_REQUEST_STATUSES as readonly string[]).includes(request.status)) return false + // A NULL expiry predates the column (or was written by an older client); + // treat it as open so a legitimate in-flight request is never dropped. + if (!request.expires_at) return true + return new Date(request.expires_at).getTime() > now.getTime() +} diff --git a/lib/billing/plan-limits.ts b/lib/billing/plan-limits.ts index f132cf8f..100153cb 100644 --- a/lib/billing/plan-limits.ts +++ b/lib/billing/plan-limits.ts @@ -68,6 +68,49 @@ export async function countTenantUsage( } } +export interface TenantPlanLimits { + /** The tenant's current plan slug (`free` when the tenant row has none). */ + planSlug: string + planName: string | null + limits: PlanLimits +} + +/** + * Resolve the limits governing a tenant RIGHT NOW, from `platform_plans` and + * nothing else (issue #546 §5). + * + * Deliberately no `is_active` filter: retiring a plan must not silently change + * what its existing subscribers are allowed to do. A missing plan row yields + * `limits: null`, which `computePlanLimitViolations` reads as unlimited — the + * same fail-open every other caller in this module uses, and the reason the old + * hardcoded `PLAN_LIMITS_FALLBACK` map in `app/actions/teacher/courses.ts` + * could be deleted instead of duplicated here. + */ +export async function getTenantPlanLimits( + admin: SupabaseClient, + tenantId: string +): Promise { + const { data: tenant } = await admin + .from('tenants') + .select('plan') + .eq('id', tenantId) + .maybeSingle() + + const planSlug = (tenant?.plan as string | null) || 'free' + + const { data: plan } = await admin + .from('platform_plans') + .select('name, limits') + .eq('slug', planSlug) + .maybeSingle() + + return { + planSlug, + planName: (plan?.name as string | null) ?? null, + limits: (plan?.limits as PlanLimits) ?? null, + } +} + /** * Pure comparison of usage against a plan's limits. `-1` (or a missing key) * means unlimited and never produces a violation. diff --git a/lib/email/templates/payment-request-expired.ts b/lib/email/templates/payment-request-expired.ts new file mode 100644 index 00000000..19150b73 --- /dev/null +++ b/lib/email/templates/payment-request-expired.ts @@ -0,0 +1,37 @@ +export interface PaymentRequestExpiredData { + schoolName: string + planName: string + amount: string + billingUrl: string + ttlDays: number +} + +/** + * Sent when an unpaid bank-transfer request hits its TTL and is closed + * (issue #546 §2). Says plainly that the request no longer holds the plan, + * because until the TTL existed it held it indefinitely. + */ +export function paymentRequestExpiredTemplate( + data: PaymentRequestExpiredData +): { subject: string; html: string } { + return { + subject: `Your ${data.planName} payment request for ${data.schoolName} has expired`, + html: ` + + +

Payment Request Expired

+

Hi,

+

The bank transfer request for the ${data.planName} plan on ${data.schoolName} (${data.amount}) has been open for ${data.ttlDays} days without a confirmed payment, so we've closed it.

+

Nothing has been charged. Your plan is unchanged for now, but this request no longer holds it — if your billing period has already ended, your school will move to the free plan on the normal schedule.

+

If you still want this plan, start a new request from the billing dashboard. If you already transferred the money, contact support and we'll match it up.

+

+ + Go to Billing + +

+
+

LMS Platform Billing

+ +`, + } +} diff --git a/lib/payments/platform-plan-change.ts b/lib/payments/platform-plan-change.ts index 714bb776..ac6fcb26 100644 --- a/lib/payments/platform-plan-change.ts +++ b/lib/payments/platform-plan-change.ts @@ -85,9 +85,11 @@ export async function applyPortalPlanChange( const { data: currentSub } = (await admin .from('platform_subscriptions') - .select('plan_id, interval') + .select('plan_id, interval, plan_override_at') .eq('tenant_id', tenantId) - .maybeSingle()) as { data: { plan_id: string; interval: string | null } | null } + .maybeSingle()) as { + data: { plan_id: string; interval: string | null; plan_override_at: string | null } | null + } // No-op guard: the incoming price maps to the plan already recorded. This is // also what terminates the webhook echo triggered by our own revert below. @@ -95,6 +97,18 @@ export async function applyPortalPlanChange( return { action: 'noop' } } + // Override guard (#546 §3). A super admin has deliberately put this tenant on + // a plan Stripe knows nothing about, so Stripe's price and our plan_id are + // SUPPOSED to disagree. Reconciling here would read the comp as a portal + // downgrade and — because the tenant is typically over the real plan's limits, + // which is why it was comped — push the subscription onto the comped plan's + // price, billing the school for its own comp. A super admin lifts this with + // clearTenantPlanOverride, and a real payment (confirmManualPayment / + // changePlan) clears it automatically. + if (currentSub?.plan_override_at) { + return { action: 'ignored', reason: 'tenant plan is under a super-admin override' } + } + const maxCourses = newPlan.limits?.max_courses ?? -1 const maxStudents = newPlan.limits?.max_students ?? -1 diff --git a/messages/en.json b/messages/en.json index 867f3bd6..c44a24bd 100644 --- a/messages/en.json +++ b/messages/en.json @@ -1922,7 +1922,10 @@ "cancelConfirmAction": "Cancel subscription", "cancelKeep": "Keep subscription", "cancelSuccess": "Your subscription will cancel at the end of your billing period", - "cancelError": "Failed to cancel subscription" + "cancelError": "Failed to cancel subscription", + "reactivatePlan": "Reactivate plan", + "reactivateSuccess": "Your subscription will continue — the scheduled cancellation was removed", + "reactivateError": "Failed to reactivate subscription" }, "upgrade": { "checkoutError": "Failed to create checkout session", diff --git a/messages/es.json b/messages/es.json index a159a57b..5f276c9d 100644 --- a/messages/es.json +++ b/messages/es.json @@ -1922,7 +1922,10 @@ "cancelConfirmAction": "Cancelar suscripción", "cancelKeep": "Mantener suscripción", "cancelSuccess": "Tu suscripción se cancelará al final de tu período de facturación", - "cancelError": "Error al cancelar la suscripción" + "cancelError": "Error al cancelar la suscripción", + "reactivatePlan": "Reactivar plan", + "reactivateSuccess": "Tu suscripción continuará: se eliminó la cancelación programada", + "reactivateError": "No se pudo reactivar la suscripción" }, "upgrade": { "checkoutError": "Error al crear la sesión de pago", diff --git a/supabase/migrations/20260726130000_platform_payment_request_expiry.sql b/supabase/migrations/20260726130000_platform_payment_request_expiry.sql new file mode 100644 index 00000000..39e6baa6 --- /dev/null +++ b/supabase/migrations/20260726130000_platform_payment_request_expiry.sql @@ -0,0 +1,36 @@ +-- Issue #546 §2 — a never-paid renewal request pauses the expiry downgrade forever. +-- +-- `expire-platform-subscriptions` skips the downgrade for any tenant holding a +-- renewal request in ('pending','instructions_sent','payment_received'), and +-- nothing ever moved a request out of those states except a super admin +-- confirming or rejecting it. Clicking "request renewal" and never paying kept +-- the paid plan, its limits and its reduced transaction fee indefinitely. +-- +-- Give every request a TTL so the pause is bounded. The cron sweeps lapsed +-- requests to 'expired' (already in the CHECK list since 20260217040000) and +-- both the duplicate-request guards and the downgrade pause ignore anything +-- past `expires_at`. + +ALTER TABLE platform_payment_requests + ADD COLUMN IF NOT EXISTS expires_at TIMESTAMPTZ; + +-- Backfill open and historical rows alike so the column can be NOT NULL. +UPDATE platform_payment_requests + SET expires_at = COALESCE(created_at, NOW()) + INTERVAL '14 days' + WHERE expires_at IS NULL; + +ALTER TABLE platform_payment_requests + ALTER COLUMN expires_at SET DEFAULT (NOW() + INTERVAL '14 days'); + +ALTER TABLE platform_payment_requests + ALTER COLUMN expires_at SET NOT NULL; + +COMMENT ON COLUMN platform_payment_requests.expires_at IS + 'TTL for an unpaid request (default: created + 14 days). Past this instant the ' + 'request stops blocking new requests and stops pausing the expiry downgrade; ' + 'the expire-platform-subscriptions cron flips it to status = ''expired''.'; + +-- The cron sweep and both duplicate guards filter open rows by expiry. +CREATE INDEX IF NOT EXISTS idx_platform_payment_requests_open_expiry + ON platform_payment_requests (expires_at) + WHERE status IN ('pending', 'instructions_sent', 'payment_received'); diff --git a/supabase/migrations/20260726130100_platform_plan_override_marker.sql b/supabase/migrations/20260726130100_platform_plan_override_marker.sql new file mode 100644 index 00000000..f9caf0b0 --- /dev/null +++ b/supabase/migrations/20260726130100_platform_plan_override_marker.sql @@ -0,0 +1,33 @@ +-- Issue #546 §3 — forceTenantPlanChange on a live Stripe subscriber bills the +-- school for the comped plan. +-- +-- The super-admin override rewrites platform_subscriptions.plan_id and never +-- touches Stripe, so the next customer.subscription.updated maps Stripe's +-- (unchanged, real) price to the real plan, misses applyPortalPlanChange's +-- no-op guard, reads it as a downgrade, finds the tenant over the real plan's +-- limits — which is why it was comped — and "reverts" Stripe onto the FORCED +-- plan's price. The school then pays for the plan it was given for free. +-- +-- Mark the override so the reconciler can recognise a comped tenant and leave +-- its Stripe subscription alone. Cleared whenever the tenant's plan is set by a +-- real payment path (confirmManualPayment / changePlan) or by a super admin +-- calling clearTenantPlanOverride — that is the override's exit. + +ALTER TABLE platform_subscriptions + ADD COLUMN IF NOT EXISTS plan_override_by UUID REFERENCES auth.users(id), + ADD COLUMN IF NOT EXISTS plan_override_at TIMESTAMPTZ; + +COMMENT ON COLUMN platform_subscriptions.plan_override_at IS + 'Set when a super admin forces this tenant onto a plan without changing Stripe. ' + 'While non-NULL, applyPortalPlanChange ignores portal price changes for this ' + 'tenant so the comped plan is never pushed onto the real Stripe subscription. ' + 'Cleared by confirmManualPayment, changePlan and clearTenantPlanOverride.'; + +COMMENT ON COLUMN platform_subscriptions.plan_override_by IS + 'auth.users.id of the super admin who applied the plan override.'; + +-- Small table; a partial index keeps the "who is comped" lookup cheap for the +-- platform panel without paying for the common NULL case. +CREATE INDEX IF NOT EXISTS idx_platform_subscriptions_plan_override + ON platform_subscriptions (plan_override_at) + WHERE plan_override_at IS NOT NULL; diff --git a/tests/unit/expire-platform-subscriptions.test.ts b/tests/unit/expire-platform-subscriptions.test.ts new file mode 100644 index 00000000..9d4ff587 --- /dev/null +++ b/tests/unit/expire-platform-subscriptions.test.ts @@ -0,0 +1,327 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest' +import type { NextRequest } from 'next/server' + +/** + * First coverage for the school-billing expiry cron (issue #546). Before this + * file `grep -rl "expire-platform-subscriptions" tests/` returned nothing, and + * the two defects asserted here were invisible: + * + * §2 an unpaid renewal request paused the downgrade FOREVER — nothing in the + * codebase ever moved a request out of an open status except a super + * admin, so "request renewal, never pay" kept the paid plan, its limits + * and its reduced platform fee indefinitely. + * §4 the grace window was computed from the subscription's period end rather + * than from when grace actually starts, so after an outage longer than + * GRACE_DAYS the same request sent "your payment is overdue" AND "you + * have been downgraded" — and both counters incremented, so the response + * looked healthy. + * + * The Supabase fake is a small in-memory store rather than a script of canned + * replies: every phase re-queries the same tables, and phase ordering (does + * phase 3 see what phase 2 just wrote?) is exactly what these bugs live in. + */ + +type Row = Record + +const db: Record = { + platform_subscriptions: [], + platform_payment_requests: [], + platform_plans: [], + tenants: [], + tenant_users: [], +} + +const emails: { to: string; subject: string }[] = [] +const downgraded: string[] = [] + +type Predicate = (row: Row) => boolean + +function embed(table: string, cols: string, rows: Row[]): Row[] { + return rows.map((row) => { + const out: Row = { ...row } + if (cols.includes('tenants(')) { + out.tenants = db.tenants.find((t) => t.id === row.tenant_id) ?? null + } + if (cols.includes('platform_plans(')) { + out.platform_plans = db.platform_plans.find((p) => p.plan_id === row.plan_id) ?? null + } + return out + }) +} + +function makeSupabase() { + function builder(table: string) { + const preds: Predicate[] = [] + let cols = '*' + let take = Infinity + let pending: { op: 'update'; values: Row } | null = null + + const rows = () => { + const matched = (db[table] || []).filter((r) => preds.every((p) => p(r))) + return matched.slice(0, take === Infinity ? undefined : take) + } + + function settle() { + if (pending) { + for (const row of rows()) Object.assign(row, pending.values) + return { data: null, error: null } + } + return { data: embed(table, cols, rows()), error: null } + } + + const b: Record = { + select(c: string) { + cols = c + return b + }, + update(values: Row) { + pending = { op: 'update', values } + return b + }, + eq(col: string, val: unknown) { + preds.push((r) => r[col] === val) + return b + }, + in(col: string, vals: unknown[]) { + preds.push((r) => vals.includes(r[col] as never)) + return b + }, + is(col: string, val: unknown) { + preds.push((r) => (val === null ? r[col] == null : r[col] === val)) + return b + }, + not(col: string, op: string, val: unknown) { + if (op !== 'is') throw new Error(`fake: unsupported not(${op})`) + preds.push((r) => (val === null ? r[col] != null : r[col] !== val)) + return b + }, + lt(col: string, val: string) { + preds.push((r) => r[col] != null && new Date(r[col] as string) < new Date(val)) + return b + }, + lte(col: string, val: string) { + preds.push((r) => r[col] != null && new Date(r[col] as string) <= new Date(val)) + return b + }, + gte(col: string, val: string) { + preds.push((r) => r[col] != null && new Date(r[col] as string) >= new Date(val)) + return b + }, + order() { + return b + }, + limit(n: number) { + take = n + return b + }, + then: (resolve: (v: unknown) => unknown) => Promise.resolve(settle()).then(resolve), + } + return b + } + + return { + from: (table: string) => builder(table), + auth: { + admin: { + getUserById: (id: string) => + Promise.resolve({ data: { user: { email: `${id}@example.com` } }, error: null }), + }, + }, + } +} + +vi.mock('@supabase/supabase-js', () => ({ createClient: () => makeSupabase() })) + +vi.mock('@/lib/email/send', () => ({ + sendEmail: (o: { to: string; subject: string }) => { + emails.push({ to: o.to, subject: o.subject }) + return Promise.resolve(true) + }, +})) + +vi.mock('@/lib/billing/downgrade-tenant', () => ({ + downgradeTenantToFree: (_c: unknown, tenantId: string) => { + downgraded.push(tenantId) + return Promise.resolve(3) + }, +})) + +import { GET } from '@/app/api/cron/expire-platform-subscriptions/route' + +const TENANT = '00000000-0000-0000-0000-000000000001' +const PLAN = 'plan-pro' +const DAY = 24 * 60 * 60 * 1000 + +const daysFromNow = (d: number) => new Date(Date.now() + d * DAY).toISOString() + +function req(secret = 'cron-secret'): NextRequest { + return { + headers: { get: (k: string) => (k === 'authorization' ? `Bearer ${secret}` : null) }, + } as unknown as NextRequest +} + +function seedSub(over: Row = {}) { + db.platform_subscriptions.push({ + tenant_id: TENANT, + plan_id: PLAN, + payment_method: 'manual_transfer', + status: 'active', + cancel_at_period_end: false, + current_period_end: daysFromNow(-1), + grace_period_end: null, + renewal_reminder_sent_at: null, + ...over, + }) + return db.platform_subscriptions[db.platform_subscriptions.length - 1] +} + +function seedRequest(over: Row = {}) { + db.platform_payment_requests.push({ + request_id: `req-${db.platform_payment_requests.length + 1}`, + tenant_id: TENANT, + plan_id: PLAN, + request_type: 'renewal', + status: 'pending', + amount: 29, + currency: 'usd', + expires_at: daysFromNow(7), + created_at: daysFromNow(-7), + ...over, + }) + return db.platform_payment_requests[db.platform_payment_requests.length - 1] +} + +beforeEach(() => { + process.env.CRON_SECRET = 'cron-secret' + process.env.NEXT_PUBLIC_SUPABASE_URL = 'http://localhost:54321' + process.env.SUPABASE_SERVICE_ROLE_KEY = 'service-role' + for (const key of Object.keys(db)) db[key] = [] + db.tenants.push({ id: TENANT, name: 'Test School', billing_status: 'active' }) + db.platform_plans.push({ plan_id: PLAN, name: 'Pro' }) + db.tenant_users.push({ tenant_id: TENANT, user_id: 'admin-1', role: 'admin', status: 'active' }) + emails.length = 0 + downgraded.length = 0 +}) + +describe('expire-platform-subscriptions: auth', () => { + it('rejects a request without the cron secret', async () => { + const res = await GET(req('wrong')) + expect(res.status).toBe(401) + expect(downgraded).toEqual([]) + }) +}) + +describe('expire-platform-subscriptions: §4 grace window after a missed week', () => { + it('gives a full grace window even when the period lapsed 20 days ago', async () => { + // Cron down for weeks (#513 is still open on whether anything on Dokploy + // calls /api/cron/* at all), then recovers. + const sub = seedSub({ current_period_end: daysFromNow(-20) }) + + const res = await GET(req()) + const body = await res.json() + + expect(body.graceStarted).toBe(1) + // The bug: graceEnd was period_end + 7d = 13 days in the PAST, and phase 3 + // re-queries in the same request, so the school was told it was overdue and + // downgraded in one pass with no chance to pay. + expect(body.downgraded).toBe(0) + expect(downgraded).toEqual([]) + expect(sub.status).toBe('past_due') + const graceMs = new Date(sub.grace_period_end as string).getTime() - Date.now() + expect(graceMs).toBeGreaterThan(6.5 * DAY) + expect(graceMs).toBeLessThan(7.5 * DAY) + }) + + it('downgrades on a later run once the grace window it was given has passed', async () => { + seedSub({ status: 'past_due', current_period_end: daysFromNow(-30), grace_period_end: daysFromNow(-1) }) + + const body = await (await GET(req())).json() + + expect(body.downgraded).toBe(1) + expect(downgraded).toEqual([TENANT]) + expect(emails.map((e) => e.subject)).toContain( + 'Your Test School school has moved to the free plan' + ) + }) + + it('still reminds a school whose period has not lapsed yet', async () => { + const sub = seedSub({ current_period_end: daysFromNow(3) }) + + const body = await (await GET(req())).json() + + expect(body).toMatchObject({ reminded: 1, graceStarted: 0, downgraded: 0 }) + expect(sub.renewal_reminder_sent_at).not.toBeNull() + }) +}) + +describe('expire-platform-subscriptions: §2 renewal-request TTL', () => { + it('pauses the downgrade while a renewal request is still open', async () => { + seedSub({ status: 'past_due', grace_period_end: daysFromNow(-1) }) + seedRequest({ expires_at: daysFromNow(5) }) + + const body = await (await GET(req())).json() + + expect(body).toMatchObject({ skippedPendingRenewal: 1, downgraded: 0, requestsExpired: 0 }) + expect(downgraded).toEqual([]) + }) + + it('expires a lapsed request, notifies, and stops pausing the downgrade', async () => { + seedSub({ status: 'past_due', grace_period_end: daysFromNow(-1) }) + const request = seedRequest({ expires_at: daysFromNow(-1) }) + + const body = await (await GET(req())).json() + + expect(request.status).toBe('expired') + expect(body).toMatchObject({ requestsExpired: 1, skippedPendingRenewal: 0, downgraded: 1 }) + expect(downgraded).toEqual([TENANT]) + expect(emails.map((e) => e.subject)).toContain( + 'Your Pro payment request for Test School has expired' + ) + }) + + it('leaves confirmed and rejected requests alone', async () => { + const confirmed = seedRequest({ status: 'confirmed', expires_at: daysFromNow(-30) }) + const rejected = seedRequest({ status: 'rejected', expires_at: daysFromNow(-30) }) + + const body = await (await GET(req())).json() + + expect(body.requestsExpired).toBe(0) + expect(confirmed.status).toBe('confirmed') + expect(rejected.status).toBe('rejected') + }) + + it('expires a lapsed upgrade request too, without pausing anything', async () => { + const upgrade = seedRequest({ request_type: 'upgrade', expires_at: daysFromNow(-2) }) + + const body = await (await GET(req())).json() + + expect(upgrade.status).toBe('expired') + expect(body.requestsExpired).toBe(1) + }) +}) + +describe('expire-platform-subscriptions: §1 cancel-then-repay', () => { + it('downgrades a subscription still flagged cancel_at_period_end', async () => { + // What confirmManualPayment used to leave behind: a paid-for period on a row + // whose stale cancel flag survived the upsert. + seedSub({ cancel_at_period_end: true, current_period_end: daysFromNow(-1) }) + + const body = await (await GET(req())).json() + + expect(body.canceled).toBe(1) + expect(downgraded).toEqual([TENANT]) + // Silent: phases 1 and 2 both filter on cancel_at_period_end = false, so + // this school got neither a reminder nor a grace window. + expect(body.reminded).toBe(0) + expect(body.graceStarted).toBe(0) + }) + + it('reminds and grants grace once the flag is cleared by the repayment', async () => { + seedSub({ cancel_at_period_end: false, current_period_end: daysFromNow(-1) }) + + const body = await (await GET(req())).json() + + expect(body).toMatchObject({ canceled: 0, graceStarted: 1, downgraded: 0 }) + expect(downgraded).toEqual([]) + }) +}) diff --git a/tests/unit/payment-request-ttl.test.ts b/tests/unit/payment-request-ttl.test.ts new file mode 100644 index 00000000..9f02bb7a --- /dev/null +++ b/tests/unit/payment-request-ttl.test.ts @@ -0,0 +1,46 @@ +import { describe, it, expect } from 'vitest' +import { + OPEN_REQUEST_STATUSES, + REQUEST_TTL_DAYS, + isRequestOpen, + requestExpiresAt, +} from '@/lib/billing/payment-request-ttl' + +/** + * Pins the TTL rules three call sites share (issue #546 §2): the two duplicate + * guards in the billing actions, the cron's expiry sweep, and the cron's + * downgrade pause. They disagreed before — the pause looked only at status, so + * an unpaid renewal held the paid plan forever. + */ +const DAY = 24 * 60 * 60 * 1000 +const NOW = new Date('2026-07-26T12:00:00.000Z') +const at = (days: number) => new Date(NOW.getTime() + days * DAY).toISOString() + +describe('payment-request TTL', () => { + it('stamps expiry TTL days out', () => { + const ms = new Date(requestExpiresAt(NOW)).getTime() - NOW.getTime() + expect(ms).toBe(REQUEST_TTL_DAYS * DAY) + }) + + it('counts every open status while the expiry is in the future', () => { + for (const status of OPEN_REQUEST_STATUSES) { + expect(isRequestOpen({ status, expires_at: at(1) }, NOW), status).toBe(true) + } + }) + + it('stops counting the instant the expiry passes, before the cron sweeps it', () => { + // Deliberate: tying "does it still count" to the sweep would hand the + // downgrade-pause leak back during any cron outage. + expect(isRequestOpen({ status: 'pending', expires_at: at(-0.001) }, NOW)).toBe(false) + }) + + it('never counts a terminal status', () => { + for (const status of ['confirmed', 'rejected', 'expired']) { + expect(isRequestOpen({ status, expires_at: at(30) }, NOW), status).toBe(false) + } + }) + + it('treats a NULL expiry as open so a legitimate request is never dropped', () => { + expect(isRequestOpen({ status: 'pending', expires_at: null }, NOW)).toBe(true) + }) +}) diff --git a/tests/unit/plan-limit-count-parity.test.ts b/tests/unit/plan-limit-count-parity.test.ts new file mode 100644 index 00000000..328d8965 --- /dev/null +++ b/tests/unit/plan-limit-count-parity.test.ts @@ -0,0 +1,179 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest' +import { createFakeSupabase, type Db } from './support/fake-supabase' + +/** + * Issue #546 §5 — four different definitions of "how many courses does this + * tenant have". + * + * lib/billing/plan-limits.ts (downgrade pre-flight) → non-archived + * lib/billing/access-cutoff.ts (cutoff reconciler) → non-archived + * app/actions/teacher/courses.ts (creation enforcement) → ALL courses + * app/actions/admin/billing.ts (the number shown) → ALL courses + * + * plus a hardcoded `PLAN_LIMITS_FALLBACK` map as a fourth source of truth for + * the limit itself. A school with 30 courses (20 archived) on Pro saw 30/100 on + * its billing page, was approved to downgrade to Starter because 10 active ≤ 15, + * and then could not create a single course because enforcement counted 30 ≥ 15 + * — while the pre-flight's error text told it to archive courses, which is + * exactly what it had already done. + */ + +const TENANT = '00000000-0000-0000-0000-000000000001' +const USER = 'user-admin-1' +const PLAN_PRO = 'plan-pro' +const PLAN_STARTER = 'plan-starter' + +let db: Db + +function makeClient() { + return createFakeSupabase(db, { + embeds: { + platform_plans: { table: 'platform_plans', localKey: 'plan_id', foreignKey: 'plan_id' }, + }, + conflictKeys: { platform_subscriptions: 'tenant_id', revenue_splits: 'tenant_id' }, + }).client +} + +vi.mock('@/lib/supabase/server', () => ({ createClient: () => Promise.resolve(makeClient()) })) +vi.mock('@/lib/supabase/admin', () => ({ createAdminClient: () => makeClient() })) +vi.mock('@/lib/supabase/tenant', () => ({ + getCurrentTenantId: () => Promise.resolve(TENANT), + getCurrentUserId: () => Promise.resolve(USER), +})) +vi.mock('@/lib/supabase/get-user-role', () => ({ getUserRole: () => Promise.resolve('admin') })) +vi.mock('next/cache', () => ({ revalidatePath: () => {} })) +vi.mock('@/lib/email/send', () => ({ sendEmail: () => Promise.resolve(true) })) +vi.mock('@/lib/billing/access-cutoff', () => ({ + reconcileAccessCutoff: () => Promise.resolve({ action: 'none' }), +})) + +import { getSubscriptionStatus } from '@/app/actions/admin/billing' +import { checkCourseLimit } from '@/app/actions/teacher/courses' +import { checkPlanLimits } from '@/lib/billing/plan-limits' +import type { SupabaseClient } from '@supabase/supabase-js' + +/** The fake implements the slice of the client these paths touch. */ +const asClient = () => makeClient() as unknown as SupabaseClient + +function seedCourses(active: number, archived: number) { + db.courses = [ + ...Array.from({ length: active }, (_, i) => ({ + course_id: i + 1, + tenant_id: TENANT, + status: i % 2 === 0 ? 'published' : 'draft', + })), + ...Array.from({ length: archived }, (_, i) => ({ + course_id: 1000 + i, + tenant_id: TENANT, + status: 'archived', + })), + // Another tenant's courses must never be counted for this one. + { course_id: 9999, tenant_id: 'other-tenant', status: 'published' }, + ] +} + +beforeEach(() => { + db = { + tenants: [{ id: TENANT, name: 'Test School', plan: 'pro', billing_status: 'active' }], + tenant_users: [{ tenant_id: TENANT, user_id: USER, role: 'admin', status: 'active' }], + platform_plans: [ + { + plan_id: PLAN_PRO, + slug: 'pro', + name: 'Pro', + is_active: true, + price_monthly: 29, + price_yearly: 290, + transaction_fee_percent: 2, + limits: { max_courses: 100, max_students: 1000 }, + }, + { + plan_id: PLAN_STARTER, + slug: 'starter', + name: 'Starter', + is_active: true, + price_monthly: 9, + price_yearly: 90, + transaction_fee_percent: 5, + limits: { max_courses: 15, max_students: 200 }, + }, + ], + platform_subscriptions: [ + { + tenant_id: TENANT, + plan_id: PLAN_PRO, + status: 'active', + interval: 'monthly', + payment_method: 'manual_transfer', + cancel_at_period_end: false, + current_period_end: new Date(Date.now() + 5 * 86_400_000).toISOString(), + }, + ], + courses: [], + platform_payment_requests: [], + revenue_splits: [], + } + seedCourses(10, 20) +}) + +describe('#546 §5 — one course count everywhere', () => { + it('agrees across the billing page, the pre-flight and creation enforcement', async () => { + const shown = await getSubscriptionStatus() + const enforcement = await checkCourseLimit() + const preflight = await checkPlanLimits(asClient(), TENANT, { slug: 'starter' }) + + expect(shown.usage.courses.current).toBe(10) + expect(enforcement.currentCount).toBe(10) + expect(preflight.usage.courses).toBe(10) + expect(new Set([shown.usage.courses.current, enforcement.currentCount, preflight.usage.courses]).size).toBe(1) + }) + + it('lets a school create courses on the plan a downgrade pre-flight approved', async () => { + // Approved: 10 active ≤ Starter's 15. Enforcement used to see 30 ≥ 15 and + // refuse, with an error telling the school to archive courses it had + // already archived. + const preflight = await checkPlanLimits(asClient(), TENANT, { slug: 'starter' }) + expect(preflight.ok).toBe(true) + + db.tenants[0].plan = 'starter' + const enforcement = await checkCourseLimit() + + expect(enforcement.plan).toBe('starter') + expect(enforcement.limit).toBe(15) + expect(enforcement.canCreate).toBe(true) + }) + + it('still blocks creation when the ACTIVE count is at the limit', async () => { + db.tenants[0].plan = 'starter' + seedCourses(15, 4) + + const enforcement = await checkCourseLimit() + + expect(enforcement.currentCount).toBe(15) + expect(enforcement.canCreate).toBe(false) + }) + + it('reads the limit from platform_plans instead of the deleted fallback map', async () => { + db.tenants[0].plan = 'starter' + // The hardcoded map said starter = 15 no matter what the row said; the + // table is now the only source of truth. + ;(db.platform_plans[1] as { limits: { max_courses: number } }).limits = { max_courses: 3 } + seedCourses(4, 0) + + const enforcement = await checkCourseLimit() + + expect(enforcement.limit).toBe(3) + expect(enforcement.canCreate).toBe(false) + }) + + it('treats -1 as unlimited', async () => { + db.tenants[0].plan = 'starter' + ;(db.platform_plans[1] as { limits: { max_courses: number } }).limits = { max_courses: -1 } + seedCourses(500, 0) + + const enforcement = await checkCourseLimit() + + expect(enforcement.limit).toBe(-1) + expect(enforcement.canCreate).toBe(true) + }) +}) diff --git a/tests/unit/platform-billing-lifecycle.test.ts b/tests/unit/platform-billing-lifecycle.test.ts new file mode 100644 index 00000000..40dd0b39 --- /dev/null +++ b/tests/unit/platform-billing-lifecycle.test.ts @@ -0,0 +1,302 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest' +import { createFakeSupabase, type Db, type Row } from './support/fake-supabase' + +/** + * First coverage for the school→platform billing actions (issue #546 §1, §2). + * + * §1 Cancelling was irreversible AND paying again did not undo it. PostgREST's + * ON CONFLICT DO UPDATE only writes the columns you supply, so + * confirmManualPayment's upsert left a stale `cancel_at_period_end = true` + * on the row: the school paid for a full year and was dropped to free at the + * end of it, with no reminder and no grace (both cron phases filter on + * `cancel_at_period_end = false`). The billing UI imported no reactivate + * action at all, and re-checkout is blocked while the sub is still active. + * + * §2 Both duplicate-request guards used `.single()`. With two matching rows + * PostgREST returns `PGRST116 / data: null`, so the guard PASSED — and since + * the two guards had different filters, creating a renewal alongside a + * pending upgrade was the ordinary way to get there. + */ + +const TENANT = '00000000-0000-0000-0000-000000000001' +const USER = 'user-admin-1' +const PLAN_PRO = 'plan-pro' +const PLAN_BUSINESS = 'plan-business' +const DAY = 24 * 60 * 60 * 1000 +const daysFromNow = (d: number) => new Date(Date.now() + d * DAY).toISOString() + +let db: Db +const stripeUpdates: { id: string; params: Record }[] = [] + +function makeClient() { + return createFakeSupabase(db, { + embeds: { + platform_plans: { table: 'platform_plans', localKey: 'plan_id', foreignKey: 'plan_id' }, + tenants: { table: 'tenants', localKey: 'tenant_id', foreignKey: 'id' }, + }, + conflictKeys: { + platform_subscriptions: 'tenant_id', + revenue_splits: 'tenant_id', + }, + }).client +} + +vi.mock('@/lib/supabase/server', () => ({ createClient: () => Promise.resolve(makeClient()) })) +vi.mock('@/lib/supabase/admin', () => ({ createAdminClient: () => makeClient() })) +vi.mock('@/lib/supabase/tenant', () => ({ + getCurrentTenantId: () => Promise.resolve(TENANT), + getCurrentUserId: () => Promise.resolve(USER), +})) +vi.mock('next/cache', () => ({ revalidatePath: () => {} })) +vi.mock('@/lib/billing/access-cutoff', () => ({ reconcileAccessCutoff: () => Promise.resolve({ action: 'none' }) })) +vi.mock('@/lib/stripe', () => ({ + getStripe: () => ({ + subscriptions: { + update: (id: string, params: Record) => { + stripeUpdates.push({ id, params }) + return Promise.resolve({ id }) + }, + retrieve: () => + Promise.resolve({ id: 'sub_live', customer: 'cus_1', items: { data: [{ id: 'si_1' }] } }), + }, + }), +})) + +import { + confirmManualPayment, + reactivateSubscription, + requestManualRenewal, + requestManualPlanUpgrade, + changePlan, +} from '@/app/actions/admin/billing' + +function seedSub(over: Row = {}) { + db.platform_subscriptions = [ + { + subscription_id: 'sub-row-1', + tenant_id: TENANT, + plan_id: PLAN_PRO, + payment_method: 'manual_transfer', + status: 'active', + interval: 'yearly', + cancel_at_period_end: false, + canceled_at: null, + current_period_start: daysFromNow(-360), + current_period_end: daysFromNow(5), + grace_period_end: null, + renewal_reminder_sent_at: null, + stripe_subscription_id: null, + plan_override_by: null, + plan_override_at: null, + ...over, + }, + ] + return db.platform_subscriptions[0] +} + +function seedRequest(over: Row = {}) { + const row: Row = { + request_id: `req-${db.platform_payment_requests.length + 1}`, + tenant_id: TENANT, + plan_id: PLAN_PRO, + requested_by: USER, + request_type: 'upgrade', + status: 'pending', + interval: 'yearly', + amount: 290, + currency: 'usd', + created_at: daysFromNow(-2), + expires_at: daysFromNow(12), + ...over, + } + db.platform_payment_requests.push(row) + return row +} + +beforeEach(() => { + stripeUpdates.length = 0 + db = { + tenants: [{ id: TENANT, name: 'Test School', plan: 'pro', billing_status: 'active' }], + tenant_users: [{ tenant_id: TENANT, user_id: USER, role: 'admin', status: 'active' }], + super_admins: [{ user_id: USER }], + platform_plans: [ + { + plan_id: PLAN_PRO, + slug: 'pro', + name: 'Pro', + price_monthly: 29, + price_yearly: 290, + transaction_fee_percent: 2, + sort_order: 3, + is_active: true, + limits: { max_courses: 100, max_students: 1000 }, + stripe_price_id_monthly: 'price_pro_m', + stripe_price_id_yearly: 'price_pro_y', + }, + { + plan_id: PLAN_BUSINESS, + slug: 'business', + name: 'Business', + price_monthly: 79, + price_yearly: 790, + transaction_fee_percent: 0, + sort_order: 4, + is_active: true, + limits: { max_courses: -1, max_students: 5000 }, + stripe_price_id_monthly: 'price_biz_m', + stripe_price_id_yearly: 'price_biz_y', + }, + ], + platform_subscriptions: [], + platform_payment_requests: [], + revenue_splits: [], + courses: [], + } +}) + +describe('#546 §1 — confirming a payment un-cancels', () => { + it('clears cancel_at_period_end and canceled_at on the renewed subscription', async () => { + const sub = seedSub({ cancel_at_period_end: true, canceled_at: daysFromNow(-3) }) + const request = seedRequest({ request_type: 'renewal', status: 'payment_received' }) + + await confirmManualPayment(request.request_id as string) + + // The bug: the upsert never named these two columns, so the stale `true` + // survived and the cron's cancel phase dropped the school to free at the + // end of the year it had just paid for. + expect(sub.cancel_at_period_end).toBe(false) + expect(sub.canceled_at).toBeNull() + expect(sub.status).toBe('active') + expect(new Date(sub.current_period_end as string).getTime()).toBeGreaterThan(Date.now()) + expect(request.status).toBe('confirmed') + }) + + it('clears a super-admin plan override, so portal changes sync again', async () => { + const sub = seedSub({ plan_override_by: 'super-1', plan_override_at: daysFromNow(-10) }) + const request = seedRequest({ request_type: 'renewal' }) + + await confirmManualPayment(request.request_id as string) + + expect(sub.plan_override_at).toBeNull() + expect(sub.plan_override_by).toBeNull() + }) +}) + +describe('#546 §1 — reactivateSubscription', () => { + it('removes a pending cancellation on a manual subscription', async () => { + const sub = seedSub({ cancel_at_period_end: true, canceled_at: daysFromNow(-1) }) + + await reactivateSubscription() + + expect(sub.cancel_at_period_end).toBe(false) + expect(sub.canceled_at).toBeNull() + expect(stripeUpdates).toEqual([]) + }) + + it('clears the cancellation on Stripe first for a Stripe subscription', async () => { + const sub = seedSub({ + payment_method: 'stripe', + stripe_subscription_id: 'sub_live', + cancel_at_period_end: true, + }) + + await reactivateSubscription() + + expect(stripeUpdates).toEqual([{ id: 'sub_live', params: { cancel_at_period_end: false } }]) + expect(sub.cancel_at_period_end).toBe(false) + }) + + it('refuses when nothing is scheduled for cancellation', async () => { + seedSub({ cancel_at_period_end: false }) + await expect(reactivateSubscription()).rejects.toThrow(/not scheduled for cancellation/) + }) + + it('refuses once the paid period has already ended', async () => { + seedSub({ cancel_at_period_end: true, current_period_end: daysFromNow(-1) }) + await expect(reactivateSubscription()).rejects.toThrow(/already ended/) + }) +}) + +describe('#546 §1 — changePlan does not leave a cancellation behind', () => { + it('sends cancel_at_period_end: false to Stripe and mirrors it locally', async () => { + const sub = seedSub({ + payment_method: 'stripe', + stripe_subscription_id: 'sub_live', + cancel_at_period_end: true, + canceled_at: daysFromNow(-2), + plan_override_at: daysFromNow(-20), + plan_override_by: 'super-1', + }) + + await changePlan(PLAN_BUSINESS, 'yearly') + + expect(stripeUpdates).toHaveLength(1) + expect(stripeUpdates[0].params).toMatchObject({ cancel_at_period_end: false }) + expect(sub.cancel_at_period_end).toBe(false) + expect(sub.canceled_at).toBeNull() + expect(sub.plan_override_at).toBeNull() + expect(sub.plan_id).toBe(PLAN_BUSINESS) + }) +}) + +describe('#546 §2 — duplicate-request guards survive two open rows', () => { + it('blocks a third request when two are already open', async () => { + seedSub() + seedRequest({ request_type: 'upgrade' }) + seedRequest({ request_type: 'renewal' }) + + // `.single()` returned PGRST116 with `data: null` here, so BOTH guards + // passed and the table grew unbounded per tenant. + await expect(requestManualPlanUpgrade(PLAN_BUSINESS, 'yearly')).rejects.toThrow(/already have a pending/) + await expect(requestManualRenewal()).rejects.toThrow(/already have a pending/) + expect(db.platform_payment_requests).toHaveLength(2) + }) + + it('blocks a renewal while an upgrade request is open', async () => { + seedSub() + seedRequest({ request_type: 'upgrade' }) + + await expect(requestManualRenewal()).rejects.toThrow(/already have a pending/) + }) + + it('ignores a lapsed request so a school is never locked out of paying', async () => { + seedSub() + seedRequest({ request_type: 'renewal', expires_at: daysFromNow(-1) }) + + await requestManualRenewal() + + expect(db.platform_payment_requests).toHaveLength(2) + const created = db.platform_payment_requests[1] + const ttlDays = (new Date(created.expires_at as string).getTime() - Date.now()) / DAY + expect(ttlDays).toBeGreaterThan(13.5) + expect(ttlDays).toBeLessThan(14.5) + }) + + it('ignores confirmed and rejected requests', async () => { + seedSub() + seedRequest({ status: 'confirmed' }) + seedRequest({ status: 'rejected' }) + + await requestManualRenewal() + + expect(db.platform_payment_requests).toHaveLength(3) + }) +}) + +describe('#546 §2 — the renewal window guard is not bypassed by a non-active status', () => { + it('rejects a renewal request 200 days before the end of a canceled subscription', async () => { + seedSub({ status: 'canceled', current_period_end: daysFromNow(200) }) + + await expect(requestManualRenewal()).rejects.toThrow(/within 30 days/) + expect(db.platform_payment_requests).toHaveLength(0) + }) + + it('still lets a school in grace pay', async () => { + seedSub({ status: 'past_due', current_period_end: daysFromNow(-3) }) + + await requestManualRenewal() + + expect(db.platform_payment_requests).toHaveLength(1) + expect(db.platform_payment_requests[0].request_type).toBe('renewal') + }) +}) diff --git a/tests/unit/platform-plan-change.test.ts b/tests/unit/platform-plan-change.test.ts index 0661a5fd..478c71de 100644 --- a/tests/unit/platform-plan-change.test.ts +++ b/tests/unit/platform-plan-change.test.ts @@ -22,7 +22,7 @@ interface PlanRow { interface FakeConfig { newPlan?: PlanRow | null oldPlan?: PlanRow | null - currentSub?: { plan_id: string; interval: string | null } | null + currentSub?: { plan_id: string; interval: string | null; plan_override_at?: string | null } | null courseCount?: number studentCount?: number adminUsers?: string[] @@ -316,3 +316,74 @@ describe('applyPortalPlanChange', () => { expect(calls.stripeUpdates).toHaveLength(0) }) }) + +/** + * Issue #546 §3. A super admin comps a school from Starter to Pro with + * `forceTenantPlanChange`, which never calls Stripe. Stripe therefore keeps + * billing Starter, and the next routine `customer.subscription.updated` carries + * the Starter price — which this module read as a Pro→Starter downgrade. The + * school is over Starter's limits (that is WHY it was comped), so the enforcement + * path "reverted" Stripe onto the PRO price: the school gets billed for the plan + * it was given for free, by the code whose stated purpose is protecting an + * invariant. #468 item 2 introduced the coupling; the dead webhook (#544) masked + * it until that was fixed. + */ +describe('applyPortalPlanChange — super-admin plan override (#546 §3)', () => { + const comped = (override: string | null) => ({ + newPlan: STARTER, + oldPlan: PRO, + currentSub: { plan_id: PRO.plan_id, interval: 'monthly', plan_override_at: override }, + // Over Starter's 15-course limit — the reason the comp exists. + courseCount: 30, + adminUsers: ['admin-a'], + }) + + it('never reprices a comped tenant on Stripe', async () => { + const { admin, calls, sendEmailFn, makeStripe } = makeFakeAdmin(comped('2026-07-20T00:00:00.000Z')) + + const result = await applyPortalPlanChange(subscriptionEvent('price_starter_m'), { + admin, + stripe: makeStripe(), + sendEmailFn, + }) + + expect(result).toMatchObject({ action: 'ignored' }) + expect(calls.stripeUpdates).toHaveLength(0) + expect(calls.updates).toHaveLength(0) + expect(calls.upserts).toHaveLength(0) + expect(calls.emails).toHaveLength(0) + }) + + it('control: without the override marker the same event reprices Stripe onto the comped plan', async () => { + const { admin, calls, sendEmailFn, makeStripe } = makeFakeAdmin(comped(null)) + + const result = await applyPortalPlanChange(subscriptionEvent('price_starter_m'), { + admin, + stripe: makeStripe(), + sendEmailFn, + }) + + expect(result.action).toBe('reverted') + const items = calls.stripeUpdates[0].params.items as { price: string }[] + expect(items[0].price).toBe('price_pro_m') + }) + + it('clearing the override lets portal changes reconcile again', async () => { + const { admin, calls, sendEmailFn, makeStripe } = makeFakeAdmin({ + newPlan: STARTER, + oldPlan: PRO, + currentSub: { plan_id: PRO.plan_id, interval: 'monthly', plan_override_at: null }, + courseCount: 5, + studentCount: 10, + }) + + const result = await applyPortalPlanChange(subscriptionEvent(), { + admin, + stripe: makeStripe(), + sendEmailFn, + }) + + expect(result.action).toBe('applied') + expect(calls.updates.find((u) => u.table === 'tenants')?.values.plan).toBe('starter') + }) +}) diff --git a/tests/unit/support/fake-supabase.ts b/tests/unit/support/fake-supabase.ts new file mode 100644 index 00000000..d1cf59ce --- /dev/null +++ b/tests/unit/support/fake-supabase.ts @@ -0,0 +1,208 @@ +/** + * Minimal in-memory Supabase/PostgREST stand-in for the platform-billing tests + * (issue #546). + * + * The billing lifecycle bugs under test are all about what a query RETURNS for + * a given table state — a `.single()` that yields `PGRST116 / data: null` once + * two rows match, an upsert that leaves an unnamed column untouched, a count + * that does or does not include archived rows. Canned per-call replies cannot + * express any of that, so the fake keeps real rows and applies real filters. + * + * Supported: select (incl. `{count:'exact', head:true}` and one-level embedded + * `table(cols)`), insert, update, upsert (on a caller-declared conflict key), + * eq/neq/in/is/not-is/lt/lte/gt/gte, order, limit, single, maybeSingle, and + * awaiting the builder directly. Everything else throws loudly rather than + * silently returning nothing. + */ + +export type Row = Record +export type Db = Record + +export interface FakeSupabaseOptions { + /** Column PostgREST would resolve an embedded `(...)` join on. */ + embeds?: Record + /** Conflict target per table, for `.upsert(..., { onConflict })`. */ + conflictKeys?: Record + /** Emails to hand back from `auth.admin.getUserById`. */ + userEmail?: (id: string) => string | null +} + +type Predicate = (row: Row) => boolean + +export function createFakeSupabase(db: Db, opts: FakeSupabaseOptions = {}) { + const embeds = opts.embeds ?? {} + const conflictKeys = opts.conflictKeys ?? {} + const writes: { table: string; op: 'insert' | 'update' | 'upsert'; values: Row }[] = [] + + function embed(cols: string, rows: Row[]): Row[] { + const attached = Object.entries(embeds).filter(([name]) => cols.includes(`${name}(`)) + if (attached.length === 0) return rows.map((r) => ({ ...r })) + return rows.map((row) => { + const out: Row = { ...row } + for (const [name, cfg] of attached) { + out[name] = + (db[cfg.table] || []).find((f) => f[cfg.foreignKey] === row[cfg.localKey]) ?? null + } + return out + }) + } + + function builder(table: string) { + const preds: Predicate[] = [] + let cols = '*' + let wantCount = false + let headOnly = false + let take = Infinity + let pending: { op: 'insert' | 'update' | 'upsert'; values: Row; onConflict?: string } | null = + null + + db[table] = db[table] || [] + + const matched = () => { + const rows = db[table].filter((r) => preds.every((p) => p(r))) + return take === Infinity ? rows : rows.slice(0, take) + } + + function applyWrite(): Row[] { + const w = pending! + writes.push({ table, op: w.op, values: { ...w.values } }) + if (w.op === 'update') { + const rows = matched() + for (const row of rows) Object.assign(row, w.values) + return rows + } + if (w.op === 'insert') { + const row = { ...w.values } + db[table].push(row) + return [row] + } + // upsert + const key = w.onConflict || conflictKeys[table] + const existing = key ? db[table].find((r) => r[key] === w.values[key]) : undefined + if (existing) { + Object.assign(existing, w.values) + return [existing] + } + const row = { ...w.values } + db[table].push(row) + return [row] + } + + function settle(): { data: unknown; error: unknown; count?: number } { + if (pending) { + const rows = applyWrite() + return { data: embed(cols, rows), error: null } + } + const rows = matched() + if (wantCount) { + return { data: headOnly ? null : embed(cols, rows), error: null, count: rows.length } + } + return { data: embed(cols, rows), error: null } + } + + const one = (allowEmpty: boolean) => { + const settled = settle() + const rows = (settled.data as Row[] | null) || [] + if (rows.length === 1) return { data: rows[0], error: null } + if (rows.length === 0) { + return allowEmpty + ? { data: null, error: null } + : { data: null, error: { code: 'PGRST116', message: 'no rows returned' } } + } + // What PostgREST really does with 2+ rows, and the reason the duplicate + // guards this suite covers silently passed. + return { + data: null, + error: { code: 'PGRST116', message: 'more than one row returned' }, + } + } + + const b: Record = { + select(c = '*', o?: { count?: string; head?: boolean }) { + cols = c + wantCount = o?.count === 'exact' + headOnly = o?.head === true + return b + }, + insert(values: Row) { + pending = { op: 'insert', values } + return b + }, + update(values: Row) { + pending = { op: 'update', values } + return b + }, + upsert(values: Row, o?: { onConflict?: string }) { + pending = { op: 'upsert', values, onConflict: o?.onConflict } + return b + }, + eq(col: string, val: unknown) { + preds.push((r) => r[col] === val) + return b + }, + neq(col: string, val: unknown) { + preds.push((r) => r[col] !== val) + return b + }, + in(col: string, vals: unknown[]) { + preds.push((r) => vals.includes(r[col] as never)) + return b + }, + is(col: string, val: unknown) { + preds.push((r) => (val === null ? r[col] == null : r[col] === val)) + return b + }, + not(col: string, op: string, val: unknown) { + if (op !== 'is') throw new Error(`fake-supabase: unsupported not(${op})`) + preds.push((r) => (val === null ? r[col] != null : r[col] !== val)) + return b + }, + lt: cmp(preds, (a, c) => a < c), + lte: cmp(preds, (a, c) => a <= c), + gt: cmp(preds, (a, c) => a > c), + gte: cmp(preds, (a, c) => a >= c), + order() { + return b + }, + limit(n: number) { + take = n + return b + }, + single: () => Promise.resolve(one(false)), + maybeSingle: () => Promise.resolve(one(true)), + then: (resolve: (v: unknown) => unknown, reject?: (e: unknown) => unknown) => + Promise.resolve(settle()).then(resolve, reject), + } + + function cmp(list: Predicate[], op: (a: number, c: number) => boolean) { + return (col: string, val: string | number) => { + list.push((r) => { + const raw = r[col] + if (raw == null) return false + const left = typeof raw === 'number' ? raw : new Date(raw as string).getTime() + const right = typeof val === 'number' ? val : new Date(val).getTime() + return op(left, right) + }) + return b + } + } + + return b + } + + return { + client: { + from: (table: string) => builder(table), + auth: { + admin: { + getUserById: (id: string) => + Promise.resolve({ + data: { user: { email: opts.userEmail ? opts.userEmail(id) : `${id}@example.com` } }, + error: null, + }), + }, + }, + }, + writes, + } +}