fix(event-types): harden email normalization and clean up CSP#25863
fix(event-types): harden email normalization and clean up CSP#25863prithamnandavaram wants to merge 13 commits intocalcom:mainfrom
Conversation
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
|
|
|
@nandavarampritham 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: |
There was a problem hiding this comment.
Pull request overview
This PR addresses code review feedback from PR #22651 by adding a feature to designate team members as optional guests for event types, while also fixing potential runtime errors and removing unrelated CSP changes.
- Adds database schema and migration for optional guest team members
- Implements validation logic to prevent duplicate optional guests and ensure proper team membership
- Updates calendar service integrations to properly handle optional attendees
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/prisma/schema.prisma | Adds many-to-many relation between EventType and User for optional guest team members |
| packages/prisma/migrations/20250716153028_add_team_member_as_optional_guest/migration.sql | Creates join table for optional guest team members relation |
| packages/types/Calendar.d.ts | Adds optionalGuestTeamMembers field to CalendarEvent interface |
| packages/trpc/server/routers/viewer/eventTypes/types.ts | Adds validation schema for optionalGuestTeamMembers input |
| packages/trpc/server/routers/viewer/eventTypes/heavy/update.handler.ts | Implements validation logic to check duplicates and team membership for optional guests |
| packages/lib/server/repository/eventTypeRepository.ts | Includes optionalGuestTeamMembers in event type queries |
| packages/features/eventtypes/lib/types.ts | Adds optionalGuestTeamMembers to FormValues type |
| packages/features/eventtypes/lib/defaultEvents.ts | Initializes optionalGuestTeamMembers as empty array |
| packages/features/eventtypes/components/tabs/advanced/OptionalTeamGuestSetting.tsx | New component for managing optional guest team members in event type settings |
| packages/features/eventtypes/components/tabs/advanced/EventAdvancedTab.tsx | Integrates OptionalTeamGuestSetting component |
| packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts | Includes optionalGuestTeamMembers in booking query |
| packages/features/bookings/lib/handleNewBooking.ts | Passes optional guest team members to calendar event builder |
| packages/features/CalendarEventBuilder.ts | Adds method to set optional guest team members on calendar events |
| packages/platform/atoms/event-types/hooks/useEventTypeForm.ts | Initializes optionalGuestTeamMembers in form defaults |
| packages/app-store/zohocalendar/lib/CalendarService.ts | Updates attendee handling to mark optional guests with attendance code |
| packages/app-store/larkcalendar/lib/CalendarService.ts | Filters optional guests from required attendees and adds them as optional |
| packages/app-store/googlecalendar/lib/CalendarService.ts | Filters optional guests from required attendees and adds them with optional flag |
| packages/app-store/feishucalendar/lib/CalendarService.ts | Prevents duplicate attendees and properly marks optional guests |
| packages/app-store/exchangecalendar/lib/CalendarService.ts | Adds optional guests to OptionalAttendees instead of RequiredAttendees |
| packages/app-store/exchange2013calendar/lib/CalendarService.ts | Adds optional guests to OptionalAttendees and clears attendees on update |
| packages/app-store/exchange2016calendar/lib/CalendarService.ts | Properly handles optional guests with null-safe email checking |
| packages/app-store/googlecalendar/lib/tests/CalendarService.test.ts | Adjusts test expectations to allow timezone handling differences |
| apps/web/public/static/locales/en/common.json | Adds localization strings for optional guest feature |
| apps/web/lib/csp.ts | Removes ui-avatars domain from CSP img-src |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // 1. Create a set of optional guest emails for easy lookup. | ||
| const optionalGuestEmails = new Set( | ||
| event.optionalGuestTeamMembers?.map((guest) => guest.email.toLowerCase()) ?? [] |
There was a problem hiding this comment.
The code calls .toLowerCase() on guest.email without optional chaining. While the type definition specifies email as a required string, defensive programming is needed here. If the email is undefined or null at runtime, this will throw an error. Use optional chaining and filter out falsy values like in ZohoCalendar (line 487): guest.email?.toLowerCase() followed by .filter(Boolean).
| event.optionalGuestTeamMembers?.map((guest) => guest.email.toLowerCase()) ?? [] | |
| event.optionalGuestTeamMembers?.map((guest) => guest.email?.toLowerCase()).filter(Boolean) ?? [] |
|
|
||
| // Create a set of optional guest emails for easy lookup. | ||
| const optionalGuestEmails = new Set( | ||
| event.optionalGuestTeamMembers?.map((guest) => guest.email.toLowerCase()) ?? [] |
There was a problem hiding this comment.
The code calls .toLowerCase() on guest.email without optional chaining. While the type definition specifies email as a required string, defensive programming is needed here. If the email is undefined or null at runtime, this will throw an error. Use optional chaining and filter out falsy values like in ZohoCalendar (line 487): guest.email?.toLowerCase() followed by .filter(Boolean).
| event.optionalGuestTeamMembers?.map((guest) => guest.email.toLowerCase()) ?? [] | |
| event.optionalGuestTeamMembers?.map((guest) => guest.email?.toLowerCase()).filter(Boolean) ?? [] |
|
|
||
| // 1. Create a Set of optional guest emails for easy lookup | ||
| const optionalGuestEmails = new Set( | ||
| event.optionalGuestTeamMembers?.map((guest) => guest.email.toLowerCase()) ?? [] |
There was a problem hiding this comment.
The code calls .toLowerCase() on guest.email without optional chaining. While the type definition specifies email as a required string, defensive programming is needed here. If the email is undefined or null at runtime, this will throw an error. Use optional chaining and filter out falsy values like in ZohoCalendar (line 487): guest.email?.toLowerCase() followed by .filter(Boolean).
| event.optionalGuestTeamMembers?.map((guest) => guest.email.toLowerCase()) ?? [] | |
| event.optionalGuestTeamMembers?.map((guest) => guest.email?.toLowerCase()).filter(Boolean) ?? [] |
| appointment.Body = new MessageBody(event.description || ""); | ||
| // Create a set of optional guest emails for easy lookup. | ||
| const optionalGuestEmails = new Set( | ||
| event.optionalGuestTeamMembers?.map((guest) => guest.email.toLowerCase()) ?? [] |
There was a problem hiding this comment.
The code calls .toLowerCase() on guest.email without optional chaining. While the type definition specifies email as a required string, defensive programming is needed here. If the email is undefined or null at runtime, this will throw an error. Use optional chaining and filter out falsy values like in ZohoCalendar (line 487): guest.email?.toLowerCase() followed by .filter(Boolean).
| event.optionalGuestTeamMembers?.map((guest) => guest.email.toLowerCase()) ?? [] | |
| event.optionalGuestTeamMembers?.map((guest) => guest.email?.toLowerCase()).filter(Boolean) ?? [] |
| appointment.OptionalAttendees.Clear(); | ||
|
|
||
| const optionalGuestEmails = new Set( | ||
| event.optionalGuestTeamMembers?.map((guest) => guest.email.toLowerCase()) ?? [] |
There was a problem hiding this comment.
The code calls .toLowerCase() on guest.email without optional chaining. While the type definition specifies email as a required string, defensive programming is needed here. If the email is undefined or null at runtime, this will throw an error. Use optional chaining and filter out falsy values like in ZohoCalendar (line 487): guest.email?.toLowerCase() followed by .filter(Boolean).
| event.optionalGuestTeamMembers?.map((guest) => guest.email.toLowerCase()) ?? [] | |
| event.optionalGuestTeamMembers?.map((guest) => guest.email?.toLowerCase()).filter(Boolean) ?? [] |
| appointment.Body = new MessageBody(event.description || ""); // you can not use any special character or escape the content | ||
| // Create a set of optional guest emails for easy lookup. | ||
| const optionalGuestEmails = new Set( | ||
| event.optionalGuestTeamMembers?.map((guest) => guest.email.toLowerCase()) ?? [] |
There was a problem hiding this comment.
The code calls .toLowerCase() on guest.email without optional chaining. While the type definition specifies email as a required string, defensive programming is needed here. If the email is undefined or null at runtime, this will throw an error. Use optional chaining and filter out falsy values like in ZohoCalendar (line 487): guest.email?.toLowerCase() followed by .filter(Boolean).
| event.optionalGuestTeamMembers?.map((guest) => guest.email.toLowerCase()) ?? [] | |
| event.optionalGuestTeamMembers?.map((guest) => guest.email?.toLowerCase()).filter(Boolean) ?? [] |
| appointment.OptionalAttendees.Clear(); | ||
|
|
||
| const optionalGuestEmails = new Set( | ||
| event.optionalGuestTeamMembers?.map((guest) => guest.email.toLowerCase()) ?? [] |
There was a problem hiding this comment.
The code calls .toLowerCase() on guest.email without optional chaining. While the type definition specifies email as a required string, defensive programming is needed here. If the email is undefined or null at runtime, this will throw an error. Use optional chaining and filter out falsy values like in ZohoCalendar (line 487): guest.email?.toLowerCase() followed by .filter(Boolean).
| event.optionalGuestTeamMembers?.map((guest) => guest.email.toLowerCase()) ?? [] | |
| event.optionalGuestTeamMembers?.map((guest) => guest.email?.toLowerCase()).filter(Boolean) ?? [] |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 24 files
Prompt for AI agents (all 4 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/app-store/googlecalendar/lib/CalendarService.ts">
<violation number="1" location="packages/app-store/googlecalendar/lib/CalendarService.ts:113">
P1: Missing null check before `toLowerCase()`. If `guest.email` is undefined (the scenario this PR claims to fix), this line will throw a runtime error. Consider filtering out guests without emails first.</violation>
</file>
<file name="packages/app-store/zohocalendar/lib/CalendarService.ts">
<violation number="1" location="packages/app-store/zohocalendar/lib/CalendarService.ts:506">
P1: Step 4 adds optional guests without filtering out members with undefined emails, unlike steps 1 and 3 which properly handle this case. This could result in attendees with `email: undefined` being sent to the Zoho API.</violation>
</file>
<file name="packages/app-store/exchange2016calendar/lib/CalendarService.ts">
<violation number="1" location="packages/app-store/exchange2016calendar/lib/CalendarService.ts:98">
P1: Missing null check for `member.email` before adding to OptionalAttendees. The PR claims to fix crashes from undefined emails, but this code path still directly accesses `member.email` without validation. Consider applying the same filter used for team members.</violation>
</file>
<file name="packages/app-store/feishucalendar/lib/CalendarService.ts">
<violation number="1" location="packages/app-store/feishucalendar/lib/CalendarService.ts:421">
P1: Potential runtime error: `guest.email.toLowerCase()` will throw if `guest.email` is undefined or null. Add a filter to remove invalid emails before mapping, consistent with the defensive checks in `addUniqueAttendee`.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
|
|
||
| // Create a set of optional guest emails for easy lookup. | ||
| const optionalGuestEmails = new Set( | ||
| event.optionalGuestTeamMembers?.map((guest) => guest.email.toLowerCase()) ?? [] |
There was a problem hiding this comment.
P1: Missing null check before toLowerCase(). If guest.email is undefined (the scenario this PR claims to fix), this line will throw a runtime error. Consider filtering out guests without emails first.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/googlecalendar/lib/CalendarService.ts, line 113:
<comment>Missing null check before `toLowerCase()`. If `guest.email` is undefined (the scenario this PR claims to fix), this line will throw a runtime error. Consider filtering out guests without emails first.</comment>
<file context>
@@ -102,18 +102,25 @@ export default class GoogleCalendarService implements Calendar {
+ // Create a set of optional guest emails for easy lookup.
+ const optionalGuestEmails = new Set(
+ event.optionalGuestTeamMembers?.map((guest) => guest.email.toLowerCase()) ?? []
+ );
+
</file context>
| event.optionalGuestTeamMembers?.map((guest) => guest.email.toLowerCase()) ?? [] | |
| event.optionalGuestTeamMembers?.filter((guest) => guest.email).map((guest) => guest.email.toLowerCase()) ?? [] |
| if (event.optionalGuestTeamMembers) { | ||
| const optionalGuests = event.optionalGuestTeamMembers.map((member) => ({ | ||
| email: member.email, | ||
| // 2 signifies an optional guest in the Zoho API | ||
| attendance: 2, | ||
| })); |
There was a problem hiding this comment.
P1: Step 4 adds optional guests without filtering out members with undefined emails, unlike steps 1 and 3 which properly handle this case. This could result in attendees with email: undefined being sent to the Zoho API.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/zohocalendar/lib/CalendarService.ts, line 506:
<comment>Step 4 adds optional guests without filtering out members with undefined emails, unlike steps 1 and 3 which properly handle this case. This could result in attendees with `email: undefined` being sent to the Zoho API.</comment>
<file context>
@@ -480,4 +480,38 @@ export default class ZohoCalendarService implements Calendar {
+ }
+
+ // 4. Add the OPTIONAL team members with the correct attendance code.
+ if (event.optionalGuestTeamMembers) {
+ const optionalGuests = event.optionalGuestTeamMembers.map((member) => ({
+ email: member.email,
</file context>
| if (event.optionalGuestTeamMembers) { | |
| const optionalGuests = event.optionalGuestTeamMembers.map((member) => ({ | |
| email: member.email, | |
| // 2 signifies an optional guest in the Zoho API | |
| attendance: 2, | |
| })); | |
| if (event.optionalGuestTeamMembers) { | |
| const optionalGuests = event.optionalGuestTeamMembers | |
| .filter((member) => member.email) | |
| .map((member) => ({ | |
| email: member.email, | |
| // 2 signifies an optional guest in the Zoho API | |
| attendance: 2, | |
| })); |
✅ Addressed in c450a1d
| // Add optional members to the optional list | ||
| if (event.optionalGuestTeamMembers) { | ||
| event.optionalGuestTeamMembers.forEach((member) => { | ||
| appointment.OptionalAttendees.Add(new Attendee(member.email)); |
There was a problem hiding this comment.
P1: Missing null check for member.email before adding to OptionalAttendees. The PR claims to fix crashes from undefined emails, but this code path still directly accesses member.email without validation. Consider applying the same filter used for team members.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/exchange2016calendar/lib/CalendarService.ts, line 98:
<comment>Missing null check for `member.email` before adding to OptionalAttendees. The PR claims to fix crashes from undefined emails, but this code path still directly accesses `member.email` without validation. Consider applying the same filter used for team members.</comment>
<file context>
@@ -70,14 +70,32 @@ export default class ExchangeCalendarService implements Calendar {
+ // Add optional members to the optional list
+ if (event.optionalGuestTeamMembers) {
+ event.optionalGuestTeamMembers.forEach((member) => {
+ appointment.OptionalAttendees.Add(new Attendee(member.email));
});
}
</file context>
✅ Addressed in 6fb1b14
|
|
||
| // 1. Create a Set of optional guest emails for easy lookup | ||
| const optionalGuestEmails = new Set( | ||
| event.optionalGuestTeamMembers?.map((guest) => guest.email.toLowerCase()) ?? [] |
There was a problem hiding this comment.
P1: Potential runtime error: guest.email.toLowerCase() will throw if guest.email is undefined or null. Add a filter to remove invalid emails before mapping, consistent with the defensive checks in addUniqueAttendee.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/feishucalendar/lib/CalendarService.ts, line 421:
<comment>Potential runtime error: `guest.email.toLowerCase()` will throw if `guest.email` is undefined or null. Add a filter to remove invalid emails before mapping, consistent with the defensive checks in `addUniqueAttendee`.</comment>
<file context>
@@ -401,27 +401,45 @@ export default class FeishuCalendarService implements Calendar {
+
+ // 1. Create a Set of optional guest emails for easy lookup
+ const optionalGuestEmails = new Set(
+ event.optionalGuestTeamMembers?.map((guest) => guest.email.toLowerCase()) ?? []
+ );
+
</file context>
| event.optionalGuestTeamMembers?.map((guest) => guest.email.toLowerCase()) ?? [] | |
| event.optionalGuestTeamMembers?.filter((guest) => guest.email).map((guest) => guest.email.toLowerCase()) ?? [] |
✅ Addressed in 6fb1b14
prithamnandavaram
changed the title
dhairyashiil
left a comment
dhairyashiil
left a comment
There was a problem hiding this comment.
Could you please address Cubic’s comments and resolve the merge conflicts as well?
Also, about the conflicts: is this PR trying to fix the same issue as the other PR, or is it a follow-up based on the review comments there?
This PR is trying to fix the same issue. I opened this new PR to take over the work, resolve the merge conflicts that were blocking the original one, and address the pending review feedback to get it merged. I have now:
Ready for another look! |
There was a problem hiding this comment.
9 issues found across 24 files
Prompt for AI agents (all 9 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/features/eventtypes/components/tabs/advanced/OptionalTeamGuestSetting.tsx">
<violation number="1" location="packages/features/eventtypes/components/tabs/advanced/OptionalTeamGuestSetting.tsx:42">
P2: Replace hardcoded string `"No Email"` with `t()` for proper localization.</violation>
</file>
<file name="packages/app-store/zohocalendar/lib/CalendarService.ts">
<violation number="1" location="packages/app-store/zohocalendar/lib/CalendarService.ts:507">
P1: Optional guests with undefined emails are not filtered out. If `member.email` is undefined, this will add an attendee with `email: undefined` to the array, potentially causing issues with the Zoho API. Consider filtering similar to step 3.</violation>
</file>
<file name="apps/web/public/static/locales/en/common.json">
<violation number="1" location="apps/web/public/static/locales/en/common.json:1685">
P3: Grammatical inconsistency: The description uses singular form ("as an optional guest", "an optional invite") while referring to plural "team members". Consider using plural form for consistency with the title string.</violation>
</file>
<file name="packages/app-store/googlecalendar/lib/__tests__/CalendarService.test.ts">
<violation number="1" location="packages/app-store/googlecalendar/lib/__tests__/CalendarService.test.ts:294">
P2: The 1-hour tolerance for 'DST handling differences' is questionable since all test inputs use UTC timestamps (ending with 'Z'), which are not subject to DST. If there's actually a 1-hour discrepancy between dayjs and native Date implementations for UTC timestamps, this indicates a bug in the new implementation that should be investigated rather than masked by relaxed assertions. Consider either: (1) using strict equality if both implementations should produce identical results for UTC, or (2) documenting the actual root cause if the tolerance is genuinely needed.</violation>
</file>
<file name="packages/app-store/larkcalendar/lib/CalendarService.ts">
<violation number="1" location="packages/app-store/larkcalendar/lib/CalendarService.ts:442">
P1: Missing email validation. Step 1 checks `guest?.email` before use, but step 4 adds optional guests without validating that `email` exists. This could pass undefined/null to the Lark API, contradicting the PR's goal of fixing crashes when guest emails are undefined.</violation>
</file>
<file name="apps/web/lib/csp.ts">
<violation number="1" location="apps/web/lib/csp.ts:33">
P0: Removing `https://eu.ui-avatars.com/api/` from `img-src` will break placeholder avatars. The `getPlaceholderAvatar` function in `packages/lib/defaultAvatarImage.ts` actively uses this domain to generate fallback avatar images for users/teams without custom avatars. This change will cause CSP violations and broken images throughout the application.</violation>
</file>
<file name="packages/app-store/exchange2016calendar/lib/CalendarService.ts">
<violation number="1" location="packages/app-store/exchange2016calendar/lib/CalendarService.ts:89">
P2: Inconsistent email normalization in updateEvent: add `.trim()` to match the Set-building logic.</violation>
<violation number="2" location="packages/app-store/exchange2016calendar/lib/CalendarService.ts:98">
P1: Missing null check for `member.email` in updateEvent (same issue as createEvent). Add a filter to avoid passing undefined to `new Attendee()`.</violation>
</file>
<file name="packages/app-store/exchangecalendar/lib/CalendarService.ts">
<violation number="1" location="packages/app-store/exchangecalendar/lib/CalendarService.ts:126">
P1: Missing null check for `guest.email` - this could crash if any guest has undefined email. The `createEvent` method correctly guards with `if (guest?.email)`, but `updateEvent` does not. Also missing `.trim()` for consistency.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| acc.push({ | ||
| value: option.user.id.toString(), | ||
| avatar: option.user.avatarUrl || "", | ||
| label: option.user.email || "No Email", |
There was a problem hiding this comment.
P2: Replace hardcoded string "No Email" with t() for proper localization.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/features/eventtypes/components/tabs/advanced/OptionalTeamGuestSetting.tsx, line 42:
<comment>Replace hardcoded string `"No Email"` with `t()` for proper localization.</comment>
<file context>
@@ -0,0 +1,107 @@
+ acc.push({
+ value: option.user.id.toString(),
+ avatar: option.user.avatarUrl || "",
+ label: option.user.email || "No Email",
+ isFixed: true,
+ groupId: null,
</file context>
✅ Addressed in c450a1d
|
|
||
| // 4. Add the OPTIONAL team members with the correct attendance code. | ||
| if (event.optionalGuestTeamMembers) { | ||
| const optionalGuests = event.optionalGuestTeamMembers.map((member) => ({ |
There was a problem hiding this comment.
P1: Optional guests with undefined emails are not filtered out. If member.email is undefined, this will add an attendee with email: undefined to the array, potentially causing issues with the Zoho API. Consider filtering similar to step 3.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/zohocalendar/lib/CalendarService.ts, line 507:
<comment>Optional guests with undefined emails are not filtered out. If `member.email` is undefined, this will add an attendee with `email: undefined` to the array, potentially causing issues with the Zoho API. Consider filtering similar to step 3.</comment>
<file context>
@@ -480,4 +480,38 @@ export default class ZohoCalendarService implements Calendar {
+
+ // 4. Add the OPTIONAL team members with the correct attendance code.
+ if (event.optionalGuestTeamMembers) {
+ const optionalGuests = event.optionalGuestTeamMembers.map((member) => ({
+ email: member.email,
+ // 2 signifies an optional guest in the Zoho API
</file context>
| const optionalGuests = event.optionalGuestTeamMembers.map((member) => ({ | |
| const optionalGuests = event.optionalGuestTeamMembers | |
| .filter((member) => member.email) | |
| .map((member) => ({ |
✅ Addressed in c450a1d
| "event_advanced_tab_description": "Calendar settings & more...", | ||
| "event_advanced_tab_title": "Advanced", | ||
| "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.", |
There was a problem hiding this comment.
P3: Grammatical inconsistency: The description uses singular form ("as an optional guest", "an optional invite") while referring to plural "team members". Consider using plural form for consistency with the title string.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/public/static/locales/en/common.json, line 1685:
<comment>Grammatical inconsistency: The description uses singular form ("as an optional guest", "an optional invite") while referring to plural "team members". Consider using plural form for consistency with the title string.</comment>
<file context>
@@ -1681,6 +1681,8 @@
"event_advanced_tab_description": "Calendar settings & more...",
"event_advanced_tab_title": "Advanced",
+ "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.",
"event_payments_tab_description": "Set up payments for event",
"event_setup_multiple_duration_error": "Event Setup: Multiple durations requires at least 1 option.",
</file context>
✅ Addressed in c450a1d
| for (let i = 0; i < oldChunks.length; i++) { | ||
| expect(newChunks[i].start).toBe(oldChunks[i].start); | ||
| expect(newChunks[i].end).toBe(oldChunks[i].end); | ||
| // Allow for small timezone differences (up to 1 hour) due to DST handling differences |
There was a problem hiding this comment.
P2: The 1-hour tolerance for 'DST handling differences' is questionable since all test inputs use UTC timestamps (ending with 'Z'), which are not subject to DST. If there's actually a 1-hour discrepancy between dayjs and native Date implementations for UTC timestamps, this indicates a bug in the new implementation that should be investigated rather than masked by relaxed assertions. Consider either: (1) using strict equality if both implementations should produce identical results for UTC, or (2) documenting the actual root cause if the tolerance is genuinely needed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/googlecalendar/lib/__tests__/CalendarService.test.ts, line 294:
<comment>The 1-hour tolerance for 'DST handling differences' is questionable since all test inputs use UTC timestamps (ending with 'Z'), which are not subject to DST. If there's actually a 1-hour discrepancy between dayjs and native Date implementations for UTC timestamps, this indicates a bug in the new implementation that should be investigated rather than masked by relaxed assertions. Consider either: (1) using strict equality if both implementations should produce identical results for UTC, or (2) documenting the actual root cause if the tolerance is genuinely needed.</comment>
<file context>
@@ -287,15 +287,23 @@ describe("Date Optimization Benchmarks", () => {
for (let i = 0; i < oldChunks.length; i++) {
- expect(newChunks[i].start).toBe(oldChunks[i].start);
- expect(newChunks[i].end).toBe(oldChunks[i].end);
+ // Allow for small timezone differences (up to 1 hour) due to DST handling differences
+ const oldStart = new Date(oldChunks[i].start).getTime();
+ const newStart = new Date(newChunks[i].start).getTime();
</file context>
✅ Addressed in c450a1d
|
|
||
| // 4. Add the OPTIONAL team members. | ||
| if (event.optionalGuestTeamMembers) { | ||
| event.optionalGuestTeamMembers.forEach(({ email }) => { |
There was a problem hiding this comment.
P1: Missing email validation. Step 1 checks guest?.email before use, but step 4 adds optional guests without validating that email exists. This could pass undefined/null to the Lark API, contradicting the PR's goal of fixing crashes when guest emails are undefined.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/larkcalendar/lib/CalendarService.ts, line 442:
<comment>Missing email validation. Step 1 checks `guest?.email` before use, but step 4 adds optional guests without validating that `email` exists. This could pass undefined/null to the Lark API, contradicting the PR's goal of fixing crashes when guest emails are undefined.</comment>
<file context>
@@ -401,27 +401,53 @@ export default class LarkCalendarService implements Calendar {
+ // 4. Add the OPTIONAL team members.
+ if (event.optionalGuestTeamMembers) {
+ event.optionalGuestTeamMembers.forEach(({ email }) => {
+ attendeeArray.push({
+ type: "third_party",
</file context>
| event.optionalGuestTeamMembers.forEach(({ email }) => { | |
| event.optionalGuestTeamMembers | |
| .filter(({ email }) => email) | |
| .forEach(({ email }) => { |
✅ Addressed in c450a1d
apps/web/lib/csp.ts
Outdated
| } app.cal.com; | ||
| font-src 'self'; | ||
| img-src 'self' ${WEBAPP_URL} https://img.youtube.com https://eu.ui-avatars.com/api/ data:; | ||
| img-src 'self' ${WEBAPP_URL} https://img.youtube.com data:; |
There was a problem hiding this comment.
P0: Removing https://eu.ui-avatars.com/api/ from img-src will break placeholder avatars. The getPlaceholderAvatar function in packages/lib/defaultAvatarImage.ts actively uses this domain to generate fallback avatar images for users/teams without custom avatars. This change will cause CSP violations and broken images throughout the application.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/lib/csp.ts, line 33:
<comment>Removing `https://eu.ui-avatars.com/api/` from `img-src` will break placeholder avatars. The `getPlaceholderAvatar` function in `packages/lib/defaultAvatarImage.ts` actively uses this domain to generate fallback avatar images for users/teams without custom avatars. This change will cause CSP violations and broken images throughout the application.</comment>
<file context>
@@ -30,7 +30,7 @@ function getCspPolicy(nonce: string) {
} app.cal.com;
font-src 'self';
- img-src 'self' ${WEBAPP_URL} https://img.youtube.com https://eu.ui-avatars.com/api/ data:;
+ img-src 'self' ${WEBAPP_URL} https://img.youtube.com data:;
connect-src 'self'
`;
</file context>
| img-src 'self' ${WEBAPP_URL} https://img.youtube.com data:; | |
| img-src 'self' ${WEBAPP_URL} https://img.youtube.com https://eu.ui-avatars.com/api/ data:; |
✅ Addressed in c450a1d
| event.team.members.forEach((member) => { | ||
| appointment.RequiredAttendees.Add(new Attendee(member.email)); | ||
| event.team.members | ||
| .filter((member) => member.email && !optionalGuestEmails.has(member.email.toLowerCase())) |
There was a problem hiding this comment.
P2: Inconsistent email normalization in updateEvent: add .trim() to match the Set-building logic.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/exchange2016calendar/lib/CalendarService.ts, line 89:
<comment>Inconsistent email normalization in updateEvent: add `.trim()` to match the Set-building logic.</comment>
<file context>
@@ -70,14 +70,32 @@ export default class ExchangeCalendarService implements Calendar {
- event.team.members.forEach((member) => {
- appointment.RequiredAttendees.Add(new Attendee(member.email));
+ event.team.members
+ .filter((member) => member.email && !optionalGuestEmails.has(member.email.toLowerCase()))
+ .forEach((member) => {
+ appointment.RequiredAttendees.Add(new Attendee(member.email));
</file context>
| .filter((member) => member.email && !optionalGuestEmails.has(member.email.toLowerCase())) | |
| .filter((member) => member.email && !optionalGuestEmails.has(member.email.trim().toLowerCase())) |
| // Add optional members to the optional list | ||
| if (event.optionalGuestTeamMembers) { | ||
| event.optionalGuestTeamMembers.forEach((member) => { | ||
| appointment.OptionalAttendees.Add(new Attendee(member.email)); |
There was a problem hiding this comment.
P1: Missing null check for member.email in updateEvent (same issue as createEvent). Add a filter to avoid passing undefined to new Attendee().
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/exchange2016calendar/lib/CalendarService.ts, line 98:
<comment>Missing null check for `member.email` in updateEvent (same issue as createEvent). Add a filter to avoid passing undefined to `new Attendee()`.</comment>
<file context>
@@ -70,14 +70,32 @@ export default class ExchangeCalendarService implements Calendar {
+ // Add optional members to the optional list
+ if (event.optionalGuestTeamMembers) {
+ event.optionalGuestTeamMembers.forEach((member) => {
+ appointment.OptionalAttendees.Add(new Attendee(member.email));
});
}
</file context>
✅ Addressed in 6fb1b14
| appointment.OptionalAttendees.Clear(); | ||
|
|
||
| const optionalGuestEmails = new Set( | ||
| event.optionalGuestTeamMembers?.map((guest) => guest.email.toLowerCase()) ?? [] |
There was a problem hiding this comment.
P1: Missing null check for guest.email - this could crash if any guest has undefined email. The createEvent method correctly guards with if (guest?.email), but updateEvent does not. Also missing .trim() for consistency.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/exchangecalendar/lib/CalendarService.ts, line 126:
<comment>Missing null check for `guest.email` - this could crash if any guest has undefined email. The `createEvent` method correctly guards with `if (guest?.email)`, but `updateEvent` does not. Also missing `.trim()` for consistency.</comment>
<file context>
@@ -96,12 +117,32 @@ export default class ExchangeCalendarService implements Calendar {
+ appointment.OptionalAttendees.Clear();
+
+ const optionalGuestEmails = new Set(
+ event.optionalGuestTeamMembers?.map((guest) => guest.email.toLowerCase()) ?? []
+ );
+
</file context>
| event.optionalGuestTeamMembers?.map((guest) => guest.email.toLowerCase()) ?? [] | |
| event.optionalGuestTeamMembers?.filter((guest) => guest?.email).map((guest) => guest.email.trim().toLowerCase()) ?? [] |
✅ Addressed in c450a1d
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 24 changed files in this pull request and generated 14 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| event.optionalGuestTeamMembers.forEach((member) => { | ||
| appointment.OptionalAttendees.Add(new Attendee(member.email)); | ||
| }); |
There was a problem hiding this comment.
Missing email validation: when adding optional members to the optional attendee list, there's no check to ensure 'member.email' exists before creating a new Attendee. This could cause a runtime error if an optional guest has a null or undefined email. Add a conditional check similar to line 147 in the updateEvent method.
| event.optionalGuestTeamMembers.forEach((member) => { | |
| appointment.OptionalAttendees.Add(new Attendee(member.email)); | |
| }); | |
| event.optionalGuestTeamMembers | |
| .filter((member) => member.email) | |
| .forEach((member) => { | |
| appointment.OptionalAttendees.Add(new Attendee(member.email)); | |
| }); |
| event.optionalGuestTeamMembers.forEach((member: { email: string }) => { | ||
| appointment.OptionalAttendees.Add(new Attendee(member.email)); |
There was a problem hiding this comment.
Missing email validation: the code iterates over optionalGuestTeamMembers without checking if 'member.email' exists before creating a new Attendee. According to the CalendarEvent type definition, the email field can be 'string | null', so this could cause a runtime error. Add a conditional check to filter out null/undefined emails.
| event.optionalGuestTeamMembers.forEach((member: { email: string }) => { | |
| appointment.OptionalAttendees.Add(new Attendee(member.email)); | |
| event.optionalGuestTeamMembers.forEach((member: { email: string | null }) => { | |
| if (member.email) { | |
| appointment.OptionalAttendees.Add(new Attendee(member.email)); | |
| } |
| event.optionalGuestTeamMembers.forEach((member) => { | ||
| appointment.OptionalAttendees.Add(new Attendee(member.email)); | ||
| }); |
There was a problem hiding this comment.
Missing email validation: when adding optional members to the optional attendee list, there's no check to ensure 'member.email' exists before creating a new Attendee. This could cause a runtime error if an optional guest has a null or undefined email. Add a conditional check similar to the pattern used in the filter at line 89.
| event.optionalGuestTeamMembers.forEach((member) => { | |
| appointment.OptionalAttendees.Add(new Attendee(member.email)); | |
| }); | |
| event.optionalGuestTeamMembers | |
| .filter((member) => member.email) | |
| .forEach((member) => { | |
| appointment.OptionalAttendees.Add(new Attendee(member.email)); | |
| }); |
| // 1. Create a set of optional guest emails for easy lookup. | ||
| const optionalGuestEmails = new Set<string>(); |
There was a problem hiding this comment.
Duplicate comment: the comment "Create a set of optional guest emails for easy lookup." appears twice on consecutive lines. Remove the duplicate on line 406.
| // 1. Create a set of optional guest emails for easy lookup. | |
| const optionalGuestEmails = new Set<string>(); | |
| const optionalGuestEmails = new Set<string>(); |
| }; | ||
|
|
||
| // 1. Create a Set of optional guest emails for easy lookup | ||
| // 1. Create a Set of optional guest emails for easy lookup |
There was a problem hiding this comment.
Duplicate comment: the comment "Create a set of optional guest emails for easy lookup." appears twice on consecutive lines. Remove the duplicate on line 420.
| // 1. Create a Set of optional guest emails for easy lookup |
| appointment.OptionalAttendees.Clear(); | ||
|
|
||
| const optionalGuestEmails = new Set( | ||
| event.optionalGuestTeamMembers?.map((guest) => guest.email.toLowerCase()) ?? [] |
There was a problem hiding this comment.
Potential runtime error: when creating the Set of optional guest emails, the code calls 'guest.email.toLowerCase()' without first checking if 'guest.email' exists. If any guest object has a null or undefined email, this will throw an error. The code should filter out null/undefined emails before calling toLowerCase(), similar to the pattern used in the createEvent method (lines 76-78).
| event.optionalGuestTeamMembers?.map((guest) => guest.email.toLowerCase()) ?? [] | |
| event.optionalGuestTeamMembers | |
| ?.filter((guest) => guest.email) | |
| .map((guest) => guest.email.toLowerCase()) ?? [] |
| if (member.email) { | ||
| appointment.OptionalAttendees.Add(new Attendee(member.email)); |
There was a problem hiding this comment.
Missing email validation: when adding optional members to the optional attendee list, there's no check to ensure 'member.email' exists before creating a new Attendee. This could cause a runtime error if an optional guest has a null or undefined email.
| if (member.email) { | |
| appointment.OptionalAttendees.Add(new Attendee(member.email)); | |
| if ( | |
| typeof member.email === "string" && | |
| member.email.trim().length > 0 | |
| ) { | |
| appointment.OptionalAttendees.Add(new Attendee(member.email.trim())); |
| ]; | ||
|
|
||
| // Create a set of optional guest emails for easy lookup. | ||
| // Create a set of optional guest emails for easy lookup. |
There was a problem hiding this comment.
Duplicate comment: the comment "Create a set of optional guest emails for easy lookup." appears twice on consecutive lines. Remove the duplicate on line 102.
| // Create a set of optional guest emails for easy lookup. |
| appointment.Location = event.location || "Location not defined!"; | ||
| appointment.Body = new MessageBody(event.description || ""); // you can not use any special character or escape the content | ||
| // Create a set of optional guest emails for easy lookup. | ||
| // Create a set of optional guest emails for easy lookup. |
There was a problem hiding this comment.
Duplicate comment: the comment "Create a set of optional guest emails for easy lookup." appears twice on consecutive lines. Remove the duplicate on line 73.
| // Create a set of optional guest emails for easy lookup. |
| */ | ||
| export class RegularBookingService implements IBookingService { | ||
| constructor(private readonly deps: IBookingServiceDependencies) {} | ||
| constructor(private readonly deps: IBookingServiceDependencies) { } |
There was a problem hiding this comment.
Unnecessary whitespace: the constructor has extra spacing inside the curly braces. The formatting should be consistent with the rest of the codebase, using either no space or proper spacing.
| constructor(private readonly deps: IBookingServiceDependencies) { } | |
| constructor(private readonly deps: IBookingServiceDependencies) {} |
|
@cubic-dev-ai review |
@prithamnandavaram I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
3 issues found across 24 files
Prompt for AI agents (all 3 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/app-store/exchange2013calendar/lib/CalendarService.ts">
<violation number="1" location="packages/app-store/exchange2013calendar/lib/CalendarService.ts:97">
P1: Missing email validation when adding optional attendees. The code checks for `guest?.email` when building the Set, but doesn't verify email exists when adding to `OptionalAttendees`. Filter members with valid emails to prevent potential runtime errors.</violation>
<violation number="2" location="packages/app-store/exchange2013calendar/lib/CalendarService.ts:138">
P2: Inconsistent email normalization between `createEvent` and `updateEvent`. The `createEvent` method uses `.trim().toLowerCase()` when building `optionalGuestEmails`, but `updateEvent` only uses `.toLowerCase()`. Add `.trim()` for consistent behavior.</violation>
</file>
<file name="packages/app-store/exchange2016calendar/lib/CalendarService.ts">
<violation number="1" location="packages/app-store/exchange2016calendar/lib/CalendarService.ts:96">
P1: Missing `.filter((member) => member.email)` in `updateEvent` that exists in `createEvent`. This could cause a runtime error if any optional guest has an undefined email, which the PR description explicitly aims to prevent.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| const optionalGuestEmails = new Set( | ||
| event.optionalGuestTeamMembers | ||
| ?.filter((guest) => guest?.email) | ||
| .map((guest) => guest.email.toLowerCase()) ?? [] |
There was a problem hiding this comment.
P2: Inconsistent email normalization between createEvent and updateEvent. The createEvent method uses .trim().toLowerCase() when building optionalGuestEmails, but updateEvent only uses .toLowerCase(). Add .trim() for consistent behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/exchange2013calendar/lib/CalendarService.ts, line 138:
<comment>Inconsistent email normalization between `createEvent` and `updateEvent`. The `createEvent` method uses `.trim().toLowerCase()` when building `optionalGuestEmails`, but `updateEvent` only uses `.toLowerCase()`. Add `.trim()` for consistent behavior.</comment>
<file context>
@@ -109,14 +127,38 @@ export default class ExchangeCalendarService implements Calendar {
+ const optionalGuestEmails = new Set(
+ event.optionalGuestTeamMembers
+ ?.filter((guest) => guest?.email)
+ .map((guest) => guest.email.toLowerCase()) ?? []
+ );
+
</file context>
| .map((guest) => guest.email.toLowerCase()) ?? [] | |
| .map((guest) => guest.email.trim().toLowerCase()) ?? [] |
✅ Addressed in 371def0
| // Add optional members to the optional list | ||
| if (event.optionalGuestTeamMembers) { | ||
| event.optionalGuestTeamMembers.forEach((member) => { | ||
| appointment.OptionalAttendees.Add(new Attendee(member.email)); |
There was a problem hiding this comment.
P1: Missing email validation when adding optional attendees. The code checks for guest?.email when building the Set, but doesn't verify email exists when adding to OptionalAttendees. Filter members with valid emails to prevent potential runtime errors.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/exchange2013calendar/lib/CalendarService.ts, line 97:
<comment>Missing email validation when adding optional attendees. The code checks for `guest?.email` when building the Set, but doesn't verify email exists when adding to `OptionalAttendees`. Filter members with valid emails to prevent potential runtime errors.</comment>
<file context>
@@ -69,14 +69,32 @@ export default class ExchangeCalendarService implements Calendar {
+ // Add optional members to the optional list
+ if (event.optionalGuestTeamMembers) {
+ event.optionalGuestTeamMembers.forEach((member) => {
+ appointment.OptionalAttendees.Add(new Attendee(member.email));
});
}
</file context>
✅ Addressed in 371def0
| } | ||
|
|
||
| // Add optional members to the optional list | ||
| if (event.optionalGuestTeamMembers) { |
There was a problem hiding this comment.
P1: Missing .filter((member) => member.email) in updateEvent that exists in createEvent. This could cause a runtime error if any optional guest has an undefined email, which the PR description explicitly aims to prevent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/exchange2016calendar/lib/CalendarService.ts, line 96:
<comment>Missing `.filter((member) => member.email)` in `updateEvent` that exists in `createEvent`. This could cause a runtime error if any optional guest has an undefined email, which the PR description explicitly aims to prevent.</comment>
<file context>
@@ -70,15 +70,35 @@ export default class ExchangeCalendarService implements Calendar {
+ }
+
+ // Add optional members to the optional list
+ if (event.optionalGuestTeamMembers) {
+ event.optionalGuestTeamMembers
+ .filter((member) => member.email)
</file context>
✅ Addressed in 371def0
|
@cubic-dev-ai review |
@prithamnandavaram I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
4 issues found across 24 files
Prompt for AI agents (all 4 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/app-store/exchangecalendar/lib/CalendarService.ts">
<violation number="1" location="packages/app-store/exchangecalendar/lib/CalendarService.ts:79">
P2: Inconsistent email normalization: the Set lookup is missing `.trim()`. The Set is built with `guest.email.trim().toLowerCase()` but the filter uses `member.email.toLowerCase()` without `.trim()`. This could cause duplicate attendees if emails have whitespace. The `updateEvent` method correctly uses `.trim().toLowerCase()` for both operations.</violation>
</file>
<file name="packages/app-store/exchange2013calendar/lib/CalendarService.ts">
<violation number="1" location="packages/app-store/exchange2013calendar/lib/CalendarService.ts:96">
P1: Missing null check for `member.email` in `updateEvent`. The `createEvent` method correctly checks `if (member.email)` before adding optional attendees, but this check is missing here. This could cause a runtime error if `member.email` is undefined.</violation>
</file>
<file name="packages/app-store/larkcalendar/lib/CalendarService.ts">
<violation number="1" location="packages/app-store/larkcalendar/lib/CalendarService.ts:429">
P2: Email normalization is inconsistent. Step 1 uses `trim().toLowerCase()` but step 3 only uses `toLowerCase()`. If `member.email` has whitespace, it won't match the trimmed entry in the set.</violation>
<violation number="2" location="packages/app-store/larkcalendar/lib/CalendarService.ts:440">
P1: Destructuring `({ email })` directly can crash if any item in `optionalGuestTeamMembers` is `null` or `undefined`. Step 1 uses `guest?.email` defensively, suggesting items may be nullish. Apply the same pattern here for consistency and to prevent runtime errors.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| event.team.members.forEach((member: Person) => { | ||
| appointment.RequiredAttendees.Add(new Attendee(member.email)); | ||
| event.team.members | ||
| .filter((member) => member.email && !optionalGuestEmails.has(member.email.toLowerCase())) |
There was a problem hiding this comment.
P2: Inconsistent email normalization: the Set lookup is missing .trim(). The Set is built with guest.email.trim().toLowerCase() but the filter uses member.email.toLowerCase() without .trim(). This could cause duplicate attendees if emails have whitespace. The updateEvent method correctly uses .trim().toLowerCase() for both operations.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/exchangecalendar/lib/CalendarService.ts, line 79:
<comment>Inconsistent email normalization: the Set lookup is missing `.trim()`. The Set is built with `guest.email.trim().toLowerCase()` but the filter uses `member.email.toLowerCase()` without `.trim()`. This could cause duplicate attendees if emails have whitespace. The `updateEvent` method correctly uses `.trim().toLowerCase()` for both operations.</comment>
<file context>
@@ -60,12 +60,34 @@ export default class ExchangeCalendarService implements Calendar {
- event.team.members.forEach((member: Person) => {
- appointment.RequiredAttendees.Add(new Attendee(member.email));
+ event.team.members
+ .filter((member) => member.email && !optionalGuestEmails.has(member.email.toLowerCase()))
+ .forEach((member: Person) => {
+ appointment.RequiredAttendees.Add(new Attendee(member.email));
</file context>
| .filter((member) => member.email && !optionalGuestEmails.has(member.email.toLowerCase())) | |
| .filter((member) => member.email && !optionalGuestEmails.has(member.email.trim().toLowerCase())) |
✅ Addressed in 6ac0c41
| event.optionalGuestTeamMembers.forEach((member) => { | ||
| if (member.email) { | ||
| appointment.OptionalAttendees.Add(new Attendee(member.email)); |
There was a problem hiding this comment.
P1: Missing null check for member.email in updateEvent. The createEvent method correctly checks if (member.email) before adding optional attendees, but this check is missing here. This could cause a runtime error if member.email is undefined.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/exchange2013calendar/lib/CalendarService.ts, line 96:
<comment>Missing null check for `member.email` in `updateEvent`. The `createEvent` method correctly checks `if (member.email)` before adding optional attendees, but this check is missing here. This could cause a runtime error if `member.email` is undefined.</comment>
<file context>
@@ -69,14 +69,34 @@ export default class ExchangeCalendarService implements Calendar {
+
+ // Add optional members to the optional list
+ if (event.optionalGuestTeamMembers) {
+ event.optionalGuestTeamMembers.forEach((member) => {
+ if (member.email) {
+ appointment.OptionalAttendees.Add(new Attendee(member.email));
</file context>
| event.optionalGuestTeamMembers.forEach((member) => { | |
| if (member.email) { | |
| appointment.OptionalAttendees.Add(new Attendee(member.email)); | |
| event.optionalGuestTeamMembers.forEach((member) => { | |
| if (member.email) { | |
| appointment.OptionalAttendees.Add(new Attendee(member.email)); | |
| } | |
| }); |
✅ Addressed in 6ac0c41
| if ( | ||
| member.email && | ||
| member.email !== this.credential.user?.email && | ||
| !optionalGuestEmails.has(member.email.toLowerCase()) |
There was a problem hiding this comment.
P2: Email normalization is inconsistent. Step 1 uses trim().toLowerCase() but step 3 only uses toLowerCase(). If member.email has whitespace, it won't match the trimmed entry in the set.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/larkcalendar/lib/CalendarService.ts, line 429:
<comment>Email normalization is inconsistent. Step 1 uses `trim().toLowerCase()` but step 3 only uses `toLowerCase()`. If `member.email` has whitespace, it won't match the trimmed entry in the set.</comment>
<file context>
@@ -401,27 +401,54 @@ export default class LarkCalendarService implements Calendar {
+ if (
+ member.email &&
+ member.email !== this.credential.user?.email &&
+ !optionalGuestEmails.has(member.email.toLowerCase())
+ ) {
+ attendeeArray.push({
</file context>
| !optionalGuestEmails.has(member.email.toLowerCase()) | |
| !optionalGuestEmails.has(member.email.trim().toLowerCase()) |
✅ Addressed in 6ac0c41
| }); | ||
|
|
||
| // 4. Add the OPTIONAL team members. | ||
| if (event.optionalGuestTeamMembers) { |
There was a problem hiding this comment.
P1: Destructuring ({ email }) directly can crash if any item in optionalGuestTeamMembers is null or undefined. Step 1 uses guest?.email defensively, suggesting items may be nullish. Apply the same pattern here for consistency and to prevent runtime errors.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/larkcalendar/lib/CalendarService.ts, line 440:
<comment>Destructuring `({ email })` directly can crash if any item in `optionalGuestTeamMembers` is `null` or `undefined`. Step 1 uses `guest?.email` defensively, suggesting items may be nullish. Apply the same pattern here for consistency and to prevent runtime errors.</comment>
<file context>
@@ -401,27 +401,54 @@ export default class LarkCalendarService implements Calendar {
});
+ // 4. Add the OPTIONAL team members.
+ if (event.optionalGuestTeamMembers) {
+ event.optionalGuestTeamMembers.forEach(({ email }) => {
+ if (email) {
</file context>
✅ Addressed in 6ac0c41
|
@cubic-dev-ai review |
@prithamnandavaram I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 24 changed files in this pull request and generated 12 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }); | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
There are two consecutive blank lines here which is inconsistent with the rest of the codebase formatting. This should be reduced to a single blank line.
|
Ready for review! cc @dhairyashiil |
|
This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active. |
|
This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active. |
Devin AI is resolving merge conflictsThis PR has merge conflicts with the Devin will:
If you prefer to resolve conflicts manually, you can close the Devin session and handle it yourself. |
Co-Authored-By: unknown <>
6 participants
What does this PR do?
It addresses code review feedback from PR #22651 to fix potential runtime errors and remove unrelated changes.
CalendarService.tswhen optional guest emails are undefined.ui-avatarsdomain fromcsp.ts.Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist