feat: add optional guest members from team#22127
feat: add optional guest members from team#22127jayantpranjal0 wants to merge 1 commit intocalcom:mainfrom
Conversation
|
Someone is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
|
|
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
github-actions
bot
added
❗️ migrations
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (06/29/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add community label" took an action on this PR • (06/29/25)1 label was added to this PR based on Keith Williams's automation. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
cubic found 6 issues across 21 files. Review them in cubic.dev
React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai to give specific feedback.
packages/features/eventtypes/components/tabs/advanced/EventAdvancedTab.tsx
Outdated
Show resolved
Hide resolved
packages/trpc/server/routers/viewer/eventTypes/update.handler.ts
Outdated
Show resolved
Hide resolved
jayantpranjal0
force-pushed
the
add_guest_team
branch
from
8422ad3 to
082b9a2
Compare
jayantpranjal0
changed the title
jayantpranjal0
force-pushed
the
add_guest_team
branch
from
082b9a2 to
04c7f07
Compare
|
@cubic-dev-ai check for any issues again |
|
@jayantpranjal0 I've started the AI code review. It'll take a few minutes to complete. |
There was a problem hiding this comment.
cubic found 4 issues across 21 files. Review them in cubic.dev
React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai to give specific feedback.
packages/features/eventtypes/components/tabs/advanced/GuestTeamMemberController.tsx
Show resolved
Hide resolved
Devanshusharma2005
left a comment
Devanshusharma2005
left a comment
There was a problem hiding this comment.
hey @jayantpranjal0 thanks for the pr. can you please address the issues suggested by cubic ?
|
Making it draft. Feel free to rfr once the comments are addressed. |
jayantpranjal0
force-pushed
the
add_guest_team
branch
from
04c7f07 to
af00797
Compare
|
@cubic-dev-ai check out this commit |
|
@jayantpranjal0 I noticed you tagged me but I'm not quite sure what you need help with. Here are some things you can ask me to do:
Please tag me again with your specific request and I'll be happy to help! |
|
@jayantpranjal0 I've started the AI code review. It'll take a few minutes to complete. |
There was a problem hiding this comment.
cubic found 3 issues across 21 files. Review them in cubic.dev
React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai to give specific feedback.
There was a problem hiding this comment.
cubic found 2 issues across 21 files. Review them in cubic.dev
React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai to give specific feedback.
jayantpranjal0
force-pushed
the
add_guest_team
branch
from
af00797 to
4ebf0e8
Compare
|
@cubic-dev-ai any reviews? |
|
@jayantpranjal0 This PR introduces the "optional guest team members" feature across the event type flow and calendar integrations. Key points from review:
No major issues found. Minor suggestions:
Overall, the implementation is sound and aligns with the described feature. |
|
The PR is ready for review. Please approve the workflows |
|
Also, wanted to know if there is a communication channel for cal.com |
|
Please note that this PR will need a video as visual evidence instead of just an image. Could you kindly add a Loom video demonstrating the feature? |
|
This PR is being marked as stale due to inactivity. |
WalkthroughThis change introduces support for designating team members as optional guests in event bookings. A new many-to-many relation between event types and users is added to the database schema, with corresponding migration and Prisma schema updates. Backend logic is updated to handle the new Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/app-store/exchangecalendar/lib/CalendarService.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-playwright". (The package "eslint-plugin-playwright" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-playwright" was referenced from the config file in ".eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. packages/app-store/larkcalendar/lib/CalendarService.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-playwright". (The package "eslint-plugin-playwright" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-playwright" was referenced from the config file in ".eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. packages/app-store/exchange2013calendar/lib/CalendarService.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-playwright". (The package "eslint-plugin-playwright" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-playwright" was referenced from the config file in ".eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (6)
packages/app-store/larkcalendar/lib/CalendarService.ts (1)
425-434: Filter out members without email addresses before mapping.This issue was already identified in a previous review. Mapping directly without filtering out members that lack an email can create attendee objects with
third_party_emailundefined, which may cause the Lark API to reject the request.- if (event.optionalGuestTeamMembers) { - const optionalGuestMembers = event.optionalGuestTeamMembers?.map( + if (event.optionalGuestTeamMembers) { + const optionalGuestMembers = event.optionalGuestTeamMembers?.filter(({ email }) => !!email).map( ({ email }): LarkEventAttendee => ({ type: "third_party", is_optional: true, third_party_email: email, }) ); attendeeArray.push(...optionalGuestMembers); }packages/app-store/exchangecalendar/lib/CalendarService.ts (2)
72-76: Use the imported Person interface for better type safety.Using an inline object type reduces type safety and consistency. The Person interface is already imported and should be used.
if (event.optionalGuestTeamMembers) { - event.optionalGuestTeamMembers.forEach((member: { email: string }) => { + event.optionalGuestTeamMembers.forEach((member: Person) => { appointment.OptionalAttendees.Add(new Attendee(member.email)); }); }
115-120: Prevent unintended data loss by making clearing conditional.Clearing optional attendees unconditionally can remove existing optional guests even when the update payload doesn't include
optionalGuestTeamMembers, leading to data loss.- appointment.OptionalAttendees.Clear(); if (event.optionalGuestTeamMembers) { + appointment.OptionalAttendees.Clear(); - event.optionalGuestTeamMembers.forEach((member: { email: string }) => { + event.optionalGuestTeamMembers.forEach((member: Person) => { appointment.OptionalAttendees.Add(new Attendee(member.email)); }); }packages/app-store/exchange2013calendar/lib/CalendarService.ts (1)
126-131: Identical logic for iterating over optionalGuestTeamMembers and adding each attendee is duplicated in both createEvent and updateEvent. Duplicated code makes future changes error-prone and harder to maintain; consider extracting this into a shared helper method.packages/prisma/schema.prisma (1)
203-203: LGTM! Many-to-many relation properly implemented.The new
optionalGuestTeamMembersrelation is correctly implemented as an explicit many-to-many relation using a join table (as evidenced by the migration file). This resolves the previous concern about Prisma's constraint on multiple implicit many-to-many relations between the same models.Also applies to: 337-337
packages/features/eventtypes/components/tabs/advanced/GuestTeamMemberController.tsx (1)
26-28: State synchronization properly implemented.The useEffect correctly addresses the previous concern about stale state by updating
isGuestTeamMembersEnabledwhenevereventType.optionalGuestTeamMemberschanges, ensuring the component stays synchronized with prop updates.
🧹 Nitpick comments (2)
apps/web/public/static/locales/en/common.json (1)
936-937: Tweak wording for grammatical correctnessMinor grammar issue – “Adding team members as an optional guest” mixes plural members with singular guest. Recommend pluralizing “guest” and adding the missing “will” for consistency with the rest of the sentence.
-"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_description": "Adding team members as optional guests will always send an optional invite, but will not check their availability.",packages/features/eventtypes/components/tabs/advanced/GuestTeamMemberController.tsx (1)
64-64: Consider adding error handling for data transformation.While the current implementation should work in practice, consider adding validation:
- Line 64:
parseInt(option.value)could benefit from validation or usingNumber()with error handling- Lines 81-82: The
|| ""fallbacks onmember.user.emailandmember.user.id.toString()suggest these could be undefined, but they should always be truthy - consider removing redundant fallbacks or adding proper validation- onChange(options.map((option) => ({ id: parseInt(option.value) }))); + onChange(options.map((option) => ({ id: Number(option.value) })).filter(item => !isNaN(item.id)));- label: member.user.email || "", - value: member.user.id.toString() || "", + label: member.user.email, + value: member.user.id.toString(),Also applies to: 78-82
📜 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(2 hunks)packages/app-store/exchange2016calendar/lib/CalendarService.ts(2 hunks)packages/app-store/exchangecalendar/lib/CalendarService.ts(2 hunks)packages/app-store/feishucalendar/lib/CalendarService.ts(1 hunks)packages/app-store/googlecalendar/lib/CalendarService.ts(1 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/GuestTeamMemberController.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/20250628001746_add_optional_guest_team_members/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)
🧰 Additional context used
🧠 Learnings (14)
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/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/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/platform/atoms/event-types/hooks/useEventTypeForm.ts (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.
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/larkcalendar/lib/CalendarService.ts (2)
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: 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/app-store/exchange2016calendar/lib/CalendarService.ts (2)
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.
packages/app-store/googlecalendar/lib/CalendarService.ts (2)
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.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/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/features/eventtypes/components/tabs/advanced/EventAdvancedTab.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/prisma/schema.prisma (2)
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/app-store/exchangecalendar/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/GuestTeamMemberController.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.
🧬 Code Graph Analysis (3)
packages/app-store/zohocalendar/lib/CalendarService.ts (1)
packages/types/Calendar.d.ts (1)
CalendarEvent(163-228)
packages/trpc/server/routers/viewer/eventTypes/update.handler.ts (1)
packages/platform/libraries/index.ts (2)
TRPCError(56-56)SchedulingType(30-30)
packages/features/eventtypes/components/tabs/advanced/GuestTeamMemberController.tsx (2)
packages/features/eventtypes/lib/types.ts (2)
EventTypeSetup(25-25)FormValues(69-168)packages/features/eventtypes/components/CheckedTeamSelect.tsx (2)
CheckedTeamSelect(47-189)CheckedSelectOption(20-29)
🔇 Additional comments (21)
packages/features/eventtypes/lib/types.ts (1)
167-167: LGTM! Type definition is appropriate for form handling.The addition of
optionalGuestTeamMembersas an array of objects withidfields is correct for form data handling, where you'd typically store selected team member IDs and enrich with additional details when building calendar events.packages/lib/defaultEvents.ts (1)
147-147: LGTM! Appropriate default value for the new feature.Setting an empty array as the default for
optionalGuestTeamMembersis the correct approach, ensuring backward compatibility and providing a sensible default when no optional guests are selected.packages/types/Calendar.d.ts (1)
224-227: LGTM! Appropriate type definition for calendar events.The
optionalGuestTeamMembersproperty is correctly defined withnamefields needed for calendar invitations. Making it optional ensures backward compatibility, and allowingnameto be null handles cases where team members may not have names set.packages/features/bookings/lib/handleNewBooking.ts (1)
1155-1155: LGTM! Proper integration into the calendar event building flow.The addition correctly integrates optional guest team members into the CalendarEventBuilder chain. Using the defensive
|| []fallback ensures robustness, and the placement before.build()follows the established builder pattern.packages/trpc/server/routers/viewer/eventTypes/types.ts (1)
89-92: LGTM! Schema definition correctly implements previous feedback.The
optionalGuestTeamMembersfield is properly defined with both.nullable()and.optional()modifiers, which addresses the previous review feedback and maintains consistency with other optional fields in the schema. The array structure withidproperties is appropriate for referencing team members.packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts (1)
175-181: LGTM! Well-structured addition to support optional guest team members.The addition of
optionalGuestTeamMembersrelation with appropriate field selection (name, email, id) follows the established pattern used for other user/member selections in this query. These fields provide the necessary data for downstream calendar invitation functionality.packages/lib/server/repository/eventType.ts (1)
517-521: LGTM! Clean addition of optional guest team members to the selection.The addition follows the existing pattern in the repository and appropriately selects only the
idfield for the optional guest team members, which aligns with the feature requirements.packages/features/CalendarEventBuilder.ts (1)
264-275: LGTM! Well-implemented builder method following established patterns.The new
withOptionalGuestTeamMembersmethod correctly follows the builder pattern, handles the optional parameter appropriately by defaulting to an empty array, and maintains consistency with other methods in the class.packages/platform/atoms/event-types/hooks/useEventTypeForm.ts (1)
58-58: LGTM! Consistent initialization pattern for optional guest team members.The initialization follows the established pattern used for other optional array fields in the default values, ensuring the form always has a defined array for
optionalGuestTeamMembers.packages/features/eventtypes/components/tabs/advanced/EventAdvancedTab.tsx (2)
66-66: LGTM! Clean import addition for the new component.The import follows the established pattern in the file.
1205-1207: LGTM! Proper conditional rendering with safe member access.The conditional rendering correctly checks for both the existence of
team?.membersand its length before rendering theGuestTeamMemberController. This addresses the previous review concern about safe property access and follows React best practices.packages/app-store/googlecalendar/lib/CalendarService.ts (1)
124-140: LGTM! Proper handling of optional guests.The implementation correctly addresses the concerns from previous reviews:
- Sets
optional: trueto properly mark guests as optional in Google Calendar- Filters out guests without email addresses
- Deduplicates attendees using case-insensitive email comparison
- Uses appropriate
responseStatus: "needsAction"for optional guestspackages/app-store/feishucalendar/lib/CalendarService.ts (1)
402-446: LGTM! Excellent deduplication logic implementation.The changes properly address the duplicate email issue by introducing separate sets to track mandatory and optional emails. The sequential processing ensures that:
- Main attendees are added as mandatory first
- Team members are added as mandatory only if not duplicates
- Optional guest team members are added only if not already present in either category
This implementation prevents the Feishu calendar service from receiving duplicate attendees with conflicting mandatory/optional status.
packages/app-store/exchange2013calendar/lib/CalendarService.ts (1)
83-87: Correct implementation for optional attendees.The logic properly adds optional guest team members to the Exchange appointment's
OptionalAttendeescollection, ensuring they receive optional invitations without availability checks.packages/prisma/schema.prisma (1)
333-333: Excellent fix for UTC timestamp handling.The change from
@default(now())to@default(dbgenerated("now() AT TIME ZONE 'UTC'"))properly addresses the previous feedback about ensuring UTC timestamps in the database and follows the "Prevent Direct NOW() Usage in Database Queries" guideline.packages/prisma/migrations/20250628001746_add_optional_guest_team_members/migration.sql (1)
1-17: Well-structured migration for many-to-many relation.The migration correctly implements the join table for the
optionalGuestTeamMembersrelation with:
- Proper foreign key constraints linking to
EventTypeanduserstables- CASCADE operations ensuring data integrity on deletions/updates
- Appropriate indexes for performance (unique composite on A,B and single on B)
- Standard Prisma naming convention for explicit many-to-many tables
packages/features/eventtypes/components/tabs/advanced/GuestTeamMemberController.tsx (2)
43-86: Well-structured UI implementation.The component properly integrates with Cal.com's design system using
SettingsToggleandCheckedTeamSelect. The conditional styling for rounded corners and responsive layout are well-implemented and follow established patterns.
17-92: Solid component implementation with good form integration.The component demonstrates good practices:
- Proper react-hook-form integration using Controller
- Appropriate null checking and early return
- State cleanup when disabling the feature (line 57)
- Consistent naming with the form schema
packages/trpc/server/routers/viewer/eventTypes/update.handler.ts (3)
94-94: Proper default value assignment for optional guest team members.The destructuring with default empty array ensures the field is always defined, preventing potential undefined reference errors in subsequent validation logic.
412-416: Host validation logic is correctly preserved.The condition properly excludes organization sub-teams from strict host membership validation while maintaining security for regular teams. The logic correctly uses the negation to ensure all hosts are team members.
423-461: Enhanced hosts processing with proper array validation and comprehensive CRUD operations.The refactored hosts logic introduces several improvements:
- Array validation: Added
Array.isArray(hosts)check prevents runtime errors- Efficient set operations: Uses Sets for O(1) lookups when categorizing hosts
- Comprehensive CRUD: Properly handles create, update, and delete operations for hosts
- Schedule ID handling: Added
scheduleIdto the update operation (line 457)The implementation correctly handles the complex many-to-many relationship updates with proper Prisma operations.
| appointment.OptionalAttendees.Clear(); | ||
| if (event.optionalGuestTeamMembers) { | ||
| event.optionalGuestTeamMembers.forEach((member: { email: string }) => { | ||
| appointment.OptionalAttendees.Add(new Attendee(member.email)); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add email validation for consistency.
Apply the same email filtering logic here as suggested for the createEvent method.
appointment.OptionalAttendees.Clear();
if (event.optionalGuestTeamMembers) {
- event.optionalGuestTeamMembers.forEach((member: { email: string }) => {
+ event.optionalGuestTeamMembers.filter(member => !!member.email).forEach((member: { email: string }) => {
appointment.OptionalAttendees.Add(new Attendee(member.email));
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| appointment.OptionalAttendees.Clear(); | |
| if (event.optionalGuestTeamMembers) { | |
| event.optionalGuestTeamMembers.forEach((member: { email: string }) => { | |
| appointment.OptionalAttendees.Add(new Attendee(member.email)); | |
| }); | |
| } | |
| appointment.OptionalAttendees.Clear(); | |
| if (event.optionalGuestTeamMembers) { | |
| event.optionalGuestTeamMembers | |
| .filter(member => !!member.email) | |
| .forEach((member: { email: string }) => { | |
| appointment.OptionalAttendees.Add(new Attendee(member.email)); | |
| }); | |
| } |
🤖 Prompt for AI Agents
In packages/app-store/exchange2016calendar/lib/CalendarService.ts around lines
127 to 132, the code adds optional attendees without validating their email
addresses. To fix this, apply the same email validation logic used in the
createEvent method before adding each optional attendee. This ensures only valid
emails are added to appointment.OptionalAttendees.
| if (event.optionalGuestTeamMembers) { | ||
| event.optionalGuestTeamMembers.forEach((member: { email: string }) => { | ||
| appointment.OptionalAttendees.Add(new Attendee(member.email)); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add email validation to prevent API errors.
Consider filtering out team members without email addresses before adding them as attendees to prevent potential Exchange API errors.
if (event.optionalGuestTeamMembers) {
- event.optionalGuestTeamMembers.forEach((member: { email: string }) => {
+ event.optionalGuestTeamMembers.filter(member => !!member.email).forEach((member: { email: string }) => {
appointment.OptionalAttendees.Add(new Attendee(member.email));
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (event.optionalGuestTeamMembers) { | |
| event.optionalGuestTeamMembers.forEach((member: { email: string }) => { | |
| appointment.OptionalAttendees.Add(new Attendee(member.email)); | |
| }); | |
| } | |
| if (event.optionalGuestTeamMembers) { | |
| event.optionalGuestTeamMembers | |
| .filter(member => !!member.email) | |
| .forEach((member: { email: string }) => { | |
| appointment.OptionalAttendees.Add(new Attendee(member.email)); | |
| }); | |
| } |
🤖 Prompt for AI Agents
In packages/app-store/exchange2016calendar/lib/CalendarService.ts around lines
84 to 88, the code adds optional guest team members as attendees without
validating their email addresses, which can cause API errors. Before adding each
member, check if the email property exists and is a valid non-empty string; only
add attendees with valid emails to prevent errors from the Exchange API.
| private getAttendees = (event: CalendarEvent) => { | ||
| const attendees = event.attendees.map((attendee) => ({ email: attendee.email })); | ||
| if (event.optionalGuestTeamMembers) { | ||
| attendees.push( | ||
| ...event.optionalGuestTeamMembers.map((member) => ({ | ||
| email: member.email, | ||
| // 2 is optional guest | ||
| attendance: 2, | ||
| })) | ||
| ); | ||
| } | ||
| return attendees; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add email validation for robustness.
The refactoring into a separate method is excellent for maintainability. Consider adding email validation to prevent potential API errors when team members lack email addresses.
private getAttendees = (event: CalendarEvent) => {
const attendees = event.attendees.map((attendee) => ({ email: attendee.email }));
if (event.optionalGuestTeamMembers) {
attendees.push(
- ...event.optionalGuestTeamMembers.map((member) => ({
+ ...event.optionalGuestTeamMembers.filter(member => !!member.email).map((member) => ({
email: member.email,
// 2 is optional guest
attendance: 2,
}))
);
}
return attendees;
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private getAttendees = (event: CalendarEvent) => { | |
| const attendees = event.attendees.map((attendee) => ({ email: attendee.email })); | |
| if (event.optionalGuestTeamMembers) { | |
| attendees.push( | |
| ...event.optionalGuestTeamMembers.map((member) => ({ | |
| email: member.email, | |
| // 2 is optional guest | |
| attendance: 2, | |
| })) | |
| ); | |
| } | |
| return attendees; | |
| }; | |
| private getAttendees = (event: CalendarEvent) => { | |
| const attendees = event.attendees.map((attendee) => ({ email: attendee.email })); | |
| if (event.optionalGuestTeamMembers) { | |
| attendees.push( | |
| ...event.optionalGuestTeamMembers.filter(member => !!member.email).map((member) => ({ | |
| email: member.email, | |
| // 2 is optional guest | |
| attendance: 2, | |
| })) | |
| ); | |
| } | |
| return attendees; | |
| }; |
🤖 Prompt for AI Agents
In packages/app-store/zohocalendar/lib/CalendarService.ts around lines 484 to
496, the getAttendees method currently maps attendees and
optionalGuestTeamMembers without validating their email addresses. To fix this,
add a check to ensure each attendee and optional guest team member has a valid,
non-empty email before including them in the returned array. This validation
will prevent potential API errors caused by missing or invalid emails.
| if (optionalGuestTeamMembers && optionalGuestTeamMembers.length > 0) { | ||
| if (optionalGuestTeamMembers.every((each) => teamMemberIds.includes(each.id))) { | ||
| data.optionalGuestTeamMembers = { | ||
| set: [], | ||
| connect: optionalGuestTeamMembers.map(({ id }) => ({ id })), | ||
| }; | ||
| } else { | ||
| throw new TRPCError({ | ||
| code: "FORBIDDEN", | ||
| message: "You can only assign team members as optional guests.", | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Validate optional guest team members authorization and data integrity.
The validation logic correctly ensures that only accepted team members can be assigned as optional guests. However, consider the edge case where optionalGuestTeamMembers might contain duplicate IDs.
🏁 Script executed:
#!/bin/bash
# Description: Check if there are any deduplication mechanisms for optionalGuestTeamMembers in the schema or input validation
# Expected: Find validation logic that prevents duplicate IDs in the array
# Search for optionalGuestTeamMembers schema validation
ast-grep --pattern 'optionalGuestTeamMembers: $_'
# Look for array deduplication patterns in the codebase
rg -A 3 -B 3 "optionalGuestTeamMembers.*unique|unique.*optionalGuestTeamMembers"Length of output: 134
Enforce deduplication for optionalGuestTeamMembers
No existing check prevents duplicate IDs in the optionalGuestTeamMembers array. Add a guard to reject or dedupe duplicates before connecting.
• File:
packages/trpc/server/routers/viewer/eventTypes/update.handler.ts (around lines 396–408)
Suggested diff:
if (optionalGuestTeamMembers && optionalGuestTeamMembers.length > 0) {
+ // Deduplicate IDs to prevent duplicate connections
+ const ids = optionalGuestTeamMembers.map(({ id }) => id);
+ const uniqueIds = [...new Set(ids)];
+ if (uniqueIds.length !== ids.length) {
+ throw new TRPCError({
+ code: "BAD_REQUEST",
+ message: "Duplicate optional guest team members are not allowed.",
+ });
+ }
if (optionalGuestTeamMembers.every((each) => teamMemberIds.includes(each.id))) {
- data.optionalGuestTeamMembers = {
- set: [],
- connect: optionalGuestTeamMembers.map(({ id }) => ({ id })),
- };
+ data.optionalGuestTeamMembers = {
+ set: [],
+ connect: uniqueIds.map((id) => ({ id })),
+ };
} else {
throw new TRPCError({
code: "FORBIDDEN",
message: "You can only assign team members as optional guests.",
});
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (optionalGuestTeamMembers && optionalGuestTeamMembers.length > 0) { | |
| if (optionalGuestTeamMembers.every((each) => teamMemberIds.includes(each.id))) { | |
| data.optionalGuestTeamMembers = { | |
| set: [], | |
| connect: optionalGuestTeamMembers.map(({ id }) => ({ id })), | |
| }; | |
| } else { | |
| throw new TRPCError({ | |
| code: "FORBIDDEN", | |
| message: "You can only assign team members as optional guests.", | |
| }); | |
| } | |
| } | |
| if (optionalGuestTeamMembers && optionalGuestTeamMembers.length > 0) { | |
| // Deduplicate IDs to prevent duplicate connections | |
| const ids = optionalGuestTeamMembers.map(({ id }) => id); | |
| const uniqueIds = [...new Set(ids)]; | |
| if (uniqueIds.length !== ids.length) { | |
| throw new TRPCError({ | |
| code: "BAD_REQUEST", | |
| message: "Duplicate optional guest team members are not allowed.", | |
| }); | |
| } | |
| if (optionalGuestTeamMembers.every((each) => teamMemberIds.includes(each.id))) { | |
| data.optionalGuestTeamMembers = { | |
| set: [], | |
| connect: uniqueIds.map((id) => ({ id })), | |
| }; | |
| } else { | |
| throw new TRPCError({ | |
| code: "FORBIDDEN", | |
| message: "You can only assign team members as optional guests.", | |
| }); | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/trpc/server/routers/viewer/eventTypes/update.handler.ts around lines
396 to 408, the code does not check for duplicate IDs in the
optionalGuestTeamMembers array. Add a validation step before the existing checks
to either reject the input if duplicates are found or remove duplicates from
optionalGuestTeamMembers. This can be done by creating a Set of IDs and
comparing its size to the array length or by filtering duplicates out. Ensure
this deduplication or validation happens before setting
data.optionalGuestTeamMembers.
|
Hi @jayantpranjal0, could you pls add a loom showing your feature and resolve merge conflicts and coderabbit comments as well? Making it draft. Feel free to rfr once the comments are addressed. |
|
Closing in favour of this #22651 |
5 participants
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):
Image Demo (if applicable):
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
/claim [CAL-5091] additional settings: "add team member as optional guest #18947
Checklist
Summary by cubic
Added the ability to include team members as optional guests for event types, allowing them to receive invites without checking their availability.