feat(event-types): add team members as optional guests #22651
Conversation
|
@husniabad is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
|
|
WalkthroughThe PR adds support for selecting team members as optional guests: DB schema and migration for a many-to-many relation, API/schema updates, form controls and UI component to toggle/select optional team guests, plumbing through event builders and repositories, and calendar-provider integrations updated to add optional attendees and avoid duplicating them as required. Localization and defaults were added. Minor unrelated environment compatibility change to CSP nonce generation was also included. Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
apps/web/public/static/locales/en/common.json (1)
1497-1498: Grammar & clarity – adjust pluralization and streamline the description
The current copy mixes singular (“an optional guest”) with plural (“team members”) and is a bit verbose. Refining it improves readability and remains consistent with neighbouring strings.- "optional_guest_team_members": "Add team members as optional guests", - "optional_guest_team_members_description": "Adding team members as an optional guest will always send an optional invite, but not check their availability.", + "optional_guest_team_members": "Add team members as optional guests", + "optional_guest_team_members_description": "Selected team members are invited as optional guests without checking their availability.",packages/features/eventtypes/components/tabs/advanced/OptionalTeamGuestSetting.tsx (3)
26-28: Consider removing redundant useEffect.The
useEffectupdatingisOptionalTeamGuestEnabledappears redundant since the initial state already derives fromeventType.optionalGuestTeamMembers.length > 0. The component will re-render when props change, making this effect unnecessary.- useEffect(() => { - setIsOptionalTeamGuestEnabled(eventType.optionalGuestTeamMembers.length > 0); - }, [eventType.optionalGuestTeamMembers]);
42-42: Ensure consistent email fallback values.The email fallback values are inconsistent:
- Line 42:
"No Email"- Line 89:
""Use consistent fallback values to prevent display inconsistencies.
- label: option.user.email || "No Email", + label: option.user.email || "",Also applies to: 89-89
82-83: Remove unnecessary null check for onChange.The
onChangefunction is provided by react-hook-form's Controller and will always be defined. The null check is unnecessary.- if (!onChange) return; onChange(options.map((option) => ({ id: parseInt(option.value) })));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
apps/web/public/static/locales/en/common.json(1 hunks)packages/app-store/exchange2013calendar/lib/CalendarService.ts(3 hunks)packages/app-store/exchange2016calendar/lib/CalendarService.ts(3 hunks)packages/app-store/exchangecalendar/lib/CalendarService.ts(4 hunks)packages/app-store/feishucalendar/lib/CalendarService.ts(1 hunks)packages/app-store/googlecalendar/lib/CalendarService.ts(2 hunks)packages/app-store/larkcalendar/lib/CalendarService.ts(1 hunks)packages/app-store/zohocalendar/lib/CalendarService.ts(2 hunks)packages/features/CalendarEventBuilder.ts(1 hunks)packages/features/bookings/lib/handleNewBooking.ts(1 hunks)packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts(1 hunks)packages/features/eventtypes/components/tabs/advanced/EventAdvancedTab.tsx(2 hunks)packages/features/eventtypes/components/tabs/advanced/OptionalTeamGuestSetting.tsx(1 hunks)packages/features/eventtypes/lib/types.ts(1 hunks)packages/lib/defaultEvents.ts(1 hunks)packages/lib/server/repository/eventType.ts(1 hunks)packages/platform/atoms/event-types/hooks/useEventTypeForm.ts(1 hunks)packages/prisma/migrations/20250716153028_add_team_member_as_optional_guest/migration.sql(1 hunks)packages/prisma/schema.prisma(2 hunks)packages/trpc/server/routers/viewer/eventTypes/types.ts(1 hunks)packages/trpc/server/routers/viewer/eventTypes/update.handler.ts(4 hunks)packages/types/Calendar.d.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (16)
📓 Common learnings
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/prisma/schema.prisma:891-891
Timestamp: 2025-07-18T08:47:01.255Z
Learning: In Cal.com's calendar integration, both Google Calendar and Outlook Calendar are designed to allow multiple eventTypeIds to share the same subscription ID (googleChannelId or outlookSubscriptionId). This is an intentional design pattern to reuse existing subscriptions for efficiency rather than creating separate subscriptions for each event type. Therefore, unique constraints like `@@unique([outlookSubscriptionId, eventTypeId])` should not be added as they would break this subscription sharing functionality.
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/prisma/schema.prisma:891-891
Timestamp: 2025-07-18T08:47:01.256Z
Learning: The Outlook Calendar integration in Cal.com intentionally reuses subscription IDs across multiple event types for efficiency. The `upsertSelectedCalendarsForEventTypeIds` method creates separate SelectedCalendar records for each eventTypeId, all sharing the same outlookSubscriptionId. This subscription sharing pattern means that unique constraints like `@@unique([outlookSubscriptionId, eventTypeId])` should not be applied as they would prevent this intended functionality.
packages/features/eventtypes/lib/types.ts (1)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
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.
packages/platform/atoms/event-types/hooks/useEventTypeForm.ts (3)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
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.
Learnt from: alishaz-polymath
PR: calcom/cal.com#22304
File: packages/features/eventtypes/components/MultiplePrivateLinksController.tsx:92-94
Timestamp: 2025-07-16T06:42:27.001Z
Learning: In the MultiplePrivateLinksController component (packages/features/eventtypes/components/MultiplePrivateLinksController.tsx), the `currentLink.maxUsageCount ?? 1` fallback in the openSettingsDialog function is intentional. Missing maxUsageCount values indicate old/legacy private links that existed before the expiration feature was added, and they should default to single-use behavior (1) for backward compatibility.
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/prisma/schema.prisma:891-891
Timestamp: 2025-07-18T08:47:01.255Z
Learning: In Cal.com's calendar integration, both Google Calendar and Outlook Calendar are designed to allow multiple eventTypeIds to share the same subscription ID (googleChannelId or outlookSubscriptionId). This is an intentional design pattern to reuse existing subscriptions for efficiency rather than creating separate subscriptions for each event type. Therefore, unique constraints like `@@unique([outlookSubscriptionId, eventTypeId])` should not be added as they would break this subscription sharing functionality.
packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts (1)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
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.
packages/features/bookings/lib/handleNewBooking.ts (1)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
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.
packages/trpc/server/routers/viewer/eventTypes/types.ts (1)
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/prisma/schema.prisma:891-891
Timestamp: 2025-07-18T08:47:01.255Z
Learning: In Cal.com's calendar integration, both Google Calendar and Outlook Calendar are designed to allow multiple eventTypeIds to share the same subscription ID (googleChannelId or outlookSubscriptionId). This is an intentional design pattern to reuse existing subscriptions for efficiency rather than creating separate subscriptions for each event type. Therefore, unique constraints like `@@unique([outlookSubscriptionId, eventTypeId])` should not be added as they would break this subscription sharing functionality.
packages/app-store/feishucalendar/lib/CalendarService.ts (1)
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/app-store/office365calendar/api/webhook.ts:120-123
Timestamp: 2025-07-18T17:57:16.370Z
Learning: The office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is specifically designed for Office365 calendar integration, not as a generic webhook handler. Therefore, it's safe to assume that fetchAvailabilityAndSetCache method will be implemented in the Office365CalendarService, making explicit validation checks unnecessary.
packages/app-store/exchange2013calendar/lib/CalendarService.ts (3)
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/app-store/office365calendar/api/webhook.ts:120-123
Timestamp: 2025-07-18T17:57:16.370Z
Learning: The office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is specifically designed for Office365 calendar integration, not as a generic webhook handler. Therefore, it's safe to assume that fetchAvailabilityAndSetCache method will be implemented in the Office365CalendarService, making explicit validation checks unnecessary.
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/prisma/schema.prisma:891-891
Timestamp: 2025-07-18T08:47:01.256Z
Learning: The Outlook Calendar integration in Cal.com intentionally reuses subscription IDs across multiple event types for efficiency. The `upsertSelectedCalendarsForEventTypeIds` method creates separate SelectedCalendar records for each eventTypeId, all sharing the same outlookSubscriptionId. This subscription sharing pattern means that unique constraints like `@@unique([outlookSubscriptionId, eventTypeId])` should not be applied as they would prevent this intended functionality.
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/prisma/schema.prisma:891-891
Timestamp: 2025-07-18T08:47:01.255Z
Learning: In Cal.com's calendar integration, both Google Calendar and Outlook Calendar are designed to allow multiple eventTypeIds to share the same subscription ID (googleChannelId or outlookSubscriptionId). This is an intentional design pattern to reuse existing subscriptions for efficiency rather than creating separate subscriptions for each event type. Therefore, unique constraints like `@@unique([outlookSubscriptionId, eventTypeId])` should not be added as they would break this subscription sharing functionality.
packages/app-store/exchangecalendar/lib/CalendarService.ts (3)
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/app-store/office365calendar/api/webhook.ts:120-123
Timestamp: 2025-07-18T17:57:16.370Z
Learning: The office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is specifically designed for Office365 calendar integration, not as a generic webhook handler. Therefore, it's safe to assume that fetchAvailabilityAndSetCache method will be implemented in the Office365CalendarService, making explicit validation checks unnecessary.
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/prisma/schema.prisma:891-891
Timestamp: 2025-07-18T08:47:01.256Z
Learning: The Outlook Calendar integration in Cal.com intentionally reuses subscription IDs across multiple event types for efficiency. The `upsertSelectedCalendarsForEventTypeIds` method creates separate SelectedCalendar records for each eventTypeId, all sharing the same outlookSubscriptionId. This subscription sharing pattern means that unique constraints like `@@unique([outlookSubscriptionId, eventTypeId])` should not be applied as they would prevent this intended functionality.
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/prisma/schema.prisma:891-891
Timestamp: 2025-07-18T08:47:01.255Z
Learning: In Cal.com's calendar integration, both Google Calendar and Outlook Calendar are designed to allow multiple eventTypeIds to share the same subscription ID (googleChannelId or outlookSubscriptionId). This is an intentional design pattern to reuse existing subscriptions for efficiency rather than creating separate subscriptions for each event type. Therefore, unique constraints like `@@unique([outlookSubscriptionId, eventTypeId])` should not be added as they would break this subscription sharing functionality.
packages/app-store/larkcalendar/lib/CalendarService.ts (1)
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/app-store/office365calendar/api/webhook.ts:120-123
Timestamp: 2025-07-18T17:57:16.370Z
Learning: The office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is specifically designed for Office365 calendar integration, not as a generic webhook handler. Therefore, it's safe to assume that fetchAvailabilityAndSetCache method will be implemented in the Office365CalendarService, making explicit validation checks unnecessary.
packages/app-store/exchange2016calendar/lib/CalendarService.ts (3)
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/app-store/office365calendar/api/webhook.ts:120-123
Timestamp: 2025-07-18T17:57:16.370Z
Learning: The office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is specifically designed for Office365 calendar integration, not as a generic webhook handler. Therefore, it's safe to assume that fetchAvailabilityAndSetCache method will be implemented in the Office365CalendarService, making explicit validation checks unnecessary.
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/prisma/schema.prisma:891-891
Timestamp: 2025-07-18T08:47:01.256Z
Learning: The Outlook Calendar integration in Cal.com intentionally reuses subscription IDs across multiple event types for efficiency. The `upsertSelectedCalendarsForEventTypeIds` method creates separate SelectedCalendar records for each eventTypeId, all sharing the same outlookSubscriptionId. This subscription sharing pattern means that unique constraints like `@@unique([outlookSubscriptionId, eventTypeId])` should not be applied as they would prevent this intended functionality.
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/prisma/schema.prisma:891-891
Timestamp: 2025-07-18T08:47:01.255Z
Learning: In Cal.com's calendar integration, both Google Calendar and Outlook Calendar are designed to allow multiple eventTypeIds to share the same subscription ID (googleChannelId or outlookSubscriptionId). This is an intentional design pattern to reuse existing subscriptions for efficiency rather than creating separate subscriptions for each event type. Therefore, unique constraints like `@@unique([outlookSubscriptionId, eventTypeId])` should not be added as they would break this subscription sharing functionality.
packages/app-store/googlecalendar/lib/CalendarService.ts (1)
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/app-store/office365calendar/api/webhook.ts:120-123
Timestamp: 2025-07-18T17:57:16.370Z
Learning: The office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is specifically designed for Office365 calendar integration, not as a generic webhook handler. Therefore, it's safe to assume that fetchAvailabilityAndSetCache method will be implemented in the Office365CalendarService, making explicit validation checks unnecessary.
packages/app-store/zohocalendar/lib/CalendarService.ts (1)
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/app-store/office365calendar/api/webhook.ts:120-123
Timestamp: 2025-07-18T17:57:16.370Z
Learning: The office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is specifically designed for Office365 calendar integration, not as a generic webhook handler. Therefore, it's safe to assume that fetchAvailabilityAndSetCache method will be implemented in the Office365CalendarService, making explicit validation checks unnecessary.
packages/features/eventtypes/components/tabs/advanced/OptionalTeamGuestSetting.tsx (2)
Learnt from: alishaz-polymath
PR: calcom/cal.com#22304
File: packages/features/eventtypes/components/MultiplePrivateLinksController.tsx:92-94
Timestamp: 2025-07-16T06:42:27.001Z
Learning: In the MultiplePrivateLinksController component (packages/features/eventtypes/components/MultiplePrivateLinksController.tsx), the `currentLink.maxUsageCount ?? 1` fallback in the openSettingsDialog function is intentional. Missing maxUsageCount values indicate old/legacy private links that existed before the expiration feature was added, and they should default to single-use behavior (1) for backward compatibility.
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
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.
apps/web/public/static/locales/en/common.json (1)
undefined
<retrieved_learning>
Learnt from: bandhan-majumder
PR: #22359
File: packages/lib/server/locales/en/common.json:1336-1339
Timestamp: 2025-07-14T16:31:45.201Z
Learning: When making localization changes for new features, it's often safer to add new strings rather than modify existing ones to avoid breaking existing functionality that depends on the original strings. This approach allows for feature-specific customization while maintaining backward compatibility.
</retrieved_learning>
packages/features/eventtypes/components/tabs/advanced/EventAdvancedTab.tsx (2)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
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.
Learnt from: alishaz-polymath
PR: calcom/cal.com#22304
File: packages/features/eventtypes/components/MultiplePrivateLinksController.tsx:92-94
Timestamp: 2025-07-16T06:42:27.001Z
Learning: In the MultiplePrivateLinksController component (packages/features/eventtypes/components/MultiplePrivateLinksController.tsx), the `currentLink.maxUsageCount ?? 1` fallback in the openSettingsDialog function is intentional. Missing maxUsageCount values indicate old/legacy private links that existed before the expiration feature was added, and they should default to single-use behavior (1) for backward compatibility.
🧬 Code Graph Analysis (2)
packages/app-store/exchangecalendar/lib/CalendarService.ts (2)
packages/trpc/server/routers/publicViewer/procedures/event.ts (1)
event(8-12)packages/types/Calendar.d.ts (1)
Person(33-45)
packages/trpc/server/routers/viewer/eventTypes/update.handler.ts (1)
packages/platform/libraries/index.ts (2)
TRPCError(56-56)SchedulingType(30-30)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: required
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Security Check
🔇 Additional comments (32)
packages/features/eventtypes/lib/types.ts (1)
168-168: LGTM! Type definition is appropriate for the feature.The
optionalGuestTeamMembersproperty is correctly defined as an array of objects with numeric IDs, which is suitable for storing references to selected team members in the form state.packages/lib/defaultEvents.ts (1)
147-147: LGTM! Proper default initialization.The empty array default for
optionalGuestTeamMembersis appropriate and follows the established pattern in the commons object for initializing optional array properties.packages/types/Calendar.d.ts (1)
224-227: LGTM! Well-structured type definition for calendar events.The
optionalGuestTeamMembersproperty is appropriately defined with:
- Required
- Optional
namefield (allowing null)- Optional property for backward compatibility
The structure correctly differs from the form type (using email/name instead of just id) as calendar events need actual contact information rather than just references.
packages/trpc/server/routers/viewer/eventTypes/types.ts (1)
90-93: LGTM! Proper schema validation definition.The
optionalGuestTeamMembersschema is well-defined with:
- Appropriate Zod validation for array of objects with numeric IDs
- Optional and nullable for backward compatibility
- Consistent structure with the form type definitions
The validation ensures type safety while maintaining flexibility for different input states.
packages/features/bookings/lib/handleNewBooking.ts (1)
1163-1163: LGTM! Clean integration with the CalendarEventBuilder pattern.The addition follows the established builder pattern used throughout the file and includes appropriate null-safety with the empty array fallback.
packages/platform/atoms/event-types/hooks/useEventTypeForm.ts (1)
83-83: LGTM! Proper form field initialization.The addition follows the existing pattern for initializing optional array fields in the form's default values, ensuring consistent form state management.
packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts (1)
175-181: LGTM! Well-structured Prisma relation selection.The addition follows the established pattern for relation selections in this query and appropriately selects the necessary fields (name, email, id) for optional guest team member functionality.
packages/lib/server/repository/eventType.ts (1)
520-524: LGTM! Well-integrated addition following established patterns.The addition of
optionalGuestTeamMembersto the select object follows the same pattern as other relations in the codebase, appropriately selecting only theidfield. This change aligns with the PR objective to support optional guest team members functionality.packages/features/CalendarEventBuilder.ts (1)
265-276: LGTM! Clean builder method implementation.The
withOptionalGuestTeamMembersmethod follows the established builder pattern perfectly, with appropriate TypeScript typing and safe defaults. The method signature aligns with the CalendarEvent interface and provides a clean API for setting optional guest team members.packages/prisma/schema.prisma (1)
206-209: Rename the new relation field for clarity & to avoid naming collisionsThe field name
EventTypeis too generic, starts with a capital letter (inconsistent with the rest of the model), and clashes semantically with the already-existingeventTypes/ownedEventTypesrelations. This will generate an awkward Prisma Client API (user.EventType) that gives no hint that it represents “optional guest” event types.- optionalGuestTeamMembers User[] @relation("optional_guest_team_members") + optionalGuestTeamMembers User[] @relation("optional_guest_team_members")No change is needed on the
EventTypeside, but onUserthe mirror field should be renamed to something descriptive, e.g.optionalGuestForEventTypes.Likely an incorrect or invalid review comment.
packages/features/eventtypes/components/tabs/advanced/EventAdvancedTab.tsx (2)
66-66: LGTM: Clean import addition.The import is properly placed in alphabetical order and follows the established pattern for local component imports.
1205-1207: LGTM: Well-implemented conditional rendering.The component is properly conditionally rendered with appropriate checks for team existence and members. The props passed (
teamandeventType) align with the component's requirements, and the placement before the round-robin setting makes logical sense.packages/app-store/larkcalendar/lib/CalendarService.ts (1)
402-448: Excellent refactor for attendee handling.The restructured
translateAttendeesmethod demonstrates several best practices:
- Efficient lookup: Using a
Setfor optional guest emails provides O(1) lookup performance- Clear separation: The 4-step approach with descriptive comments makes the logic easy to follow
- Duplication prevention: Properly filters required team members to exclude those marked as optional guests
- Proper flagging: Correctly sets
is_optionalbased on attendee typeThis refactor aligns well with the broader pattern across calendar services for handling optional guest team members.
packages/app-store/exchange2013calendar/lib/CalendarService.ts (3)
72-96: Well-implemented attendee handling in createEvent.The changes properly implement the optional guest team member feature:
- Efficient lookup: Set-based approach for checking optional guest emails
- Proper segregation: Required team members are filtered to exclude optional guests, preventing duplication
- Correct API usage: Uses Exchange's
RequiredAttendeesandOptionalAttendeescollections appropriately- Clear logic flow: Well-commented steps make the intent obvious
128-155: Comprehensive updateEvent implementation.The changes ensure proper attendee management during updates:
- Prevents accumulation: Clearing existing attendees before adding new ones avoids stale entries
- Consistent logic: Uses the same efficient attendee handling as
createEvent- Complete coverage: Handles both required and optional attendees consistently
This thorough approach ensures the update operation maintains data integrity.
104-104: Good improvement: Added descriptive type field.Setting the
typeto"exchange2013_calendar"instead of an empty string provides better service identification and debugging capabilities.packages/app-store/googlecalendar/lib/CalendarService.ts (3)
104-107: LGTM! Efficient email lookup implementation.The use of a
Setwith lowercase conversion provides optimal O(1) lookup performance and ensures case-insensitive email matching.
111-116: Good deduplication logic.The enhanced filter correctly prevents team members from being added as both required and optional attendees, maintaining clean attendee lists.
130-147: Excellent implementation of optional guest handling.The code correctly sets
optional: trueandresponseStatus: "needsAction"for optional attendees, with proper deduplication logic to prevent conflicts with existing attendees.packages/app-store/exchange2016calendar/lib/CalendarService.ts (4)
73-90: Consistent implementation with proper deduplication.The optional guest handling follows the established pattern from Google Calendar integration, correctly preventing team members from being added as both required and optional attendees.
93-97: Correct use of Exchange API for optional attendees.The code properly utilizes
appointment.OptionalAttendees.Add()to distinguish optional guests from required attendees in the Exchange calendar system.
129-156: Good practice of clearing existing attendees in updates.The implementation correctly clears both required and optional attendees before adding new ones, preventing stale data accumulation. The attendee handling logic consistently mirrors the
createEventmethod.
105-105: Good addition of explicit type identifier.Setting the
typefield to"exchange2016_calendar"improves clarity and makes it easier to identify the calendar integration used for event creation.packages/app-store/exchangecalendar/lib/CalendarService.ts (3)
63-87: Consistent implementation with proper TypeScript typing.The optional guest handling follows the established pattern with correct type annotations. The deduplication logic properly prevents conflicts between required and optional attendees.
117-143: Proper attendee management in updates.The implementation correctly clears existing attendees before reconstruction, preventing data inconsistencies. The logic consistently mirrors the
createEventmethod pattern.
95-95: Good addition of explicit type identifiers.Setting the
typefield to"exchange_calendar"in both methods improves consistency and makes it easier to identify the calendar integration used.Also applies to: 154-154
packages/app-store/feishucalendar/lib/CalendarService.ts (2)
404-423: Excellent refactoring with improved deduplication.The introduction of the
addUniqueAttendeehelper function and Set-based deduplication significantly improves code organization and prevents duplicate attendees. The case-insensitive email tracking is a good practice.
424-444: Well-structured 4-step attendee processing.The systematic approach with clear step documentation makes the attendee handling logic easy to understand and maintain. The filtering correctly prevents duplication and excludes the current user from team member invitations.
packages/prisma/migrations/20250716153028_add_team_member_as_optional_guest/migration.sql (1)
1-18: Well-structured many-to-many relationship migration.The migration follows Prisma conventions with proper indexing and CASCADE constraints. The unique index on
(A, B)prevents duplicate relationships while the index onBoptimizes queries from the user side.packages/app-store/zohocalendar/lib/CalendarService.ts (1)
470-470: Good refactoring to extract attendee logic.Moving the attendee construction logic to a dedicated method improves code organization and maintainability.
packages/trpc/server/routers/viewer/eventTypes/update.handler.ts (2)
394-418: Well-implemented validation for optional guest team members.The validation logic properly handles:
- Duplicate detection using Set for efficiency
- Team membership verification to ensure data integrity
- Clear error messages for different failure scenarios
- Correct Prisma relation update pattern with set/connect
432-471: Good defensive programming for host updates.The added conditional check
if (hosts && Array.isArray(hosts))prevents unintended modifications to the hosts relation when the field is not included in the update request. This is an important improvement for API robustness.
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (07/20/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add community label" took an action on this PR • (07/20/25)1 label was added to this PR based on Keith Williams's automation. |
2f2e97f to
ad8e8a6
Compare
ad8e8a6 to
bf05524
Compare
Issue fixed, please check again |
|
This PR is being marked as stale due to inactivity. |
bf05524 to
336a5da
Compare
|
Hi @kart1ka I have updated the PR, could you please review |
|
Good work @husniabad! |
Thank you @kart1ka for approving this PR, I hope you and the team to merge it soon 🙏 |
|
@husniabad Could you please resolve the merge conflicts? |
2f0cb60 to
f87a6d7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/lib/csp.ts (1)
39-50: Document the magic number.22 bytes is a good choice (base64 encodes to 32 chars). Add a quick inline note so future edits don’t accidentally change CSP expectations.
- const BYTE_LEN = 22; + const BYTE_LEN = 22; // 22 bytes → 32 base64 chars (no padding) for CSP nonce
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
apps/web/lib/csp.ts(1 hunks)apps/web/public/static/locales/en/common.json(1 hunks)packages/app-store/exchange2013calendar/lib/CalendarService.ts(3 hunks)packages/app-store/exchange2016calendar/lib/CalendarService.ts(3 hunks)packages/app-store/exchangecalendar/lib/CalendarService.ts(4 hunks)packages/app-store/feishucalendar/lib/CalendarService.ts(1 hunks)packages/app-store/googlecalendar/lib/CalendarService.ts(2 hunks)packages/app-store/larkcalendar/lib/CalendarService.ts(1 hunks)packages/app-store/zohocalendar/lib/CalendarService.ts(2 hunks)packages/features/CalendarEventBuilder.ts(1 hunks)packages/features/bookings/lib/handleNewBooking.ts(1 hunks)packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts(1 hunks)packages/features/eventtypes/components/tabs/advanced/EventAdvancedTab.tsx(2 hunks)packages/features/eventtypes/components/tabs/advanced/OptionalTeamGuestSetting.tsx(1 hunks)packages/features/eventtypes/lib/types.ts(1 hunks)packages/lib/defaultEvents.ts(1 hunks)packages/lib/server/repository/eventTypeRepository.ts(2 hunks)packages/platform/atoms/event-types/hooks/useEventTypeForm.ts(1 hunks)packages/prisma/migrations/20250716153028_add_team_member_as_optional_guest/migration.sql(1 hunks)packages/prisma/schema.prisma(2 hunks)packages/trpc/server/routers/viewer/eventTypes/types.ts(1 hunks)packages/trpc/server/routers/viewer/eventTypes/update.handler.ts(3 hunks)packages/types/Calendar.d.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (21)
- packages/features/eventtypes/lib/types.ts
- packages/lib/defaultEvents.ts
- packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts
- packages/platform/atoms/event-types/hooks/useEventTypeForm.ts
- packages/features/CalendarEventBuilder.ts
- packages/trpc/server/routers/viewer/eventTypes/types.ts
- packages/app-store/larkcalendar/lib/CalendarService.ts
- packages/features/eventtypes/components/tabs/advanced/EventAdvancedTab.tsx
- packages/app-store/exchangecalendar/lib/CalendarService.ts
- packages/features/bookings/lib/handleNewBooking.ts
- packages/app-store/exchange2016calendar/lib/CalendarService.ts
- packages/types/Calendar.d.ts
- packages/prisma/schema.prisma
- packages/app-store/exchange2013calendar/lib/CalendarService.ts
- packages/app-store/feishucalendar/lib/CalendarService.ts
- packages/prisma/migrations/20250716153028_add_team_member_as_optional_guest/migration.sql
- packages/features/eventtypes/components/tabs/advanced/OptionalTeamGuestSetting.tsx
- apps/web/public/static/locales/en/common.json
- packages/app-store/googlecalendar/lib/CalendarService.ts
- packages/app-store/zohocalendar/lib/CalendarService.ts
- packages/trpc/server/routers/viewer/eventTypes/update.handler.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
apps/web/lib/csp.tspackages/lib/server/repository/eventTypeRepository.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/csp.tspackages/lib/server/repository/eventTypeRepository.ts
**/*Repository.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Repository files must include
Repositorysuffix, prefix with technology if applicable (e.g.,PrismaAppRepository.ts), and use PascalCase matching the exported class
Files:
packages/lib/server/repository/eventTypeRepository.ts
🧠 Learnings (9)
📓 Common learnings
Learnt from: sean-brydon
PR: calcom/cal.com#22618
File: packages/trpc/server/routers/viewer/eventTypes/utils/transformUtils.ts:113-113
Timestamp: 2025-08-05T07:42:06.335Z
Learning: In Cal.com's getUserEventGroups handler refactor (PR #22618), the membershipCount field for team event groups is intentionally set to 0 in the new createTeamEventGroup function, as confirmed by sean-brydon (PR author). This preserves the same behavior as the old implementation that was being refactored, maintaining backward compatibility. While other parts of the codebase may use actual member counts, this specific implementation maintains the previous behavior.
Learnt from: sean-brydon
PR: calcom/cal.com#22618
File: packages/trpc/server/routers/viewer/eventTypes/utils/transformUtils.ts:113-113
Timestamp: 2025-08-05T07:42:06.335Z
Learning: In Cal.com's event type system, the membershipCount field for team event groups is intentionally set to 0, as confirmed by sean-brydon (PR author). This behavior was preserved during the refactor from the old getUserEventGroups.handler.ts implementation to the new createTeamEventGroup function in transformUtils.ts. This is not a bug but the intended behavior that maintains consistency with the previous implementation.
Learnt from: sean-brydon
PR: calcom/cal.com#22618
File: packages/trpc/server/routers/viewer/eventTypes/utils/transformUtils.ts:113-113
Timestamp: 2025-08-05T07:42:06.335Z
Learning: In Cal.com's event type system, the membershipCount field for team event groups is intentionally hard-coded to 0, as confirmed by sean-brydon. This behavior was preserved during the refactor from the old getUserEventGroups.handler.ts implementation to the new createTeamEventGroup function in transformUtils.ts. This is not a bug but the intended behavior.
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/prisma/schema.prisma:891-891
Timestamp: 2025-07-18T08:47:01.264Z
Learning: In Cal.com's calendar integration, both Google Calendar and Outlook Calendar are designed to allow multiple eventTypeIds to share the same subscription ID (googleChannelId or outlookSubscriptionId). This is an intentional design pattern to reuse existing subscriptions for efficiency rather than creating separate subscriptions for each event type. Therefore, unique constraints like `@unique([outlookSubscriptionId, eventTypeId])` should not be added as they would break this subscription sharing functionality.
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/prisma/schema.prisma:891-891
Timestamp: 2025-07-18T08:47:01.264Z
Learning: The Outlook Calendar integration in Cal.com intentionally reuses subscription IDs across multiple event types for efficiency. The `upsertSelectedCalendarsForEventTypeIds` method creates separate SelectedCalendar records for each eventTypeId, all sharing the same outlookSubscriptionId. This subscription sharing pattern means that unique constraints like `@unique([outlookSubscriptionId, eventTypeId])` should not be applied as they would prevent this intended functionality.
📚 Learning: 2025-07-28T11:50:23.946Z
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.ts : Ensure the `credential.key` field is never returned from tRPC endpoints or APIs
Applied to files:
apps/web/lib/csp.ts
📚 Learning: 2025-08-05T07:42:06.335Z
Learnt from: sean-brydon
PR: calcom/cal.com#22618
File: packages/trpc/server/routers/viewer/eventTypes/utils/transformUtils.ts:113-113
Timestamp: 2025-08-05T07:42:06.335Z
Learning: In Cal.com's getUserEventGroups handler refactor (PR #22618), the membershipCount field for team event groups is intentionally set to 0 in the new createTeamEventGroup function, as confirmed by sean-brydon (PR author). This preserves the same behavior as the old implementation that was being refactored, maintaining backward compatibility. While other parts of the codebase may use actual member counts, this specific implementation maintains the previous behavior.
Applied to files:
packages/lib/server/repository/eventTypeRepository.ts
📚 Learning: 2025-08-05T07:42:06.335Z
Learnt from: sean-brydon
PR: calcom/cal.com#22618
File: packages/trpc/server/routers/viewer/eventTypes/utils/transformUtils.ts:113-113
Timestamp: 2025-08-05T07:42:06.335Z
Learning: In Cal.com's event type system, the membershipCount field for team event groups is intentionally set to 0, as confirmed by sean-brydon (PR author). This behavior was preserved during the refactor from the old getUserEventGroups.handler.ts implementation to the new createTeamEventGroup function in transformUtils.ts. This is not a bug but the intended behavior that maintains consistency with the previous implementation.
Applied to files:
packages/lib/server/repository/eventTypeRepository.ts
📚 Learning: 2025-08-05T07:42:06.335Z
Learnt from: sean-brydon
PR: calcom/cal.com#22618
File: packages/trpc/server/routers/viewer/eventTypes/utils/transformUtils.ts:113-113
Timestamp: 2025-08-05T07:42:06.335Z
Learning: In Cal.com's event type system, the membershipCount field for team event groups is intentionally hard-coded to 0, as confirmed by sean-brydon. This behavior was preserved during the refactor from the old getUserEventGroups.handler.ts implementation to the new createTeamEventGroup function in transformUtils.ts. This is not a bug but the intended behavior.
Applied to files:
packages/lib/server/repository/eventTypeRepository.ts
📚 Learning: 2025-07-28T11:50:23.946Z
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.ts : For Prisma queries, only select data you need; never use `include`, always use `select`
Applied to files:
packages/lib/server/repository/eventTypeRepository.ts
📚 Learning: 2025-08-08T09:27:23.870Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:195-216
Timestamp: 2025-08-08T09:27:23.870Z
Learning: In PR calcom/cal.com#22919, file packages/features/calAIPhone/providers/retellAI/services/AgentService.ts, the updateAgentConfiguration method intentionally does not persist the optional `name` parameter to the repository for now, per maintainer (Udit-takkar). Future reviews should not flag this unless requirements change.
Applied to files:
packages/lib/server/repository/eventTypeRepository.ts
📚 Learning: 2025-08-07T18:42:34.056Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/lib/server/repository/PrismaPhoneNumberRepository.ts:412-417
Timestamp: 2025-08-07T18:42:34.056Z
Learning: In Cal.com codebase, the coding guideline requiring explicit `select` clauses instead of `include` for Prisma queries applies to read operations but not to update operations. Update operations don't need explicit select clauses.
Applied to files:
packages/lib/server/repository/eventTypeRepository.ts
📚 Learning: 2025-08-08T09:29:11.665Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts:118-143
Timestamp: 2025-08-08T09:29:11.665Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (TypeScript), the AIPhoneServiceAgentListItem is required to include user.email in listAgents responses (per maintainer Udit-takkar). Future reviews should not flag this as unnecessary PII unless requirements change.
Applied to files:
packages/lib/server/repository/eventTypeRepository.ts
⏰ Context from checks skipped due to timeout of 90000ms. 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 (4)
packages/lib/server/repository/eventTypeRepository.ts (3)
302-306: AI summary vs code: minimal-data method does not include optionalGuestTeamMembersThe AI summary mentions adding optionalGuestTeamMembers to findAllByUpIdWithMinimalData, but this select currently includes only eventTypeSelect and hashedLink. If minimal views/UI need to show existing optional guests, consider adding the id-only selection here; otherwise update the PR summary for accuracy.
Proposed change:
const select = { ...eventTypeSelect, hashedLink: hashedLinkSelect, + optionalGuestTeamMembers: { select: { id: true } }, };To ensure we don’t duplicate what eventTypeSelect might already include, check its contents:
#!/bin/bash # Locate and inspect eventTypeSelect definition and callers fd -a 'eventTypeSelect.ts' | while read -r f; do echo "==> $f" rg -n 'optionalGuestTeamMembers|export const eventTypeSelect' "$f" -n -A 3 done # Find usages of findAllByUpIdWithMinimalData to confirm consumer needs rg -n --hidden --glob '!node_modules' 'findAllByUpIdWithMinimalData' -A 3Likely an incorrect or invalid review comment.
866-870: Approve minimal id-only select for optionalGuestTeamMembers
Selecting only id in eventTypeRepository keeps the payload minimal and aligns with how the viewer and update routes consume this data. Downstream booking flows (via getEventTypesFromDB) still fetch email and name for calendar-provider integrations, and the UI maps the returned IDs to full member details in the OptionalTeamGuestSetting component. No further changes needed.
565-569: optionalGuestTeamMembers select is safe as implementedAll downstream consumers either only need an
idor explicitly fetch additional fields:
- UI (OptionalTeamGuestSetting.tsx) only reads
.lengthand IDs to toggle settings.- tRPC schema (
viewer/eventTypes/types.ts) definesoptionalGuestTeamMembersasarray<{id: number}>.- Booking flow (
getEventTypesFromDB.ts) uses its own query selectingid,name, andNo changes required.
apps/web/lib/csp.ts (1)
41-48: No client-side bundling risk detectedWe verified that
getCspNonceis only imported inapps/web/middleware.ts(server‐only), and there are no imports ofapps/web/lib/cspin any client components, pages, or hooks. The runtime guard aroundcrypto/node:cryptocovers both environments, so no further changes are needed here.
f87a6d7 to
55a564f
Compare
Adds a setting to event types to automatically invite selected team members as optional attendees to meetings. This does not check their availability. Closes CAL-5091
55a564f to
c2da2f1
Compare
|
Hi @kart1ka I updated the PR and fixed merge conflicts as well I addressed the test failure The test failure was not related to my PR, most probably due to the previous merged PR, I fixed it in this file: |
| const oldHostsSet = new Set(eventType.hosts.map((oldHost) => oldHost.userId)); | ||
| const newHostsSet = new Set(hosts.map((oldHost) => oldHost.userId)); | ||
| // weights were already enabled or are enabled now | ||
| const isWeightsEnabled = |
| const BYTE_LEN = 22; | ||
| const bytes = new Uint8Array(BYTE_LEN); | ||
|
|
||
| return nonce; | ||
| // Prefer Web Crypto in both browser and modern Node runtimes | ||
| const hasWebCrypto = | ||
| typeof (globalThis as any).crypto !== "undefined" && | ||
| typeof (globalThis as any).crypto.getRandomValues === "function"; | ||
|
|
||
| if (hasWebCrypto) { | ||
| (globalThis as any).crypto.getRandomValues(bytes); | ||
| } else { | ||
| // Node.js environment (including tests) | ||
| // eslint-disable-next-line @typescript-eslint/no-var-requires | ||
| const nodeCrypto = require("node:crypto") as typeof import("node:crypto"); | ||
| if (nodeCrypto.webcrypto?.getRandomValues) { | ||
| nodeCrypto.webcrypto.getRandomValues(bytes); | ||
| } else { | ||
| nodeCrypto.randomFillSync(bytes); | ||
| } | ||
| } | ||
|
|
||
| return buildNonce(bytes); |
There was a problem hiding this comment.
This one has nothing to do with the implementation feature; I added it because tests were failing due to another merged PR the time I pushed
Adds a setting to event types to automatically invite selected team members as optional attendees to meetings. This does not check their availability. Closes CAL-5091
What does this PR do?
Visual Demo (For contributors especially)
A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).
Video Demo (if applicable):
Add team member as optional guest - Watch Video
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
/claim [CAL-5091] additional settings: "add team member as optional guest #18947
Checklist