fix: Allow team admins to see hidden fields and UTM tracking data #23878
Conversation
- Add team admin permission check for booking data visibility - Team admins can now view hidden booking field answers - Team admins can now view UTM tracking parameters - Support for managed events (parent team admins can view child event data) - Maintain backward compatibility with existing host permissions Fixes team admin access to booking details as requested in issue.
|
@naaa760 is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughServer-side: imported Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 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 |
github-actions
bot
added
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 (3)
apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx (2)
178-194: Team‑admin check works; consider parallelizing and deduping IDs.Two sequential DB calls per request; minor latency. Also avoid duplicate calls when IDs are equal.
Apply:
- const checkIfUserIsTeamAdmin = async (userId?: number | null) => { + const checkIfUserIsTeamAdmin = async (userId?: number | null) => { if (!userId) return false; - - const teamId = eventType.team?.id; - if (teamId) { - const teamAdminResult = await isTeamAdmin(userId, teamId); - if (teamAdminResult) return true; - } - - const parentTeamId = eventType.parent?.teamId; - if (parentTeamId) { - const parentTeamAdminResult = await isTeamAdmin(userId, parentTeamId); - if (parentTeamAdminResult) return true; - } - - return false; + const ids = Array.from( + new Set([eventType.team?.id, eventType.parent?.teamId].filter(Boolean) as number[]) + ); + if (ids.length === 0) return false; + if (ids.length === 1) return !!(await isTeamAdmin(userId, ids[0])); + const [a, b] = await Promise.all([isTeamAdmin(userId, ids[0]), isTeamAdmin(userId, ids[1])]); + return !!a || !!b; };
218-225: Server-side hidden-field filtering is good; avoid in-place mutation.Safer to rebuild responses immutably to prevent iterator side-effects.
Use:
- if (!canViewHiddenData) { - for (const key in bookingInfo.responses) { - const field = eventTypeRaw.bookingFields.find((field) => field.name === key); - if (field && !!field.hidden) { - delete bookingInfo.responses[key]; - } - } - } + if (!canViewHiddenData) { + const hidden = new Set( + eventTypeRaw.bookingFields.filter((f) => f?.hidden).map((f) => f.name) + ); + bookingInfo.responses = Object.fromEntries( + Object.entries(bookingInfo.responses).filter(([name]) => !hidden.has(name)) + ) as typeof bookingInfo.responses; + }apps/web/modules/bookings/views/bookings-single-view.tsx (1)
388-388: Remove unused variable_canCancelAndReschedule.Dead code; keep surface clean.
- const _canCancelAndReschedule = !eventType?.disableCancelling && !eventType?.disableRescheduling;
📜 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(5 hunks)apps/web/modules/bookings/views/bookings-single-view.tsx(6 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.tsxapps/web/modules/bookings/views/bookings-single-view.getServerSideProps.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.tsxapps/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/modules/bookings/views/bookings-single-view.tsxapps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx
🧠 Learnings (3)
📚 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
📚 Learning: 2025-09-12T11:23:34.118Z
Learnt from: hariombalhara
PR: calcom/cal.com#23736
File: packages/features/bookings/lib/reschedule/determineReschedulePreventionRedirect.ts:73-84
Timestamp: 2025-09-12T11:23:34.118Z
Learning: In the Cal.com codebase, the forceRescheduleForCancelledBooking flag historically affects both CANCELLED and REJECTED booking statuses, despite its name suggesting it should only affect cancelled bookings. This behavior existed before PR #23736 and was preserved during the refactoring.
Applied to files:
apps/web/modules/bookings/views/bookings-single-view.tsx
📚 Learning: 2025-08-26T08:08:23.395Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23343
File: packages/features/insights/server/trpc-router.ts:1080-1101
Timestamp: 2025-08-26T08:08:23.395Z
Learning: In packages/features/insights/server/trpc-router.ts, when filtering personal event types (userId provided, no teamId, not isAll), the query correctly uses user.id (authenticated user) instead of the input userId parameter for security reasons. This prevents users from accessing other users' personal event types by passing arbitrary user IDs.
Applied to files:
apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx
🧬 Code graph analysis (1)
apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx (1)
packages/lib/server/queries/teams/index.ts (1)
isTeamAdmin(390-410)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (9)
apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx (4)
14-14: Import is correct (named export).Looks good; aligns with packages/lib/server/queries/teams export.
197-199: Permission aggregation is correct.OR-ing host/admin is the right boundary for visibility.
230-241: Correctly gates internal note presets.Fetching only when authorized prevents leaking presets.
255-281: Sanitize bookingInfo.tracking from SSR propsbookingInfo.tracking is still returned in props even when canViewHiddenData is false — this leaks UTM/tracking data in the page JSON.
const isPlatformBooking = eventType.users[0]?.isPlatformManaged || eventType.team?.createdByOAuthClientId; - return { + const bookingInfoForClient = + canViewHiddenData + ? bookingInfo + : ({ ...bookingInfo, tracking: undefined } as typeof bookingInfo); + + return { props: { orgSlug: currentOrgDomain, themeBasis: eventType.team ? eventType.team.slug : eventType.users[0]?.username, hideBranding: isPlatformBooking ? true : await shouldHideBrandingForEvent({ eventTypeId: eventType.id, team: eventType?.parent?.team ?? eventType?.team, owner: eventType.users[0] ?? null, organizationId: session?.user?.profile?.organizationId ?? session?.user?.org?.id ?? null, }), profile, eventType, recurringBookings: await getRecurringBookings(bookingInfo.recurringEventId), dynamicEventName: bookingInfo?.eventType?.eventName || "", - bookingInfo, + bookingInfo: bookingInfoForClient, previousBooking: sanitizedPreviousBooking, paymentStatus: payment, ...(tz && { tz }), userTimeFormat, requiresLoginToUpdate, rescheduledToUid, isLoggedInUserHost, canViewHiddenData, internalNotePresets: internalNotes, }, };apps/web/modules/bookings/views/bookings-single-view.tsx (5)
107-114: Prop plumb-through looks right.canViewHiddenData is correctly threaded from SSR.
474-474: Class reordering is harmless.No behavioral changes.
560-560: Grid/RTL class tweak is fine.No functional impact; improves RTL alignment.
710-749: UI gate for UTMs looks correct; ensure SSR sanitization is applied.With the server change, UTMs won’t exist in props for unauthorized viewers.
Confirm that after the SSR change, the toggle never renders for unauthorized users because utmParams will be undefined.
1084-1084: Alert class reordering is OK.No behavior change.
|
thanks for your work @naaa760 , can you pls attach a before and after also? |
Alternative Visual Demo Approach:Code Review:The changes are minimal and focused:
Manual Testing:
Expected Visual Changes:Before: Team admins see only regular booking fields, no UTM section The implementation handles all the requirements:
|
|
@naaa760 Thanks for the PR !! You need to show it to all team members and not just admins. The title of the ticket was wrong(I just fixed it), though description was correct. Also, it is an absolute requirement to have before and after video/loom for the fix. So, if you are unable to run it locally, let's solve that. |
Previously only team admins could view hidden booking field answers and UTM tracking parameters. Now all team members can see this data, providing better transparency within teams. - Changed from isTeamAdmin to isTeamMember check - Team members (not just admins) can now view hidden booking fields - Team members can now view UTM tracking parameters - Maintains host permission (organizers can still see everything) - Supports managed events (parent team members can view child event data)
|
neha seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
@hariombalhara Unfortunately I'm having trouble running the app locally right now to create the before/after video demo. Quick summary of what changed:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
apps/web/next.config.js (2)
5-5: Avoid unrelated PrismaPlugin removal; verify builds don’t regress.Commenting out the Prisma monorepo workaround can break Next.js builds that rely on Prisma’s path resolution. If this was only to work around a local dev issue, please revert or guard it behind an env flag.
Apply this guarded re-enable:
-// const { PrismaPlugin } = require("@prisma/nextjs-monorepo-workaround-plugin"); +const { PrismaPlugin } = require("@prisma/nextjs-monorepo-workaround-plugin"); +const ENABLE_PRISMA_PLUGIN = process.env.DISABLE_PRISMA_PLUGIN !== "1";Then in webpack:
-// config.plugins = [...config.plugins, new PrismaPlugin()]; +if (ENABLE_PRISMA_PLUGIN) { + config.plugins = [...config.plugins, new PrismaPlugin()]; +}Run a CI build on this PR branch to confirm no Prisma client/module resolution errors appear.
258-258: Prisma plugin registration is commented out.Same concern as above; either re-enable or feature‑flag it to avoid accidental removal in main.
apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx (2)
229-241: Internal note presets now visible to all who canViewHiddenData; confirm this expansion.Previously host-gated; now any team member (under current logic) can access presets. If that wasn’t intended, restrict to host or admin.
Restrict to host/admin:
-async function getInternalNotePresets(teamId: number | null) { - if (!teamId || !canViewHiddenData) return []; +async function getInternalNotePresets(teamId: number | null) { + if (!teamId) return []; + const canUsePresets = isLoggedInUserHost || (await checkIfUserIsTeamAdmin(userId)); + if (!canUsePresets) return [];
178-195: Micro: reduce DB round trips for membership/admin checks.If you keep member-based logic, you can check both teamId and parentTeamId in one query to cut latency.
Example (pseudo):
// queries/teams.ts export async function isMemberOfAny(userId: number, teamIds: number[]) { return !!(await prisma.membership.findFirst({ where: { userId, teamId: { in: teamIds }, accepted: true } })); }Then call with available IDs.
📜 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(5 hunks)apps/web/next.config.js(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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/next.config.jsapps/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
**/*.{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.tsx
🧠 Learnings (2)
📓 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-26T08:08:23.395Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23343
File: packages/features/insights/server/trpc-router.ts:1080-1101
Timestamp: 2025-08-26T08:08:23.395Z
Learning: In packages/features/insights/server/trpc-router.ts, when filtering personal event types (userId provided, no teamId, not isAll), the query correctly uses user.id (authenticated user) instead of the input userId parameter for security reasons. This prevents users from accessing other users' personal event types by passing arbitrary user IDs.
Applied to files:
apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx
🧬 Code graph analysis (1)
apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx (1)
packages/features/ee/teams/lib/queries.ts (1)
isTeamMember(422-430)
⏰ 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: Type check / check-types
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (4)
apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx (4)
197-199: Align canViewHiddenData with final policy.Currently: host OR member. If “admins only,” compute using admin check; if “all members,” this is fine but update copy/docs.
Suggested admin‑only computation:
-const isLoggedInUserTeamMember = await checkIfUserIsTeamMember(userId); -const canViewHiddenData = isLoggedInUserHost || isLoggedInUserTeamMember; +const isLoggedInUserTeamAdmin = await checkIfUserIsTeamAdmin(userId); +const canViewHiddenData = isLoggedInUserHost || isLoggedInUserTeamAdmin;
218-225: LGTM: server-side stripping of hidden responses when unauthorized.Solid defensive filter to ensure hidden fields don’t leak to clients lacking canViewHiddenData.
279-280: Prop surface change: canViewHiddenData is used for hidden/UTM data; isLoggedInUserHost remains for host-only actions.Verified: apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx defines both flags and passes them as props (lines ~196–199 and 276–280); bookings-single-view.tsx uses canViewHiddenData for UTM/internal-note visibility (line 710) and still assigns isLoggedInUserHost → isHost for host-specific UI (line 162). No changes required.
178-195: Permissions broadened to all team members (not just admins). Confirm intended.checkIfUserIsTeamMember in apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx (lines 178–195) returns true for any member of eventType.team or eventType.parent and is used to set canViewHiddenData. If admin‑only access is required, replace the isTeamMember checks with isTeamAdmin; otherwise update the PR title/description to reflect member-level visibility.
Admin‑only diff:
-const checkIfUserIsTeamMember = async (userId?: number | null) => { +const checkIfUserIsTeamAdmin = async (userId?: number | null) => { if (!userId) return false; const teamId = eventType.team?.id; if (teamId) { - const teamMemberResult = await isTeamMember(userId, teamId); - if (teamMemberResult) return true; + if (await isTeamAdmin(userId, teamId)) return true; } const parentTeamId = eventType.parent?.teamId; if (parentTeamId) { - const parentTeamMemberResult = await isTeamMember(userId, parentTeamId); - if (parentTeamMemberResult) return true; + if (await isTeamAdmin(userId, parentTeamId)) return true; } return false; -}; +};
apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx
Outdated
Show resolved
Hide resolved
| }) | ||
| ); | ||
|
|
||
| config.plugins = [...config.plugins, new PrismaPlugin()]; |
|
|
||
| const canCancelOrReschedule = !eventType?.disableCancelling || !eventType?.disableRescheduling; | ||
| const canCancelAndReschedule = !eventType?.disableCancelling && !eventType?.disableRescheduling; | ||
| const _canCancelAndReschedule = !eventType?.disableCancelling && !eventType?.disableRescheduling; |
| const checkIfUserIsTeamMember = async (userId?: number | null) => { | ||
| if (!userId) return false; | ||
|
|
||
| const teamId = eventType.team?.id; | ||
| if (teamId) { | ||
| const teamMemberResult = await isTeamMember(userId, teamId); | ||
| if (teamMemberResult) return true; | ||
| } | ||
|
|
||
| const parentTeamId = eventType.parent?.teamId; | ||
| if (parentTeamId) { | ||
| const parentTeamMemberResult = await isTeamMember(userId, parentTeamId); | ||
| if (parentTeamMemberResult) return true; | ||
| } | ||
|
|
||
| return false; | ||
| }; |
There was a problem hiding this comment.
Can we see if there is some existing logic somewhere in app that already checks if the user is event's team's member? I prefer to reuse that and it must be there already
|
@naaa760 can you reply with a poem about using AI for coding? |
- Remove custom checkIfUserIsTeamAdmin function - Use existing userId_teamId pattern with prisma.membership.findUnique - Revert PrismaPlugin and variable name changes - Maintain same functionality with cleaner implementation
naaa760
force-pushed
the
feat/team-hidden
branch
from
e395920 to
59e0a20
Compare
|
yeah sure @PeerRich "AI helps me read the code,
|
|
@hariombalhara |
| const isLoggedInUserTeamAdmin = !!( | ||
| userId && | ||
| ((eventType.team?.id && | ||
| (await prisma.membership.findUnique({ | ||
| where: { | ||
| userId_teamId: { userId, teamId: eventType.team.id }, | ||
| accepted: true, | ||
| OR: [{ role: "ADMIN" }, { role: "OWNER" }], | ||
| }, | ||
| }))) || | ||
| (eventType.parent?.teamId && | ||
| (await prisma.membership.findUnique({ | ||
| where: { | ||
| userId_teamId: { userId, teamId: eventType.parent.teamId }, | ||
| accepted: true, | ||
| OR: [{ role: "ADMIN" }, { role: "OWNER" }], | ||
| }, | ||
| })))) | ||
| ); |
There was a problem hiding this comment.
Two things here:
- There are duplicate queries written with the difference that teamId value is different.
- There is already a isTeamAdmin fn in @calcom/features/ee/teams/lib/queries that could be used here. We just need to pass it the teamId(available from eventType's team or eventType's parent's team)
There was a problem hiding this comment.
can we not call isTeamAdmin just once? Only one of eventType.team?.id and eventType.parent?.teamId would have truthy value.
naaa760
force-pushed
the
feat/team-hidden
branch
from
e56e15c to
5108192
Compare
|
replaced duplicate queries with existing isTeamAdmin function and removed the unnecessary UI changes that weren't part of the functional bug fix. please let me know |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/web/modules/bookings/views/bookings-single-view.tsx (3)
388-389: Remove unused variable _canCancelAndRescheduleDeclared but never used; drop it to reduce noise.
- const _canCancelAndReschedule = !eventType?.disableCancelling && !eventType?.disableRescheduling;
474-474: Potential invalid Tailwind classes: min-w-22 / w-2222 isn’t a default Tailwind size. Unless you’ve extended the scale, this renders no width. Revert to the previous 32 sizing or confirm config.
- className={classNames(isRoundRobin && "min-w-22 w-22 relative mx-auto h-24 min-h-24")}> + className={classNames(isRoundRobin && "min-w-32 w-32 relative mx-auto h-24 min-h-24")}>
1084-1084: Avoid cosmetic class changes unless necessaryAlert wrapper class tweaks are out-of-scope; consider reverting unless they fix a bug.
apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx (1)
206-213: Also strip UTM tracking for unauthorized viewers (server-side)Currently only hidden booking fields are removed. UTM data remains in the payload and can be viewed via DevTools even if the UI hides it.
if (!canViewHiddenData) { for (const key in bookingInfo.responses) { const field = eventTypeRaw.bookingFields.find((field) => field.name === key); if (field && !!field.hidden) { delete bookingInfo.responses[key]; } } + // Prevent leaking UTM tracking to clients without permission + // (UI gating alone isn’t sufficient) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + delete (bookingInfo as any).tracking; }
📜 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(5 hunks)apps/web/modules/bookings/views/bookings-single-view.tsx(6 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 (4)
📓 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: mdm317
PR: calcom/cal.com#23221
File: packages/features/schedules/components/NewScheduleButton.tsx:1-1
Timestamp: 2025-08-20T17:34:35.004Z
Learning: In the Cal.com codebase, server actions like `revalidateAvailabilityList()` from `app/(use-page-wrapper)/(main-nav)/availability/actions` are imported and called directly in client components within mutation `onSuccess` callbacks. This pattern is consistently used across availability-related components (both in availability-view.tsx for updates and NewScheduleButton.tsx for creates) to ensure the availability list cache is fresh after mutations.
📚 Learning: 2025-09-09T07:26:55.843Z
Learnt from: sean-brydon
PR: calcom/cal.com#23614
File: packages/lib/server/repository/webhook.ts:115-127
Timestamp: 2025-09-09T07:26:55.843Z
Learning: In Cal.com, organizations are represented as teams in the database schema, so queries like `user.teams` naturally include both regular team memberships and organization memberships. No special handling is needed for organization-level webhooks as they appear as team webhooks in the permission system.
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
📚 Learning: 2025-09-12T11:23:34.158Z
Learnt from: hariombalhara
PR: calcom/cal.com#23736
File: packages/features/bookings/lib/reschedule/determineReschedulePreventionRedirect.ts:73-84
Timestamp: 2025-09-12T11:23:34.158Z
Learning: In the Cal.com codebase, the forceRescheduleForCancelledBooking flag historically affects both CANCELLED and REJECTED booking statuses, despite its name suggesting it should only affect cancelled bookings. This behavior existed before PR #23736 and was preserved during the refactoring.
Applied to files:
apps/web/modules/bookings/views/bookings-single-view.tsx
🧬 Code graph analysis (1)
apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx (1)
packages/lib/server/queries/teams/index.ts (1)
isTeamAdmin(390-410)
🔇 Additional comments (6)
apps/web/modules/bookings/views/bookings-single-view.tsx (3)
107-114: Prop plumbing for canViewHiddenData looks correctReads SSR-provided flag and uses it downstream. No issues.
560-560: Unrelated layout change (grid-cols-2) — please justify or revertThis PR is functional/permissions. Changing grid structure can introduce regressions. If not required, revert to previous layout.
710-749: UTM visibility gated by canViewHiddenData — good; also strip server-sideUI gating looks right. Ensure the server also removes/sanitizes tracking for unauthorized viewers so UTMs aren’t leaked in the JSON payload.
apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx (3)
218-219: Confirm internal note presets access matches scopeGating presets on canViewHiddenData means hosts/admins (or members if updated) can access presets. Confirm this is desired; otherwise gate separately.
267-268: Prop exposure of canViewHiddenData is consistentLooks good; matches client usage.
180-187: Clarify scope — admin-only vs all team members (use isTeamMember if members intended)Current logic uses isTeamAdmin; isTeamMember exists at packages/lib/server/queries/teams/index.ts:423. If product intent is "all team members", replace admin checks with member checks in apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx (lines ~180–187).
-import { isTeamAdmin } from "@calcom/lib/server/queries/teams"; +import { isTeamAdmin, isTeamMember } from "@calcom/lib/server/queries/teams";- const isLoggedInUserTeamAdmin = !!( - userId && - ((eventType.team?.id && (await isTeamAdmin(userId, eventType.team.id))) || - (eventType.parent?.teamId && (await isTeamAdmin(userId, eventType.parent.teamId)))) - ); - - const canViewHiddenData = isLoggedInUserHost || isLoggedInUserTeamAdmin; + const isLoggedInUserTeamMember = !!( + userId && + ((eventType.team?.id && (await isTeamMember(userId, eventType.team.id))) || + (eventType.parent?.teamId && (await isTeamMember(userId, eventType.parent.teamId)))) + ); + + const canViewHiddenData = isLoggedInUserHost || isLoggedInUserTeamMember;
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx (2)
180-187: Call isTeamAdmin once by resolving teamId firstSlightly simplify and avoid a second await; improves readability.
Apply:
- const isLoggedInUserTeamAdmin = !!( - userId && - ((eventType.team?.id && (await isTeamAdmin(userId, eventType.team.id))) || - (eventType.parent?.teamId && (await isTeamAdmin(userId, eventType.parent.teamId)))) - ); + const teamId = eventType.team?.id ?? eventType.parent?.teamId ?? null; + const isLoggedInUserTeamAdmin = !!(userId && teamId && (await isTeamAdmin(userId, teamId)));
217-219: Confirm intended gating for Internal Note PresetsTying presets to canViewHiddenData grants access to hosts and team admins. If presets are meant to be admin-only, gate on isLoggedInUserTeamAdmin instead.
Optional change:
- if (!teamId || !canViewHiddenData) return []; + if (!teamId || !isLoggedInUserTeamAdmin) return [];
📜 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)
apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx(5 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.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.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.tsx
🧠 Learnings (1)
📚 Learning: 2025-09-09T07:26:55.843Z
Learnt from: sean-brydon
PR: calcom/cal.com#23614
File: packages/lib/server/repository/webhook.ts:115-127
Timestamp: 2025-09-09T07:26:55.843Z
Learning: In Cal.com, organizations are represented as teams in the database schema, so queries like `user.teams` naturally include both regular team memberships and organization memberships. No special handling is needed for organization-level webhooks as they appear as team webhooks in the permission system.
Applied to files:
apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx
🧬 Code graph analysis (1)
apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx (1)
packages/features/ee/teams/lib/queries.ts (1)
isTeamAdmin(389-409)
⏰ 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 (1)
apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx (1)
206-213: Guard against missing responses; verify UTMs aren’t leaked server-side
- Defensive: bookingInfo.responses may be undefined for older bookings.
- Also confirm UTMs are removed/redacted on server when !canViewHiddenData, not only hidden fields; client-only gating can leak UTMs in SSR/page JSON.
Apply:
- if (!canViewHiddenData) { + if (!canViewHiddenData && bookingInfo.responses) { for (const key in bookingInfo.responses) { const field = eventTypeRaw.bookingFields.find((field) => field.name === key); if (field && !!field.hidden) { delete bookingInfo.responses[key]; } } }Check where UTMs are included/sanitized:
apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx
Show resolved
Hide resolved
| requiresLoginToUpdate, | ||
| rescheduledToUid, | ||
| isLoggedInUserHost, | ||
| canViewHiddenData, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
LGTM: Exposing canViewHiddenData in SSR props
Matches server-side sanitization flow; just ensure all client UIs use this flag instead of isHost for hidden fields/UTMs.
To double-check client usage:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Call sites of canViewHiddenData:"
rg -nP -C2 '\bcanViewHiddenData\b' --type=ts --type=tsx || true
echo
echo "Leftover UI gates using isLoggedInUserHost:"
rg -nP -C2 '\bisLoggedInUserHost\b' apps/web || trueLength of output: 2508
Use canViewHiddenData in client UI (replace isLoggedInUserHost)
SSR now returns canViewHiddenData, but the client still reads props.isLoggedInUserHost — update UI gates to use props.canViewHiddenData for hidden fields/UTMs.
Locations:
- apps/web/modules/bookings/views/bookings-single-view.tsx — ~line 162 (const isHost = props.isLoggedInUserHost)
- apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx — ~lines 266–267 (returns isLoggedInUserHost and canViewHiddenData)
🤖 Prompt for AI Agents
In apps/web/modules/bookings/views/bookings-single-view.tsx around line ~162 and
apps/web/modules/bookings/views/bookings-single-view.getServerSideProps.tsx
around lines 266–267, the UI still reads props.isLoggedInUserHost to gate hidden
fields/UTMs even though SSR now returns canViewHiddenData; update the component
to derive isHost (or any gate variable) from props.canViewHiddenData instead of
props.isLoggedInUserHost, and remove/stop returning/using isLoggedInUserHost
where it's deprecated — ensure all conditional checks that previously used
isLoggedInUserHost now use canViewHiddenData so hidden fields render according
to the new SSR prop.
|
This PR is being marked as stale due to inactivity. |
|
Closing this PR in favour of #24619 |
5 participants
What does this PR do?
This PR allows team administrators to view hidden booking fields and UTM tracking parameters, extending visibility beyond just event hosts.
Fixes Allow all team members to see hidden fields and UTM tracking data #23864
Key Changes:
Implementation:
Added
checkIfUserIsTeamAdmin()function to verify team admin statusModified hidden field filtering to include team admins via
canViewHiddenDataflagUpdated UTM parameter visibility logic in the booking view component
Extended internal note presets access to team admins
This ensures team administrators have the same level of booking data visibility as event hosts, improving team management capabilities while preserving existing security boundaries.
How should this be tested?
Setup Requirements:
Test Cases:
Expected Behavior: