fix: address code review findings for billing feature (#142)

Kikolator · claude · web-flow · commit 6e62bbe6a3c8 · 2026-04-01T10:06:11.000Z
## Summary Review fixes for #141 that were pushed after the PR was merged. - Fix comma-operator bug in Stripe product ID resolution (could fail silently) - Paginate cron query with batches of 500 for multi-tenant scale - Warn admin when custom price change doesn't sync to Stripe subscription - Surface warnings in add/edit member forms instead of swallowing them - Use plan currency instead of hardcoded EUR in member detail - Use Postgres enum type for `billing_mode` instead of text+CHECK - Calculate credit validity as next anniversary date, not fixed 30 days - Add TODO comments for post-migration type regen ## Test plan - [x] 351 unit tests pass, type-check clean - [ ] Verify Stripe product ID resolution works for plans without existing Stripe product - [ ] Verify cron processes members in batches correctly - [ ] Verify warning appears when changing custom price on Stripe-billed member 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
diff --git a/apps/web/app/(app)/admin/members/actions.ts b/apps/web/app/(app)/admin/members/actions.ts
@@ -46,8 +46,27 @@ export async function updateMember(memberId: string, input: unknown) {
   }
 
   const { supabase, spaceId } = await getSpaceId();
+  const admin = createAdminClient();
   const d = parsed.data;
 
+  // Check if custom price changed on a Stripe-billed member
+  // TODO: remove unknown casts after running `supabase gen types` with billing_mode migration
+  const { data: currentRaw } = await admin
+    .from("members")
+    .select("custom_price_cents")
+    .eq("id", memberId)
+    .eq("space_id", spaceId)
+    .single();
+  const current = currentRaw as unknown as {
+    billing_mode?: string;
+    custom_price_cents: number | null;
+  } | null;
+
+  const priceChanged =
+    current &&
+    current.billing_mode === "stripe" &&
+    d.customPriceCents !== current.custom_price_cents;
+
   const { error } = await supabase
     .from("members")
     .update({
@@ -81,6 +100,14 @@ export async function updateMember(memberId: string, input: unknown) {
   }
 
   revalidatePath("/admin/members");
+
+  if (priceChanged) {
+    return {
+      success: true as const,
+      warning: "Custom price updated in the database. The active Stripe subscription was not changed — update it manually in the Stripe dashboard if needed.",
+    };
+  }
+
   return { success: true as const };
 }
 
@@ -280,10 +307,17 @@ export async function addMember(input: unknown) {
 
       let priceId: string;
       if (d.customPriceCents != null && d.customPriceCents !== plan.price_cents) {
-        // Create a custom Stripe price for this member
-        const productId = plan.stripe_product_id
-          ?? (await ensureStripePriceExists(plan, stripeAccountId, spaceId),
-             (await admin.from("plans").select("stripe_product_id").eq("id", plan.id).single()).data?.stripe_product_id);
+        // Ensure Stripe product exists, then resolve the product ID
+        let productId = plan.stripe_product_id;
+        if (!productId) {
+          await ensureStripePriceExists(plan, stripeAccountId, spaceId);
+          const { data: refreshedPlan } = await admin
+            .from("plans")
+            .select("stripe_product_id")
+            .eq("id", plan.id)
+            .single();
+          productId = refreshedPlan?.stripe_product_id ?? null;
+        }
 
         if (!productId) throw new Error("Could not resolve Stripe product");
 
@@ -341,8 +375,9 @@ export async function addMember(input: unknown) {
   } else {
     // Manual billing — grant initial credits immediately
     try {
-      const validUntil = new Date();
-      validUntil.setDate(validUntil.getDate() + 30);
+      // Credits valid until same day next month
+      const now = new Date();
+      const validUntil = new Date(now.getFullYear(), now.getMonth() + 1, now.getDate());
 
       await grantMonthlyCredits({
         spaceId,
diff --git a/apps/web/app/(app)/admin/members/add-member-form.tsx b/apps/web/app/(app)/admin/members/add-member-form.tsx
@@ -69,6 +69,13 @@ export function AddMemberForm({ open, onOpenChange, plans }: AddMemberFormProps)
       const result = await addMember(data);
       if (!result.success) {
         setServerError(result.error);
+      } else if ("warning" in result && result.warning) {
+        // Show warning but still close — member was created
+        setServerError(result.warning);
+        setTimeout(() => {
+          reset();
+          onOpenChange(false);
+        }, 4000);
       } else {
         reset();
         onOpenChange(false);
diff --git a/apps/web/app/(app)/admin/members/member-detail.tsx b/apps/web/app/(app)/admin/members/member-detail.tsx
@@ -66,6 +66,7 @@ interface MemberDetailProps {
   member: Member;
   profile: ProfileEntry;
   planName: string;
+  planCurrency: string;
   deskName: string | null;
   notes: MemberNote[];
   profileMap: Record<string, ProfileEntry>;
@@ -117,6 +118,7 @@ export function MemberDetail({
   member,
   profile,
   planName,
+  planCurrency,
   deskName,
   notes,
   profileMap,
@@ -196,7 +198,7 @@ export function MemberDetail({
             {member.custom_price_cents != null && (
               <DetailRow
                 label="Custom price"
-                value={`${(member.custom_price_cents / 100).toFixed(2)} EUR`}
+                value={`${(member.custom_price_cents / 100).toFixed(2)} ${planCurrency.toUpperCase()}`}
               />
             )}
             <DetailRow
diff --git a/apps/web/app/(app)/admin/members/member-form.tsx b/apps/web/app/(app)/admin/members/member-form.tsx
@@ -148,6 +148,9 @@ export function MemberForm({ open, onOpenChange, member, plans, desks, deskAssig
       const result = await updateMember(member.id, data);
       if (!result.success) {
         setServerError(result.error);
+      } else if ("warning" in result && result.warning) {
+        setServerError(result.warning);
+        setTimeout(() => onOpenChange(false), 4000);
       } else {
         onOpenChange(false);
       }
diff --git a/apps/web/app/(app)/admin/members/members-table.tsx b/apps/web/app/(app)/admin/members/members-table.tsx
@@ -637,6 +637,7 @@ export function MembersTable({
           member={detailMember}
           profile={detailProfile}
           planName={detailMember.plan?.name ?? "Unknown"}
+          planCurrency={plans.find((p) => p.id === detailMember.plan_id)?.currency ?? "eur"}
           deskName={detailDeskName}
           notes={detailNotes}
           profileMap={profileMap}
diff --git a/apps/web/app/(app)/admin/members/page.tsx b/apps/web/app/(app)/admin/members/page.tsx
@@ -57,6 +57,7 @@ export default async function MembersPage() {
 
   return (
     <MembersTable
+      // TODO: remove casts after running `supabase gen types` with the billing_mode migration
       members={(members ?? []).map((m) => ({
         ...m,
         billing_mode: (m as Record<string, unknown>).billing_mode as string | null ?? null,
diff --git a/apps/web/app/api/cron/renew-credits/route.ts b/apps/web/app/api/cron/renew-credits/route.ts
@@ -3,6 +3,19 @@ import { createLogger } from "@cowork/shared";
 import { createAdminClient } from "@/lib/supabase/admin";
 import { grantMonthlyCredits, expireRenewableCredits } from "@/lib/credits/grant";
 
+const BATCH_SIZE = 500;
+
+/** Calculate the next anniversary date from a join date. */
+function getNextAnniversary(joinedAt: Date, today: Date): Date {
+  const nextMonth = today.getMonth() + 1;
+  const nextYear = today.getFullYear() + (nextMonth > 11 ? 1 : 0);
+  const joinDay = joinedAt.getDate();
+  // Clamp to last day of next month if join day exceeds it
+  const lastDayOfNextMonth = new Date(nextYear, (nextMonth % 12) + 1, 0).getDate();
+  const day = Math.min(joinDay, lastDayOfNextMonth);
+  return new Date(nextYear, nextMonth % 12, day);
+}
+
 /**
  * Daily cron: renew credits for manual-billing members on their monthly anniversary.
  * Runs every day at midnight UTC. Configured in vercel.json.
@@ -19,7 +32,6 @@ export async function GET(request: Request) {
   const today = new Date();
   const dayOfMonth = today.getDate();
 
-  // Find manual-billing active members whose joined_at day matches today.
   // For months shorter than the join day (e.g., joined on 31st, current month
   // has 28 days), we renew on the last day of the month.
   const lastDayOfMonth = new Date(
@@ -29,65 +41,70 @@ export async function GET(request: Request) {
   ).getDate();
   const isLastDay = dayOfMonth === lastDayOfMonth;
 
-  // Query members: day of month matches, OR it's the last day and their
-  // join day exceeds this month's length
-  const { data: members, error: queryError } = await admin
-    .from("members")
-    .select("id, user_id, plan_id, space_id, joined_at")
-    .eq("billing_mode", "manual")
-    .eq("status", "active");
-
-  if (queryError) {
-    logger.error("Failed to query manual members", { error: queryError.message });
-    return NextResponse.json({ error: "Query failed" }, { status: 500 });
-  }
-
-  if (!members || members.length === 0) {
-    return NextResponse.json({ processed: 0 });
-  }
-
   let processed = 0;
   let errors = 0;
+  let offset = 0;
+
+  // Paginated query to handle large member counts across all tenants
+  while (true) {
+    const { data: members, error: queryError } = await admin
+      .from("members")
+      .select("id, user_id, plan_id, space_id, joined_at")
+      .eq("billing_mode", "manual")
+      .eq("status", "active")
+      .order("id")
+      .range(offset, offset + BATCH_SIZE - 1);
+
+    if (queryError) {
+      logger.error("Failed to query manual members", { error: queryError.message, offset });
+      return NextResponse.json({ error: "Query failed" }, { status: 500 });
+    }
 
-  for (const member of members) {
-    if (!member.joined_at) continue;
-
-    const joinDay = new Date(member.joined_at).getDate();
-
-    // Check if today is this member's renewal day
-    const isDueToday =
-      joinDay === dayOfMonth ||
-      (isLastDay && joinDay > lastDayOfMonth);
-
-    if (!isDueToday) continue;
-
-    try {
-      // Expire previous month's credits
-      await expireRenewableCredits({
-        spaceId: member.space_id,
-        userId: member.user_id,
-      });
-
-      // Grant new credits
-      const validUntil = new Date();
-      validUntil.setDate(validUntil.getDate() + 30);
-
-      await grantMonthlyCredits({
-        spaceId: member.space_id,
-        userId: member.user_id,
-        planId: member.plan_id,
-        stripeInvoiceId: `manual_renewal_${member.id}_${today.toISOString().slice(0, 10)}`,
-        validUntil,
-      });
-
-      processed++;
-    } catch (err) {
-      errors++;
-      logger.error("Failed to renew credits for manual member", {
-        memberId: member.id,
-        error: err instanceof Error ? err.message : String(err),
-      });
+    if (!members || members.length === 0) break;
+
+    for (const member of members) {
+      if (!member.joined_at) continue;
+
+      const joinDate = new Date(member.joined_at);
+      const joinDay = joinDate.getDate();
+
+      // Check if today is this member's renewal day
+      const isDueToday =
+        joinDay === dayOfMonth ||
+        (isLastDay && joinDay > lastDayOfMonth);
+
+      if (!isDueToday) continue;
+
+      try {
+        // Expire previous month's credits
+        await expireRenewableCredits({
+          spaceId: member.space_id,
+          userId: member.user_id,
+        });
+
+        // Grant new credits valid until next anniversary
+        const validUntil = getNextAnniversary(joinDate, today);
+
+        await grantMonthlyCredits({
+          spaceId: member.space_id,
+          userId: member.user_id,
+          planId: member.plan_id,
+          stripeInvoiceId: `manual_renewal_${member.id}_${today.toISOString().slice(0, 10)}`,
+          validUntil,
+        });
+
+        processed++;
+      } catch (err) {
+        errors++;
+        logger.error("Failed to renew credits for manual member", {
+          memberId: member.id,
+          error: err instanceof Error ? err.message : String(err),
+        });
+      }
     }
+
+    if (members.length < BATCH_SIZE) break;
+    offset += BATCH_SIZE;
   }
 
   logger.info("Manual credit renewal completed", { processed, errors });
diff --git a/packages/db/supabase/migrations/20260331173515_add_billing_mode_custom_price.sql b/packages/db/supabase/migrations/20260331173515_add_billing_mode_custom_price.sql
@@ -5,12 +5,17 @@
 -- ============================================================================
 
 -- ==========================================================================
--- 1. Add columns
+-- 1. Create enum type
+-- ==========================================================================
+
+CREATE TYPE billing_mode AS ENUM ('stripe', 'manual');
+
+-- ==========================================================================
+-- 2. Add columns
 -- ==========================================================================
 
 ALTER TABLE members
-  ADD COLUMN billing_mode text NOT NULL DEFAULT 'stripe'
-    CHECK (billing_mode IN ('stripe', 'manual')),
+  ADD COLUMN billing_mode billing_mode NOT NULL DEFAULT 'stripe',
   ADD COLUMN custom_price_cents integer
     CHECK (custom_price_cents IS NULL OR custom_price_cents >= 0);
 
@@ -22,7 +27,8 @@ UPDATE members
     AND stripe_customer_id IS NULL;
 
 -- ==========================================================================
--- 2. Rollback
+-- 3. Rollback
 -- ==========================================================================
 -- ALTER TABLE members DROP COLUMN custom_price_cents;
 -- ALTER TABLE members DROP COLUMN billing_mode;
+-- DROP TYPE IF EXISTS billing_mode;
