Skip to content

Commit beb273d

Browse files
committed
refactor: reorg free plan regrant to be less racy
1 parent a4695e8 commit beb273d

5 files changed

Lines changed: 286 additions & 45 deletions

File tree

apps/backend/src/app/api/latest/payments/products/[customer_type]/[customer_id]/route.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
import { ensureClientCanAccessCustomer, ensureCustomerExists, ensureProductIdOrInlineProduct, grantProductToCustomer, isActiveSubscription, productToInlineProduct } from "@/lib/payments";
1+
import { ensureClientCanAccessCustomer, ensureCustomerExists, ensureProductIdOrInlineProduct, grantProductToCustomer, isActiveSubscription, isAddOnProduct, productToInlineProduct } from "@/lib/payments";
22
import { getOwnedProductsForCustomer, getSubscriptionMapForCustomer } from "@/lib/payments/customer-data";
33
import { getPrismaClientForTenancy } from "@/prisma-client";
44
import { createSmartRouteHandler } from "@/route-handlers/smart-route-handler";
55
import { KnownErrors } from "@stackframe/stack-shared";
66
import { customerProductsListResponseSchema } from "@stackframe/stack-shared/dist/interface/crud/products";
77
import { adaptSchema, clientOrHigherAuthTypeSchema, inlineProductSchema, serverOrHigherAuthTypeSchema, yupBoolean, yupNumber, yupObject, yupString } from "@stackframe/stack-shared/dist/schema-fields";
88
import { StatusError } from "@stackframe/stack-shared/dist/utils/errors";
9-
import { typedEntries, typedFromEntries, typedKeys } from "@stackframe/stack-shared/dist/utils/objects";
9+
import { typedEntries, typedFromEntries } from "@stackframe/stack-shared/dist/utils/objects";
1010
import { stringCompare } from "@stackframe/stack-shared/dist/utils/strings";
1111

