Skip to content

Commit 761b780

Browse files
authored
fix(bookings): refund all seat payments when cancelling a paid seated… (calcom#29685)
* fix(bookings): refund all seat payments when cancelling a paid seated booking A paid seated booking creates one Payment row per seat on the same bookingId (each seat's payment_intent.succeeded webhook flips only its own payment via handlePaymentSuccess). processPaymentRefund only refunded payment.find((p) => p.success) — the first successful payment — so fully cancelling a multi-seat paid booking with an auto-refund policy refunded a single seat and left the other attendees charged with no error surfaced. Refund every successful, not-yet-refunded payment instead. Tests: unit regression in processPaymentRefund.test.ts (all payments refunded, verified red before the fix); integration test in handleCancelBooking.test.ts proving a full cancel of a seated multi-payment booking hands all payments to the refund path. * fix(bookings): make seat refunds resilient and drop dayjs in test Address CodeRabbit review on calcom#29685: - processPaymentRefund now attempts every seat's refund independently and throws an aggregate error, so one provider failure no longer skips the remaining seats. - Add a regression test for that resilience, and use native Date instead of dayjs for fixed-offset test timestamps.
1 parent 009f91d commit 761b780

3 files changed

Lines changed: 178 additions & 2 deletions

File tree

packages/features/bookings/lib/handleCancelBooking/test/handleCancelBooking.test.ts

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,111 @@ describe("Cancel Booking", () => {
289289
expect(processPaymentRefund).toHaveBeenCalled();
290290
});
291291

292+
test("Should pass all seat payments to processPaymentRefund when fully cancelling a paid seated booking", async () => {
293+
const handleCancelBooking = (await import("@calcom/features/bookings/lib/handleCancelBooking")).default;
294+
295+
const booker = getBooker({
296+
email: "booker@example.com",
297+
name: "Booker",
298+
});
299+
300+
const organizer = getOrganizer({
301+
name: "Organizer",
302+
email: "organizer@example.com",
303+
id: 101,
304+
schedules: [TestData.schedules.IstWorkHours],
305+
credentials: [getGoogleCalendarCredential()],
306+
selectedCalendars: [TestData.selectedCalendars.google],
307+
});
308+
309+
const uidOfBookingToBeCancelled = "seated-multi-pay-cancel";
310+
const idOfBookingToBeCancelled = 1099;
311+
const { dateString: plus1DateString } = getDate({ dateIncrement: 1 });
312+
const booking = {
313+
id: idOfBookingToBeCancelled,
314+
uid: uidOfBookingToBeCancelled,
315+
eventTypeId: 1,
316+
userId: 101,
317+
responses: {
318+
email: booker.email,
319+
name: booker.name,
320+
location: { optionValue: "", value: BookingLocations.CalVideo },
321+
},
322+
status: BookingStatus.ACCEPTED,
323+
startTime: `${plus1DateString}T05:00:00.000Z`,
324+
endTime: `${plus1DateString}T05:15:00.000Z`,
325+
metadata: {
326+
videoCallUrl: "https://existing-daily-video-call-url.example.com",
327+
},
328+
attendees: [{ timeZone: "Asia/Kolkata", email: booker.email }],
329+
};
330+
331+
await createBookingScenario(
332+
getScenarioData({
333+
eventTypes: [
334+
{
335+
id: 1,
336+
slotInterval: 30,
337+
length: 30,
338+
seatsPerTimeSlot: 3,
339+
users: [{ id: 101 }],
340+
},
341+
],
342+
// Two seats on the same booking → two Payment rows sharing one bookingId.
343+
payment: [
344+
{
345+
amount: 1000,
346+
bookingId: idOfBookingToBeCancelled,
347+
currency: "usd",
348+
data: {},
349+
externalId: "ext_seat_1",
350+
fee: 0,
351+
refunded: false,
352+
success: true,
353+
uid: "seat-1-pay",
354+
},
355+
{
356+
amount: 5000,
357+
bookingId: idOfBookingToBeCancelled,
358+
currency: "usd",
359+
data: {},
360+
externalId: "ext_seat_2",
361+
fee: 0,
362+
refunded: false,
363+
success: true,
364+
uid: "seat-2-pay",
365+
},
366+
],
367+
bookings: [booking],
368+
organizer,
369+
apps: [TestData.apps["daily-video"]],
370+
})
371+
);
372+
mockSuccessfulVideoMeetingCreation({
373+
metadataLookupKey: "dailyvideo",
374+
videoMeetingData: { id: "MOCK_ID", password: "MOCK_PASS", url: `http://mock-dailyvideo.example.com/meeting-1` },
375+
});
376+
mockCalendarToHaveNoBusySlots("googlecalendar", {
377+
create: { id: "MOCKED_GOOGLE_CALENDAR_EVENT_ID" },
378+
});
379+
380+
// Full cancel (no seatReferenceUid) by the host. Clear first so the count is order-independent.
381+
vi.mocked(processPaymentRefund).mockClear();
382+
await handleCancelBooking({
383+
bookingData: {
384+
id: idOfBookingToBeCancelled,
385+
uid: uidOfBookingToBeCancelled,
386+
cancelledBy: organizer.email,
387+
cancellationReason: "No reason",
388+
},
389+
});
390+
391+
expect(processPaymentRefund).toHaveBeenCalledTimes(1);
392+
const refundArg = vi.mocked(processPaymentRefund).mock.calls[0][0];
393+
expect(refundArg.booking.payment).toHaveLength(2);
394+
expect(refundArg.booking.payment.map((p) => p.externalId).sort()).toEqual(["ext_seat_1", "ext_seat_2"]);
395+
});
396+
292397
test("Should block cancelling past bookings", async () => {
293398
const handleCancelBooking = (await import("@calcom/features/bookings/lib/handleCancelBooking")).default;
294399

packages/features/bookings/lib/payment/processPaymentRefund.test.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,57 @@ describe("processPaymentRefund", () => {
116116
);
117117
});
118118

119+
it("should refund every successful payment on a multi-seat booking, not just the first", async () => {
120+
mockedGetPaymentAppData.mockReturnValue(mockAppData);
121+
await prismock.app.create({ data: mockApp });
122+
await prismock.credential.create({ data: mockCredential });
123+
124+
// A paid seated booking fans out into one Payment row per seat on the same bookingId.
125+
const multiSeatBooking = {
126+
...mockBooking,
127+
payment: [
128+
{ ...mockPayment[0], id: 1, uid: "seat-1-pay", externalId: "ext-seat-1" },
129+
{ ...mockPayment[0], id: 2, uid: "seat-2-pay", externalId: "ext-seat-2", amount: 5000 },
130+
],
131+
};
132+
133+
const mockNow = new Date(mockStartTime.getTime() - 8 * 24 * 60 * 60 * 1000);
134+
vi.useFakeTimers();
135+
vi.setSystemTime(mockNow);
136+
137+
await processPaymentRefund({ booking: multiSeatBooking, teamId: 1 });
138+
139+
expect(handlePaymentRefund).toHaveBeenCalledTimes(2);
140+
expect(handlePaymentRefund).toHaveBeenCalledWith(1, expect.objectContaining({ appId: "123" }));
141+
expect(handlePaymentRefund).toHaveBeenCalledWith(2, expect.objectContaining({ appId: "123" }));
142+
});
143+
144+
it("should attempt every refund even when one fails, then surface an aggregate error", async () => {
145+
mockedGetPaymentAppData.mockReturnValue(mockAppData);
146+
await prismock.app.create({ data: mockApp });
147+
await prismock.credential.create({ data: mockCredential });
148+
vi.mocked(handlePaymentRefund)
149+
.mockRejectedValueOnce(new Error("provider down"))
150+
.mockResolvedValueOnce(undefined);
151+
152+
const multiSeatBooking = {
153+
...mockBooking,
154+
payment: [
155+
{ ...mockPayment[0], id: 1, uid: "seat-1-pay", externalId: "ext-seat-1" },
156+
{ ...mockPayment[0], id: 2, uid: "seat-2-pay", externalId: "ext-seat-2", amount: 5000 },
157+
],
158+
};
159+
160+
const mockNow = new Date(mockStartTime.getTime() - 8 * 24 * 60 * 60 * 1000);
161+
vi.useFakeTimers();
162+
vi.setSystemTime(mockNow);
163+
164+
await expect(processPaymentRefund({ booking: multiSeatBooking, teamId: 1 })).rejects.toThrow();
165+
166+
// Both seats are attempted even though the first refund failed.
167+
expect(handlePaymentRefund).toHaveBeenCalledTimes(2);
168+
});
169+
119170
it("should not process refund if past the refund deadline", async () => {
120171
mockedGetPaymentAppData.mockReturnValueOnce(mockAppData);
121172
await prismock.app.create({ data: mockApp });

packages/features/bookings/lib/payment/processPaymentRefund.ts

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,10 @@ export const processPaymentRefund = async ({
2626
const { startTime, eventType, payment } = booking;
2727
if (!teamId && !eventType?.owner) return;
2828

29-
const successPayment = payment.find((p) => p.success);
29+
// A paid seated booking creates one Payment per seat on the same bookingId, so refund every
30+
// successful (not-yet-refunded) payment — not just the first — otherwise the other seats stay charged.
31+
const successfulPayments = payment.filter((p) => p.success && !p.refunded);
32+
const [successPayment] = successfulPayments;
3033
if (!successPayment) return;
3134

3235
const eventTypeMetadata = EventTypeMetaDataSchema.parse(eventType?.metadata);
@@ -80,5 +83,22 @@ export const processPaymentRefund = async ({
8083
dayjs(startTime).businessDaysSubtract(refundDaysCount);
8184
if (dayjs().isAfter(refundDeadline)) return;
8285
}
83-
await handlePaymentRefund(successPayment.id, paymentAppCredential);
86+
// Attempt every refund independently so one provider failure doesn't leave the remaining seats
87+
// charged, then surface an aggregate error for the caller to log/report.
88+
const refundErrors: unknown[] = [];
89+
for (const paymentToRefund of successfulPayments) {
90+
try {
91+
await handlePaymentRefund(paymentToRefund.id, paymentAppCredential);
92+
} catch (error) {
93+
refundErrors.push(error);
94+
}
95+
}
96+
97+
if (refundErrors.length) {
98+
throw new Error(
99+
`Failed to refund ${refundErrors.length} of ${successfulPayments.length} payment(s): ${refundErrors
100+
.map((error) => (error instanceof Error ? error.message : String(error)))
101+
.join("; ")}`
102+
);
103+
}
84104
};

0 commit comments

Comments
 (0)