fix(payments): prevent paid-but-not-activated on Stripe webhook retry
handleCheckoutCompleted marked the payment 'completed' BEFORE activating the subscription, but the idempotency guard keys off payment.status === 'completed'. So if activateSubscription threw after the payment was marked completed, the Stripe webhook retry would hit the guard, skip activation, and leave a paid-but-never-activated subscription. Fix: activate FIRST, then mark completed — a mid-activation failure leaves the payment 'pending' so the retry re-runs cleanly. Guard activateSubscription with an idempotency check (return early when already active) so a retry after a successful activate but failed mark-completed cannot re-fire revenue events (PostHog subscription_activated + Meta CAPI Purchase), re-consume referral credit, or double-insert Full-plan brands. Admin manual-activate already pre-rejects 'active', so legitimate first activations are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -261,6 +261,14 @@ export class StripeService {
|
||||
const paymentIntentId =
|
||||
typeof session.payment_intent === "string" ? session.payment_intent : null;
|
||||
|
||||
// Activate FIRST, then mark the payment completed. If activation throws, the
|
||||
// payment stays 'pending', so a Stripe webhook retry re-runs this handler
|
||||
// cleanly instead of hitting the completed-guard above and skipping
|
||||
// activation forever — which would leave a paid-but-never-activated
|
||||
// subscription. activateSubscription is idempotent, so a retry after a
|
||||
// successful activate but failed mark-completed won't double-count revenue.
|
||||
const activated = await this.subscriptionsService.activateSubscription(payment.subscriptionId);
|
||||
|
||||
await this.db
|
||||
.update(payments)
|
||||
.set({
|
||||
@@ -270,8 +278,6 @@ export class StripeService {
|
||||
})
|
||||
.where(eq(payments.id, paymentId));
|
||||
|
||||
const activated = await this.subscriptionsService.activateSubscription(payment.subscriptionId);
|
||||
|
||||
this.posthog.captureForUser(payment.userId, "payment_success", {
|
||||
method: "stripe",
|
||||
payment_id: paymentId,
|
||||
|
||||
@@ -288,4 +288,43 @@ describe("SubscriptionsService", () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("activateSubscription", () => {
|
||||
it("is idempotent: skips re-activation when already active (no revenue double-count)", async () => {
|
||||
// A Stripe webhook retry must not re-fire revenue events, re-consume
|
||||
// referral credit, or re-insert brands on an already-active subscription.
|
||||
const activeSub = {
|
||||
id: "sub-1",
|
||||
userId: "user-1",
|
||||
planId: "plan-1",
|
||||
status: "active",
|
||||
billingPeriod: "monthly",
|
||||
endDate: new Date(),
|
||||
};
|
||||
const db = {
|
||||
select: vi.fn().mockReturnValue({
|
||||
from: vi.fn().mockReturnThis(),
|
||||
where: vi.fn().mockReturnThis(),
|
||||
limit: vi.fn().mockReturnValue([activeSub]),
|
||||
}),
|
||||
update: vi.fn(),
|
||||
insert: vi.fn(),
|
||||
};
|
||||
const posthog = {
|
||||
captureForUser: vi.fn(),
|
||||
capture: vi.fn(),
|
||||
flush: vi.fn().mockResolvedValue(undefined),
|
||||
};
|
||||
const metaCapi = { sendPurchase: vi.fn().mockResolvedValue(undefined) };
|
||||
const service = new SubscriptionsService(db as any, posthog as any, metaCapi as any);
|
||||
|
||||
const result = await service.activateSubscription("sub-1");
|
||||
|
||||
expect(result).toBe(activeSub);
|
||||
expect(db.update).not.toHaveBeenCalled();
|
||||
expect(db.insert).not.toHaveBeenCalled();
|
||||
expect(posthog.captureForUser).not.toHaveBeenCalled();
|
||||
expect(metaCapi.sendPurchase).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -3,6 +3,7 @@ import {
|
||||
ConflictException,
|
||||
Inject,
|
||||
Injectable,
|
||||
Logger,
|
||||
NotFoundException,
|
||||
} from "@nestjs/common";
|
||||
import { and, desc, eq, inArray, or } from "drizzle-orm";
|
||||
@@ -20,6 +21,8 @@ import { PostHogService } from "../posthog/posthog.service";
|
||||
|
||||
@Injectable()
|
||||
export class SubscriptionsService {
|
||||
private readonly logger = new Logger(SubscriptionsService.name);
|
||||
|
||||
constructor(
|
||||
@Inject(DATABASE) private db: Database,
|
||||
private posthog: PostHogService,
|
||||
@@ -123,6 +126,17 @@ export class SubscriptionsService {
|
||||
|
||||
if (!sub) throw new NotFoundException("Abonelik bulunamadı");
|
||||
|
||||
// Idempotency guard. A Stripe webhook retry (or any double-delivery) must
|
||||
// not re-activate an already-active subscription: that would reset the
|
||||
// period from now, re-consume referral credit, double-insert Full-plan
|
||||
// brands, and double-count revenue (PostHog subscription_activated + Meta
|
||||
// CAPI Purchase). Legitimate first activations always run on a 'pending'
|
||||
// sub; admin manual activation pre-rejects 'active' before reaching here.
|
||||
if (sub.status === "active") {
|
||||
this.logger.warn(`activateSubscription: ${subscriptionId} already active — skipping`);
|
||||
return sub;
|
||||
}
|
||||
|
||||
const now = new Date();
|
||||
const endDate = new Date(now);
|
||||
if (sub.billingPeriod === "yearly") {
|
||||
|
||||
Reference in New Issue
Block a user