From 0306157a602bd7bca574975a4fe2a90a5e6bda7c Mon Sep 17 00:00:00 2001 From: Moheeb Zara Date: Mon, 18 May 2026 13:11:09 -0700 Subject: [PATCH] refactor(stripe): extract + unit-test webhook decision logic (R2) The webhook's money/status mapping (Basil field relocations, period extraction, customer-id unwrap, status patch, invoice->payment row, minor->major) lived inline in the route with zero test coverage - the biggest untested critical path in the codebase. Extracted to a pure, SDK-free lib/stripe/webhook-logic.ts (structural input types) and refactored the route to only do signature verify + network + DB writes. 12 new tests cover the Basil edges and link/unlink + defaults. Behavior unchanged; suite 514. --- __tests__/stripe-webhook-logic.test.ts | 132 ++++++++++++++++++++++++ app/api/stripe/webhook/[space]/route.ts | 83 ++++++--------- lib/stripe/webhook-logic.ts | 101 ++++++++++++++++++ 3 files changed, 262 insertions(+), 54 deletions(-) create mode 100644 __tests__/stripe-webhook-logic.test.ts create mode 100644 lib/stripe/webhook-logic.ts diff --git a/__tests__/stripe-webhook-logic.test.ts b/__tests__/stripe-webhook-logic.test.ts new file mode 100644 index 0000000..6cb5259 --- /dev/null +++ b/__tests__/stripe-webhook-logic.test.ts @@ -0,0 +1,132 @@ +import { describe, it, expect } from 'vitest' +import { + isoFromUnix, + customerIdOf, + subscriptionPeriodEnd, + invoiceMetadataMemberId, + invoiceLinePeriodEnd, + memberStatusPatch, + stripeInvoiceToPaymentRow, + minorToMajor, +} from '@/lib/stripe/webhook-logic' + +describe('isoFromUnix', () => { + it('converts a positive unix seconds value to ISO', () => { + expect(isoFromUnix(1_700_000_000)).toBe(new Date(1_700_000_000_000).toISOString()) + }) + it('returns null for 0, negative, null, undefined, NaN', () => { + for (const v of [0, -1, null, undefined, NaN]) { + expect(isoFromUnix(v as number)).toBeNull() + } + }) +}) + +describe('customerIdOf', () => { + it('handles string id, expanded object, empty, and nullish', () => { + expect(customerIdOf('cus_1')).toBe('cus_1') + expect(customerIdOf({ id: 'cus_2' })).toBe('cus_2') + expect(customerIdOf('')).toBeNull() + expect(customerIdOf(null)).toBeNull() + expect(customerIdOf(undefined)).toBeNull() + expect(customerIdOf({ id: null })).toBeNull() + }) +}) + +describe('subscriptionPeriodEnd (Basil: period on item, not subscription)', () => { + it('reads item 0 current_period_end', () => { + expect(subscriptionPeriodEnd({ items: { data: [{ current_period_end: 1_700_000_000 }] } })).toBe( + new Date(1_700_000_000_000).toISOString(), + ) + }) + it('null when no items / no field', () => { + expect(subscriptionPeriodEnd({ items: { data: [] } })).toBeNull() + expect(subscriptionPeriodEnd({})).toBeNull() + expect(subscriptionPeriodEnd(null)).toBeNull() + expect(subscriptionPeriodEnd({ items: { data: [null] } })).toBeNull() + }) +}) + +describe('invoiceMetadataMemberId (Basil: under parent.subscription_details)', () => { + it('reads the relocated metadata', () => { + expect( + invoiceMetadataMemberId({ parent: { subscription_details: { metadata: { member_id: 'm1' } } } }), + ).toBe('m1') + }) + it('undefined when any level is missing or empty', () => { + expect(invoiceMetadataMemberId({})).toBeUndefined() + expect(invoiceMetadataMemberId({ parent: null })).toBeUndefined() + expect(invoiceMetadataMemberId({ parent: { subscription_details: { metadata: { member_id: '' } } } })).toBeUndefined() + }) +}) + +describe('invoiceLinePeriodEnd', () => { + it('reads line 0 period end, null when absent', () => { + expect(invoiceLinePeriodEnd({ lines: { data: [{ period: { end: 1_700_000_000 } }] } })).toBe( + new Date(1_700_000_000_000).toISOString(), + ) + expect(invoiceLinePeriodEnd({ lines: { data: [] } })).toBeNull() + expect(invoiceLinePeriodEnd({ lines: null })).toBeNull() + expect(invoiceLinePeriodEnd({})).toBeNull() + }) +}) + +describe('memberStatusPatch (only current<->late)', () => { + it('current adds last_paid_at; late does not; null/undefined -> no patch', () => { + expect(memberStatusPatch('current', '2026-01-01T00:00:00.000Z')).toEqual({ + status: 'current', + last_paid_at: '2026-01-01T00:00:00.000Z', + }) + expect(memberStatusPatch('late', '2026-01-01T00:00:00.000Z')).toEqual({ status: 'late' }) + expect(memberStatusPatch(null, 'x')).toBeNull() + expect(memberStatusPatch(undefined, 'x')).toBeNull() + }) +}) + +describe('stripeInvoiceToPaymentRow', () => { + it('maps minor->major, upcases currency, links by member presence', () => { + const row = stripeInvoiceToPaymentRow({ + inv: { id: 'in_1', amount_paid: 2500, currency: 'usd', customer_email: 'a@b.test' }, + spaceId: 's1', + memberId: 'm1', + eventId: 'evt_1', + nowIso: '2026-01-01T00:00:00.000Z', + }) + expect(row).toMatchObject({ + space_id: 's1', + member_id: 'm1', + platform: 'stripe', + amount: 25, + currency: 'USD', + status: 'linked', + link_status: 'linked', + external_id: 'in_1', + payer_email: 'a@b.test', + from_identifier: 'a@b.test', + transaction_date: '2026-01-01T00:00:00.000Z', + raw_data: { event_id: 'evt_1', invoice: 'in_1' }, + }) + }) + it('unlinked + safe defaults when member/amount/currency/email missing', () => { + const row = stripeInvoiceToPaymentRow({ + inv: { id: 'in_2' }, + spaceId: 's1', + memberId: null, + eventId: 'evt_2', + nowIso: '2026-01-01T00:00:00.000Z', + }) + expect(row.status).toBe('unlinked') + expect(row.link_status).toBe('unlinked') + expect(row.amount).toBe(0) + expect(row.currency).toBe('USD') + expect(row.description).toBe('Stripe membership dues') + expect(row.payer_email).toBeNull() + }) +}) + +describe('minorToMajor', () => { + it('divides by 100, treats nullish as 0', () => { + expect(minorToMajor(2500)).toBe(25) + expect(minorToMajor(null)).toBe(0) + expect(minorToMajor(undefined)).toBe(0) + }) +}) diff --git a/app/api/stripe/webhook/[space]/route.ts b/app/api/stripe/webhook/[space]/route.ts index c4d0f39..d257fe4 100644 --- a/app/api/stripe/webhook/[space]/route.ts +++ b/app/api/stripe/webhook/[space]/route.ts @@ -13,6 +13,15 @@ import { createAdminClient } from '@/lib/supabase/admin' import { getStripe } from '@/lib/stripe/client' import { getStripeConfig, readStripeSecret } from '@/lib/stripe/config' import { duesMemberStatus, graceExceeded } from '@/lib/stripe-logic' +import { + customerIdOf, + subscriptionPeriodEnd, + invoiceMetadataMemberId, + invoiceLinePeriodEnd, + memberStatusPatch, + stripeInvoiceToPaymentRow, + minorToMajor, +} from '@/lib/stripe/webhook-logic' import { renderDuesEmail, duesDedupeKey, @@ -22,10 +31,6 @@ import { export const runtime = 'nodejs' export const dynamic = 'force-dynamic' -function isoFromUnix(s: number | null | undefined): string | null { - return typeof s === 'number' && s > 0 ? new Date(s * 1000).toISOString() : null -} - export async function POST( req: NextRequest, { params }: { params: Promise<{ space: string }> }, @@ -182,19 +187,13 @@ export async function POST( } async function applySubscription(sub: Stripe.Subscription) { - const customerId = typeof sub.customer === 'string' ? sub.customer : sub.customer?.id ?? null + const customerId = customerIdOf(sub.customer) const memberId = await resolveMemberId( (sub.metadata?.member_id as string | undefined) || undefined, customerId, ) if (!memberId) return - // API 2026-04-22.dahlia (Basil 2025-03-31): the billing period moved off - // the top-level Subscription onto each subscription item. Dues are a - // single-price subscription, so item 0 carries the period end. - const periodEnd = isoFromUnix( - (sub.items?.data?.[0] as unknown as { current_period_end?: number } | undefined) - ?.current_period_end, - ) + const periodEnd = subscriptionPeriodEnd(sub) await admin.from('member_billing').upsert( { @@ -210,11 +209,10 @@ export async function POST( ) const desired = duesMemberStatus(sub.status, graceExceeded(periodEnd, graceDays)) - if (desired) { + const patch = memberStatusPatch(desired, new Date().toISOString()) + if (patch) { // Only move between current<->late; never resurrect inactive or // auto-approve unverified via billing. - const patch: Record = { status: desired } - if (desired === 'current') patch.last_paid_at = new Date().toISOString() await admin .from('space_members') .update(patch) @@ -251,16 +249,8 @@ export async function POST( // for the same invoice — handling both would double the ledger). case 'invoice.paid': { const inv = event.data.object as Stripe.Invoice - const customerId = typeof inv.customer === 'string' ? inv.customer : inv.customer?.id ?? null - // API 2026-04-22.dahlia (Basil): invoice subscription + its metadata - // moved under invoice.parent.subscription_details. - const invParent = (inv as unknown as { - parent?: { subscription_details?: { metadata?: Record } } - }).parent - const memberId = await resolveMemberId( - invParent?.subscription_details?.metadata?.member_id || undefined, - customerId, - ) + const customerId = customerIdOf(inv.customer) + const memberId = await resolveMemberId(invoiceMetadataMemberId(inv), customerId) // Ledger idempotency: invoice.paid + a retry are distinct event ids // but the same invoice; never double-record one charge. const { data: dupe } = await admin @@ -271,21 +261,15 @@ export async function POST( .eq('external_id', inv.id) .maybeSingle() if (!dupe) { - await admin.from('payments').insert({ - space_id: spaceId, - member_id: memberId, - platform: 'stripe', - amount: (inv.amount_paid ?? 0) / 100, - currency: (inv.currency ?? 'usd').toUpperCase(), - description: inv.description ?? 'Stripe membership dues', - status: memberId ? 'linked' : 'unlinked', - link_status: memberId ? 'linked' : 'unlinked', - external_id: inv.id, - payer_email: inv.customer_email ?? null, - from_identifier: inv.customer_email ?? null, - transaction_date: new Date().toISOString(), - raw_data: { event_id: event.id, invoice: inv.id }, - }) + await admin.from('payments').insert( + stripeInvoiceToPaymentRow({ + inv, + spaceId, + memberId, + eventId: event.id, + nowIso: new Date().toISOString(), + }), + ) } if (memberId) { await admin @@ -295,17 +279,12 @@ export async function POST( .eq('space_id', spaceId) .in('status', ['current', 'late']) } - const paidPeriodEnd = isoFromUnix( - (inv as unknown as { - lines?: { data?: Array<{ period?: { end?: number } }> } - }).lines?.data?.[0]?.period?.end, - ) await enqueueDues('dues_renewed', { memberId, fallbackEmail: inv.customer_email ?? null, - amount: (inv.amount_paid ?? 0) / 100, + amount: minorToMajor(inv.amount_paid), currency: (inv.currency ?? 'usd').toUpperCase(), - periodEnd: paidPeriodEnd, + periodEnd: invoiceLinePeriodEnd(inv), invoiceId: inv.id, }) break @@ -315,18 +294,14 @@ export async function POST( // updated (status past_due past grace) through applySubscription. case 'invoice.payment_failed': { const inv = event.data.object as Stripe.Invoice - const customerId = typeof inv.customer === 'string' ? inv.customer : inv.customer?.id ?? null - const invParent = (inv as unknown as { - parent?: { subscription_details?: { metadata?: Record } } - }).parent const memberId = await resolveMemberId( - invParent?.subscription_details?.metadata?.member_id || undefined, - customerId, + invoiceMetadataMemberId(inv), + customerIdOf(inv.customer), ) await enqueueDues('dues_payment_failed', { memberId, fallbackEmail: inv.customer_email ?? null, - amount: (inv.amount_due ?? 0) / 100, + amount: minorToMajor(inv.amount_due), currency: (inv.currency ?? 'usd').toUpperCase(), invoiceId: inv.id, }) diff --git a/lib/stripe/webhook-logic.ts b/lib/stripe/webhook-logic.ts new file mode 100644 index 0000000..7457fed --- /dev/null +++ b/lib/stripe/webhook-logic.ts @@ -0,0 +1,101 @@ +// Pure decision + mapping logic for the Stripe webhook. No Stripe SDK, no +// Supabase, no I/O — structural input types only, so every branch is unit- +// testable in isolation. The route handler keeps only signature verify, +// network calls, and DB writes; all "what should we derive/record" logic +// lives here. Status mapping itself stays in lib/stripe-logic.ts +// (duesMemberStatus / graceExceeded); this module is the webhook-shaped glue +// around it, including the Stripe Basil (2025-03-31 / 2026-04-22.dahlia) +// field relocations that are easy to get wrong and must be tested. + +type CustomerRef = string | { id?: string | null } | null | undefined + +type SubscriptionLike = { + items?: { data?: Array<{ current_period_end?: number } | null | undefined> } +} + +type InvoiceLike = { + id?: string | null + amount_paid?: number | null + amount_due?: number | null + currency?: string | null + description?: string | null + customer_email?: string | null + parent?: { subscription_details?: { metadata?: Record | null } | null } | null + lines?: { data?: Array<{ period?: { end?: number } | null } | null> } | null +} + +export function isoFromUnix(s: number | null | undefined): string | null { + return typeof s === 'number' && s > 0 ? new Date(s * 1000).toISOString() : null +} + +// Stripe customer is either an id string or an expanded object. +export function customerIdOf(customer: CustomerRef): string | null { + if (typeof customer === 'string') return customer || null + return customer?.id ?? null +} + +// Basil moved the billing period off the top-level Subscription onto each +// item. Dues are a single-price subscription, so item 0 carries it. +export function subscriptionPeriodEnd(sub: SubscriptionLike | null | undefined): string | null { + return isoFromUnix(sub?.items?.data?.[0]?.current_period_end) +} + +// Basil moved invoice subscription metadata under +// invoice.parent.subscription_details.metadata. +export function invoiceMetadataMemberId(inv: InvoiceLike | null | undefined): string | undefined { + return inv?.parent?.subscription_details?.metadata?.member_id || undefined +} + +// Next renewal date for the renewal receipt: the paid invoice line's period +// end. Absent on some invoices -> null (email simply omits the date). +export function invoiceLinePeriodEnd(inv: InvoiceLike | null | undefined): string | null { + return isoFromUnix(inv?.lines?.data?.[0]?.period?.end) +} + +// Build the space_members patch for a billing-driven status change. Only +// current<->late ever moves (the route additionally constrains the UPDATE to +// rows already in those states, so inactive/unverified are never touched). +// Returns null when there is no status to apply. +export function memberStatusPatch( + desired: 'current' | 'late' | null | undefined, + nowIso: string, +): { status: 'current' | 'late'; last_paid_at?: string } | null { + if (!desired) return null + if (desired === 'current') return { status: 'current', last_paid_at: nowIso } + return { status: 'late' } +} + +// Stripe amounts are in minor units; the payments ledger stores major units +// (numeric, matching the existing PayPal/CSV rows). Centralized so the cents +// division + currency casing + link status can be tested once. +export function stripeInvoiceToPaymentRow(args: { + inv: InvoiceLike + spaceId: string + memberId: string | null + eventId: string + nowIso: string +}) { + const { inv, spaceId, memberId, eventId, nowIso } = args + const linked = memberId ? 'linked' : 'unlinked' + return { + space_id: spaceId, + member_id: memberId, + platform: 'stripe' as const, + amount: (inv.amount_paid ?? 0) / 100, + currency: (inv.currency ?? 'usd').toUpperCase(), + description: inv.description ?? 'Stripe membership dues', + status: linked, + link_status: linked, + external_id: inv.id ?? null, + payer_email: inv.customer_email ?? null, + from_identifier: inv.customer_email ?? null, + transaction_date: nowIso, + raw_data: { event_id: eventId, invoice: inv.id ?? null }, + } +} + +// Major-units amount for the dues email (amount_paid for renewals, +// amount_due for failed-payment notices). +export function minorToMajor(amount: number | null | undefined): number { + return (amount ?? 0) / 100 +}