Skip to content

Comments

fix: managed event types inherit team hideBranding setting#23562

Merged
emrysal merged 8 commits intomainfrom
devin/1756921252-disable-cta-when-branding-disabled
Sep 13, 2025
Merged

fix: managed event types inherit team hideBranding setting#23562
emrysal merged 8 commits intomainfrom
devin/1756921252-disable-cta-when-branding-disabled

Conversation

@joeauyeung
Copy link
Contributor

What does this PR do?

Fixes an issue where managed event types were not properly inheriting their team's hideBranding setting, causing the "create your own booking link with Cal" CTA to appear on booking success pages even when the team had branding disabled.

Problem: Managed event types (child event types) don't have direct team relationships - they inherit team context through their parentId. The booking success page wasn't passing this parent team data to the branding functions, so managed event types couldn't inherit the team's branding settings.

Solution:

  • Added getParentTeamForBranding helper function to query parent team data when needed
  • Modified shouldHideBrandingForEvent call to use parent team data when eventType.team is null but eventType.parent?.teamId exists
  • Follows existing pattern used elsewhere in the same file: eventType.team?.id ?? eventType.parent?.teamId

Link to Devin run: https://app.devin.ai/sessions/558321e957d540b1bfc5c4eaf04a0758
Requested by: @joeauyeung

How should this be tested?

Test Steps:

  1. Create a team and enable "Disable Cal.com branding" in team appearance settings
  2. Create a managed event type for that team
  3. Book an event through the managed event type (child event type)
  4. On the booking success page, verify that the CTA "create your own booking link with Cal" is not displayed
  5. Test with non-managed event types to ensure they continue working correctly

Expected behavior:

  • Managed event types should inherit the team's branding settings
  • When team has hideBranding: true, the CTA should be hidden on booking success pages
  • Non-managed event types should continue to work as before

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox. N/A - internal branding logic fix
  • I confirm automated tests are in place that prove my fix is effective or that my feature works. Testing requires full booking flow which is covered by existing E2E tests

Human Review Checklist

Critical items to verify:

  • Functionality test: Create team with branding disabled → managed event type → book event → verify CTA is hidden
  • Performance impact: Review if additional database query for parent team data affects page load times
  • Edge cases: Test behavior when parent team doesn't exist or has null hideBranding values
  • Code correctness: Verify the logic follows existing patterns in the codebase (line 236 uses same pattern)
  • Non-regression: Ensure regular event types still work correctly

Checklist

  • My code follows the style guidelines of this project
  • I have self-reviewed the code and tested type checking passes
  • My changes generate no new TypeScript warnings
  • The solution follows existing patterns in the codebase

devin-ai-integration bot and others added 2 commits September 3, 2025 18:30
- Add getParentTeamForBranding helper to query parent team data
- Update shouldHideBrandingForEvent to use parent team when eventType.team is null
- Ensures CTA 'create your own booking link with Cal' respects team branding settings
- Follows existing pattern used by eventType.team?.id ?? eventType.parent?.teamId

Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 3, 2025

Walkthrough

Expanded the Prisma select in apps/web/lib/booking.ts to include eventType.parent.team with its hideBranding and that team’s parent hideBranding. Updated apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx to pass eventType?.parent?.team (falling back to eventType?.team) into shouldHideBrandingForEvent. No exported/public API signatures were changed.

Possibly related PRs


📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 57c5778 and 5bf5cf2.

📒 Files selected for processing (1)
  • apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch devin/1756921252-disable-cta-when-branding-disabled

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@keithwillcode keithwillcode added core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO labels Sep 3, 2025
@vercel
Copy link

vercel bot commented Sep 3, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
cal Ignored Ignored Sep 8, 2025 5:45pm
cal-eu Ignored Ignored Sep 8, 2025 5:45pm

@github-actions
Copy link
Contributor

github-actions bot commented Sep 3, 2025

E2E results are ready!

@joeauyeung joeauyeung marked this pull request as ready for review September 5, 2025 16:31
@joeauyeung joeauyeung requested a review from a team as a code owner September 5, 2025 16:31
@graphite-app graphite-app bot requested a review from a team September 5, 2025 16:31
@dosubot dosubot bot added bookings area: bookings, availability, timezones, double booking 🐛 bug Something isn't working labels Sep 5, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
apps/web/lib/booking.ts (2)

172-185: Replace Prisma include with select (repo guideline)
We avoid include in Prisma. Switch to select and explicitly pick the fields you consume later (data and attendee.*), keeping types intact.

