feat: add time-based cancellation fees for no-show fee events#23595
feat: add time-based cancellation fees for no-show fee events#23595alishaz-polymath merged 43 commits intomainfrom
Conversation
- Add configurable time threshold (minutes/hours/days) for cancellation fees - Show warnings during booking submission and cancellation - Automatically charge fees when bookings cancelled within threshold - Exempt organizer/admin cancellations from fees - Extend existing no-show fee infrastructure Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
- Use Record<string, unknown> instead of any for metadata type - Add proper type assertions for nested metadata properties - Maintain type safety while avoiding ESLint no-explicit-any warning Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
- Add proper type guard for metadata JsonValue from Prisma - Ensure metadata is object before casting to Record<string, unknown> - Fixes TypeScript error on line 390 in handleCancelBooking.ts Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
WalkthroughAdds end-to-end no-show cancellation fee support: UI (CancelBooking) shows an acknowledgement when per-app event metadata and booking payment indicate a last-minute fee; Stripe app settings expose controls and schema for enabling/configuring the fee; server-side adds shouldChargeNoShowCancellationFee, handleNoShowFee, processNoShowFeeOnCancellation, and branches cancellation handling by payment option; TRPC chargeCard handler delegates to handleNoShowFee; data fetching and repository methods include payment.appId and booking.startTime; translations, unit tests, and Playwright E2E tests added. Possibly related PRs
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
keithwillcode
added
core
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
…est coverage - Fix userId in attendee cancellation test to properly test fee charging - Ensure all test files follow Cal.com patterns with proper mocking - Add comprehensive coverage for time thresholds, role exemptions, and UI warnings - All tests pass locally with proper timezone handling Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
alishaz-polymath
dismissed stale reviews from CarinaWolli and themself
Stale
dosubot
bot
added
billing
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/trpc/server/routers/viewer/payments/chargeCard.handler.ts (1)
24-35: Authorize caller and select the correct HOLD payment; handle empty payments.
- Enforce access for
ctx.user.- Find the latest capturable HOLD payment instead of relying on
[0].- Fail fast if none found (idempotency-safe).
- if (booking.payment[0].success) { - throw new TRPCError({ - code: "BAD_REQUEST", - message: `The no show fee for ${booking.id} has already been charged.`, - }); - } + const hasAccess = await bookingRepository.doesUserIdHaveAccessToBooking({ + userId: ctx.user.id, + bookingId: booking.id, + }); + if (!hasAccess) { + throw new TRPCError({ code: "FORBIDDEN", message: "Not authorized to charge this booking." }); + } + + const holdPayment = booking.payment?.find( + (p) => p?.paymentOption === "HOLD" && p?.success === false + ); + if (!holdPayment) { + throw new TRPCError({ + code: "BAD_REQUEST", + message: "No capturable hold payment found for this booking.", + }); + } try { - await handleNoShowFee({ - booking, - payment: booking.payment[0], - }); + await handleNoShowFee({ booking, payment: holdPayment });apps/web/components/booking/CancelBooking.tsx (1)
75-86: Type safety for eventTypeMetadata and app metadata accessUsing
Record<string, unknown>forces unsafe property access and will fight TS when reading nested fields. Strongly type this with the shared Zod schema and narrow app metadata. Also, props coming from GSSP serializeDateto string; reflect that in the type to avoid mismatches.Apply:
@@ -import { shouldChargeNoShowCancellationFee } from "@calcom/lib/payment/shouldChargeNoShowCancellationFee"; +import { shouldChargeNoShowCancellationFee } from "@calcom/lib/payment/shouldChargeNoShowCancellationFee"; +import type { z } from "zod"; +import { eventTypeMetaDataSchemaWithTypedApps } from "@calcom/prisma/zod-utils"; @@ booking: { @@ - startTime: Date; + startTime: string | Date; @@ - } | null; + } | null; }; @@ - eventTypeMetadata?: Record<string, unknown> | null; + eventTypeMetadata?: z.infer<typeof eventTypeMetaDataSchemaWithTypedApps> | null; @@ -const getAppMetadata = (appId: string): Record<string, unknown> | null => { - if (!eventTypeMetadata?.apps || !appId) return null; - const apps = eventTypeMetadata.apps as Record<string, unknown>; - return (apps[appId] as Record<string, unknown>) || null; -}; +const getAppMetadata = (appId: string | null | undefined) => { + if (!eventTypeMetadata?.apps || !appId) return null; + return eventTypeMetadata.apps[appId as keyof typeof eventTypeMetadata.apps] ?? null; +}; @@ -const timeValue = booking?.payment?.appId - ? (getAppMetadata(booking.payment.appId) as Record<string, unknown> | null)?.autoChargeNoShowFeeTimeValue - : null; -const timeUnit = booking?.payment?.appId - ? (getAppMetadata(booking.payment.appId) as Record<string, unknown> | null)?.autoChargeNoShowFeeTimeUnit - : null; +const appMeta = getAppMetadata(booking?.payment?.appId) as + | { + autoChargeNoShowFeeTimeValue?: number | null; + autoChargeNoShowFeeTimeUnit?: "minutes" | "hours" | "days" | null; + } + | null; +const timeValue = appMeta?.autoChargeNoShowFeeTimeValue ?? null; +const timeUnit = appMeta?.autoChargeNoShowFeeTimeUnit ?? null;Also applies to: 110-111, 131-143
♻️ Duplicate comments (1)
apps/web/components/booking/CancelBooking.tsx (1)
259-261: Disable conditions are correctly combinedButton disables when host reason is missing or the attendee hasn’t acknowledged the fee. Matches the earlier intent.
Also applies to: 160-164
🧹 Nitpick comments (34)
apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx (1)
182-194: Select now includes appId; add deterministic ordering to avoid picking an arbitrary payment.Without an orderBy, findFirst may return a stale attempt; this can mis-drive appId-based logic/UI.
Apply:
const payment = await prisma.payment.findFirst({ where: { bookingId: bookingInfo.id, }, + orderBy: { createdAt: "desc" }, select: { appId: true, success: true, refunded: true, currency: true, amount: true, paymentOption: true, }, });If unsure Payment has createdAt, search the Prisma schema to confirm and adjust the field if named differently.
packages/features/bookings/lib/getBookingToDelete.ts (2)
25-26: Confirm locale is consumed; drop if unused to keep payload lean.
50-51: teamId duplication—ensure it’s needed beyond team.id; otherwise remove.packages/app-store/stripepayment/zod.ts (1)
17-17: Tighten schema: integer, min bound, and require value/unit when auto-charge is enabled.Prevents misconfiguration and aligns with UI intent.
Apply:
-export const autoChargeNoShowFeeTimeUnitEnum = z.enum(["minutes", "hours", "days"]); +export const autoChargeNoShowFeeTimeUnitEnum = z.enum(["minutes", "hours", "days"]); @@ - autoChargeNoShowFeeIfCancelled: z.boolean().optional(), - autoChargeNoShowFeeTimeValue: z.number().optional(), - autoChargeNoShowFeeTimeUnit: autoChargeNoShowFeeTimeUnitEnum.optional(), + autoChargeNoShowFeeIfCancelled: z.boolean().optional(), + autoChargeNoShowFeeTimeValue: z.number().int().min(1).optional(), + autoChargeNoShowFeeTimeUnit: autoChargeNoShowFeeTimeUnitEnum.optional(), - }) -); + }).superRefine((data, ctx) => { + if (data.autoChargeNoShowFeeIfCancelled) { + if (data.autoChargeNoShowFeeTimeValue == null) { + ctx.addIssue({ code: z.ZodIssueCode.custom, path: ["autoChargeNoShowFeeTimeValue"], message: "required" }); + } + if (data.autoChargeNoShowFeeTimeUnit == null) { + ctx.addIssue({ code: z.ZodIssueCode.custom, path: ["autoChargeNoShowFeeTimeUnit"], message: "required" }); + } + } + }) +);Also applies to: 28-31
packages/app-store/stripepayment/components/EventTypeAppSettingsInterface.tsx (2)
80-85: Typo: autoChange → autoCharge.Keeps naming consistent with feature terminology.
Apply:
- const autoChangeTimeUnitOptions = [ + const autoChargeTimeUnitOptions = [ { value: autoChargeNoShowFeeTimeUnitEnum.enum.minutes, label: t("minutes") }, { value: autoChargeNoShowFeeTimeUnitEnum.enum.hours, label: t("hours") }, { value: autoChargeNoShowFeeTimeUnitEnum.enum.days, label: t("days") }, ];And update references below accordingly.
221-268: Make inputs fully controlled, avoid NaN, and honor disabled state on the checkbox.Prevents React controlled/uncontrolled warnings and accidental NaN in app data.
Apply:
- <CheckboxField - checked={autoChargeNoShowFeeIfCancelled} - onChange={(e) => setAppData("autoChargeNoShowFeeIfCancelled", e.target.checked)} - description={t("auto_charge_for_last_minute_cancellation")} - /> + <CheckboxField + checked={autoChargeNoShowFeeIfCancelled} + disabled={disabled} + onChange={(e) => setAppData("autoChargeNoShowFeeIfCancelled", e.target.checked)} + description={t("auto_charge_for_last_minute_cancellation")} + /> @@ - <TextField + <TextField labelSrOnly type="number" className={classNames( "border-default my-0 block w-16 text-sm [appearance:textfield] ltr:mr-2 rtl:ml-2" )} placeholder="2" disabled={disabled} - min={0} - defaultValue={autoChargeNoShowFeeTimeValue} + min={0} required={autoChargeNoShowFeeIfCancelled} value={autoChargeNoShowFeeTimeValue ?? ""} - onChange={(e) => - setAppData("autoChargeNoShowFeeTimeValue", parseInt(e.currentTarget.value)) - } + onChange={(e) => { + const v = e.currentTarget.value; + setAppData( + "autoChargeNoShowFeeTimeValue", + v === "" ? undefined : parseInt(v, 10) + ); + }} /> @@ - <Select - options={autoChangeTimeUnitOptions} + <Select + options={autoChargeTimeUnitOptions} isSearchable={false} isDisabled={disabled} onChange={(option) => setAppData("autoChargeNoShowFeeTimeUnit", option?.value)} - value={ - autoChangeTimeUnitOptions.find((opt) => opt.value === autoChargeNoShowFeeTimeUnit) || - autoChangeTimeUnitOptions[0] - } - defaultValue={ - autoChangeTimeUnitOptions.find((opt) => opt.value === autoChargeNoShowFeeTimeUnit) || - autoChangeTimeUnitOptions[0] - } + value={ + autoChargeTimeUnitOptions.find((opt) => opt.value === autoChargeNoShowFeeTimeUnit) || + autoChargeTimeUnitOptions[0] + } />packages/lib/payment/shouldChargeNoShowCancellationFee.ts (1)
24-35: Type access on metadata is overly complex; simplify the indexing.Reduces casts and improves readability.
Apply:
- const cancellationFeeEnabled = - eventTypeMetadata?.apps?.[paymentAppId as keyof typeof eventTypeMetadata.apps] - ?.autoChargeNoShowFeeIfCancelled; - const paymentOption = - eventTypeMetadata?.apps?.[paymentAppId as keyof typeof eventTypeMetadata.apps]?.paymentOption; - const timeValue = - eventTypeMetadata?.apps?.[paymentAppId as keyof typeof eventTypeMetadata.apps] - ?.autoChargeNoShowFeeTimeValue; - const timeUnit = - eventTypeMetadata?.apps?.[paymentAppId as keyof typeof eventTypeMetadata.apps] - ?.autoChargeNoShowFeeTimeUnit; + const appCfg = (eventTypeMetadata?.apps as Record<string, any> | undefined)?.[paymentAppId]; + const cancellationFeeEnabled = appCfg?.autoChargeNoShowFeeIfCancelled; + const paymentOption = appCfg?.paymentOption; + const timeValue = appCfg?.autoChargeNoShowFeeTimeValue; + const timeUnit = appCfg?.autoChargeNoShowFeeTimeUnit as string | undefined;packages/lib/payment/shouldChargeNoShowCancellationFee.test.ts (2)
14-14: Align test name with function under test.-describe("shouldChargeCancellationFee", () => { +describe("shouldChargeNoShowCancellationFee", () => {
167-187: Add boundary test for equality at threshold.Covers
now === thresholdto lock behavior.it("should return false when timeUnit is missing", () => { const mockEventTypeMetadata = { apps: { stripe: { autoChargeNoShowFeeIfCancelled: true, paymentOption: "HOLD", autoChargeNoShowFeeTimeValue: 24, }, }, }; const result = shouldChargeNoShowCancellationFee({ eventTypeMetadata: mockEventTypeMetadata as EventTypeMetadata, booking: { startTime: new Date("2024-09-01T12:00:00Z") }, payment: mockPayment, }); expect(result).toBe(false); }); + + it("should return true when cancelled exactly at the threshold", () => { + // now = 2024-09-01T10:00:00Z (set in beforeAll), threshold = start - 2h + const startTime = new Date("2024-09-01T12:00:00Z"); + const result = shouldChargeNoShowCancellationFee({ + eventTypeMetadata: mockEventTypeMetadata as EventTypeMetadata, + booking: { startTime }, + payment: mockPayment, + }); + expect(result).toBe(true); + }); });apps/web/public/static/locales/en/common.json (1)
3614-3617: Standardize “no‑show” hyphenation for consistency.Other strings (e.g., "No-show Fee") use a hyphen.
- "cancel_booking_acknowledge_no_show_fee": "I acknowledge that by cancelling the booking within {{timeValue}} {{timeUnit}} of the start time I will be charged the no show fee of {{amount, currency}}", + "cancel_booking_acknowledge_no_show_fee": "I acknowledge that by cancelling the booking within {{timeValue}} {{timeUnit}} of the start time I will be charged the no-show fee of {{amount, currency}}",packages/features/bookings/lib/handleCancelBooking.ts (1)
581-581: Prefer named export.Switch to
export { handler }for clearer imports and better tree-shaking. Low priority due to widespread call sites.packages/features/bookings/lib/handleCancelBooking/test/handleCancelBooking.test.ts (4)
777-780: Freeze time to make threshold assertions deterministic.Use
vi.setSystemTimeso the 30-minute window is stable across environments.- const now = new Date(); - const startTime = new Date(now.getTime() + 30 * 60 * 1000); + const now = new Date("2025-01-01T10:00:00.000Z"); + vi.setSystemTime(now); + const startTime = new Date(now.getTime() + 30 * 60 * 1000);
759-763: Remove unsupportedidfrom getBooker call.
getBookerdoesn’t acceptid; this arg is ignored and can confuse future readers.- const booker = getBooker({ - email: "booker@example.com", - name: "Booker", - id: 999, - }); + const booker = getBooker({ email: "booker@example.com", name: "Booker" });
861-869: Assert that a charge actually happened.Add a DB assertion that the HOLD payment was captured (
success === true) to prevent false positives.const result = await handleCancelBooking({ bookingData: { id: idOfBookingToBeCancelled, uid: uidOfBookingToBeCancelled, cancelledBy: booker.email, cancellationReason: "Attendee cancelled within time threshold", }, userId: 999, }); expect(result.success).toBe(true); + // Verify capture + const payment = await prisma.payment.findFirst({ + where: { bookingId: idOfBookingToBeCancelled }, + orderBy: { id: "desc" }, + select: { success: true }, + }); + expect(payment?.success).toBe(true);
513-623: Add boundary/unit coverage for cancellation window.Consider tests for: exactly-on-threshold, minutes and days units, and DST transitions.
I can add cases for MINUTES/HOURS/DAYS and a DST-crossing startTime to harden the logic.
Also applies to: 624-755
packages/trpc/server/routers/viewer/payments/chargeCard.handler.ts (2)
18-22: Use TRPCError(NOT_FOUND) for missing booking.Surface a proper TRPC error to clients.
- if (!booking) { - throw new Error("Booking not found"); - } + if (!booking) { + throw new TRPCError({ code: "NOT_FOUND", message: "Booking not found" }); + }
37-41: Preserve error details when available.Bubble up
TRPCErrorinstances; only wrap unknowns.- } catch (error) { - console.error(error); - throw new TRPCError({ - code: "INTERNAL_SERVER_ERROR", - message: `Failed to charge no show fee for ${booking.id}`, - }); - } + } catch (error) { + console.error(error); + if (error instanceof TRPCError) throw error; + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: `Failed to charge no show fee for ${booking.id}`, + }); + }apps/web/components/booking/__tests__/CancelBooking.cancellationFee.test.tsx (5)
28-43: Remove unused next-i18next mock to reduce noise.CancelBooking uses useLocale; this mock isn’t consumed here.
-vi.mock("next-i18next", () => ({ - useTranslation: () => ({ - t: (key: string, options?: Record<string, unknown>) => { - if (key === "cancellation_fee_warning_cancel") { - const opts = options as { - time?: string; - unit?: string; - amount?: number; - formatParams?: { amount?: { currency?: string } }; - }; - return `Cancelling within ${opts?.time} ${opts?.unit} will result in a ${opts?.amount} ${opts?.formatParams?.amount?.currency} cancellation fee being charged to your card.`; - } - return key; - }, - }), -})); +// next-i18next not needed in this test; UI uses useLocale()
106-115: Rename misleading fixture; it actually has fee enabled.The object named “WithoutFee” still sets autoChargeNoShowFeeIfCancelled: true, which confuses test intent.
-const mockEventTypeMetadataWithoutFee = { +const mockEventTypeMetadataOutsideThreshold = { apps: { stripe: { autoChargeNoShowFeeIfCancelled: true, autoChargeNoShowFeeTimeValue: 1, autoChargeNoShowFeeTimeUnit: "hours" as const, paymentOption: "HOLD" as const, }, }, }; ... - eventTypeMetadata={mockEventTypeMetadataWithoutFee} + eventTypeMetadata={mockEventTypeMetadataOutsideThreshold}Also applies to: 171-179
163-166: Make assertion robust to singular/plural (“1 hour” vs “1 hours”).Avoid hard-coding a potentially grammatically incorrect string.
-expect( - screen.getByText(/I acknowledge that cancelling within 1 hours will result in a/) -).toBeInTheDocument(); +expect( + screen.getByText(/I acknowledge that cancelling within 1 hour(s)? will result in a/i) +).toBeInTheDocument();
149-166: Also assert CTA gating (button disabled until acknowledged).Covers the most important UX guard.
render( <CancelBooking @@ /> ); -expect( - screen.getByText(/I acknowledge that cancelling within 1 hour(s)? will result in a/i) -).toBeInTheDocument(); +const ack = screen.getByText(/I acknowledge that cancelling within 1 hour\(s\)\? will result in a/i); +expect(ack).toBeInTheDocument(); +const confirm = screen.getByTestId("confirm_cancel"); +expect(confirm).toBeDisabled();
13-26: Trim unused mocks (trpc/useRouter) for leaner tests.They aren’t exercised by this component path.
-vi.mock("@calcom/trpc", () => ({ ... })) -vi.mock("next/router", () => ({ ... })) +// Remove unused mocks; keep test focused on CancelBooking branchAlso applies to: 72-77
packages/lib/payment/processNoShowFeeOnCancellation.ts (3)
29-41: Extend exemption to org-level admins (if team belongs to an org).When teamId maps to an org, org ADMIN/OWNER should also bypass charges.
if (cancelledByUserId && booking.eventType?.teamId) { const membership = await MembershipRepository.findUniqueByUserIdAndTeamId({ userId: cancelledByUserId, teamId: booking.eventType.teamId, }); if (membership && (membership.role === "ADMIN" || membership.role === "OWNER")) { log.info( `Booking ${booking.uid} was cancelled by team admin/owner (${cancelledByUserId}), skipping no-show fee` ); return; } + + // Optional: check parent organization membership as an admin/owner + // const org = await new TeamRepository(prisma).findParentOrganizationByTeamId(booking.eventType.teamId); + // if (org) { + // const orgMembership = await MembershipRepository.findUniqueByUserIdAndTeamId({ + // userId: cancelledByUserId, + // teamId: org.id, + // }); + // if (orgMembership && (orgMembership.role === "ADMIN" || orgMembership.role === "OWNER")) { + // log.info(`Booking ${booking.uid} was cancelled by org admin/owner (${cancelledByUserId}), skipping no-show fee`); + // return; + // } + // } }
73-76: Preserve error stack; include cause.Re-wrapping loses stack/context.
- } catch (error) { - log.error(`Error charging no-show fee for booking ${booking.uid}`, error); - throw new Error(`Failed to charge no-show fee with error ${error}`); - } + } catch (error) { + log.error(`Error charging no-show fee for booking ${booking.uid}`, error); + // Prefer preserving original error; if wrapping, use cause + if (error instanceof Error) throw error; + throw new Error(`Failed to charge no-show fee`, { cause: error }); + }
18-19: Add structured logs with booking/payment ids for observability.Makes support/debugging easier.
-const log = logger.getSubLogger({ prefix: ["processNoShowFeeOnCancellation"] }); +const log = logger.getSubLogger({ prefix: [`[processNoShowFeeOnCancellation uid=${booking.uid}]`] }); @@ - log.info(`Date is not valid for no-show fee to charge for booking ${booking.uid}`); + log.info(`Not charging no-show fee: outside threshold (bookingUid=${booking.uid})`); @@ - await handleNoShowFee({ + log.info(`Charging no-show fee (bookingId=${booking.id}, paymentId=${paymentToCharge.id})`); + await handleNoShowFee({ booking, payment: paymentToCharge, });Also applies to: 62-66, 67-73
apps/web/playwright/cancellation-fee-warning.e2e.ts (1)
36-37: Prefer data-testid or more resilient assertions over text snippets.Reduces i18n coupling and copy fragility. If adding testids isn’t feasible now, at least soften the matcher.
-await expect(page.getByText(/I acknowledge that if I cancel within 2 hours/)).toBeVisible(); +await expect(page.getByText(/I acknowledge that .* cancel within 2 hours/i)).toBeVisible(); @@ -await expect(page.getByText(/Cancelling within 24 hours will result in a/)).toBeVisible(); +await expect(page.getByText(/Cancelling within 24 hours/i)).toBeVisible(); @@ -await expect(page.getByText(/I acknowledge that if I cancel within/)).toBeHidden(); +await expect(page.getByText(/I acknowledge that .* cancel within/i)).toHaveCount(0);Also applies to: 64-65, 92-92
apps/web/components/booking/CancelBooking.tsx (3)
137-143: Localize unit label instead of injecting raw enum valuesPassing raw
"minutes" | "hours" | "days"into a translation placeholder risks untranslated output. Map to i18n keys and pass the localized label.@@ -const timeUnit = booking?.payment?.appId - ? (getAppMetadata(booking.payment.appId) as Record<string, unknown> | null)?.autoChargeNoShowFeeTimeUnit - : null; +const unitKey = timeUnit ? ({ minutes: "minutes", hours: "hours", days: "days" } as const)[timeUnit] : null; +const timeUnitLabel = unitKey ? t(unitKey) : ""; @@ - description={t("cancel_booking_acknowledge_no_show_fee", { - timeValue, - timeUnit, - amount: booking.payment.amount / 100, - formatParams: { amount: { currency: booking.payment.currency } }, - })} + description={t("cancel_booking_acknowledge_no_show_fee", { + timeValue, + timeUnit: timeUnitLabel, + amount: booking.payment.amount / 100, + formatParams: { amount: { currency: booking.payment.currency } }, + })}Also applies to: 233-249
233-249: Make the checkbox controlled for predictable UI stateBind
checkedto state to avoid uncontrolled/controlled transitions and ensure SSR hydration consistency.-<CheckboxField +<CheckboxField + checked={acknowledgeCancellationNoShowFee} description={t("cancel_booking_acknowledge_no_show_fee", { @@ onChange={(e) => setAcknowledgeCancellationNoShowFee(e.target.checked)}
113-116: Prefer named export for componentsFor non-page components, named exports improve tree-shaking and refactors.
-export default function CancelBooking(props: Props) { +export function CancelBooking(props: Props) {And update imports accordingly.
packages/lib/payment/handleNoShowFee.ts (5)
65-66: Typo: eventTypeMetdata → eventTypeMetadataMinor naming fix improves readability and prevents future confusion.
- const eventTypeMetdata = eventTypeMetaDataSchemaWithTypedApps.parse(booking.eventType?.metadata ?? {}); + const eventTypeMetadata = eventTypeMetaDataSchemaWithTypedApps.parse(booking.eventType?.metadata ?? {}); @@ - await sendNoShowFeeChargedEmail(attendeesListPromises[0], evt, eventTypeMetdata); + await sendNoShowFeeChargedEmail(attendeesList[0], evt, eventTypeMetadata);Also applies to: 167-167
67-71: Normalize precondition errors (consistency and localization)Errors thrown before the
tryblock won’t be localized. Either wrap preconditions in the sametry/catchor throwErrorWithCodeso upstream can map them.Option A (wrap in try/catch) or Option B (use
ErrorWithCodewith stable codes). Happy to draft either approach if you confirm the preferred pattern in this module.Also applies to: 110-120, 140-155
3-5: Remove stray eslint-disable-next-lineThe
// eslint-disable-next-linebefore the import is unnecessary and suppresses useful checks.-// eslint-disable-next-line import { PaymentServiceMap } from "@calcom/app-store/payment.services.generated";
95-103: Localize fallback organizer nameHard-coded
"Nameless"can leak into user emails. Prefer a localized fallback or the email local-part.- name: booking.user?.name || "Nameless", + name: booking.user?.name || tOrganizer("nameless_user"),If
nameless_userdoesn’t exist, I can add it to locales.
159-179: Emit observability signal for success/failureAdd structured telemetry (success/failure, appId, paymentId, teamId/userId) for alerting and dashboards.
I can wire a
logger.info+ metrics counter here or integrate with your existing telemetry service if you point me to it.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (17)
apps/web/components/booking/CancelBooking.tsx(6 hunks)apps/web/components/booking/__tests__/CancelBooking.cancellationFee.test.tsx(1 hunks)apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx(1 hunks)apps/web/modules/bookings/views/bookings-single-view.tsx(1 hunks)apps/web/playwright/cancellation-fee-warning.e2e.ts(1 hunks)apps/web/public/static/locales/en/common.json(3 hunks)packages/app-store/stripepayment/components/EventTypeAppSettingsInterface.tsx(5 hunks)packages/app-store/stripepayment/zod.ts(2 hunks)packages/features/bookings/lib/getBookingToDelete.ts(2 hunks)packages/features/bookings/lib/handleCancelBooking.ts(2 hunks)packages/features/bookings/lib/handleCancelBooking/test/handleCancelBooking.test.ts(1 hunks)packages/lib/payment/handleNoShowFee.ts(1 hunks)packages/lib/payment/processNoShowFeeOnCancellation.ts(1 hunks)packages/lib/payment/shouldChargeNoShowCancellationFee.test.ts(1 hunks)packages/lib/payment/shouldChargeNoShowCancellationFee.ts(1 hunks)packages/lib/server/repository/booking.ts(1 hunks)packages/trpc/server/routers/viewer/payments/chargeCard.handler.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
apps/web/components/booking/__tests__/CancelBooking.cancellationFee.test.tsxapps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsxpackages/app-store/stripepayment/components/EventTypeAppSettingsInterface.tsxapps/web/components/booking/CancelBooking.tsxapps/web/modules/bookings/views/bookings-single-view.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
apps/web/components/booking/__tests__/CancelBooking.cancellationFee.test.tsxpackages/features/bookings/lib/handleCancelBooking/test/handleCancelBooking.test.tspackages/features/bookings/lib/getBookingToDelete.tspackages/lib/server/repository/booking.tsapps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsxpackages/lib/payment/shouldChargeNoShowCancellationFee.test.tspackages/app-store/stripepayment/zod.tsapps/web/playwright/cancellation-fee-warning.e2e.tspackages/features/bookings/lib/handleCancelBooking.tspackages/lib/payment/shouldChargeNoShowCancellationFee.tspackages/lib/payment/processNoShowFeeOnCancellation.tspackages/app-store/stripepayment/components/EventTypeAppSettingsInterface.tsxapps/web/components/booking/CancelBooking.tsxpackages/trpc/server/routers/viewer/payments/chargeCard.handler.tsapps/web/modules/bookings/views/bookings-single-view.tsxpackages/lib/payment/handleNoShowFee.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
apps/web/components/booking/__tests__/CancelBooking.cancellationFee.test.tsxpackages/features/bookings/lib/handleCancelBooking/test/handleCancelBooking.test.tspackages/features/bookings/lib/getBookingToDelete.tspackages/lib/server/repository/booking.tsapps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsxpackages/lib/payment/shouldChargeNoShowCancellationFee.test.tspackages/app-store/stripepayment/zod.tsapps/web/playwright/cancellation-fee-warning.e2e.tspackages/features/bookings/lib/handleCancelBooking.tspackages/lib/payment/shouldChargeNoShowCancellationFee.tspackages/lib/payment/processNoShowFeeOnCancellation.tspackages/app-store/stripepayment/components/EventTypeAppSettingsInterface.tsxapps/web/components/booking/CancelBooking.tsxpackages/trpc/server/routers/viewer/payments/chargeCard.handler.tsapps/web/modules/bookings/views/bookings-single-view.tsxpackages/lib/payment/handleNoShowFee.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/bookings/lib/handleCancelBooking/test/handleCancelBooking.test.tspackages/features/bookings/lib/getBookingToDelete.tspackages/lib/server/repository/booking.tspackages/lib/payment/shouldChargeNoShowCancellationFee.test.tspackages/app-store/stripepayment/zod.tsapps/web/playwright/cancellation-fee-warning.e2e.tspackages/features/bookings/lib/handleCancelBooking.tspackages/lib/payment/shouldChargeNoShowCancellationFee.tspackages/lib/payment/processNoShowFeeOnCancellation.tspackages/trpc/server/routers/viewer/payments/chargeCard.handler.tspackages/lib/payment/handleNoShowFee.ts
🧠 Learnings (3)
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Applied to files:
apps/web/components/booking/__tests__/CancelBooking.cancellationFee.test.tsxapps/web/playwright/cancellation-fee-warning.e2e.tspackages/app-store/stripepayment/components/EventTypeAppSettingsInterface.tsxapps/web/modules/bookings/views/bookings-single-view.tsx
📚 Learning: 2025-08-21T13:44:06.805Z
Learnt from: supalarry
PR: calcom/cal.com#23217
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts:93-94
Timestamp: 2025-08-21T13:44:06.805Z
Learning: In apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, repository functions that use explicit Prisma select clauses (like getEventTypeWithSeats) are used for specific purposes and don't need to include all EventType fields like bookingRequiresAuthentication. These methods don't feed into the general OutputEventTypesService_2024_06_14 flow.
Applied to files:
packages/features/bookings/lib/getBookingToDelete.tspackages/lib/server/repository/booking.ts
📚 Learning: 2025-08-21T12:28:42.018Z
Learnt from: alishaz-polymath
PR: calcom/cal.com#23247
File: packages/features/webhooks/lib/factory/WebhookPayloadFactory.ts:274-282
Timestamp: 2025-08-21T12:28:42.018Z
Learning: In BookingPaymentInitiatedDTO and other webhook DTOs in packages/features/webhooks/lib/dto/types.ts, the booking field is a restricted structure containing only specific fields (id, eventTypeId, userId) rather than the full database booking object, so there are no security or PII leakage concerns when passing the booking object to buildEventPayload.
Applied to files:
packages/lib/server/repository/booking.tsapps/web/modules/bookings/views/bookings-single-view.tsx
🧬 Code graph analysis (11)
apps/web/components/booking/__tests__/CancelBooking.cancellationFee.test.tsx (1)
apps/web/components/booking/CancelBooking.tsx (1)
CancelBooking(113-316)
packages/features/bookings/lib/handleCancelBooking/test/handleCancelBooking.test.ts (1)
apps/web/test/utils/bookingScenario/bookingScenario.ts (9)
getBooker(2220-2234)getOrganizer(1520-1576)TestData(1239-1511)getGoogleCalendarCredential(1192-1200)getDate(1093-1140)createBookingScenario(978-1009)getScenarioData(1578-1664)mockSuccessfulVideoMeetingCreation(2075-2093)mockCalendarToHaveNoBusySlots(1960-1973)
packages/lib/payment/shouldChargeNoShowCancellationFee.test.ts (2)
packages/prisma/zod-utils.ts (2)
EventTypeMetadata(146-146)eventTypeMetaDataSchemaWithTypedApps(138-144)packages/lib/payment/shouldChargeNoShowCancellationFee.ts (1)
shouldChargeNoShowCancellationFee(5-58)
apps/web/playwright/cancellation-fee-warning.e2e.ts (1)
apps/web/playwright/lib/testUtils.ts (1)
selectFirstAvailableTimeSlotNextMonth(109-117)
packages/features/bookings/lib/handleCancelBooking.ts (1)
packages/lib/payment/processNoShowFeeOnCancellation.ts (1)
processNoShowFeeOnCancellation(9-77)
packages/lib/payment/shouldChargeNoShowCancellationFee.ts (1)
packages/prisma/zod-utils.ts (1)
eventTypeMetaDataSchemaWithTypedApps(138-144)
packages/lib/payment/processNoShowFeeOnCancellation.ts (3)
packages/lib/payment/handleNoShowFee.ts (1)
handleNoShowFee(19-179)packages/prisma/zod-utils.ts (1)
eventTypeMetaDataSchemaWithTypedApps(138-144)packages/lib/payment/shouldChargeNoShowCancellationFee.ts (1)
shouldChargeNoShowCancellationFee(5-58)
packages/app-store/stripepayment/components/EventTypeAppSettingsInterface.tsx (1)
packages/app-store/stripepayment/zod.ts (1)
autoChargeNoShowFeeTimeUnitEnum(17-17)
apps/web/components/booking/CancelBooking.tsx (1)
packages/lib/payment/shouldChargeNoShowCancellationFee.ts (1)
shouldChargeNoShowCancellationFee(5-58)
packages/trpc/server/routers/viewer/payments/chargeCard.handler.ts (2)
packages/lib/server/repository/booking.ts (1)
BookingRepository(117-993)packages/lib/payment/handleNoShowFee.ts (1)
handleNoShowFee(19-179)
packages/lib/payment/handleNoShowFee.ts (5)
packages/prisma/zod-utils.ts (1)
eventTypeMetaDataSchemaWithTypedApps(138-144)packages/types/Calendar.d.ts (1)
CalendarEvent(163-226)packages/lib/server/repository/credential.ts (1)
CredentialRepository(29-289)packages/app-store/payment.services.generated.ts (1)
PaymentServiceMap(5-12)packages/emails/email-manager.ts (1)
sendNoShowFeeChargedEmail(741-748)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (15)
packages/lib/payment/shouldChargeNoShowCancellationFee.test.ts (1)
68-136: Good scenario coverage.Hours/minutes/days, disabled flags, and missing fields are covered well.
Also applies to: 148-166
apps/web/public/static/locales/en/common.json (3)
2314-2314: LGTM: add plain “days”.Matches timeUnit emitted by logic.
3590-3597: LGTM: image validation strings.Clear, actionable messages.
3614-3616: i18n keys and timeUnit casing verified
CancelBooking and its tests correctly usecancel_booking_acknowledge_no_show_fee(and related keys) and pass lower-case timeUnit values (minutes,hours,days).packages/features/bookings/lib/handleCancelBooking.ts (3)
26-26: LGTM: import for cancellation no‑show flow.Keeps the payment logic modular.
302-319: Seat cancellations skip fee/refund by early return — expected?When removing only an attendee seat, we bypass refund/no-show logic. Confirm this is the intended policy.
Also applies to: 458-469
458-469: Enforce single paymentOption per booking
Before branching on ON_BOOKING vs HOLD in handleCancelBooking, assert that all payments share the same option. E.g.:const opts = new Set(bookingToDelete.payment.map(p => p.paymentOption)); if (opts.size > 1) throw new Error( `Expected one paymentOption for booking ${bookingToDelete.id}, got ${[...opts].join(", ")}` );packages/lib/payment/processNoShowFeeOnCancellation.ts (2)
21-26: Organizer self-cancel exemption: good.Clear early-return keeps downstream payment logic safe.
52-60: Metadata parsing and eligibility check: solid.Consistent with eventTypeMetaDataSchemaWithTypedApps and shouldChargeNoShowCancellationFee usage.
apps/web/playwright/cancellation-fee-warning.e2e.ts (1)
7-9: Parallel mode + per-test users: good isolation.Teardown looks correct for avoiding cross-test state.
apps/web/components/booking/CancelBooking.tsx (2)
144-156: Eligibility gating logic looks goodHost exemption, startTime, and payment guards are correct before delegating to
shouldChargeNoShowCancellationFee.
75-86: Verify runtime type of startTimeProps from GSSP serialize
Dateto ISO string. Ensure upstream passes an ISO string (with timezone) or convert toDatehere before use.Would you like me to add a small runtime guard like:
const bookingStart = typeof booking.startTime === "string" ? new Date(booking.startTime) : booking.startTime;and pass
booking: { ...booking, startTime: bookingStart }to the helper?packages/lib/payment/handleNoShowFee.ts (3)
121-138: Credential resolution and org fallback look solidGood defensive fallback to parent org credentials. Logging is appropriate.
145-158: Dynamic payment service resolution is correctKeying by
app.dirNameand checking for implementation/service presence is sound.
159-166: Idempotent charge and success email flow looks correctCharging with
booking.idas idempotency input and emailing first attendee post-success is appropriate.
packages/features/bookings/lib/handleCancelBooking/test/handleCancelBooking.test.ts
Show resolved
Hide resolved
alishaz-polymath
deleted the
devin/1757001996-no-show-cancellation-fees
branch
* feat: add time-based cancellation fees for no-show fee events - Add configurable time threshold (minutes/hours/days) for cancellation fees - Show warnings during booking submission and cancellation - Automatically charge fees when bookings cancelled within threshold - Exempt organizer/admin cancellations from fees - Extend existing no-show fee infrastructure Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com> * fix: replace any type with proper type guards for metadata - Use Record<string, unknown> instead of any for metadata type - Add proper type assertions for nested metadata properties - Maintain type safety while avoiding ESLint no-explicit-any warning Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com> * fix: handle JsonValue type compatibility in shouldChargeCancellationFee - Add proper type guard for metadata JsonValue from Prisma - Ensure metadata is object before casting to Record<string, unknown> - Fixes TypeScript error on line 390 in handleCancelBooking.ts Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com> * Refactor Devin changes to Stripe options * Undo Devin changes to advanced tab * Add translations * Pass props to CancelBooking * Display no show fee charge for attendee * WIP * Anstract shouldChargeNoSHowCancellationFee * Abstract `handleNoShowFee` * Add to * Refactor `chargeCard.handler` * Remove Devin code * Type fix in `shouldChargeNoShowCancellationFee` * Create `processNoSHowFeeOnCancellation` * Process no show fee on cancellation * Type fix * Skip processing no show fee if organizer or admin is cancelling * Add translation * Dynamically get and in * Remove unused translations * Undo dev change * Refactor logic * Type fix * remove any * revert WEBAPP_URL_FOR_OAUTH * Clean up console.log remnants * test: add comprehensive tests for time-based cancellation fees - Add unit tests for shouldChargeCancellationFee function with time thresholds and role exemptions - Add handleCancelBooking integration tests for organizer/team admin exemptions - Add handleCancelBooking test for attendee fee charging within time threshold - Add UI component tests for cancellation fee warning display - Add E2E test for cancellation fee warning during booking flow All 23 new tests pass successfully, covering: - Time-based logic with different units (minutes, hours, days) - Role-based exemptions (organizer, team admin vs regular attendee) - Payment charging integration with HOLD payment option - UI warning display during booking submission and cancellation - Edge cases like invalid metadata and past bookings Fixed lint issues: - Replaced 'any' types with proper Record<string, unknown> - Fixed Playwright test.skip() usage with ESLint disable comments Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com> * test: add comprehensive tests for time-based cancellation fees - Add unit tests for shouldChargeCancellationFee function with time thresholds and role exemptions - Add handleCancelBooking integration tests for organizer/team admin exemptions - Add handleCancelBooking test for attendee fee charging within time threshold - Add UI component tests for cancellation fee warning display - Add E2E test for cancellation fee warning during booking flow All unit and integration tests pass successfully. E2E test implementation complete. Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com> * test: add comprehensive tests for time-based cancellation fees - Add unit tests for shouldChargeCancellationFee function with time thresholds and role exemptions - Add handleCancelBooking integration tests for organizer/team admin exemptions - Add handleCancelBooking test for attendee fee charging within time threshold - Add UI component tests for cancellation fee warning display - Add E2E test for cancellation fee warning during booking flow - Fix TypeScript issues and prettier formatting - All tests follow existing Cal.com patterns and pass locally Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com> * test: add comprehensive unit and E2E tests for time-based cancellation fees - Add unit tests for shouldChargeCancellationFee function with time thresholds and role exemptions - Add handleCancelBooking integration tests for organizer/team admin exemptions and attendee fee charging - Add UI component tests for cancellation fee warning display in CancelBooking component - Add E2E test for cancellation fee warning during booking flow - Fix TypeScript errors in CancelBooking.tsx with proper type annotations - All tests follow existing Cal.com patterns and use proper mocking/test utilities Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com> * Revert dev change * Add missing translation * Remove Devin code * Revert Devin changes * test: fix attendee cancellation test logic and ensure comprehensive test coverage - Fix userId in attendee cancellation test to properly test fee charging - Ensure all test files follow Cal.com patterns with proper mocking - Add comprehensive coverage for time thresholds, role exemptions, and UI warnings - All tests pass locally with proper timezone handling Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com> * Fix tests * Remove reverted PR translations --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Syed Ali Shahbaz <52925846+alishaz-polymath@users.noreply.github.com>
5 participants
What does this PR do?
Extends Cal.com's existing no-show fee system to support time-based cancellation fees. When a booking is cancelled within a configurable time frame (minutes/hours/days), the system automatically charges the held no-show fee. The feature includes:
https://www.loom.com/share/822045a284c948299e98a58d77a32b52
Requested by: joe@cal.com (@joeauyeung)
Link to Devin run: https://app.devin.ai/sessions/37ae4e54c6d44a2cb304c6c844588374
Key Implementation Details
Schema Extensions
stripepayment/zod.tswithcancellationFeeEnabled,cancellationFeeTimeValue, andcancellationFeeTimeUnitfieldsEventType.metadata.apps.stripestructure for consistencyUI Components
Backend Logic
shouldChargeCancellationFeefunction inhandleCancelBooking.tsHow should this be tested?
Prerequisites
Test Scenarios
Configuration Testing
Cancellation Fee Charging
Role-based Exemptions
Edge Cases
Critical Review Areas
Prisma.JsonValuemetadata. Review:Mandatory Tasks (DO NOT REMOVE)
Checklist
yarn type-check:ci --force)TZ=UTC yarn test)