fix: disable cal branding for user, team, organisation#24375
fix: disable cal branding for user, team, organisation#24375dhairyashiil wants to merge 90 commits intocalcom:mainfrom
Conversation
|
@dhairyashiil is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
github-actions
bot
added
consumer
emails
keithwillcode
added
the
community-interns
WalkthroughAdds an optional Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
dosubot
bot
added
the
bookings
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/features/bookings/lib/handleCancelBooking.ts (1)
233-256: Drop the second profile fetch and reuseownerProfile.We already load the organizer’s profile at Line 233, so this second
profile.findFirstcall just repeats that lookup and adds latency for every cancellation (especially inside hot paths like bulk cancellations). Reuse the existingownerProfile?.organizationIdinstead of issuing another query.
📜 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)
packages/features/bookings/lib/getBookingToDelete.ts(2 hunks)packages/features/bookings/lib/handleCancelBooking.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/getBookingToDelete.tspackages/features/bookings/lib/handleCancelBooking.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/getBookingToDelete.tspackages/features/bookings/lib/handleCancelBooking.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/getBookingToDelete.tspackages/features/bookings/lib/handleCancelBooking.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/features/bookings/lib/getBookingToDelete.ts
📚 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/getBookingToDelete.ts
🧬 Code graph analysis (1)
packages/features/bookings/lib/handleCancelBooking.ts (2)
packages/lib/hideBranding.ts (1)
shouldHideBrandingForEvent(76-113)packages/emails/email-manager.ts (1)
sendCancelledEmailsAndSMS(476-534)
|
For cancelation emails: hide.branding.for.cancelation.mov |
|
For confirmation booking email: hide.branding.for.confirmation.mov |
|
for seated bookings: Screen.Recording.2025-10-09.at.9.48.25.PM.mov |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/features/bookings/lib/handleSeats/reschedule/owner/moveSeatedBookingToNewTimeSlot.ts (1)
34-62: Defer branding lookups until email actually sendsWe always compute
teamForBranding, profile, andhideBrandingeven whennoEmail === trueorisConfirmedByDefaultis false. That path ends up skipping the send, so we burn two extra DB roundtrips for nothing. Please hoist this block inside the email gate so we only hit Prisma when we truly need the branding decision.packages/features/bookings/lib/handleSeats/reschedule/owner/combineTwoSeatedBookings.ts (1)
138-178: Factor shared branding lookup into a helperThis
teamForBranding+shouldHideBrandingForEventblock is now copy/pasted across multiple reschedule flows. Please extract it into a shared helper so we have a single place to evolve the branding heuristics (team parent, org profile lookup, etc.) and avoid subtle drift the next time we tweak it.
📜 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/features/bookings/lib/handleBookingRequested.ts(3 hunks)packages/features/bookings/lib/handleConfirmation.ts(6 hunks)packages/features/bookings/lib/handleSeats/reschedule/attendee/attendeeRescheduleSeatedBooking.ts(4 hunks)packages/features/bookings/lib/handleSeats/reschedule/owner/combineTwoSeatedBookings.ts(3 hunks)packages/features/bookings/lib/handleSeats/reschedule/owner/moveSeatedBookingToNewTimeSlot.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/features/bookings/lib/handleConfirmation.tspackages/features/bookings/lib/handleBookingRequested.tspackages/features/bookings/lib/handleSeats/reschedule/attendee/attendeeRescheduleSeatedBooking.tspackages/features/bookings/lib/handleSeats/reschedule/owner/combineTwoSeatedBookings.tspackages/features/bookings/lib/handleSeats/reschedule/owner/moveSeatedBookingToNewTimeSlot.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/handleConfirmation.tspackages/features/bookings/lib/handleBookingRequested.tspackages/features/bookings/lib/handleSeats/reschedule/attendee/attendeeRescheduleSeatedBooking.tspackages/features/bookings/lib/handleSeats/reschedule/owner/combineTwoSeatedBookings.tspackages/features/bookings/lib/handleSeats/reschedule/owner/moveSeatedBookingToNewTimeSlot.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/handleConfirmation.tspackages/features/bookings/lib/handleBookingRequested.tspackages/features/bookings/lib/handleSeats/reschedule/attendee/attendeeRescheduleSeatedBooking.tspackages/features/bookings/lib/handleSeats/reschedule/owner/combineTwoSeatedBookings.tspackages/features/bookings/lib/handleSeats/reschedule/owner/moveSeatedBookingToNewTimeSlot.ts
🧬 Code graph analysis (5)
packages/features/bookings/lib/handleConfirmation.ts (3)
packages/lib/getTeamIdFromEventType.ts (1)
getTeamIdFromEventType(3-29)packages/lib/getOrgIdFromMemberOrTeamId.ts (1)
getOrgIdFromMemberOrTeamId(47-58)packages/lib/hideBranding.ts (1)
shouldHideBrandingForEvent(76-113)
packages/features/bookings/lib/handleBookingRequested.ts (3)
packages/lib/hideBranding.ts (1)
shouldHideBrandingForEvent(76-113)packages/emails/email-manager.ts (2)
sendOrganizerRequestEmail(433-436)sendAttendeeRequestEmailAndSMS(451-454)packages/prisma/zod-utils.ts (1)
EventTypeMetadata(135-135)
packages/features/bookings/lib/handleSeats/reschedule/attendee/attendeeRescheduleSeatedBooking.ts (2)
packages/lib/hideBranding.ts (1)
shouldHideBrandingForEvent(76-113)packages/emails/email-manager.ts (1)
sendRescheduledSeatEmailAndSMS(323-342)
packages/features/bookings/lib/handleSeats/reschedule/owner/combineTwoSeatedBookings.ts (2)
packages/lib/hideBranding.ts (1)
shouldHideBrandingForEvent(76-113)packages/emails/email-manager.ts (1)
sendRescheduledEmailsAndSMS(318-321)
packages/features/bookings/lib/handleSeats/reschedule/owner/moveSeatedBookingToNewTimeSlot.ts (1)
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). (2)
- GitHub Check: Linters / lint
- GitHub Check: Tests / Unit
|
For declined bookings: Screen.Recording.2025-10-10.at.12.41.24.AM.mov |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 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 (3)
packages/features/bookings/lib/handleBookingRequested.ts(3 hunks)packages/features/bookings/lib/handleConfirmation.ts(6 hunks)packages/trpc/server/routers/viewer/bookings/confirm.handler.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/features/bookings/lib/handleBookingRequested.tspackages/trpc/server/routers/viewer/bookings/confirm.handler.tspackages/features/bookings/lib/handleConfirmation.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/handleBookingRequested.tspackages/trpc/server/routers/viewer/bookings/confirm.handler.tspackages/features/bookings/lib/handleConfirmation.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/handleBookingRequested.tspackages/trpc/server/routers/viewer/bookings/confirm.handler.tspackages/features/bookings/lib/handleConfirmation.ts
🧠 Learnings (2)
📚 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/handleConfirmation.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/features/bookings/lib/handleBookingRequested.ts (3)
packages/lib/hideBranding.ts (1)
shouldHideBrandingForEvent(76-113)packages/emails/email-manager.ts (2)
sendOrganizerRequestEmail(433-436)sendAttendeeRequestEmailAndSMS(451-454)packages/prisma/zod-utils.ts (1)
EventTypeMetadata(135-135)
packages/trpc/server/routers/viewer/bookings/confirm.handler.ts (3)
packages/lib/hideBranding.ts (1)
shouldHideBrandingForEvent(76-113)packages/emails/email-manager.ts (1)
sendDeclinedEmailsAndSMS(456-474)packages/prisma/zod-utils.ts (1)
EventTypeMetadata(135-135)
packages/features/bookings/lib/handleConfirmation.ts (3)
packages/lib/getTeamIdFromEventType.ts (1)
getTeamIdFromEventType(3-29)packages/lib/getOrgIdFromMemberOrTeamId.ts (1)
getOrgIdFromMemberOrTeamId(47-58)packages/lib/hideBranding.ts (1)
shouldHideBrandingForEvent(76-113)
packages/trpc/server/routers/viewer/bookings/confirm.handler.ts
Outdated
Show resolved
Hide resolved
|
@hbjORbj the tests were just flaky, all passing when I merge latest main into this branch |
hbjORbj
left a comment
hbjORbj
left a comment
There was a problem hiding this comment.
I see lots of formatting changes in this PR, making the review quite difficult. I suggest using Devin or some AI to revert the formatting changes so it's easy to spot the actual changes this PR brings
|
|
||
| async function getBooking(bookingId: number) { | ||
| const bookingRepository = new BookingRepository(prisma); | ||
| const booking = await bookingRepository.findByIdIncludeDestinationCalendar(bookingId); |
There was a problem hiding this comment.
so, now can we remove findByIdIncludeDestinationCalendar?
There was a problem hiding this comment.
yeah I think so, doesnt seem to be used anywhere
Devin AI is resolving merge conflictsThis PR has merge conflicts with the Devin will:
If you prefer to resolve conflicts manually, you can close the Devin session and handle it yourself. |
Co-Authored-By: rajiv@cal.com <sahalrajiv6900@gmail.com>
Co-Authored-By: rajiv@cal.com <sahalrajiv6900@gmail.com>
Co-Authored-By: rajiv@cal.com <sahalrajiv6900@gmail.com>
|
Done! I've removed the unused Commit: 489f077 |
|
@hbjORbj I gave Devin a task to remove linting changes, can you please have a look once again now |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 61 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/features/ee/managed-event-types/reassignment/services/ManagedEventManualReassignmentService.ts">
<violation number="1" location="packages/features/ee/managed-event-types/reassignment/services/ManagedEventManualReassignmentService.ts:735">
P2: `withHideBranding(calEvent)` defaults hideBranding to false because this method never sets calEvent.hideBranding or passes an explicit value. As a result, branding suppression from org/team/user settings won’t apply to reassignment emails. Compute hideBranding (e.g., via shouldHideBrandingForEventUsingProfile) and pass it into withHideBranding or set it on the builder.</violation>
</file>
<file name="packages/trpc/server/routers/viewer/bookings/editLocation.handler.ts">
<violation number="1" location="packages/trpc/server/routers/viewer/bookings/editLocation.handler.ts:314">
P2: When eventTypeId is null, this forces hideBranding to false and skips user/organization checks, so bookings without eventTypeId can incorrectly display branding even if the organizer/org hides branding. Consider still resolving hideBranding using owner/team data and only log a warning for missing eventTypeId.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
...atures/ee/managed-event-types/reassignment/services/ManagedEventManualReassignmentService.ts
Outdated
Show resolved
Hide resolved
Ryukemeister
added
ready-for-e2e
run-ci
Ryukemeister
left a comment
Ryukemeister
left a comment
There was a problem hiding this comment.
hey @dhairyashiil, as discussed we should move this into multiple smaller PRs. this is insanely large and has a higher chance of breaking. i'll create follow up PRs for this. for now we can close this, thanks!
8 participants
What does this PR do?
This PR implements the ability to hide Cal.com branding at the user, team, and organization levels. The branding visibility is determined hierarchically - if an organization has branding hidden, all teams and users under it inherit that setting.
Key Changes
The implementation adds a
shouldHideBrandingForEventfunction that checks branding settings in this priority order:hideBrandingsetting (if applicable)hideBrandingsetting (if applicable)hideBrandingsettingEmail functions now use a
CalendarEventWithBrandingtype to ensure branding state is explicitly passed. AwithHideBrandinghelper wraps calendar events with the branding flag.Files Modified
hideBrandingfield toCalendarEventtypeVisual Demo (For contributors especially)
User:
hide.branding.normal.user.mov
Team:
hide.branding.team.user.mov
Organisation:
hide.branding.organisation.user.mov
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Set up a user with
hideBranding: truein their profileCreate a booking with that user as the organizer
Verify the confirmation email does not show Cal.com branding
Set up a team with
hideBranding: trueCreate a booking for a team event type
Verify the confirmation email does not show Cal.com branding
Set up an organization with
hideBranding: trueCreate bookings for users/teams under that organization
Verify branding is hidden for all bookings under the organization
Human Review Checklist
shouldHideBrandingForEventlogic correctly handles all edge cases (null team, null owner, etc.)hideBrandingaddGuests.handler.tsandconfirm.handler.tsare safehideBrandingfields