1212
export const GET = createSmartRouteHandler({
@@ -82,7 +82,7 @@ export const GET = createSmartRouteHandler({
8282
if (product.prices === "include-by-default") continue;
8383
const hasIntervalPrice = typedEntries(product.prices).some(([, price]) => price.interval);
8484
if (!hasIntervalPrice) continue;
85-
if (product.isAddOnTo && typedKeys(product.isAddOnTo).length > 0) continue;
85+
if (isAddOnProduct(product)) continue;
8686

8787
const inlineProduct = productToInlineProduct(product);
8888
const intervalPrices = typedFromEntries(

apps/backend/src/app/api/latest/payments/products/[customer_type]/[customer_id]/switch/route.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { SubscriptionStatus } from "@/generated/prisma/client";
2-
import { ensureClientCanAccessCustomer, ensureCustomerExists, getDefaultCardPaymentMethodSummary, getStripeCustomerForCustomerOrNull, isActiveSubscription } from "@/lib/payments";
2+
import { ensureClientCanAccessCustomer, ensureCustomerExists, getDefaultCardPaymentMethodSummary, getStripeCustomerForCustomerOrNull, isActiveSubscription, isAddOnProduct } from "@/lib/payments";
33
import { bulldozerWriteSubscription } from "@/lib/payments/bulldozer-dual-write";
44
import { getOwnedProductsForCustomer, getSubscriptionMapForCustomer } from "@/lib/payments/customer-data";
55
import { upsertProductVersion } from "@/lib/product-versions";
@@ -9,7 +9,7 @@ import { createSmartRouteHandler } from "@/route-handlers/smart-route-handler";
99
import { KnownErrors } from "@stackframe/stack-shared";
1010
import { adaptSchema, clientOrHigherAuthTypeSchema, yupBoolean, yupNumber, yupObject, yupString } from "@stackframe/stack-shared/dist/schema-fields";
1111
import { StackAssertionError, StatusError } from "@stackframe/stack-shared/dist/utils/errors";
12-
import { getOrUndefined, typedEntries, typedKeys } from "@stackframe/stack-shared/dist/utils/objects";
12+
import { getOrUndefined, typedEntries } from "@stackframe/stack-shared/dist/utils/objects";
1313
import { typedToUppercase } from "@stackframe/stack-shared/dist/utils/strings";
1414
import Stripe from "stripe";
1515

@@ -72,7 +72,7 @@ export const POST = createSmartRouteHandler({
7272
if (body.from_product_id === body.to_product_id) {
7373
throw new StatusError(400, "Product is already active.");
7474
}
75-
if (toProduct.isAddOnTo && typedKeys(toProduct.isAddOnTo).length > 0) {
75+
if (isAddOnProduct(toProduct)) {
7676
throw new StatusError(400, "Add-on products cannot be selected for plan switching.");
7777
}
7878
const fromIsIncludeByDefault = fromProduct.prices === "include-by-default";

apps/backend/src/lib/payments.tsx

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,19 @@ export function isActiveSubscription(subscription: { status: string }): boolean
121121
return s === "active" || s === SubscriptionStatus.active || s === "trialing" || s === SubscriptionStatus.trialing;
122122
}
123123

124+
/**
125+
* True when the given product config / snapshot declares itself as an add-on
126+
* to one or more other products. Add-ons share a product line with their base
127+
* plan but don't satisfy "base plan owned" invariants on their own.
128+
*
129+
* The predicate normalises the three ways a product can signal "not an
130+
* add-on" (absent, explicitly `false`, or an empty record) so callers don't
131+
* have to reimplement the check.
132+
*/
133+
export function isAddOnProduct(product: { isAddOnTo?: false | Record<string, true> | null }): boolean {
134+
return product.isAddOnTo != null && product.isAddOnTo !== false && Object.keys(product.isAddOnTo).length > 0;
135+
}
136+
124137
type OwnedProducts = OwnedProductsRow["ownedProducts"];
125138

126139
/**
Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
import { randomUUID } from "node:crypto";
2+
import { describe, expect, it } from "vitest";
3+
import { getOrUndefined } from "@stackframe/stack-shared/dist/utils/objects";
4+
import { bulldozerWriteSubscription } from "@/lib/payments/bulldozer-dual-write";
5+
import { getSubscriptionMapForCustomer } from "@/lib/payments/customer-data";
6+
// eslint-disable-next-line @typescript-eslint/no-deprecated -- idiomatic way to get the internal tenancy today (see plan-entitlements.ts)
7+
import { DEFAULT_BRANCH_ID, getSoleTenancyFromProjectBranch } from "@/lib/tenancies";
8+
import { getPrismaClientForTenancy } from "@/prisma-client";
9+
import { ensureFreePlanForBillingTeam } from "./ensure-free-plan";
10+
11+
// Uses the real internal tenancy (relies on its seeded free/team/growth/
12+
// extra-seats product config) and random UUIDs as billing team IDs.
13+
// Subscription rows aren't FK-checked against the Team table, so inserting
14+
// a sub for a non-existent team works and keeps tests side-effect-free on
15+
// real teams.
16+
describe.sequential("ensureFreePlanForBillingTeam (real DB)", () => {
17+
async function getInternal() {
18+
const tenancy = await getSoleTenancyFromProjectBranch("internal", DEFAULT_BRANCH_ID, true);
19+
if (tenancy == null) throw new Error("Internal billing tenancy not found");
20+
const prisma = await getPrismaClientForTenancy(tenancy);
21+
return { tenancy, prisma };
22+
}
23+
24+
// Returns subs that haven't ended yet — matches the "occupies the product
25+
// line" semantics of `ensureFreePlanForBillingTeam`'s predicate, which is
26+
// endedAt-based (not status-based) to mirror the Subscription TimeFold.
27+
async function getUnendedSubsForTeam(tenancyId: string, billingTeamId: string, prisma: unknown) {
28+
const subMap = await getSubscriptionMapForCustomer({
29+
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- mirrors `payments.test.tsx`: PrismaClient is structurally compatible with PrismaClientTransaction here
30+
prisma: prisma as any,
31+
tenancyId,
32+
customerType: "team",
33+
customerId: billingTeamId,
34+
});
35+
const nowMillis = Date.now();
36+
return Object.values(subMap).filter((s) => s.endedAtMillis == null || s.endedAtMillis > nowMillis);
37+
}
38+
39+
async function seedSub(options: {
40+
tenancyId: string,
41+
billingTeamId: string,
42+
productId: string,
43+
productSnapshot: unknown,
44+
status?: "active" | "trialing" | "incomplete" | "past_due",
45+
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- see `getUnendedSubsForTeam`
46+
prisma: any,
47+
}) {
48+
const now = new Date();
49+
await bulldozerWriteSubscription(options.prisma, {
50+
id: randomUUID(),
51+
tenancyId: options.tenancyId,
52+
customerId: options.billingTeamId,
53+
customerType: "TEAM",
54+
productId: options.productId,
55+
priceId: null,
56+
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- ProductSnapshot is a structural JSON type; bulldozerWriteSubscription will stamp it into the stored row as-is.
57+
product: options.productSnapshot as any,
58+
quantity: 1,
59+
stripeSubscriptionId: `stripe-${randomUUID()}`,
60+
status: options.status ?? "active",
61+
currentPeriodStart: now,
62+
currentPeriodEnd: new Date(now.getTime() + 30 * 24 * 3600 * 1000),
63+
cancelAtPeriodEnd: false,
64+
canceledAt: null,
65+
endedAt: null,
66+
refundedAt: null,
67+
creationSource: "PURCHASE_PAGE",
68+
createdAt: now,
69+
});
70+
}
71+
72+
it("fast path: no-op when team already owns an active base plan in the line", async () => {
73+
const { tenancy, prisma } = await getInternal();
74+
const billingTeamId = randomUUID();
75+
76+
const teamProduct = getOrUndefined(tenancy.config.payments.products, "team");
77+
if (teamProduct == null) throw new Error("Internal tenancy missing `team` product");
78+
79+
await seedSub({
80+
tenancyId: tenancy.id,
81+
billingTeamId,
82+
productId: "team",
83+
productSnapshot: teamProduct,
84+
prisma,
85+
});
86+
87+
await ensureFreePlanForBillingTeam(billingTeamId);
88+
89+
const subs = await getUnendedSubsForTeam(tenancy.id, billingTeamId, prisma);
90+
expect(subs).toHaveLength(1);
91+
expect(subs[0].productId).toBe("team");
92+
});
93+
94+
it("regression: an `incomplete` paid sub still occupies the line — no free regrant", async () => {
95+
// Reproduces the Stripe webhook race the endedAt-based predicate
96+
// defends against: `subscription.created` lands first with
97+
// `status=incomplete` and no `endedAt`; the subsequent `invoice.paid`
98+
// flips it to `active`. Between those two webhooks, `ensureFree...`
99+
// must treat the incomplete sub as occupying the line — gating on
100+
// `status` alone would regrant free on top and leave the customer
101+
// with both subs active (exactly the chauncey-team dashboard bug).
102+
const { tenancy, prisma } = await getInternal();
103+
const billingTeamId = randomUUID();
104+
105+
const teamProduct = getOrUndefined(tenancy.config.payments.products, "team");
106+
if (teamProduct == null) throw new Error("Internal tenancy missing `team` product");
107+
108+
await seedSub({
109+
tenancyId: tenancy.id,
110+
billingTeamId,
111+
productId: "team",
112+
productSnapshot: teamProduct,
113+
status: "incomplete",
114+
prisma,
115+
});
116+
117+
await ensureFreePlanForBillingTeam(billingTeamId);
118+
119+
const subs = await getUnendedSubsForTeam(tenancy.id, billingTeamId, prisma);
120+
expect(subs).toHaveLength(1);
121+
expect(subs[0].productId).toBe("team");
122+
});
123+
124+
it("slow path: creates a free sub when team has no prior sub in the line", async () => {
125+
const { tenancy, prisma } = await getInternal();
126+
const billingTeamId = randomUUID();
127+
128+
await ensureFreePlanForBillingTeam(billingTeamId);
129+
130+
const subs = await getUnendedSubsForTeam(tenancy.id, billingTeamId, prisma);
131+
expect(subs).toHaveLength(1);
132+
expect(subs[0].productId).toBe("free");
133+
});
134+
135+
it("idempotent: sequential double-call creates exactly one free sub", async () => {
136+
const { tenancy, prisma } = await getInternal();
137+
const billingTeamId = randomUUID();
138+
139+
await ensureFreePlanForBillingTeam(billingTeamId);
140+
await ensureFreePlanForBillingTeam(billingTeamId);
141+
142+
const subs = await getUnendedSubsForTeam(tenancy.id, billingTeamId, prisma);
143+
expect(subs).toHaveLength(1);
144+
expect(subs[0].productId).toBe("free");
145+
});
146+
147+
it("slow path race: concurrent Promise.all calls create exactly one free sub", async () => {
148+
// Exercises the SERIALIZABLE slow path's retry-on-conflict behaviour —
149+
// both invocations enter the tx concurrently, one commits, the other
150+
// retries under a fresh snapshot, sees the committed row, and skips.
151+
const { tenancy, prisma } = await getInternal();
152+
const billingTeamId = randomUUID();
153+
154+
await Promise.all([
155+
ensureFreePlanForBillingTeam(billingTeamId),
156+
ensureFreePlanForBillingTeam(billingTeamId),
157+
]);
158+
159+
const subs = await getUnendedSubsForTeam(tenancy.id, billingTeamId, prisma);
160+
expect(subs).toHaveLength(1);
161+
expect(subs[0].productId).toBe("free");
162+
});
163+
164+
it("add-on does not count as a base plan — free is still regranted", async () => {
165+
const { tenancy, prisma } = await getInternal();
166+
const billingTeamId = randomUUID();
167+
168+
// `extra-seats` is an add-on (isAddOnTo: { team, growth }) but lives in
169+
// the same product line as the free plan. It must NOT short-circuit the
170+
// fast path; the team should still get a free sub on top.
171+
const extraSeatsProduct = getOrUndefined(tenancy.config.payments.products, "extra-seats");
172+
if (extraSeatsProduct == null) throw new Error("Internal tenancy missing `extra-seats` product");
173+
174+
await seedSub({
175+
tenancyId: tenancy.id,
176+
billingTeamId,
177+
productId: "extra-seats",
178+
productSnapshot: extraSeatsProduct,
179+
prisma,
180+
});
181+
182+
await ensureFreePlanForBillingTeam(billingTeamId);
183+
184+
const subs = await getUnendedSubsForTeam(tenancy.id, billingTeamId, prisma);
185+
const productIds = new Set(subs.map((s) => s.productId));
186+
expect(subs).toHaveLength(2);
187+
expect(productIds.has("free")).toBe(true);
188+
expect(productIds.has("extra-seats")).toBe(true);
189+
});
190+
});

0 commit comments

Comments
 (0)