fix: handle collective multiple host on destinationCalendar #8
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Author
Greptile OverviewConfidence Score: 1/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant Client
participant handleNewBooking
participant EventManager
participant CalendarManager
participant GoogleCalendar
participant Office365Calendar
participant Database
Client->>handleNewBooking: Create booking request
handleNewBooking->>handleNewBooking: Load users & hosts
alt Collective Event Type
handleNewBooking->>handleNewBooking: Collect destinationCalendar from all team members
handleNewBooking->>handleNewBooking: Build destinationCalendar array
else Single/Round Robin
handleNewBooking->>handleNewBooking: Use organizer's destinationCalendar as array[0]
end
handleNewBooking->>EventManager: create(evt with destinationCalendar[])
loop For each destinationCalendar in array
EventManager->>EventManager: Find credential by credentialId
alt Credential not in memory
EventManager->>Database: Fetch credential by ID
Database-->>EventManager: Return credential
end
EventManager->>CalendarManager: createEvent(credential, event, externalId)
CalendarManager->>GoogleCalendar: createEvent(event, credentialId)
GoogleCalendar->>GoogleCalendar: Find calendar by credentialId
GoogleCalendar-->>CalendarManager: Return event result
CalendarManager-->>EventManager: Return EventResult with externalId & credentialId
end
EventManager-->>handleNewBooking: Return array of created events
handleNewBooking->>Database: Save booking with references
handleNewBooking-->>Client: Return booking confirmation
|
Comment on lines
+118
to
+119
| const [mainHostDestinationCalendar] = evt.destinationCalendar ?? []; | ||
| if (evt.location === MeetLocationType && mainHostDestinationCalendar.integration !== "google_calendar") { |
There was a problem hiding this comment.
logic: Potential null pointer exception - mainHostDestinationCalendar can be undefined when destinationCalendar is an empty array, causing crash when accessing .integration
Suggested change
| const [mainHostDestinationCalendar] = evt.destinationCalendar ?? []; | |
| if (evt.location === MeetLocationType && mainHostDestinationCalendar.integration !== "google_calendar") { | |
| const [mainHostDestinationCalendar] = evt.destinationCalendar ?? []; | |
| if (evt.location === MeetLocationType && mainHostDestinationCalendar?.integration !== "google_calendar") { |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/core/EventManager.ts
Line: 118:119
Comment:
**logic:** Potential null pointer exception - `mainHostDestinationCalendar` can be undefined when `destinationCalendar` is an empty array, causing crash when accessing `.integration`
```suggestion
const [mainHostDestinationCalendar] = evt.destinationCalendar ?? [];
if (evt.location === MeetLocationType && mainHostDestinationCalendar?.integration !== "google_calendar") {
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
254
to
+256
| const selectedCalendar = externalCalendarId | ||
| ? externalCalendarId | ||
| : event.destinationCalendar?.externalId; | ||
| : event.destinationCalendar?.find((cal) => cal.externalId === externalCalendarId)?.externalId; |
There was a problem hiding this comment.
logic: Circular logic bug - when externalCalendarId is provided, the fallback searches for a calendar with the same externalCalendarId, which will never match. Should search by credentialId instead or just use the provided value
Suggested change
| const selectedCalendar = externalCalendarId | |
| ? externalCalendarId | |
| : event.destinationCalendar?.externalId; | |
| : event.destinationCalendar?.find((cal) => cal.externalId === externalCalendarId)?.externalId; | |
| const selectedCalendar = externalCalendarId || "primary"; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/app-store/googlecalendar/lib/CalendarService.ts
Line: 254:256
Comment:
**logic:** Circular logic bug - when `externalCalendarId` is provided, the fallback searches for a calendar with the same `externalCalendarId`, which will never match. Should search by `credentialId` instead or just use the provided value
```suggestion
const selectedCalendar = externalCalendarId || "primary";
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+315
to
+317
| const calendarId = externalCalendarId | ||
| ? externalCalendarId | ||
| : event.destinationCalendar?.find((cal) => cal.externalId === externalCalendarId)?.externalId; |
There was a problem hiding this comment.
logic: Same circular logic bug as in updateEvent - searching by externalCalendarId in the find condition will never match
Suggested change
| const calendarId = externalCalendarId | |
| ? externalCalendarId | |
| : event.destinationCalendar?.find((cal) => cal.externalId === externalCalendarId)?.externalId; | |
| const calendarId = externalCalendarId | |
| ? externalCalendarId | |
| : event.destinationCalendar?.[0]?.externalId; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/app-store/googlecalendar/lib/CalendarService.ts
Line: 315:317
Comment:
**logic:** Same circular logic bug as in `updateEvent` - searching by `externalCalendarId` in the find condition will never match
```suggestion
const calendarId = externalCalendarId
? externalCalendarId
: event.destinationCalendar?.[0]?.externalId;
```
How can I resolve this? If you propose a fix, please make it concise.| this.log = logger.getChildLogger({ prefix: [`[[lib] ${this.integrationName}`] }); | ||
| } | ||
|
|
||
| async createEvent(event: CalendarEvent): Promise<NewCalendarEventType> { |
There was a problem hiding this comment.
syntax: Missing credentialId parameter - signature doesn't match the interface Calendar.createEvent(event, credentialId) defined in packages/types/Calendar.d.ts:221
Suggested change
| async createEvent(event: CalendarEvent): Promise<NewCalendarEventType> { | |
| async createEvent(event: CalendarEvent, credentialId: number): Promise<NewCalendarEventType> { |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/app-store/office365calendar/lib/CalendarService.ts
Line: 72:72
Comment:
**syntax:** Missing `credentialId` parameter - signature doesn't match the interface `Calendar.createEvent(event, credentialId)` defined in `packages/types/Calendar.d.ts:221`
```suggestion
async createEvent(event: CalendarEvent, credentialId: number): Promise<NewCalendarEventType> {
```
How can I resolve this? If you propose a fix, please make it concise.| }); | ||
| }; | ||
|
|
||
| async createEvent(event: CalendarEvent): Promise<NewCalendarEventType> { |
There was a problem hiding this comment.
syntax: Missing credentialId parameter - signature doesn't match the interface Calendar.createEvent(event, credentialId) defined in packages/types/Calendar.d.ts:221
Suggested change
| async createEvent(event: CalendarEvent): Promise<NewCalendarEventType> { | |
| async createEvent(event: CalendarEvent, credentialId: number): Promise<NewCalendarEventType> { |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/app-store/larkcalendar/lib/CalendarService.ts
Line: 125:125
Comment:
**syntax:** Missing `credentialId` parameter - signature doesn't match the interface `Calendar.createEvent(event, credentialId)` defined in `packages/types/Calendar.d.ts:221`
```suggestion
async createEvent(event: CalendarEvent, credentialId: number): Promise<NewCalendarEventType> {
```
How can I resolve this? If you propose a fix, please make it concise.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Test 4
Replicated from ai-code-review-evaluation/cal.com-greptile#4