Merge pull request 'refactor: Clean up subscription admin override and Stripe integration (#205)' (#218) from issue-205-clean-subscription-admin-override into main

Reviewed-on: #218

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({

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<Subscription> {
+  async create(data: CreateSubscriptionRequest & { stripeCustomerId?: string | null }): Promise<Subscription> {
     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<Subscription> {
     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,

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.
+   * Create or return existing Stripe customer for a subscription.
-   * When an admin overrides a user's tier without Stripe, a placeholder ID is stored.
+   * Admin-set subscriptions have NULL stripeCustomerId. On first Stripe payment,
-   * This method creates a real Stripe customer and updates the subscription record.
+   * 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<string> {
-    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);
-
+    try {
-    await this.repository.update(subscription.id, {
+      await this.repository.update(subscription.id, { stripeCustomerId: stripeCustomer.id });
-      stripeCustomerId: stripeCustomer.id,
+      logger.info('Created Stripe customer for subscription', {
-    });
-
-    logger.info('Stripe customer created for admin-overridden 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
+      // Ensure Stripe customer exists (creates one for admin-set subscriptions)
-      const stripeCustomerId = await this.resolveStripeCustomerId(currentSubscription, email);
+      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<void> {
-    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
-    }
   }
 
   /**
@@ -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<any[]> {
     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);

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;

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<void> {
+    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
    */

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_%';

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;