fix: fetching calendar links for seated events#24265
fix: fetching calendar links for seated events#24265dhairyashiil wants to merge 6 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. |
WalkthroughService now fetches bookings using a new repository method Pre-merge checks and finishing touches✅ Passed checks (5 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 |
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 (2)
apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts(1 hunks)packages/features/bookings/lib/getCalendarLinks.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{service,repository}.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Avoid dot-suffixes like
.service.tsor.repository.tsfor new files; reserve.test.ts,.spec.ts,.types.tsfor their specific purposes
Files:
apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.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:
apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.tspackages/features/bookings/lib/getCalendarLinks.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/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.tspackages/features/bookings/lib/getCalendarLinks.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.tspackages/features/bookings/lib/getCalendarLinks.ts
🧠 Learnings (3)
📓 Common learnings
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/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.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/getCalendarLinks.ts
🧬 Code graph analysis (1)
packages/features/bookings/lib/getCalendarLinks.ts (1)
packages/features/eventtypes/lib/eventNaming.ts (1)
nameObjectSchema(7-10)
⏰ 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: Install dependencies / Yarn install & cache
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (3)
packages/features/bookings/lib/getCalendarLinks.ts (2)
173-186: LGTM! Data source unification improves consistency.The changes correctly unify the data source for dynamic titles, attendee names, and booking fields by using
bookingData. This ensures consistency across calendar link generation and properly supports seated events.
171-171: Ensure correct attendee selection for calendar link dataThe code in packages/features/bookings/lib/getCalendarLinks.ts line 171 uses booking.attendees[0].bookingSeat.data when present. For seated events with multiple attendees, confirm that the first attendee always provides the right calendar link data—or adjust this to pick the intended attendee’s bookingSeat.
apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts (1)
1112-1114: LGTM — bookingSeat is included but will be null for non-seated events, so existing calendar links remain unaffected.
apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts
Outdated
Show resolved
Hide resolved
…methods - Convert getByUidWithAttendeesWithBookingSeatAndUserAndEvent to use select - Convert getByIdWithAttendeesWithBookingSeatAndUserAndEvent to use select - Add type casting for responses and metadata fields - Select only necessary fields from user, attendees, and bookingSeat relations Addresses code review comment in PR #24265 Co-Authored-By: Dhairyashil Shinde <dhairyashil10101010@gmail.com>
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 (3)
apps/api/v2/src/ee/bookings/2024-08-13/bookings.repository.ts(1 hunks)apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts(1 hunks)packages/features/bookings/lib/getCalendarLinks.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{service,repository}.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Avoid dot-suffixes like
.service.tsor.repository.tsfor new files; reserve.test.ts,.spec.ts,.types.tsfor their specific purposes
Files:
apps/api/v2/src/ee/bookings/2024-08-13/bookings.repository.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:
apps/api/v2/src/ee/bookings/2024-08-13/bookings.repository.tspackages/features/bookings/lib/getCalendarLinks.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/api/v2/src/ee/bookings/2024-08-13/bookings.repository.tspackages/features/bookings/lib/getCalendarLinks.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
apps/api/v2/src/ee/bookings/2024-08-13/bookings.repository.tspackages/features/bookings/lib/getCalendarLinks.ts
🧠 Learnings (3)
📓 Common learnings
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.
📚 Learning: 2025-08-19T09:47:49.478Z
Learnt from: eunjae-lee
PR: calcom/cal.com#23166
File: packages/prisma/migrations/20250818151914_routing_form_response_denormalized_backfill2/migration.sql:65-66
Timestamp: 2025-08-19T09:47:49.478Z
Learning: The Booking table has a unique constraint and index on the uid column (defined as `uid String unique` in schema.prisma), so JOINs on Booking.uid have optimal performance and don't require additional indexing.
Applied to files:
apps/api/v2/src/ee/bookings/2024-08-13/bookings.repository.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/getCalendarLinks.ts
🧬 Code graph analysis (1)
packages/features/bookings/lib/getCalendarLinks.ts (1)
packages/features/eventtypes/lib/eventNaming.ts (1)
nameObjectSchema(7-10)
⏰ 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: Type check / check-types
- GitHub Check: Linters / lint
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (5)
packages/features/bookings/lib/getCalendarLinks.ts (5)
10-10: LGTM!The
Prismatype import is necessary for the updated type signatures.
148-150: LGTM!The type changes from
Prisma.JsonObjecttoPrisma.JsonValueforresponsesandmetadataimprove flexibility, and the addition of theattendeesarray with nestedbookingSeat.datacorrectly supports the new data source for seated events.
171-176: Previous type safety concern has been addressed.The runtime type validation correctly prevents errors when
rawBookingDatais null, primitive, or an array. This implementation matches the previously suggested fix.
178-179: LGTM!The usage of
bookingDatainstead ofbooking.responsesis consistent throughout the function, correctly implementing the unified data source for both regular and seated events.Also applies to: 183-183, 188-188
171-176: No changes needed. getCalendarLinks intentionally uses the first attendee (the booking host) to source bookingSeat.data for personalization; existing tests confirm this behavior.
| } | ||
|
|
||
| async getByUidForCalendarLinks(uid: string) { | ||
| return this.dbRead.prisma.booking.findUnique({ |
There was a problem hiding this comment.
Created this separate function because it is faster and more efficient than getByUidWithAttendeesAndUserAndEvent, as it only fetches the fields needed for calendar links, reducing database load.
Also, didn’t want to modify getByUidWithAttendeesAndUserAndEvent as per CoderabbitAI’s comment, since this function is used in other parts of the codebase as well.
Let me know, if this works and other things I should update
pallava-joshi
left a comment
There was a problem hiding this comment.
type checks are failing. also implement code rabbit suggestions.
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (all 1 issues)
Understand the root cause of the following 1 issues and fix them.
<file name="apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts">
<violation number="1" location="apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts:1112">
getByUidForCalendarLinks strips eventTypeId, so this new call always fails the eventType check and the endpoint now throws for every booking.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (all 1 issues)
Understand the root cause of the following 1 issues and fix them.
<file name="packages/features/bookings/lib/getCalendarLinks.ts">
<violation number="1" location="packages/features/bookings/lib/getCalendarLinks.ts:171">
When pulling seated booking data we must read bookingSeat.data.responses; using the whole object leaves dynamic titles/attendee fields undefined and breaks calendar link personalization.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
|
|
||
| if (eventType.isDynamic && booking.responses.title) { | ||
| evtName = booking.responses.title as string; | ||
| const rawBookingData = booking.attendees?.[0]?.bookingSeat?.data || booking.responses; |
There was a problem hiding this comment.
When pulling seated booking data we must read bookingSeat.data.responses; using the whole object leaves dynamic titles/attendee fields undefined and breaks calendar link personalization.
Prompt for AI agents
Address the following comment on packages/features/bookings/lib/getCalendarLinks.ts at line 171:
<comment>When pulling seated booking data we must read bookingSeat.data.responses; using the whole object leaves dynamic titles/attendee fields undefined and breaks calendar link personalization.</comment>
<file context>
@@ -167,16 +168,24 @@ export const getCalendarLinks = ({
- if (eventType.isDynamic && booking.responses.title) {
- evtName = booking.responses.title as string;
+ const rawBookingData = booking.attendees?.[0]?.bookingSeat?.data || booking.responses;
+ const bookingData = (
+ rawBookingData && typeof rawBookingData === "object" && !Array.isArray(rawBookingData)
</file context>
| const rawBookingData = booking.attendees?.[0]?.bookingSeat?.data || booking.responses; | |
| const rawBookingData = (booking.attendees?.[0]?.bookingSeat?.data as Prisma.JsonObject | undefined)?.responses || booking.responses; |
|
This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active. |
Ryukemeister
left a comment
There was a problem hiding this comment.
@dhairyashiil thanks for creating the PR, had some questions:
- in the PR I can see you've attached a working video but what i'm curious about is why was it not working before? just wanna what were we doing here previously?
- can you also update the e2e tests for calendar links we have here
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/calendar-links.e2e-spec.ts
What does this PR do?
Working Video:
Screen.Recording.2025-10-04.at.9.55.23.PM.mov