diff --git a/backend/src/features/subscriptions/api/subscriptions.controller.ts b/backend/src/features/subscriptions/api/subscriptions.controller.ts index e4c6f70..5702bea 100644 --- a/backend/src/features/subscriptions/api/subscriptions.controller.ts +++ b/backend/src/features/subscriptions/api/subscriptions.controller.ts @@ -220,7 +220,7 @@ export class SubscriptionsController { return; } - // Update payment method via service (handles admin_override_ customer IDs) + // Update payment method via service (creates Stripe customer if needed) await this.service.updatePaymentMethod(userId, paymentMethodId, email); reply.status(200).send({ diff --git a/backend/src/features/subscriptions/data/subscriptions.repository.ts b/backend/src/features/subscriptions/data/subscriptions.repository.ts index 23e83dd..49793f8 100644 --- a/backend/src/features/subscriptions/data/subscriptions.repository.ts +++ b/backend/src/features/subscriptions/data/subscriptions.repository.ts @@ -27,7 +27,7 @@ export class SubscriptionsRepository { /** * Create a new subscription */ - async create(data: CreateSubscriptionRequest & { stripeCustomerId: string }): Promise { + async create(data: CreateSubscriptionRequest & { stripeCustomerId?: string | null }): Promise { const query = ` INSERT INTO subscriptions ( user_id, stripe_customer_id, tier, billing_cycle @@ -38,7 +38,7 @@ export class SubscriptionsRepository { const values = [ data.userId, - data.stripeCustomerId, + data.stripeCustomerId ?? null, data.tier, data.billingCycle, ]; @@ -579,18 +579,16 @@ export class SubscriptionsRepository { client?: any ): Promise { const queryClient = client || this.pool; - // Generate a placeholder Stripe customer ID since admin override bypasses Stripe - const placeholderCustomerId = `admin_override_${userId}_${Date.now()}`; const query = ` INSERT INTO subscriptions ( user_id, stripe_customer_id, tier, billing_cycle, status ) - VALUES ($1, $2, $3, 'monthly', 'active') + VALUES ($1, NULL, $2, 'monthly', 'active') RETURNING * `; - const values = [userId, placeholderCustomerId, tier]; + const values = [userId, tier]; try { const result = await queryClient.query(query, values); @@ -623,7 +621,7 @@ export class SubscriptionsRepository { return { id: row.id, userId: row.user_id, - stripeCustomerId: row.stripe_customer_id, + stripeCustomerId: row.stripe_customer_id ?? null, stripeSubscriptionId: row.stripe_subscription_id || undefined, tier: row.tier, billingCycle: row.billing_cycle || undefined, diff --git a/backend/src/features/subscriptions/domain/subscriptions.service.ts b/backend/src/features/subscriptions/domain/subscriptions.service.ts index 9e5fffe..0a96f55 100644 --- a/backend/src/features/subscriptions/domain/subscriptions.service.ts +++ b/backend/src/features/subscriptions/domain/subscriptions.service.ts @@ -166,35 +166,42 @@ export class SubscriptionsService { } /** - * Resolve admin_override_ placeholder customer IDs to real Stripe customers. - * When an admin overrides a user's tier without Stripe, a placeholder ID is stored. - * This method creates a real Stripe customer and updates the subscription record. + * Create or return existing Stripe customer for a subscription. + * Admin-set subscriptions have NULL stripeCustomerId. On first Stripe payment, + * the customer is created in-place. Includes cleanup of orphaned Stripe customer + * if the DB update fails after customer creation. */ - private async resolveStripeCustomerId( + private async ensureStripeCustomer( subscription: Subscription, email: string ): Promise { - if (!subscription.stripeCustomerId.startsWith('admin_override_')) { + if (subscription.stripeCustomerId) { return subscription.stripeCustomerId; } - logger.info('Replacing admin_override_ placeholder with real Stripe customer', { - subscriptionId: subscription.id, - userId: subscription.userId, - }); - const stripeCustomer = await this.stripeClient.createCustomer(email); - - await this.repository.update(subscription.id, { - stripeCustomerId: stripeCustomer.id, - }); - - logger.info('Stripe customer created for admin-overridden subscription', { - subscriptionId: subscription.id, - stripeCustomerId: stripeCustomer.id, - }); - - return stripeCustomer.id; + try { + await this.repository.update(subscription.id, { stripeCustomerId: stripeCustomer.id }); + logger.info('Created Stripe customer for subscription', { + subscriptionId: subscription.id, + stripeCustomerId: stripeCustomer.id, + }); + return stripeCustomer.id; + } catch (error) { + // Attempt cleanup of orphaned Stripe customer + try { + await this.stripeClient.deleteCustomer(stripeCustomer.id); + logger.warn('Rolled back orphaned Stripe customer after DB update failure', { + stripeCustomerId: stripeCustomer.id, + }); + } catch (cleanupError: any) { + logger.error('Failed to cleanup orphaned Stripe customer', { + stripeCustomerId: stripeCustomer.id, + cleanupError: cleanupError.message, + }); + } + throw error; + } } /** @@ -216,8 +223,8 @@ export class SubscriptionsService { throw new Error('No subscription found for user'); } - // Resolve admin_override_ placeholder to real Stripe customer if needed - const stripeCustomerId = await this.resolveStripeCustomerId(currentSubscription, email); + // Ensure Stripe customer exists (creates one for admin-set subscriptions) + const stripeCustomerId = await this.ensureStripeCustomer(currentSubscription, email); // Determine price ID from environment variables const priceId = this.getPriceId(newTier, billingCycle); @@ -292,6 +299,10 @@ export class SubscriptionsService { throw new Error('No subscription found for user'); } + if (!currentSubscription.stripeCustomerId) { + throw new Error('Cannot cancel subscription without active Stripe billing'); + } + if (!currentSubscription.stripeSubscriptionId) { throw new Error('No active Stripe subscription to cancel'); } @@ -339,6 +350,10 @@ export class SubscriptionsService { throw new Error('No subscription found for user'); } + if (!currentSubscription.stripeCustomerId) { + throw new Error('Cannot reactivate subscription without active Stripe billing'); + } + if (!currentSubscription.stripeSubscriptionId) { throw new Error('No active Stripe subscription to reactivate'); } @@ -802,17 +817,8 @@ export class SubscriptionsService { * Sync subscription tier to user_profiles table */ private async syncTierToUserProfile(userId: string, tier: SubscriptionTier): Promise { - try { - await this.userProfileRepository.updateSubscriptionTier(userId, tier); - logger.info('Subscription tier synced to user profile', { userId, tier }); - } catch (error: any) { - logger.error('Failed to sync tier to user profile', { - userId, - tier, - error: error.message, - }); - // Don't throw - we don't want to fail the subscription operation if sync fails - } + await this.userProfileRepository.updateSubscriptionTier(userId, tier); + logger.info('Subscription tier synced to user profile', { userId, tier }); } /** @@ -968,7 +974,7 @@ export class SubscriptionsService { throw new Error('No subscription found for user'); } - const stripeCustomerId = await this.resolveStripeCustomerId(subscription, email); + const stripeCustomerId = await this.ensureStripeCustomer(subscription, email); await this.stripeClient.updatePaymentMethod(stripeCustomerId, paymentMethodId); } @@ -978,7 +984,7 @@ export class SubscriptionsService { async getInvoices(userId: string): Promise { try { const subscription = await this.repository.findByUserId(userId); - if (!subscription?.stripeCustomerId || subscription.stripeCustomerId.startsWith('admin_override_')) { + if (!subscription?.stripeCustomerId) { return []; } return this.stripeClient.listInvoices(subscription.stripeCustomerId); diff --git a/backend/src/features/subscriptions/domain/subscriptions.types.ts b/backend/src/features/subscriptions/domain/subscriptions.types.ts index 7c34020..298fe9b 100644 --- a/backend/src/features/subscriptions/domain/subscriptions.types.ts +++ b/backend/src/features/subscriptions/domain/subscriptions.types.ts @@ -19,7 +19,7 @@ export type DonationStatus = 'pending' | 'succeeded' | 'failed' | 'canceled'; export interface Subscription { id: string; userId: string; - stripeCustomerId: string; + stripeCustomerId: string | null; stripeSubscriptionId?: string; tier: SubscriptionTier; billingCycle?: BillingCycle; @@ -74,7 +74,7 @@ export interface CreateSubscriptionRequest { export interface SubscriptionResponse { id: string; userId: string; - stripeCustomerId: string; + stripeCustomerId: string | null; stripeSubscriptionId?: string; tier: SubscriptionTier; billingCycle?: BillingCycle; @@ -118,7 +118,7 @@ export interface CreateTierVehicleSelectionRequest { // Service layer types export interface UpdateSubscriptionData { - stripeCustomerId?: string; + stripeCustomerId?: string | null; stripeSubscriptionId?: string; tier?: SubscriptionTier; billingCycle?: BillingCycle; diff --git a/backend/src/features/subscriptions/external/stripe/stripe.client.ts b/backend/src/features/subscriptions/external/stripe/stripe.client.ts index 597b8bb..cf1050f 100644 --- a/backend/src/features/subscriptions/external/stripe/stripe.client.ts +++ b/backend/src/features/subscriptions/external/stripe/stripe.client.ts @@ -260,6 +260,24 @@ export class StripeClient { } } + /** + * Delete a Stripe customer (used for cleanup of orphaned customers) + */ + async deleteCustomer(customerId: string): Promise { + try { + logger.info('Deleting Stripe customer', { customerId }); + await this.stripe.customers.del(customerId); + logger.info('Stripe customer deleted', { customerId }); + } catch (error: any) { + logger.error('Failed to delete Stripe customer', { + customerId, + error: error.message, + code: error.code, + }); + throw error; + } + } + /** * Retrieve a subscription by ID */ diff --git a/backend/src/features/subscriptions/migrations/002_nullable_stripe_customer_id.sql b/backend/src/features/subscriptions/migrations/002_nullable_stripe_customer_id.sql new file mode 100644 index 0000000..86dd540 --- /dev/null +++ b/backend/src/features/subscriptions/migrations/002_nullable_stripe_customer_id.sql @@ -0,0 +1,11 @@ +-- Migration: Make stripe_customer_id NULLABLE +-- Removes the NOT NULL constraint that forced admin_override_ placeholder values. +-- Admin-set subscriptions (no Stripe billing) use NULL instead of sentinel strings. +-- PostgreSQL UNIQUE constraint allows multiple NULLs (SQL standard). + +-- Drop NOT NULL constraint on stripe_customer_id +ALTER TABLE subscriptions ALTER COLUMN stripe_customer_id DROP NOT NULL; + +-- Clean up existing admin_override_ placeholder values to NULL +UPDATE subscriptions SET stripe_customer_id = NULL +WHERE stripe_customer_id LIKE 'admin_override_%'; diff --git a/frontend/src/features/subscription/types/subscription.types.ts b/frontend/src/features/subscription/types/subscription.types.ts index f8a6ac4..045063e 100644 --- a/frontend/src/features/subscription/types/subscription.types.ts +++ b/frontend/src/features/subscription/types/subscription.types.ts @@ -5,7 +5,7 @@ export type SubscriptionStatus = 'active' | 'past_due' | 'canceled' | 'unpaid'; export interface Subscription { id: string; userId: string; - stripeCustomerId: string; + stripeCustomerId: string | null; stripeSubscriptionId?: string; tier: SubscriptionTier; billingCycle?: BillingCycle;