Conversation
|
@hackice20 is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds an optional boolean Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
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. Comment |
github-actions
bot
added
emails
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/types/Calendar.d.ts (1)
226-226: Additive type change looks good; consider grouping near other "hide" flags.*Optional nit: place
hideBranding?: booleanalongsidehideCalendarNotes/hideCalendarEventDetailsfor discoverability.packages/trpc/server/routers/viewer/bookings/confirm.handler.ts (1)
414-417: Bug: message is always "confirmed".Template literal makes the condition truthy. Use the boolean directly.
- const message = `Booking ${confirmed}` ? "confirmed" : "rejected"; + const message = confirmed ? "confirmed" : "rejected";
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
packages/emails/src/templates/BaseScheduledEmail.tsx(1 hunks)packages/features/CalendarEventBuilder.ts(1 hunks)packages/features/bookings/lib/handleNewBooking.ts(1 hunks)packages/features/bookings/lib/payment/getBooking.ts(1 hunks)packages/trpc/server/routers/viewer/bookings/confirm.handler.ts(1 hunks)packages/types/Calendar.d.ts(1 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:
packages/emails/src/templates/BaseScheduledEmail.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:
packages/emails/src/templates/BaseScheduledEmail.tsxpackages/features/bookings/lib/handleNewBooking.tspackages/types/Calendar.d.tspackages/features/CalendarEventBuilder.tspackages/features/bookings/lib/payment/getBooking.tspackages/trpc/server/routers/viewer/bookings/confirm.handler.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:
packages/emails/src/templates/BaseScheduledEmail.tsxpackages/features/bookings/lib/handleNewBooking.tspackages/types/Calendar.d.tspackages/features/CalendarEventBuilder.tspackages/features/bookings/lib/payment/getBooking.tspackages/trpc/server/routers/viewer/bookings/confirm.handler.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/handleNewBooking.tspackages/types/Calendar.d.tspackages/features/CalendarEventBuilder.tspackages/features/bookings/lib/payment/getBooking.tspackages/trpc/server/routers/viewer/bookings/confirm.handler.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-27T16:39:38.192Z
Learning: anglerfishlyy successfully implemented CAL-3076 email invitation feature for Cal.com team event-types in PR #23312. The feature allows inviting people via email directly from assignment flow, with automatic team invitation if email doesn't belong to existing team member. Implementation includes Host type modifications (userId?: number, email?: string, isPending?: boolean), CheckedTeamSelect component updates with CreatableSelect, TRPC schema validation with zod email validation, and integration with existing teamInvite system.
📚 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:
packages/features/bookings/lib/handleNewBooking.tspackages/features/bookings/lib/payment/getBooking.tspackages/trpc/server/routers/viewer/bookings/confirm.handler.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/features/bookings/lib/payment/getBooking.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 webhook DTOs in packages/features/webhooks/lib/dto/types.ts, the booking fields are restricted structures containing only specific fields (id, eventTypeId, userId, and sometimes additional fields like startTime or smsReminderNumber) rather than full database booking objects, so there are no security or PII leakage concerns when using these booking objects in webhook payloads.
Applied to files:
packages/features/bookings/lib/payment/getBooking.ts
📚 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/payment/getBooking.ts
🔇 Additional comments (5)
packages/emails/src/templates/BaseScheduledEmail.tsx (1)
67-67: Confirm footer is also hidden when branding is disabled.You now set
hideLogobased onplatformClientId || hideBranding. If BaseEmailHtml has a separatehideFooter/similar prop, pass it too; otherwise verifyhideLogocontrols both logo and footer as intended by the PR.packages/features/bookings/lib/payment/getBooking.ts (2)
37-41: Good: minimal Prisma select for owner.Selecting only
owner.hideBrandingfollows our “select-only fields you need” guideline.
197-198: Webhook payload surface change — verify external webhook consumers accept the new fieldevt is spread into EventPayloadType payloads; adding hideBranding will add a new boolean to webhook payloads. Verify external consumers tolerate unknown fields or explicitly add hideBranding to the webhook DTO/docs.
Locations:
- packages/features/bookings/lib/payment/getBooking.ts:197-198 (hideBranding added)
- packages/trpc/server/routers/viewer/bookings/confirm.handler.ts:379-381 (webhookData = { ...evt, ... }); 402 (related hideBranding usage)
- packages/trpc/server/routers/viewer/webhook/testTrigger.handler.ts:21 (EventPayloadType usage)
packages/features/CalendarEventBuilder.ts (1)
269-275: Builder extension LGTM.
withHideBrandingis consistent with existing builder patterns and keeps the flag optional.packages/features/bookings/lib/handleNewBooking.ts (1)
1273-1274: Plumbing flag is correct — DB loader selects owner.hideBranding; confirm default fallback.getEventTypesFromDBSelect includes owner.hideBranding (packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts), so getEventType(...) will have that field when loaded from the DB and .withHideBranding(!!eventType.owner?.hideBranding) is valid. getEventType can fall back to getDefaultEvent(...) for slug-only cases — confirm the default event includes owner.hideBranding (or accept the optional chaining).
| hideBranding: !!booking.eventType?.owner?.hideBranding, | ||
| ...(platformClientParams ? platformClientParams : {}), |
There was a problem hiding this comment.
Limit Prisma owner selection to the needed field.
Upstream you use owner: true in the eventType select; restrict it to avoid pulling extra PII.
Apply this diff in the booking query’s eventType.select:
- owner: true,
+ owner: { select: { hideBranding: true } },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| hideBranding: !!booking.eventType?.owner?.hideBranding, | |
| ...(platformClientParams ? platformClientParams : {}), | |
| eventType: { | |
| select: { | |
| // ...other selected fields... | |
| owner: { select: { hideBranding: true } }, | |
| }, | |
| }, |
🤖 Prompt for AI Agents
In packages/trpc/server/routers/viewer/bookings/confirm.handler.ts around lines
248-249, the booking query currently selects the entire owner object via `owner:
true`; change the Prisma select to only request the specific field(s) needed
(e.g. replace `owner: true` with `owner: { select: { hideBranding: true } }`) so
PII isn’t pulled unnecessarily; ensure any code that reads the value still
accesses the same property path.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
supalarry
left a comment
supalarry
left a comment
There was a problem hiding this comment.
Thank you for the contribution! I want to brainstorm about the solution - we already have a function shouldHideBrandingForEvent that also takes into account hideBranding setting not only on individual user event but also team events and also checks organization settings so we need to check more than just 1 person's individual settings to fix this for all users.
My instinct tells that we probably need to use shouldHideBrandingForEvent instead of saying hideBranding: !!booking.eventType?.owner?.hideBranding
CarinaWolli
added
High priority
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx(4 hunks)apps/web/modules/bookings/views/bookings-single-view.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/modules/bookings/views/bookings-single-view.getServerSideProps.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/modules/bookings/views/bookings-single-view.getServerSideProps.tsxapps/web/modules/bookings/views/bookings-single-view.tsx
**/*.{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/modules/bookings/views/bookings-single-view.getServerSideProps.tsxapps/web/modules/bookings/views/bookings-single-view.tsx
🧠 Learnings (5)
📓 Common learnings
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-27T16:39:38.192Z
Learning: anglerfishlyy successfully implemented CAL-3076 email invitation feature for Cal.com team event-types in PR #23312. The feature allows inviting people via email directly from assignment flow, with automatic team invitation if email doesn't belong to existing team member. Implementation includes Host type modifications (userId?: number, email?: string, isPending?: boolean), CheckedTeamSelect component updates with CreatableSelect, TRPC schema validation with zod email validation, and integration with existing teamInvite system.
📚 Learning: 2025-08-29T22:57:31.407Z
Learnt from: bandhan-majumder
PR: calcom/cal.com#23454
File: packages/features/bookings/Booker/components/EventMeta.tsx:16-16
Timestamp: 2025-08-29T22:57:31.407Z
Learning: In Cal.com's Booker architecture, components become client-side through BookerStoreProvider.tsx which has "use client". Any component using useBookerStoreContext automatically runs on the client and should use client-appropriate utilities like markdownToSafeHTMLClient, regardless of whether they have explicit "use client" directives.
Applied to files:
apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx
📚 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 webhook DTOs in packages/features/webhooks/lib/dto/types.ts, the booking fields are restricted structures containing only specific fields (id, eventTypeId, userId, and sometimes additional fields like startTime or smsReminderNumber) rather than full database booking objects, so there are no security or PII leakage concerns when using these booking objects in webhook payloads.
Applied to files:
apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx
📚 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:
apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx
📚 Learning: 2025-07-15T12:59:34.389Z
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Applied to files:
apps/web/modules/bookings/views/bookings-single-view.tsx
🧬 Code graph analysis (2)
apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx (1)
packages/lib/server/queries/bookings/canViewSensitiveBookingData.ts (1)
canViewSensitiveBookingData(47-90)
apps/web/modules/bookings/views/bookings-single-view.tsx (1)
packages/lib/server/queries/bookings/canViewSensitiveBookingData.ts (1)
canViewSensitiveBookingData(47-90)
🔇 Additional comments (5)
apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx (3)
15-15: Good import: using centralized permission check.Importing
canViewSensitiveBookingDataaligns the page with server-side privacy rules.
229-240: Correct gating of internal note presets.Short-circuiting when
!teamId || !canViewSensitiveDataprevents unnecessary DB reads and exposure.
278-279: Prop plumbed correctly.
canViewSensitiveBookingDatais included in SSR props; matches the client usage.apps/web/modules/bookings/views/bookings-single-view.tsx (2)
107-107: Prop wiring looks good.Destructuring
canViewSensitiveBookingDatafromPagePropsaligns with the new SSR prop.
705-744: UI gate for UTMs is correct, but rely on server-side scrub too.Showing UTMs only when
canViewSensitiveBookingDatais true is right. Ensure the server also removesbookingInfo.trackingfor unauthorized viewers to avoid SSR leakage. See server comment.
| // Check if user can view sensitive booking data (hidden fields and UTM parameters) | ||
| const canViewSensitiveData = userId | ||
| ? await canViewSensitiveBookingData(userId, { | ||
| id: bookingInfo.id, | ||
| userId: bookingInfo.user?.id || null, | ||
| eventType: { | ||
| id: eventType.id, | ||
| teamId: eventType.teamId, | ||
| schedulingType: eventType.schedulingType, | ||
| userId: eventType.userId, | ||
| hosts: eventType.hosts, | ||
| team: eventType.team, | ||
| parent: eventType.parent, | ||
| }, | ||
| attendees: bookingInfo.attendees, | ||
| }) | ||
| : false; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Privacy leak: UTMs still serialized to the client when access is denied.
Even though the UI hides UTMs, bookingInfo.tracking remains in the SSR props, so unauthorized viewers can read it from the page source/devtools. Strip UTMs server‑side when !canViewSensitiveData.
Apply this diff near your existing sanitization block:
// Check if user can view sensitive booking data (hidden fields and UTM parameters)
const canViewSensitiveData = userId
? await canViewSensitiveBookingData(userId, {
...
})
: false;
- if (!canViewSensitiveData) {
+ if (!canViewSensitiveData) {
// Removing hidden fields from responses
for (const key in bookingInfo.responses) {
const field = eventTypeRaw.bookingFields.find((field) => field.name === key);
if (field && !!field.hidden) {
delete bookingInfo.responses[key];
}
}
+ // Also remove UTM/tracking data from props to prevent leakage
+ // (client UI already gates display, but SSR serialization would expose it otherwise)
+ if ("tracking" in bookingInfo) {
+ // Preserve shape by nulling rather than deleting, to avoid unexpected reads
+ (bookingInfo as typeof bookingInfo & { tracking?: unknown }).tracking = undefined;
+ }
}Optionally, precompute hidden field names to avoid O(n*m) lookups:
- for (const key in bookingInfo.responses) {
- const field = eventTypeRaw.bookingFields.find((field) => field.name === key);
- if (field && !!field.hidden) {
- delete bookingInfo.responses[key];
- }
- }
+ const hidden = new Set(eventTypeRaw.bookingFields.filter((f) => f.hidden).map((f) => f.name));
+ for (const key in bookingInfo.responses) {
+ if (hidden.has(key)) delete bookingInfo.responses[key];
+ }Run this script to find other client surfaces reading bookingInfo.tracking so you can confirm they’re also privacy-gated or sanitized server-side:
Also applies to: 216-224
🏁 Script executed:
#!/bin/bash
rg -nP -C3 '\bbookingInfo\.tracking\b|tracking:\s*bookingInfo\.tracking' --type=ts --type=tsxLength of output: 27
🏁 Script executed:
#!/bin/bash
# Search for any usage of bookingInfo.tracking across .ts and .tsx files
rg -nP -C3 '\bbookingInfo\.tracking\b|tracking:\s*bookingInfo\.tracking' -g '*.ts' -g '*.tsx'Length of output: 645
Strip UTM/tracking data for unauthorized users
In apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx, immediately after your hidden-fields sanitization (!canViewSensitiveData), add removal of bookingInfo.tracking so it isn’t serialized to the client:
if (!canViewSensitiveData) {
// Removing hidden fields from responses
const hidden = new Set(eventTypeRaw.bookingFields.filter((f) => f.hidden).map((f) => f.name));
for (const key in bookingInfo.responses) {
if (hidden.has(key)) delete bookingInfo.responses[key];
}
+ // Also null out UTM/tracking to prevent leakage via SSR props
+ ;(bookingInfo as typeof bookingInfo & { tracking?: unknown }).tracking = undefined;
}Optional refactor: precompute hidden as shown above to avoid repeated .find() lookups.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Check if user can view sensitive booking data (hidden fields and UTM parameters) | |
| const canViewSensitiveData = userId | |
| ? await canViewSensitiveBookingData(userId, { | |
| id: bookingInfo.id, | |
| userId: bookingInfo.user?.id || null, | |
| eventType: { | |
| id: eventType.id, | |
| teamId: eventType.teamId, | |
| schedulingType: eventType.schedulingType, | |
| userId: eventType.userId, | |
| hosts: eventType.hosts, | |
| team: eventType.team, | |
| parent: eventType.parent, | |
| }, | |
| attendees: bookingInfo.attendees, | |
| }) | |
| : false; | |
| if (!canViewSensitiveData) { | |
| // Removing hidden fields from responses | |
| const hidden = new Set( | |
| eventTypeRaw.bookingFields | |
| .filter((f) => f.hidden) | |
| .map((f) => f.name) | |
| ); | |
| for (const key in bookingInfo.responses) { | |
| if (hidden.has(key)) delete bookingInfo.responses[key]; | |
| } | |
| // Also null out UTM/tracking to prevent leakage via SSR props | |
| ;(bookingInfo as typeof bookingInfo & { tracking?: unknown }).tracking = undefined; | |
| } |
🤖 Prompt for AI Agents
In apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx
around lines 198 to 215, after you sanitize hidden fields when
!canViewSensitiveData you must also remove bookingInfo.tracking so UTM/tracking
data is not serialized to the client; update the branch where sensitive data is
denied to set bookingInfo.tracking = undefined (or delete bookingInfo.tracking)
immediately after the hidden-fields sanitization, and optionally precompute the
hidden lookup into a local variable to avoid repeated .find() calls when
computing hidden fields.
kart1ka
left a comment
kart1ka
left a comment
There was a problem hiding this comment.
This does not work. Cal.com footer is still being shown in the emails even with the "Disable branding" toggled on. See the video attached. Also there are lots of type errors and a test failing.
@kart1ka it is working on the basic user-feat. But not for teams. I will try to look into this. Would love to get advice and guidance. If you can accept my linkedin request.working for generic usersdemo.mp4 |
hide branding for team emails@volnei @kart1ka @supalarry @CarinaWolliScreencast.From.2025-10-01.19-18-18.mp4 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/features/bookings/lib/handleNewBooking.ts (3)
2201-2201: Consider using property shorthandThe object property assignment
hideBranding: hideBrandingcan be simplified to justhideBrandingusing ES6 property shorthand.- hideBranding: hideBranding, + hideBranding,
2382-2382: Consider using property shorthandThe object property assignment
hideBranding: hideBrandingcan be simplified to justhideBrandingusing ES6 property shorthand.- hideBranding: hideBranding, + hideBranding,
2394-2394: Consider using property shorthandThe object property assignment
hideBranding: hideBrandingcan be simplified to justhideBrandingusing ES6 property shorthand.- hideBranding: hideBranding, + hideBranding,
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/features/bookings/lib/handleNewBooking.ts(6 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/handleNewBooking.ts
**/*.{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:
packages/features/bookings/lib/handleNewBooking.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:
packages/features/bookings/lib/handleNewBooking.ts
🧬 Code graph analysis (1)
packages/features/bookings/lib/handleNewBooking.ts (1)
packages/lib/hideBranding.ts (1)
shouldHideBrandingForEvent(76-113)
🔇 Additional comments (2)
packages/features/bookings/lib/handleNewBooking.ts (2)
59-59: LGTM: Import follows best practicesThe named import aligns with the coding guidelines.
1321-1321: LGTM: hideBranding correctly passed to event builderThe branding flag is properly threaded through the CalendarEventBuilder.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/features/bookings/lib/handleBookingRequested.ts (1)
95-101: Pass hideBranding into immediate emailssendOrganizerRequestEmail / sendAttendeeRequestEmailAndSMS likely read from evt; ensure the flag is present to actually hide branding.
Apply this diff:
- await sendOrganizerRequestEmail({ ...evt }, booking?.eventType?.metadata as EventTypeMetadata); + await sendOrganizerRequestEmail({ ...evt, hideBranding }, booking?.eventType?.metadata as EventTypeMetadata); await sendAttendeeRequestEmailAndSMS( - { ...evt }, + { ...evt, hideBranding }, evt.attendees[0], booking?.eventType?.metadata as EventTypeMetadata );
🧹 Nitpick comments (6)
packages/trpc/server/routers/viewer/bookings/get.handler.ts (4)
72-76: Strengthen BookingField typing/validationBookingField is permissive. Validate shape before use (name: string, hidden?: boolean) to avoid unexpected keys.
Example with zod:
const BookingFieldSchema = z.object({ name: z.string(), hidden: z.boolean().optional(), }).strict(); type BookingField = z.infer<typeof BookingFieldSchema>;
698-732: Good: batched bookingFields fetch; add item-level validation and small cacheThe batch fetch is solid. Add per-item validation and consider a tiny LRU per process to avoid re-parsing for repeated eventTypeIds within the same request burst.
const fields = normalizeBookingFields(eventType.bookingFields).filter(f => typeof f?.name === 'string');
713-726: Normalizer should reject malformed itemsCurrently accepts any array; ensure items have string name to prevent mismatches during filtering.
const arr = Array.isArray(parsed) ? parsed : []; return arr.filter((x): x is BookingField => !!x && typeof x.name === 'string');
744-760: Per-booking permission check may be chatty; consider batchingcanViewSensitiveBookingData likely hits DB each time. If possible, precompute membership/host context (e.g., user’s team/org memberships and attendee email set) and pass hints or add an API that accepts multiple bookings.
Would you like a follow-up PR to batch/short‑circuit common-positive cases (e.g., when user is host for many bookings)?
packages/features/bookings/lib/handleBookingRequested.ts (2)
102-106: Reuse previously computed orgIdIf you compute orgId above, reuse it here to avoid a second DB call.
- const orgId = await getOrgIdFromMemberOrTeamId({ - memberId: booking.userId, - teamId: booking.eventType?.teamId, - }); + // orgId computed above
31-40: Type nullability consistencyowner.hideBranding is typed as boolean while team.* are boolean | null. Make owner.hideBranding boolean | null (or coerce to boolean at boundary) for consistency with DB and callers.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
packages/features/bookings/lib/handleBookingRequested.ts(4 hunks)packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts(2 hunks)packages/features/bookings/lib/payment/getBooking.ts(5 hunks)packages/trpc/server/routers/viewer/bookings/get.handler.ts(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/payment/getBooking.tspackages/features/bookings/lib/handleBookingRequested.tspackages/trpc/server/routers/viewer/bookings/get.handler.ts
**/*.{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:
packages/features/bookings/lib/payment/getBooking.tspackages/features/bookings/lib/handleBookingRequested.tspackages/trpc/server/routers/viewer/bookings/get.handler.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:
packages/features/bookings/lib/payment/getBooking.tspackages/features/bookings/lib/handleBookingRequested.tspackages/trpc/server/routers/viewer/bookings/get.handler.ts
🧠 Learnings (2)
📚 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/trpc/server/routers/viewer/bookings/get.handler.ts
📚 Learning: 2025-09-03T11:54:05.409Z
Learnt from: supalarry
PR: calcom/cal.com#23514
File: apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts:579-582
Timestamp: 2025-09-03T11:54:05.409Z
Learning: In calcom/cal.com bookings repository methods, when Prisma select uses `eventType: true`, all eventType fields including seatsShowAttendees are automatically included in the selection. Explicit field selection is not required when using `true` for nested relations.
Applied to files:
packages/trpc/server/routers/viewer/bookings/get.handler.ts
🧬 Code graph analysis (3)
packages/features/bookings/lib/payment/getBooking.ts (1)
packages/lib/hideBranding.ts (1)
shouldHideBrandingForEvent(76-113)
packages/features/bookings/lib/handleBookingRequested.ts (1)
packages/lib/hideBranding.ts (1)
shouldHideBrandingForEvent(76-113)
packages/trpc/server/routers/viewer/bookings/get.handler.ts (1)
packages/lib/server/queries/bookings/canViewSensitiveBookingData.ts (1)
canViewSensitiveBookingData(47-90)
⏰ 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). (3)
- GitHub Check: Tests / Unit
- GitHub Check: Linters / lint
- GitHub Check: Type check / check-types
🔇 Additional comments (7)
packages/features/bookings/lib/payment/getBooking.ts (4)
6-6: Importing shared branding logic — LGTMCentralizing the decision via shouldHideBrandingForEvent is the right move.
41-43: Minimal owner select — LGTMSelecting only id and hideBranding keeps the query lean. As per coding guidelines.
90-96: Team branding fields selection — LGTMSelecting team.hideBranding and parent.hideBranding is sufficient for downstream logic. As per coding guidelines.
228-229: Propagating hideBranding into CalendarEvent — LGTMThis enables email/templates to respect the flag.
packages/trpc/server/routers/viewer/bookings/get.handler.ts (2)
545-547: Selecting Team.parentId for ownership resolution — LGTMRequired for MANAGED events’ parent team check; minimal select kept. As per coding guidelines.
14-14: Plumbing + assignment — LGTMImport/use of canViewSensitiveBookingData and final responses assignment look correct.
Also applies to: 805-806
packages/features/bookings/lib/handleBookingRequested.ts (1)
142-143: Workflow scheduling receives hideBranding — LGTMThis ensures workflow-driven emails respect branding.
| // Calculate hide branding setting using comprehensive logic | ||
| const hideBranding = booking?.eventType?.id | ||
| ? await shouldHideBrandingForEvent({ | ||
| eventTypeId: booking.eventType.id, | ||
| team: booking.eventType.team | ||
| ? { | ||
| hideBranding: booking.eventType.team.hideBranding, | ||
| parent: booking.eventType.team.parent | ||
| ? { | ||
| hideBranding: booking.eventType.team.parent.hideBranding, | ||
| } | ||
| : null, | ||
| } | ||
| : null, | ||
| owner: booking.eventType.owner | ||
| ? { | ||
| id: booking.eventType.owner.id, | ||
| hideBranding: booking.eventType.owner.hideBranding, | ||
| } | ||
| : null, | ||
| organizationId: booking.eventType.team?.parentId || null, | ||
| }).catch(() => { | ||
| // Fallback to simple logic if comprehensive check fails | ||
| return !!booking.eventType?.owner?.hideBranding; | ||
| }) | ||
| : false; | ||
|
|
There was a problem hiding this comment.
Compute orgId first and broaden fallback; ensure parity with getBooking
For user events, organizationId is always null here, so shouldHideBrandingForEvent won’t load owner profile; result may be wrong vs getBooking.ts. Also, catch fallback ignores team branding.
Apply this diff:
- // Calculate hide branding setting using comprehensive logic
- const hideBranding = booking?.eventType?.id
+ // Resolve orgId once (used for branding + webhooks)
+ const orgId = await getOrgIdFromMemberOrTeamId({
+ memberId: booking.userId,
+ teamId: booking.eventType?.teamId,
+ });
+
+ // Calculate hide branding setting using comprehensive logic
+ const hideBranding = booking?.eventType?.id
? await shouldHideBrandingForEvent({
eventTypeId: booking.eventType.id,
team: booking.eventType.team
? {
hideBranding: booking.eventType.team.hideBranding,
parent: booking.eventType.team.parent
? {
hideBranding: booking.eventType.team.parent.hideBranding,
}
: null,
}
: null,
owner: booking.eventType.owner
? {
id: booking.eventType.owner.id,
hideBranding: booking.eventType.owner.hideBranding,
}
: null,
- organizationId: booking.eventType.team?.parentId || null,
- }).catch(() => {
- // Fallback to simple logic if comprehensive check fails
- return !!booking.eventType?.owner?.hideBranding;
- })
+ organizationId: orgId || null,
+ }).catch(() =>
+ Boolean(
+ booking.eventType?.team?.hideBranding ??
+ booking.eventType?.team?.parent?.hideBranding ??
+ booking.eventType?.owner?.hideBranding
+ )
+ )
: false;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Calculate hide branding setting using comprehensive logic | |
| const hideBranding = booking?.eventType?.id | |
| ? await shouldHideBrandingForEvent({ | |
| eventTypeId: booking.eventType.id, | |
| team: booking.eventType.team | |
| ? { | |
| hideBranding: booking.eventType.team.hideBranding, | |
| parent: booking.eventType.team.parent | |
| ? { | |
| hideBranding: booking.eventType.team.parent.hideBranding, | |
| } | |
| : null, | |
| } | |
| : null, | |
| owner: booking.eventType.owner | |
| ? { | |
| id: booking.eventType.owner.id, | |
| hideBranding: booking.eventType.owner.hideBranding, | |
| } | |
| : null, | |
| organizationId: booking.eventType.team?.parentId || null, | |
| }).catch(() => { | |
| // Fallback to simple logic if comprehensive check fails | |
| return !!booking.eventType?.owner?.hideBranding; | |
| }) | |
| : false; | |
| // Resolve orgId once (used for branding + webhooks) | |
| const orgId = await getOrgIdFromMemberOrTeamId({ | |
| memberId: booking.userId, | |
| teamId: booking.eventType?.teamId, | |
| }); | |
| // Calculate hide branding setting using comprehensive logic | |
| const hideBranding = booking?.eventType?.id | |
| ? await shouldHideBrandingForEvent({ | |
| eventTypeId: booking.eventType.id, | |
| team: booking.eventType.team | |
| ? { | |
| hideBranding: booking.eventType.team.hideBranding, | |
| parent: booking.eventType.team.parent | |
| ? { | |
| hideBranding: booking.eventType.team.parent.hideBranding, | |
| } | |
| : null, | |
| } | |
| : null, | |
| owner: booking.eventType.owner | |
| ? { | |
| id: booking.eventType.owner.id, | |
| hideBranding: booking.eventType.owner.hideBranding, | |
| } | |
| : null, | |
| organizationId: orgId || null, | |
| }).catch(() => | |
| Boolean( | |
| booking.eventType?.team?.hideBranding ?? | |
| booking.eventType?.team?.parent?.hideBranding ?? | |
| booking.eventType?.owner?.hideBranding | |
| ) | |
| ) | |
| : false; |
🤖 Prompt for AI Agents
In packages/features/bookings/lib/handleBookingRequested.ts around lines 66 to
92, compute organizationId first (const organizationId =
booking.eventType.team?.parentId ?? null) and pass it into
shouldHideBrandingForEvent; call shouldHideBrandingForEvent with the same
owner/team shape as now but ensure owner is passed when present; broaden the
catch fallback to return booking.eventType?.owner?.hideBranding ||
booking.eventType?.team?.hideBranding || false so team branding is considered
when the comprehensive check fails; make these changes to match the parity and
behavior of getBooking.
| const hideBranding = booking.eventType?.id | ||
| ? await shouldHideBrandingForEvent({ | ||
| eventTypeId: booking.eventType.id, | ||
| team: booking.eventType.team | ||
| ? { | ||
| hideBranding: booking.eventType.team.hideBranding, | ||
| parent: booking.eventType.team.parent | ||
| ? { | ||
| hideBranding: booking.eventType.team.parent.hideBranding, | ||
| } | ||
| : null, | ||
| } | ||
| : null, | ||
| owner: booking.eventType.owner | ||
| ? { | ||
| id: booking.eventType.owner.id, | ||
| hideBranding: booking.eventType.owner.hideBranding, | ||
| } | ||
| : null, | ||
| organizationId: organizerOrganizationId ?? booking.eventType.team?.parentId ?? null, | ||
| }).catch(() => !!booking.eventType?.owner?.hideBranding) | ||
| : false; | ||
|
|
There was a problem hiding this comment.
Catch fallback ignores team branding; broaden fallback
If shouldHideBrandingForEvent throws, falling back only to owner.hideBranding can show branding for team events that should hide it. Consider team and parent flags too.
Apply this diff:
- }).catch(() => !!booking.eventType?.owner?.hideBranding)
+ }).catch(() =>
+ Boolean(
+ booking.eventType?.team?.hideBranding ??
+ booking.eventType?.team?.parent?.hideBranding ??
+ booking.eventType?.owner?.hideBranding
+ )
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const hideBranding = booking.eventType?.id | |
| ? await shouldHideBrandingForEvent({ | |
| eventTypeId: booking.eventType.id, | |
| team: booking.eventType.team | |
| ? { | |
| hideBranding: booking.eventType.team.hideBranding, | |
| parent: booking.eventType.team.parent | |
| ? { | |
| hideBranding: booking.eventType.team.parent.hideBranding, | |
| } | |
| : null, | |
| } | |
| : null, | |
| owner: booking.eventType.owner | |
| ? { | |
| id: booking.eventType.owner.id, | |
| hideBranding: booking.eventType.owner.hideBranding, | |
| } | |
| : null, | |
| organizationId: organizerOrganizationId ?? booking.eventType.team?.parentId ?? null, | |
| }).catch(() => !!booking.eventType?.owner?.hideBranding) | |
| : false; | |
| const hideBranding = booking.eventType?.id | |
| ? await shouldHideBrandingForEvent({ | |
| eventTypeId: booking.eventType.id, | |
| team: booking.eventType.team | |
| ? { | |
| hideBranding: booking.eventType.team.hideBranding, | |
| parent: booking.eventType.team.parent | |
| ? { | |
| hideBranding: booking.eventType.team.parent.hideBranding, | |
| } | |
| : null, | |
| } | |
| : null, | |
| owner: booking.eventType.owner | |
| ? { | |
| id: booking.eventType.owner.id, | |
| hideBranding: booking.eventType.owner.hideBranding, | |
| } | |
| : null, | |
| organizationId: organizerOrganizationId ?? booking.eventType.team?.parentId ?? null, | |
| }).catch(() => | |
| Boolean( | |
| booking.eventType?.team?.hideBranding ?? | |
| booking.eventType?.team?.parent?.hideBranding ?? | |
| booking.eventType?.owner?.hideBranding | |
| ) | |
| ) | |
| : false; |
🤖 Prompt for AI Agents
In packages/features/bookings/lib/payment/getBooking.ts around lines 164 to 186,
the catch fallback currently returns only booking.eventType?.owner?.hideBranding
when shouldHideBrandingForEvent throws, which ignores team and parent
hideBranding flags; change the catch to compute a robust fallback that checks
owner.hideBranding OR team.hideBranding OR team.parent.hideBranding (in that
precedence or combined OR) and return that boolean so team or parent settings
are respected when the service call fails.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/app-store/wipemycalother/lib/reschedule.ts (1)
109-113: Guard against empty attendees.
mainAttendee may be undefined; use optional chaining to avoid a crash.-const tAttendees = await getTranslation(mainAttendee.locale ?? "en", "common"); +const tAttendees = await getTranslation(mainAttendee?.locale ?? "en", "common");packages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts (2)
152-163: Addslugtoeventselect; it’s used later.
event.slugis referenced for builder type and webhook payload but not selected; it will be undefined. Include it in the select.if (bookingToReschedule.eventTypeId) { event = await prisma.eventType.findUniqueOrThrow({ select: { title: true, schedulingType: true, - recurringEvent: true, + recurringEvent: true, + slug: true, }, where: { id: bookingToReschedule.eventTypeId, }, }); }Also applies to: 218-230, 336-340
320-327: Pass the correct metadata source.
You selected metadata on bookingToReschedule.eventType, not onevent. Use that to avoid dropping metadata in emails.- await sendRequestRescheduleEmailAndSMS( + await sendRequestRescheduleEmailAndSMS( builder.calendarEvent, { rescheduleLink: builder.rescheduleLink, }, - eventType?.metadata as EventTypeMetadata + bookingToReschedule.eventType?.metadata as EventTypeMetadata );packages/app-store/vital/lib/reschedule.ts (1)
109-113: Avoid NPE when attendees array is empty.
Use optional chaining when deriving the locale.-const tAttendees = await getTranslation(mainAttendee.locale ?? "en", "common"); +const tAttendees = await getTranslation(mainAttendee?.locale ?? "en", "common");
🧹 Nitpick comments (11)
packages/features/bookings/lib/handleCancelBooking.ts (1)
86-106: Remove unreachableeventTypeIdfallback
SincegetBookingToDeletealways selects a non-nulleventType(with itsid), the?.id || 0fallback in handleCancelBooking.ts is never used—replace it withbookingToDelete.eventType.idfor clarity.packages/app-store/wipemycalother/lib/reschedule.ts (4)
37-64: Trim Prisma select to only needed fields.
parentId and teamId on eventType aren’t referenced here; remove to cut overfetch. Team.parentId is used; keep that.eventType: { select: { - id: true, - teamId: true, - parentId: true, + id: true, metadata: true, hideOrganizerEmail: true, customReplyToEmail: true, owner: { select: { id: true, hideBranding: true, }, }, team: { select: { id: true, name: true, - parentId: true, + parentId: true, hideBranding: true, parent: { select: { hideBranding: true, }, }, }, }, }, },
185-209: Use builder API for branding and de‑dup mapping.
Prefer builder.withHideBranding(hideBranding) and consider extracting the booking→branding context mapping to a helper to avoid copy/paste across reschedule handlers.-const hideBranding = bookingToReschedule.eventType?.id +const hideBranding = bookingToReschedule.eventType?.id ? await shouldHideBrandingForEvent({ /* ... */ }).catch( () => !!bookingToReschedule.eventType?.owner?.hideBranding ) : false; -builder.calendarEvent.hideBranding = hideBranding; +builder.withHideBranding?.(hideBranding) ?? (builder.calendarEvent.hideBranding = hideBranding);
176-182: Log unknown error shapes, too.
Current catch drops non-Error values. Log a stringified fallback.-} catch (error) { - if (error instanceof Error) { - logger.error(error.message); - } -} +} catch (error) { + logger.error(error instanceof Error ? error.message : String(error)); +}
227-227: Prefer named export over default.
Improves tree‑shaking and refactors. Change to a named export if call sites permit.-export default Reschedule; +export { Reschedule };packages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts (2)
295-319: Use builder API and extract shared branding context mapping.
Call builder.withHideBranding and factor the eventType→branding-context mapping to a helper to avoid duplication across three files.-builder.calendarEvent.hideBranding = hideBranding; +builder.withHideBranding?.(hideBranding) ?? (builder.calendarEvent.hideBranding = hideBranding);
320-327: Wrap email send with error handling (parity with other flows).
Avoid failing the handler on transient email errors; log and continue.-await sendRequestRescheduleEmailAndSMS(/* ... */); +try { + await sendRequestRescheduleEmailAndSMS(/* ... */); +} catch (e) { + logger.error("sendRequestRescheduleEmailAndSMS failed", safeStringify(e)); +}packages/app-store/vital/lib/reschedule.ts (4)
37-64: Slim Prisma select to what’s used.
customReplyToEmail and eventType.parentId aren’t used in this module; drop them. Keep team.parentId (used for orgId).eventType: { select: { id: true, - parentId: true, metadata: true, hideOrganizerEmail: true, - customReplyToEmail: true, owner: { select: { id: true, hideBranding: true, }, }, team: { select: { id: true, name: true, parentId: true, hideBranding: true, parent: { select: { hideBranding: true, }, }, }, }, }, },
185-209: Use builder API and consider extracting shared mapping.
Set branding via builder.withHideBranding and DRY up the eventType→branding-context construction.-builder.calendarEvent.hideBranding = hideBranding; +builder.withHideBranding?.(hideBranding) ?? (builder.calendarEvent.hideBranding = hideBranding);
176-182: Harden error logging.
Don’t drop non-Error throwables.-} catch (error) { - // FIXME: error logging - non-Error type errors are currently discarded - if (error instanceof Error) { - logger.error(error.message); - } -} +} catch (error) { + logger.error(error instanceof Error ? error.message : String(error)); +}
227-227: Prefer named export.
Switch to named export if consumers allow.-export default Reschedule; +export { Reschedule };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
packages/app-store/vital/lib/reschedule.ts(3 hunks)packages/app-store/wipemycalother/lib/reschedule.ts(3 hunks)packages/features/bookings/lib/getBookingToDelete.ts(2 hunks)packages/features/bookings/lib/handleCancelBooking.ts(5 hunks)packages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/app-store/wipemycalother/lib/reschedule.tspackages/features/bookings/lib/getBookingToDelete.tspackages/features/bookings/lib/handleCancelBooking.tspackages/app-store/vital/lib/reschedule.tspackages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts
**/*.{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:
packages/app-store/wipemycalother/lib/reschedule.tspackages/features/bookings/lib/getBookingToDelete.tspackages/features/bookings/lib/handleCancelBooking.tspackages/app-store/vital/lib/reschedule.tspackages/trpc/server/routers/viewer/bookings/requestReschedule.handler.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:
packages/app-store/wipemycalother/lib/reschedule.tspackages/features/bookings/lib/getBookingToDelete.tspackages/features/bookings/lib/handleCancelBooking.tspackages/app-store/vital/lib/reschedule.tspackages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts
🧠 Learnings (4)
📚 Learning: 2025-09-03T11:54:05.409Z
Learnt from: supalarry
PR: calcom/cal.com#23514
File: apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts:579-582
Timestamp: 2025-09-03T11:54:05.409Z
Learning: In calcom/cal.com bookings repository methods, when Prisma select uses `eventType: true`, all eventType fields including seatsShowAttendees are automatically included in the selection. Explicit field selection is not required when using `true` for nested relations.
Applied to files:
packages/app-store/wipemycalother/lib/reschedule.tspackages/features/bookings/lib/getBookingToDelete.tspackages/app-store/vital/lib/reschedule.tspackages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts
📚 Learning: 2025-09-03T11:54:05.409Z
Learnt from: supalarry
PR: calcom/cal.com#23514
File: apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts:579-582
Timestamp: 2025-09-03T11:54:05.409Z
Learning: In calcom/cal.com bookings repository methods, when Prisma include uses `eventType: true`, all eventType fields including seatsShowAttendees are automatically included in the selection. Explicit field selection is not required when using `true` for nested relations.
Applied to files:
packages/app-store/wipemycalother/lib/reschedule.tspackages/features/bookings/lib/getBookingToDelete.tspackages/app-store/vital/lib/reschedule.tspackages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts
📚 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/trpc/server/routers/viewer/bookings/requestReschedule.handler.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 webhook DTOs in packages/features/webhooks/lib/dto/types.ts, the booking fields are restricted structures containing only specific fields (id, eventTypeId, userId, and sometimes additional fields like startTime or smsReminderNumber) rather than full database booking objects, so there are no security or PII leakage concerns when using these booking objects in webhook payloads.
Applied to files:
packages/features/bookings/lib/getBookingToDelete.ts
🧬 Code graph analysis (4)
packages/app-store/wipemycalother/lib/reschedule.ts (1)
packages/lib/hideBranding.ts (1)
shouldHideBrandingForEvent(76-113)
packages/features/bookings/lib/handleCancelBooking.ts (1)
packages/lib/hideBranding.ts (1)
shouldHideBrandingForEvent(76-113)
packages/app-store/vital/lib/reschedule.ts (1)
packages/lib/hideBranding.ts (1)
shouldHideBrandingForEvent(76-113)
packages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts (1)
packages/lib/hideBranding.ts (1)
shouldHideBrandingForEvent(76-113)
🔇 Additional comments (11)
packages/features/bookings/lib/handleCancelBooking.ts (3)
8-8: LGTM!The imports are correctly added to support the hideBranding functionality.
Also applies to: 24-24
322-322: LGTM!The computed
hideBrandingvalue is correctly added to theevtobject, ensuring cancellation emails respect the branding setting.
381-381: LGTM!The
hideBrandingflag is correctly passed tosendCancelledReminders, ensuring workflow reminders also respect the branding setting.packages/features/bookings/lib/getBookingToDelete.ts (3)
57-62: Hierarchical branding fields are correctly consumed.
The cancellation flow forwardsteam.parent.hideBrandingintoshouldHideBrandingForEventand uses it inresolveHideBrandingas theorganizationHideBrandingsource, confirming parent branding settings are honored.
47-47: Confirm necessity ofeventType.owner.idin deletion flow
No references toowner.idwere found in the booking deletion or cancellation logic—please verify that this field is actually consumed, or remove it to avoid unnecessary data fetching.
43-43: No changes needed: new fields are consumed
EventType.id is used in the hide-branding calculation, owner.id in the host/owner check, and team.parent.hideBranding in the hierarchical branding logic.packages/app-store/wipemycalother/lib/reschedule.ts (1)
8-8: LGTM: central branding logic imported.
Importing from @calcom/lib/hideBranding keeps branding decisions consistent.packages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts (3)
51-112: Prisma select looks good; keep “select” (no “include”) and avoid overfetch.
Fields chosen are used below (bookingFields, hideOrganizerEmail, customReplyToEmail, metadata, owner/team branding). No change requested.
252-291: Good switch to Promise.allSettled for external deletions.
Prevents one failure from cancelling the whole set.
1-396: Compliance check passed: no credential.key exposure in requestReschedule.handler.ts.packages/app-store/vital/lib/reschedule.ts (1)
8-8: LGTM: centralized branding import.
Keeps logic consistent across app-store integrations.
| log.debug("builder.calendarEvent", safeStringify(builder.calendarEvent)); | ||
|
|
There was a problem hiding this comment.
Reduce PII in debug logs.
builder.calendarEvent includes attendee/organizer PII. Log only essentials or behind a dev flag.
-log.debug("builder.calendarEvent", safeStringify(builder.calendarEvent));
+log.debug("calendarEvent snapshot", safeStringify({ uid: bookingToReschedule.uid, hideBranding: builder.calendarEvent.hideBranding }));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| log.debug("builder.calendarEvent", safeStringify(builder.calendarEvent)); | |
| log.debug( | |
| "calendarEvent snapshot", | |
| safeStringify({ | |
| uid: bookingToReschedule.uid, | |
| hideBranding: builder.calendarEvent.hideBranding, | |
| }), | |
| ); |
🤖 Prompt for AI Agents
In packages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts
around lines 293-294, the debug log prints builder.calendarEvent which contains
attendee/organizer PII; replace this with logging only minimal non-PII fields
(e.g., event id, start/end times, status) or a sanitized summary (mask or omit
email/names), and gate the full verbose output behind an explicit dev flag
(e.g., check NODE_ENV==='development' or a FEATURE_DEV_LOGS flag) so PII is
never logged in production.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/trpc/server/routers/viewer/bookings/util.ts (1)
26-47: Consider migrating fromincludetoselectforeventType.As per coding guidelines, Prisma queries in this codebase should use
selectinstead ofinclude. While this pattern predates your changes, consider refactoringeventTypeto useselectwith explicit field enumeration.Example refactor:
eventType: { - include: { + select: { + id: true, + title: true, + // ... enumerate other needed EventType fields team: { select: { id: true, name: true, parentId: true, hideBranding: true, parent: { select: { hideBranding: true, }, }, }, }, owner: { select: { id: true, hideBranding: true, }, }, }, },Based on coding guidelines.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
packages/features/bookings/lib/handleConfirmation.ts(14 hunks)packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts(4 hunks)packages/trpc/server/routers/viewer/bookings/editLocation.handler.ts(3 hunks)packages/trpc/server/routers/viewer/bookings/util.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/trpc/server/routers/viewer/bookings/editLocation.handler.tspackages/features/bookings/lib/handleConfirmation.tspackages/trpc/server/routers/viewer/bookings/util.tspackages/trpc/server/routers/viewer/bookings/addGuests.handler.ts
**/*.{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:
packages/trpc/server/routers/viewer/bookings/editLocation.handler.tspackages/features/bookings/lib/handleConfirmation.tspackages/trpc/server/routers/viewer/bookings/util.tspackages/trpc/server/routers/viewer/bookings/addGuests.handler.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:
packages/trpc/server/routers/viewer/bookings/editLocation.handler.tspackages/features/bookings/lib/handleConfirmation.tspackages/trpc/server/routers/viewer/bookings/util.tspackages/trpc/server/routers/viewer/bookings/addGuests.handler.ts
🧠 Learnings (4)
📚 Learning: 2025-08-21T13:55:23.470Z
Learnt from: alishaz-polymath
PR: calcom/cal.com#23247
File: packages/features/webhooks/lib/service/FormWebhookService.ts:0-0
Timestamp: 2025-08-21T13:55:23.470Z
Learning: In the new webhook architecture for Cal.com, schedulePayload is considered legacy code that doesn't fit the clean architecture. The new architecture keeps webhook scheduling logic within the Service layer, specifically through a new method WebhookService.scheduleDelayedWebhooks, rather than using the old centralized schedulePayload helper.
Applied to files:
packages/features/bookings/lib/handleConfirmation.ts
📚 Learning: 2025-09-03T11:54:05.409Z
Learnt from: supalarry
PR: calcom/cal.com#23514
File: apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts:579-582
Timestamp: 2025-09-03T11:54:05.409Z
Learning: In calcom/cal.com bookings repository methods, when Prisma select uses `eventType: true`, all eventType fields including seatsShowAttendees are automatically included in the selection. Explicit field selection is not required when using `true` for nested relations.
Applied to files:
packages/features/bookings/lib/handleConfirmation.tspackages/trpc/server/routers/viewer/bookings/addGuests.handler.ts
📚 Learning: 2025-09-03T11:54:05.409Z
Learnt from: supalarry
PR: calcom/cal.com#23514
File: apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts:579-582
Timestamp: 2025-09-03T11:54:05.409Z
Learning: In calcom/cal.com bookings repository methods, when Prisma include uses `eventType: true`, all eventType fields including seatsShowAttendees are automatically included in the selection. Explicit field selection is not required when using `true` for nested relations.
Applied to files:
packages/features/bookings/lib/handleConfirmation.tspackages/trpc/server/routers/viewer/bookings/addGuests.handler.ts
📚 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/handleConfirmation.ts
🧬 Code graph analysis (3)
packages/trpc/server/routers/viewer/bookings/editLocation.handler.ts (4)
packages/lib/getOrgIdFromMemberOrTeamId.ts (1)
getOrgIdFromMemberOrTeamId(47-58)packages/lib/hideBranding.ts (1)
shouldHideBrandingForEvent(76-113)packages/types/Calendar.d.ts (1)
CalendarEvent(163-227)packages/emails/email-manager.ts (1)
sendLocationChangeEmailsAndSMS(640-671)
packages/features/bookings/lib/handleConfirmation.ts (1)
packages/lib/hideBranding.ts (1)
shouldHideBrandingForEvent(76-113)
packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts (2)
packages/lib/getOrgIdFromMemberOrTeamId.ts (1)
getOrgIdFromMemberOrTeamId(47-58)packages/lib/hideBranding.ts (1)
shouldHideBrandingForEvent(76-113)
⏰ 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: Tests / Unit
🔇 Additional comments (3)
packages/trpc/server/routers/viewer/bookings/util.ts (2)
33-45: LGTM! Branding fields added correctly.The additions of
hideBrandingto team, nested parent, and owner selections properly support the PR's branding propagation objective. The fields align with the updated type definition.
117-124: Type definition correctly reflects the runtime shape.The updated
BookingsProcedureContexttype accurately represents the branding fields added to the Prisma query. Optional and nullable annotations are appropriate.packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts (1)
170-198: Branding flag plumbing LGTM.Computing
hideBrandingonce viashouldHideBrandingForEventand threading it into the calendar event ensures the downstream email templates can reliably honor the “Disable Cal.com branding” toggle even for org/team contexts. Nicely done.
| const team = booking.eventType?.team | ||
| ? { | ||
| hideBranding: booking.eventType.team.hideBranding ?? null, | ||
| parent: booking.eventType.team.parent | ||
| ? { | ||
| hideBranding: booking.eventType.team.parent.hideBranding ?? null, | ||
| } | ||
| : null, | ||
| } | ||
| : null; | ||
|
|
||
| const owner = booking.eventType?.owner?.id | ||
| ? { | ||
| id: booking.eventType.owner.id, | ||
| hideBranding: booking.eventType.owner.hideBranding ?? null, | ||
| } | ||
| : null; | ||
|
|
||
| const organizationId = booking.eventType?.team?.parentId ?? fallbackOrganizationId; | ||
|
|
||
| try { | ||
| return await shouldHideBrandingForEvent({ | ||
| eventTypeId, | ||
| team, | ||
| owner, | ||
| organizationId, | ||
| }); |
There was a problem hiding this comment.
Team branding calculation drops required identifiers
When we rebuild the team payload here we strip out id (and parentId). shouldHideBrandingForEvent → shouldHideBrandingForTeamEvent dereferences those identifiers to walk the team/org hierarchy; with them missing the helper falls back to false, so team bookings keep the Cal.com footer. Please retain the identifiers (or pass the Prisma object through) so the helper can correctly evaluate team branding.
type BookingWithBranding = {
eventTypeId?: number | null;
eventType?: {
id?: number | null;
owner?: {
id?: number | null;
hideBranding?: boolean | null;
} | null;
team?: {
+ id?: number | null;
hideBranding?: boolean | null;
parentId?: number | null;
parent?: {
+ id?: number | null;
hideBranding?: boolean | null;
} | null;
} | null;
} | null;
};
const team = booking.eventType?.team
? {
+ id: booking.eventType.team.id ?? null,
+ parentId: booking.eventType.team.parentId ?? null,
hideBranding: booking.eventType.team.hideBranding ?? null,
parent: booking.eventType.team.parent
? {
+ id: booking.eventType.team.parent.id ?? null,
hideBranding: booking.eventType.team.parent.hideBranding ?? null,
}
: null,
}
: null;(Also extend the Prisma select for team.parent so parent.id is available.)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const team = booking.eventType?.team | |
| ? { | |
| hideBranding: booking.eventType.team.hideBranding ?? null, | |
| parent: booking.eventType.team.parent | |
| ? { | |
| hideBranding: booking.eventType.team.parent.hideBranding ?? null, | |
| } | |
| : null, | |
| } | |
| : null; | |
| const owner = booking.eventType?.owner?.id | |
| ? { | |
| id: booking.eventType.owner.id, | |
| hideBranding: booking.eventType.owner.hideBranding ?? null, | |
| } | |
| : null; | |
| const organizationId = booking.eventType?.team?.parentId ?? fallbackOrganizationId; | |
| try { | |
| return await shouldHideBrandingForEvent({ | |
| eventTypeId, | |
| team, | |
| owner, | |
| organizationId, | |
| }); | |
| const team = booking.eventType?.team | |
| ? { | |
| id: booking.eventType.team.id ?? null, | |
| parentId: booking.eventType.team.parentId ?? null, | |
| hideBranding: booking.eventType.team.hideBranding ?? null, | |
| parent: booking.eventType.team.parent | |
| ? { | |
| id: booking.eventType.team.parent.id ?? null, | |
| hideBranding: booking.eventType.team.parent.hideBranding ?? null, | |
| } | |
| : null, | |
| } | |
| : null; |
| let hideBranding = false; | ||
| if (booking.eventType?.id) { | ||
| const orgId = await getOrgIdFromMemberOrTeamId({ | ||
| memberId: booking.userId ?? null, | ||
| teamId: booking.eventType.team?.id ?? booking.eventType.teamId ?? null, | ||
| }); | ||
|
|
||
| hideBranding = await shouldHideBrandingForEvent({ | ||
| eventTypeId: booking.eventType.id, | ||
| team: booking.eventType.team | ||
| ? { | ||
| hideBranding: booking.eventType.team.hideBranding ?? null, | ||
| parent: booking.eventType.team.parent | ||
| ? { | ||
| hideBranding: booking.eventType.team.parent.hideBranding ?? null, | ||
| } | ||
| : null, | ||
| } | ||
| : null, | ||
| owner: booking.eventType.owner | ||
| ? { | ||
| id: booking.eventType.owner.id, | ||
| hideBranding: booking.eventType.owner.hideBranding ?? null, | ||
| } | ||
| : null, | ||
| organizationId: orgId ?? null, | ||
| }).catch(() => !!booking.eventType?.owner?.hideBranding); | ||
| } | ||
|
|
||
| const evtWithBranding: CalendarEvent = { | ||
| ...evt, | ||
| hideBranding, | ||
| }; |
There was a problem hiding this comment.
Preserve existing hideBranding when eventType data is missing
We rebuild the event with hideBranding hard-set to false before we even know whether we can recompute it. When booking.eventType is null or not eagerly loaded (e.g., legacy bookings whose event type has been deleted), buildCalEventFromBooking still surfaces the archived hideBranding flag, but this block overwrites it to false, causing branding to leak back into follow-up emails/SMS for those bookings.
Initialize from the existing event so we only override when we successfully recompute:
- let hideBranding = false;
+ let hideBranding = evt.hideBranding ?? false;After that, keep the recompute logic as-is so we still update the flag when we have the necessary context.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let hideBranding = false; | |
| if (booking.eventType?.id) { | |
| const orgId = await getOrgIdFromMemberOrTeamId({ | |
| memberId: booking.userId ?? null, | |
| teamId: booking.eventType.team?.id ?? booking.eventType.teamId ?? null, | |
| }); | |
| hideBranding = await shouldHideBrandingForEvent({ | |
| eventTypeId: booking.eventType.id, | |
| team: booking.eventType.team | |
| ? { | |
| hideBranding: booking.eventType.team.hideBranding ?? null, | |
| parent: booking.eventType.team.parent | |
| ? { | |
| hideBranding: booking.eventType.team.parent.hideBranding ?? null, | |
| } | |
| : null, | |
| } | |
| : null, | |
| owner: booking.eventType.owner | |
| ? { | |
| id: booking.eventType.owner.id, | |
| hideBranding: booking.eventType.owner.hideBranding ?? null, | |
| } | |
| : null, | |
| organizationId: orgId ?? null, | |
| }).catch(() => !!booking.eventType?.owner?.hideBranding); | |
| } | |
| const evtWithBranding: CalendarEvent = { | |
| ...evt, | |
| hideBranding, | |
| }; | |
| let hideBranding = evt.hideBranding ?? false; | |
| if (booking.eventType?.id) { | |
| const orgId = await getOrgIdFromMemberOrTeamId({ | |
| memberId: booking.userId ?? null, | |
| teamId: booking.eventType.team?.id ?? booking.eventType.teamId ?? null, | |
| }); | |
| hideBranding = await shouldHideBrandingForEvent({ | |
| eventTypeId: booking.eventType.id, | |
| team: booking.eventType.team | |
| ? { | |
| hideBranding: booking.eventType.team.hideBranding ?? null, | |
| parent: booking.eventType.team.parent | |
| ? { | |
| hideBranding: booking.eventType.team.parent.hideBranding ?? null, | |
| } | |
| : null, | |
| } | |
| : null, | |
| owner: booking.eventType.owner | |
| ? { | |
| id: booking.eventType.owner.id, | |
| hideBranding: booking.eventType.owner.hideBranding ?? null, | |
| } | |
| : null, | |
| organizationId: orgId ?? null, | |
| }).catch(() => !!booking.eventType?.owner?.hideBranding); | |
| } | |
| const evtWithBranding: CalendarEvent = { | |
| ...evt, | |
| hideBranding, | |
| }; |
🤖 Prompt for AI Agents
In packages/trpc/server/routers/viewer/bookings/editLocation.handler.ts around
lines 269 to 301, the code currently initializes hideBranding to false and then
always sets evtWithBranding.hideBranding to that default, which overwrites any
existing archived hideBranding when booking.eventType is missing; instead
initialize hideBranding from evt.hideBranding (preserve existing value), then
only recompute and override it when booking.eventType exists and
shouldHideBrandingForEvent resolves successfully (keep the .catch fallback
behavior but ensure it does not run when eventType is absent).
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/features/credentials/handleDeleteCredential.ts (1)
255-364: Provide full team context toshouldHideBrandingForEvent.
shouldHideBrandingForEventultimately callsshouldHideBrandingForTeamEvent, which expects at least the team’sid/parentIdcontext for proper inheritance checks. Here we trim the team down to{ hideBranding, parent: { hideBranding } }, so any logic relying onteam.id,team.parentId, or deeper ancestry now seesundefinedand can no longer respect organization-level overrides. Please pass the already-selectedid/parentIdfields (and keep the parent object’sidif needed) instead of this minimal shape to avoid false negatives when resolving branding inheritance.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
packages/features/credentials/handleDeleteCredential.ts(5 hunks)packages/lib/builders/CalendarEvent/class.ts(1 hunks)packages/lib/server/queries/bookings/canViewSensitiveBookingData.ts(1 hunks)packages/trpc/server/routers/viewer/bookings/get.handler.ts(6 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/trpc/server/routers/viewer/bookings/get.handler.tspackages/lib/builders/CalendarEvent/class.tspackages/lib/server/queries/bookings/canViewSensitiveBookingData.tspackages/features/credentials/handleDeleteCredential.ts
**/*.{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:
packages/trpc/server/routers/viewer/bookings/get.handler.tspackages/lib/builders/CalendarEvent/class.tspackages/lib/server/queries/bookings/canViewSensitiveBookingData.tspackages/features/credentials/handleDeleteCredential.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:
packages/trpc/server/routers/viewer/bookings/get.handler.tspackages/lib/builders/CalendarEvent/class.tspackages/lib/server/queries/bookings/canViewSensitiveBookingData.tspackages/features/credentials/handleDeleteCredential.ts
🧠 Learnings (3)
📚 Learning: 2025-09-03T11:54:05.409Z
Learnt from: supalarry
PR: calcom/cal.com#23514
File: apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts:579-582
Timestamp: 2025-09-03T11:54:05.409Z
Learning: In calcom/cal.com bookings repository methods, when Prisma select uses `eventType: true`, all eventType fields including seatsShowAttendees are automatically included in the selection. Explicit field selection is not required when using `true` for nested relations.
Applied to files:
packages/trpc/server/routers/viewer/bookings/get.handler.tspackages/features/credentials/handleDeleteCredential.ts
📚 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/trpc/server/routers/viewer/bookings/get.handler.ts
📚 Learning: 2025-09-03T11:54:05.409Z
Learnt from: supalarry
PR: calcom/cal.com#23514
File: apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts:579-582
Timestamp: 2025-09-03T11:54:05.409Z
Learning: In calcom/cal.com bookings repository methods, when Prisma include uses `eventType: true`, all eventType fields including seatsShowAttendees are automatically included in the selection. Explicit field selection is not required when using `true` for nested relations.
Applied to files:
packages/features/credentials/handleDeleteCredential.ts
🧬 Code graph analysis (2)
packages/trpc/server/routers/viewer/bookings/get.handler.ts (1)
packages/lib/server/queries/bookings/canViewSensitiveBookingData.ts (1)
canViewSensitiveBookingData(44-87)
packages/features/credentials/handleDeleteCredential.ts (2)
packages/lib/getOrgIdFromMemberOrTeamId.ts (1)
getOrgIdFromMemberOrTeamId(47-58)packages/lib/hideBranding.ts (1)
shouldHideBrandingForEvent(76-113)
⏰ 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). (4)
- GitHub Check: Type check / check-types
- GitHub Check: Tests / Unit
- GitHub Check: Linters / lint
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (1)
packages/lib/builders/CalendarEvent/class.ts (1)
38-38: LGTM! Clean additive change.The optional
hideBrandingproperty follows the established pattern of otherhide*flags in the class and integrates seamlessly with the Object.assign-based constructor.
|
@kart1ka can you please review this now |
The PR is not ready. It does not work when "Disable branding" is toggled on at organization level. Also there are a lot of type errors. |
hackice20
force-pushed
the
fix/branding-disabled-in-settings
branch
from
e770f90 to
3bcce02
Compare
5 participants
What does this PR do?
I ensure emails respect the “Disable Cal.com branding” setting by:
hideBranding?: booleantoCalendarEventhideBrandingwhen building/assembling events (new booking, confirm, payment)BaseScheduledEmailto hide the footer/logo whenhideBrandingis trueScope is minimal and limited to plumbing the flag and honoring it in email templates.
Visual Demo
Tip: Use MailHog to view emails in dev.
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Environment:
EMAIL_SERVER_HOST=localhost,EMAIL_SERVER_PORT=1025E2E_TEST_MAILHOG_ENABLED=1Minimal data:
Steps:
Expected:
Checklist