-    seatAttendee = await prisma.bookingSeat.findUnique({
-      where: {
-        referenceUid: seatReferenceUid,
-      },
-      include: {
-        attendee: {
-          select: {
-            name: true,
-            email: true,
-            phoneNumber: true,
-          },
-        },
-      },
-    });
+    seatAttendee = await prisma.bookingSeat.findUnique({
+      where: { referenceUid: seatReferenceUid },
+      select: {
+        id: true,
+        bookingId: true,
+        attendeeId: true,
+        referenceUid: true,
+        data: true,
+        attendee: {
+          select: {
+            name: true,
+            email: true,
+            phoneNumber: true,
+          },
+        },
+      },
+    });

96-111: Confirm platform-booking detection for managed (parent-team) events
If “platform booking” should also apply when the parent team is created by an OAuth client, consider reading createdByOAuthClientId from the parent team and extending the check. If that’s not desired behavior, ignore.

       parent: {
         select: {
-          hideBranding: true,
+          hideBranding: true,
         },
       },
       createdByOAuthClientId: true,
     },
   },
   parent: {
     select: {
       teamId: true,
-      team: {
+      team: {
         select: {
           hideBranding: true,
+          createdByOAuthClientId: true,
           parent: {
             select: {
               hideBranding: true,
             },
           },
         },
       },
     },
   },
-  const isPlatformBooking = eventType.users[0]?.isPlatformManaged || eventType.team?.createdByOAuthClientId;
+  const isPlatformBooking =
+    eventType.users[0]?.isPlatformManaged ||
+    eventType.team?.createdByOAuthClientId ||
+    eventType.parent?.team?.createdByOAuthClientId;

Also applies to: 231-233

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between ada8ffd and 13627db.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (2)
  • apps/web/lib/booking.ts (1 hunks)
  • apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • apps/web/lib/booking.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:

  • apps/web/lib/booking.ts
  • apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.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/lib/booking.ts
  • apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx
**/*.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.tsx
🧠 Learnings (7)
📓 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.
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.
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.
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.
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.
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.
📚 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:

  • apps/web/lib/booking.ts
  • apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx
📚 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:

  • apps/web/lib/booking.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:

  • apps/web/lib/booking.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:

  • apps/web/lib/booking.ts
  • 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/lib/booking.ts
📚 Learning: 2025-08-21T12:28:42.018Z
Learnt from: alishaz-polymath
PR: calcom/cal.com#23247
File: packages/features/webhooks/lib/factory/WebhookPayloadFactory.ts:274-282
Timestamp: 2025-08-21T12:28:42.018Z
Learning: In 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/lib/booking.ts
🔇 Additional comments (2)
apps/web/lib/booking.ts (1)

99-108: Branding inheritance data selection looks correct

Selecting only hideBranding on the parent team (and its parent) keeps the payload minimal and adheres to our “select-only” Prisma guideline. This should enable shouldHideBrandingForEvent to respect branding on managed/child event types.

apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx (1)

241-242: Correctly prefer parent team for branding decisions

Passing eventType.parent.team ?? eventType.team to shouldHideBrandingForEvent aligns with the managed-event inheritance model and the new selection in getEventTypesFromDB.

Please confirm shouldHideBrandingForEvent accepts a partial team shape (only hideBranding and parent.hideBranding) when the parent team is used.

: await shouldHideBrandingForEvent({
eventTypeId: eventType.id,
team: eventType.team,
team: eventType?.parent?.team ? eventType.parent.team : eventType.team,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: eventType?.parent?.team ?? eventType.team

yarn.lock Outdated
jimp: ^0.16.1
lingo.dev: ^0.111.1
next-collect: ^0.2.1
next-i18next: ^15.4.2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated changes to yarn.lock. Let's revert these. Otherwise it is good

@hariombalhara hariombalhara self-requested a review September 8, 2025 10:20
Copy link
Member

@hariombalhara hariombalhara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to remove yarn.lock changes.

@github-actions github-actions bot marked this pull request as draft September 8, 2025 10:21
@joeauyeung joeauyeung marked this pull request as ready for review September 8, 2025 16:57
@dosubot dosubot bot added the teams area: teams, round robin, collective, managed event-types label Sep 8, 2025
Copy link
Contributor

@emrysal emrysal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Todo: optimise caching to make sure this isn't hit every time.

@emrysal emrysal merged commit d7397d4 into main Sep 13, 2025
83 of 86 checks passed
@emrysal emrysal deleted the devin/1756921252-disable-cta-when-branding-disabled branch September 13, 2025 07:35
@coderabbitai coderabbitai bot mentioned this pull request Sep 29, 2025
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bookings area: bookings, availability, timezones, double booking 🐛 bug Something isn't working core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO ready-for-e2e size/S teams area: teams, round robin, collective, managed event-types

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants