-
Notifications
You must be signed in to change notification settings - Fork 0
Automated Test: fix/handle-collective-multiple-host-destinations #375
Automated Test: fix/handle-collective-multiple-host-destinations #375
Conversation
📝 WalkthroughWalkthroughThis PR refactors calendar management to support multiple destination calendars per booking and event. The Changes
Sequence DiagramsequenceDiagram
participant EM as EventManager
participant CS as CalendarService<br/>(Google/Lark/O365)
participant CDB as Credential DB
participant CAL as External<br/>Calendar API
EM->>EM: Iterate destinationCalendar array
loop For each calendar entry
EM->>EM: Extract credentialId from entry
alt credentialId present
EM->>CDB: Lookup credential by ID
CDB-->>EM: Resolve credential
else credentialId absent
EM->>EM: Match by integration type<br/>from all credentials
end
EM->>CS: createEvent(calEvent,<br/>credentialId)
CS->>CS: Select calendar by<br/>credentialId match
CS->>CAL: Insert/Update event<br/>on selected calendar
CAL-->>CS: Event created with<br/>externalId
CS-->>EM: Return result with<br/>credentialId & externalId
EM->>EM: Store calendar reference
end
EM->>EM: Return EventResult with<br/>multiple references
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/app-store/googlecalendar/lib/CalendarService.ts (1)
165-178:⚠️ Potential issue | 🟠 MajorMissing
"primary"fallback in theevents.patchcall.At Line 154, the
events.insertcall usesselectedCalendar || "primary", but the subsequentevents.patchat Line 168 passescalendarId: selectedCalendarwithout the"primary"fallback. IfselectedCalendarisundefined(no matchingcredentialIdindestinationCalendar), the patch call will receiveundefinedas the calendar ID.Proposed fix
calendar.events.patch({ // Update the same event but this time we know the hangout link - calendarId: selectedCalendar, + calendarId: selectedCalendar || "primary", auth: myGoogleAuth,
🤖 Fix all issues with AI agents
In `@packages/app-store/googlecalendar/lib/CalendarService.ts`:
- Around line 315-317: The current assignment for calendarId in deleteEvent uses
a tautological find (find(cal => cal.externalId === externalCalendarId)) which
is dead code when externalCalendarId is falsy; change the fallback to select a
valid externalId from event.destinationCalendar (e.g.,
event.destinationCalendar?.[0]?.externalId or another appropriate selector) so
calendarId = externalCalendarId ? externalCalendarId :
event.destinationCalendar?.[0]?.externalId; ensure you update the code
referencing calendarId (the variable named calendarId in the deleteEvent logic)
so the fallback returns an actual externalId instead of using the impossible
equality check.
- Around line 254-256: The selectedCalendar fallback uses
event.destinationCalendar?.find(cal => cal.externalId === externalCalendarId)
which never matches when externalCalendarId is falsy; update the logic in the
update flow (symbol: selectedCalendar) to fall back to
mainHostDestinationCalendar?.externalId || "primary" instead of searching for a
matching undefined id, and apply the same fix to the identical fallback in
deleteEvent (and ensure calendar.events.update / calendarId is passed a defined
value); reference createEvent which already uses mainHostDestinationCalendar for
the intended pattern.
- Line 87: Multiple calendar provider classes still implement createEvent
without the required credentialId parameter; update each provider's createEvent
signature to match the Calendar interface: createEvent(event: CalendarEvent,
credentialId: number): Promise<NewCalendarEventType>, e.g. in classes/modules
for office365calendar, larkcalendar, zoho-bigin, zohocrm, sendgrid, salesforce,
exchangecalendar, exchange2013, exchange2016, hubspot, closecom, and basecamp3;
then propagate the new parameter into internal calls and any callers/tests
(adjust parameter passing and typings) so implementations and type definitions
align with the Calendar interface.
In `@packages/core/EventManager.ts`:
- Around line 117-121: The code accesses mainHostDestinationCalendar.integration
without guarding for an undefined mainHostDestinationCalendar; update the check
around evt.destinationCalendar and mainHostDestinationCalendar (from the
destructuring on evt.destinationCalendar and the subsequent if using
MeetLocationType) to first ensure mainHostDestinationCalendar exists (or use
optional chaining) before reading .integration, e.g., guard on
mainHostDestinationCalendar truthiness or use
mainHostDestinationCalendar?.integration in the condition so no TypeError is
thrown when destinationCalendar is null/empty.
- Around line 504-551: The code calls updateEvent(credential, ...) without
ensuring credential is defined; change the logic in the EventManager loop over
calendarReference so you only call result.push(updateEvent(...)) when credential
is truthy: after resolving credential from this.calendarCredentials or from the
DB (credentialFromDB), add an if (credential) guard around the
result.push(updateEvent(credential, event, bookingRefUid, calenderExternalId))
call (leave the alternative loop over credentials by type unchanged), so
undefined credentials are not passed to updateEvent; ensure the same presence
check used in createAllCalendarEvents is applied here.
In `@packages/features/bookings/lib/handleCancelBooking.ts`:
- Around line 444-468: The recurring-event deletion currently runs inside the
outer for loop over bookingCalendarReference and re-iterates all user
credentials, causing N×M×B deletions; fix it by either (A) using the
already-resolved calendarCredential and its calendar (from the getCalendar call)
to delete only the current reference's event for each updBooking (call
calendar.deleteEvent with the specific calendarCredential/external id and push
the result to apiDeletes), or (B) move the entire recurring-event branch that
iterates updatedBookings and calls deleteEvent outside the for loop so it runs
once per bookingToDelete (use bookingToDelete.recurringEventId and
updatedBookings there) — ensure you remove the fallback iteration over
bookingToDelete.user.credentials and only call getCalendar once per intended
credential.
- Around line 453-459: The loop currently uses updatedBookings and calls
references.find(ref => ref.type.includes("_calendar")) which only returns the
first calendar reference per booking; change this to collect all matching refs
(e.g., references.filter(ref => ref.type.includes("_calendar"))) and iterate
each matching bookingRef, extracting uid and externalCalendarId and calling
calendar.deleteEvent(uid, evt, externalCalendarId) for every ref, pushing each
result into apiDeletes (or map to promises and concat) so events on all
destination calendars are deleted.
In `@packages/features/bookings/lib/handleNewBooking.ts`:
- Around line 1063-1079: The destination calendars for team events are being
dropped because evt.destinationCalendar is set to null when neither
eventType.destinationCalendar nor organizerUser.destinationCalendar exist, so
the subsequent evt.destinationCalendar?.push(...) is skipped; update the
initialization for evt.destinationCalendar (or just before the push) to ensure
it's an array in COLLECTIVE team events—either set destinationCalendar to []
instead of null when isTeamEventType and eventType.schedulingType ===
"COLLECTIVE", or add a guard like evt.destinationCalendar =
evt.destinationCalendar ?? [] before calling
evt.destinationCalendar.push(...teamDestinationCalendars) so
teamDestinationCalendars are always appended.
In `@packages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts`:
- Around line 240-242: The destinationCalendar for the webhook payload is only
using bookingToReschedule?.destinationCalendar and can become empty; change the
construction to fall back to user.destinationCalendar when
bookingToReschedule?.destinationCalendar is absent (i.e., set
destinationCalendar to [bookingToReschedule.destinationCalendar] if present else
[user.destinationCalendar] if present else []), updating the requestReschedule
handler where destinationCalendar is built so it matches the pattern used in
confirm.handler.ts and deleteCredential.handler.ts.
In `@packages/types/Calendar.d.ts`:
- Line 221: The createEvent signature changed to include credentialId but many
implementations still accept only (event: CalendarEvent); update
BaseCalendarService.createEvent and all concrete classes
(BasecampCalendarService, CloseComCalendarService, ExchangeCalendarService
(2013/2016/generic), HubspotCalendarService, LarkCalendarService,
Office365CalendarService, SalesforceCalendarService, SendgridCalendarService,
ZohoCrmCalendarService, ZohoBiginCalendarService) to match async
createEvent(event: CalendarEvent, credentialId: number):
Promise<NewCalendarEventType> (or make credentialId optional in the interface
and implementations if some integrations don’t need it), and ensure any callers
(CalendarManager already passes credential.id) continue to compile and that
unused credentialId is simply accepted/ignored where not needed.
🧹 Nitpick comments (6)
packages/features/bookings/lib/handleCancelBooking.ts (3)
418-420: Nit: plural name for a filtered collection.
bookingCalendarReferenceholds an array from.filter()but uses a singular name. Consider renaming tobookingCalendarReferencesfor clarity.✏️ Suggested rename
- const bookingCalendarReference = bookingToDelete.references.filter((reference) => + const bookingCalendarReferences = bookingToDelete.references.filter((reference) => reference.type.includes("_calendar") );Apply the same rename on lines 422 and 423.
528-532: DuplicateddestinationCalendarconstruction — consider extracting a helper.This exact ternary chain is repeated from lines 251-255. A small helper or a single variable assignment earlier would eliminate the duplication.
♻️ Example extraction
Define once near the top of the handler:
const destinationCalendar = bookingToDelete?.destinationCalendar ? [bookingToDelete.destinationCalendar] : bookingToDelete?.user.destinationCalendar ? [bookingToDelete.user.destinationCalendar] : [];Then reference
destinationCalendarin bothevtobjects.
462-468: ConsiderPromise.allSettledinstead ofPromise.allfor resilient recurring deletions.
Promise.allshort-circuits on the first rejection, so remaining in-flight deletions are silently abandoned.Promise.allSettledwould let all deletions complete and let you log each failure individually — consistent with thePromise.allSettledpattern already used on line 656.♻️ Suggested change
- try { - await Promise.all(promises); - } catch (error) { - if (error instanceof Error) { - logger.error(error.message); - } - } + const settled = await Promise.allSettled(promises); + for (const result of settled) { + if (result.status === "rejected") { + logger.error(`Calendar deletion failed: ${result.reason}`); + } + }packages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts (1)
105-105: Dead code:bookingToRescheduleis guaranteed non-null here.
findFirstOrThrowon line 43 will throw if no booking is found, making this guard unreachable.packages/core/EventManager.ts (1)
596-608: Error fallback produces objects withcredentialId: 0— consider usingnullorundefined.Using
0as a sentinel for a missingcredentialIdcould cause issues downstream if code assumes a truthy/valid credential ID.nullorundefinedwould be more explicit.packages/features/bookings/lib/handleNewBooking.ts (1)
688-689: Consider using the importedSchedulingTypeenum instead of string literals.
SchedulingType.COLLECTIVEandSchedulingType.ROUND_ROBINare imported at Line 71. Using the enum values instead of"COLLECTIVE"/"ROUND_ROBIN"string literals (also at Lines 992 and 1077) would be more type-safe and refactor-friendly.
| @@ -84,7 +84,7 @@ export default class GoogleCalendarService implements Calendar { | |||
| }; | |||
| }; | |||
|
|
|||
| async createEvent(calEventRaw: CalendarEvent): Promise<NewCalendarEventType> { | |||
| async createEvent(calEventRaw: CalendarEvent, credentialId: number): Promise<NewCalendarEventType> { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Search for createEvent invocations
rg -n "\.createEvent\(" --type=ts -C2 -g '!**/node_modules/**'Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 472
🏁 Script executed:
# Check the actual signature at line 87 in CalendarService.ts
cat -n packages/app-store/googlecalendar/lib/CalendarService.ts | sed -n '85,95p'Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 573
🏁 Script executed:
# Search for all createEvent method definitions
rg -n "createEvent\s*\(" --type=ts -A1 -g '!**/node_modules/**' | grep -E "(async|createEvent)"Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 3606
🏁 Script executed:
# Check the Calendar interface/type definition
cat -n packages/types/Calendar.d.ts | sed -n '215,230p'Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 666
🏁 Script executed:
# Check if CalendarService implementations extend a base class
rg -n "extends.*Calendar|implements.*Calendar" --type=ts packages/app-store/googlecalendar/lib/CalendarService.ts packages/app-store/office365calendar/lib/CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 345
🏁 Script executed:
# Check CalendarManager to see how it instantiates and calls createEvent
cat -n packages/core/CalendarManager.ts | sed -n '230,245p'Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 841
🏁 Script executed:
# Check which other implementations also declare implements Calendar
rg -n "export.*class.*implements Calendar" --type=ts packages/app-store/Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1708
🏁 Script executed:
# Check a few of the other implementations to see their actual signature
for file in packages/app-store/office365calendar/lib/CalendarService.ts packages/app-store/larkcalendar/lib/CalendarService.ts; do
echo "=== $file ==="
sed -n '57,60p;125,130p' "$file" 2>/dev/null || sed -n '50,70p;120,135p' "$file" 2>/dev/null
doneRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1094
🏁 Script executed:
# Check EventManager.ts to see if those createEvent references are calls to the Calendar interface method
cat -n packages/core/EventManager.ts | sed -n '365,405p'Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1785
🏁 Script executed:
# Verify the createEvent function in EventManager (likely a helper function, not the Calendar method)
rg -n "^(async\s+)?function\s+createEvent|^(const|let)\s+createEvent\s*=" packages/core/EventManager.ts -B2 -A5Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 72
🏁 Script executed:
# Search for createEvent function definition in EventManager
rg -n "const createEvent|function createEvent" packages/core/EventManager.ts -A10Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 72
🏁 Script executed:
# Check imports at the top of EventManager.ts
head -50 packages/core/EventManager.ts | cat -nRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 2538
🏁 Script executed:
# Check the createEvent function in CalendarManager.ts
rg -n "^export.*function createEvent|^export const createEvent" packages/core/CalendarManager.ts -A30Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1358
🏁 Script executed:
# Or search more broadly for the createEvent export
cat -n packages/core/CalendarManager.ts | sed -n '1,50p'Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 2345
🏁 Script executed:
# Check if there are TypeScript compilation errors by looking for type errors
# First, let's verify if the other implementations are being called through the Calendar interface
cat -n packages/app-store/_utils/getCalendar.ts | head -50Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1972
🏁 Script executed:
# Check which calendar implementation is loaded by getCalendar
cat -n packages/app-store/_utils/getCalendar.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1972
Update all calendar provider implementations to match the interface signature with credentialId parameter.
The Calendar interface in packages/types/Calendar.d.ts requires createEvent(event: CalendarEvent, credentialId: number), and GoogleCalendarService correctly implements this. However, 12+ other calendar implementations still have the old signature without credentialId: office365calendar, larkcalendar, zoho-bigin, zohocrm, sendgrid, salesforce, exchangecalendar, exchange2013, exchange2016, hubspot, closecom, and basecamp3. These violate the interface contract and must be updated to include the credentialId parameter.
🤖 Prompt for AI Agents
In `@packages/app-store/googlecalendar/lib/CalendarService.ts` at line 87,
Multiple calendar provider classes still implement createEvent without the
required credentialId parameter; update each provider's createEvent signature to
match the Calendar interface: createEvent(event: CalendarEvent, credentialId:
number): Promise<NewCalendarEventType>, e.g. in classes/modules for
office365calendar, larkcalendar, zoho-bigin, zohocrm, sendgrid, salesforce,
exchangecalendar, exchange2013, exchange2016, hubspot, closecom, and basecamp3;
then propagate the new parameter into internal calls and any callers/tests
(adjust parameter passing and typings) so implementations and type definitions
align with the Calendar interface.
| const selectedCalendar = externalCalendarId | ||
| ? externalCalendarId | ||
| : event.destinationCalendar?.externalId; | ||
| : event.destinationCalendar?.find((cal) => cal.externalId === externalCalendarId)?.externalId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's see the context around lines 254-256 in the current file
head -300 packages/app-store/googlecalendar/lib/CalendarService.ts | tail -100Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 3311
🏁 Script executed:
# Check the git history for the old implementation
git log --oneline -20 -- packages/app-store/googlecalendar/lib/CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 168
🏁 Script executed:
# Search for calls to updateEvent to see if externalCalendarId is always provided
rg "updateEvent" --type ts -B 2 -A 2 packages/Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 11261
🏁 Script executed:
# Get the commit that introduced this change
git show bd797c1ea --stat | head -50Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 3015
🏁 Script executed:
# Look at lines 315-317 (deleteEvent) to compare
sed -n '310,325p' packages/app-store/googlecalendar/lib/CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 599
🏁 Script executed:
# Check if there's a fallback to "primary" in deleteEvent
sed -n '315,330p' packages/app-store/googlecalendar/lib/CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 689
Bug: tautological fallback — find will never match when externalCalendarId is falsy.
When externalCalendarId is falsy (the else branch), the code searches for cal.externalId === externalCalendarId, which is effectively cal.externalId === undefined — this will never match a real calendar. As a result, selectedCalendar will be undefined, and calendar.events.update at line 261 receives calendarId: undefined.
The intended logic was likely to fall back to the main host's destination calendar (destructured at line 100-103), similar to the pattern in createEvent at line 154. Use mainHostDestinationCalendar?.externalId || "primary" as the fallback instead.
Note: deleteEvent has an identical bug at lines 315-317 but is partially protected by the fallback at line 322 (calendarId ? calendarId : defaultCalendarId). updateEvent has no such safety net.
🤖 Prompt for AI Agents
In `@packages/app-store/googlecalendar/lib/CalendarService.ts` around lines 254 -
256, The selectedCalendar fallback uses event.destinationCalendar?.find(cal =>
cal.externalId === externalCalendarId) which never matches when
externalCalendarId is falsy; update the logic in the update flow (symbol:
selectedCalendar) to fall back to mainHostDestinationCalendar?.externalId ||
"primary" instead of searching for a matching undefined id, and apply the same
fix to the identical fallback in deleteEvent (and ensure calendar.events.update
/ calendarId is passed a defined value); reference createEvent which already
uses mainHostDestinationCalendar for the intended pattern.
| const calendarId = externalCalendarId | ||
| ? externalCalendarId | ||
| : event.destinationCalendar?.find((cal) => cal.externalId === externalCalendarId)?.externalId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same tautological fallback bug as updateEvent.
Identical issue: when externalCalendarId is falsy, find((cal) => cal.externalId === externalCalendarId) will never match. However, in deleteEvent the impact is mitigated by the fallback at Line 322 (calendarId ? calendarId : defaultCalendarId), which falls back to "primary".
Still, the find clause is dead code and should be corrected for clarity and correctness.
Proposed fix
const calendarId = externalCalendarId
? externalCalendarId
- : event.destinationCalendar?.find((cal) => cal.externalId === externalCalendarId)?.externalId;
+ : event.destinationCalendar?.find((cal) => cal.credentialId)?.externalId;📝 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.
| const calendarId = externalCalendarId | |
| ? externalCalendarId | |
| : event.destinationCalendar?.find((cal) => cal.externalId === externalCalendarId)?.externalId; | |
| const calendarId = externalCalendarId | |
| ? externalCalendarId | |
| : event.destinationCalendar?.find((cal) => cal.credentialId)?.externalId; |
🤖 Prompt for AI Agents
In `@packages/app-store/googlecalendar/lib/CalendarService.ts` around lines 315 -
317, The current assignment for calendarId in deleteEvent uses a tautological
find (find(cal => cal.externalId === externalCalendarId)) which is dead code
when externalCalendarId is falsy; change the fallback to select a valid
externalId from event.destinationCalendar (e.g.,
event.destinationCalendar?.[0]?.externalId or another appropriate selector) so
calendarId = externalCalendarId ? externalCalendarId :
event.destinationCalendar?.[0]?.externalId; ensure you update the code
referencing calendarId (the variable named calendarId in the deleteEvent logic)
so the fallback returns an actual externalId instead of using the impossible
equality check.
| // @NOTE: destinationCalendar it's an array now so as a fallback we will only check the first one | ||
| const [mainHostDestinationCalendar] = evt.destinationCalendar ?? []; | ||
| if (evt.location === MeetLocationType && mainHostDestinationCalendar.integration !== "google_calendar") { | ||
| evt["location"] = "integrations:daily"; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TypeError: accessing .integration on potentially undefined value.
If evt.destinationCalendar is null or an empty array, mainHostDestinationCalendar will be undefined, and mainHostDestinationCalendar.integration will throw a TypeError at runtime.
Proposed fix
// `@NOTE`: destinationCalendar it's an array now so as a fallback we will only check the first one
const [mainHostDestinationCalendar] = evt.destinationCalendar ?? [];
- if (evt.location === MeetLocationType && mainHostDestinationCalendar.integration !== "google_calendar") {
+ if (evt.location === MeetLocationType && mainHostDestinationCalendar?.integration !== "google_calendar") {
evt["location"] = "integrations:daily";
}📝 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.
| // @NOTE: destinationCalendar it's an array now so as a fallback we will only check the first one | |
| const [mainHostDestinationCalendar] = evt.destinationCalendar ?? []; | |
| if (evt.location === MeetLocationType && mainHostDestinationCalendar.integration !== "google_calendar") { | |
| evt["location"] = "integrations:daily"; | |
| } | |
| // `@NOTE`: destinationCalendar it's an array now so as a fallback we will only check the first one | |
| const [mainHostDestinationCalendar] = evt.destinationCalendar ?? []; | |
| if (evt.location === MeetLocationType && mainHostDestinationCalendar?.integration !== "google_calendar") { | |
| evt["location"] = "integrations:daily"; | |
| } |
🤖 Prompt for AI Agents
In `@packages/core/EventManager.ts` around lines 117 - 121, The code accesses
mainHostDestinationCalendar.integration without guarding for an undefined
mainHostDestinationCalendar; update the check around evt.destinationCalendar and
mainHostDestinationCalendar (from the destructuring on evt.destinationCalendar
and the subsequent if using MeetLocationType) to first ensure
mainHostDestinationCalendar exists (or use optional chaining) before reading
.integration, e.g., guard on mainHostDestinationCalendar truthiness or use
mainHostDestinationCalendar?.integration in the condition so no TypeError is
thrown when destinationCalendar is null/empty.
| for (const reference of calendarReference) { | ||
| const { uid: bookingRefUid, externalCalendarId: bookingExternalCalendarId } = reference; | ||
| let calenderExternalId: string | null = null; | ||
| if (bookingExternalCalendarId) { | ||
| calenderExternalId = bookingExternalCalendarId; | ||
| } | ||
|
|
||
| if (reference.credentialId) { | ||
| credential = this.calendarCredentials.filter( | ||
| (credential) => credential.id === reference?.credentialId | ||
| )[0]; | ||
| if (!credential) { | ||
| // Fetch credential from DB | ||
| const credentialFromDB = await prisma.credential.findUnique({ | ||
| include: { | ||
| app: { | ||
| select: { | ||
| slug: true, | ||
| }, | ||
| }, | ||
| }, | ||
| where: { | ||
| id: reference.credentialId, | ||
| }, | ||
| }); | ||
| if (credentialFromDB && credentialFromDB.app?.slug) { | ||
| credential = { | ||
| appName: credentialFromDB?.app.slug ?? "", | ||
| id: credentialFromDB.id, | ||
| type: credentialFromDB.type, | ||
| key: credentialFromDB.key, | ||
| userId: credentialFromDB.userId, | ||
| teamId: credentialFromDB.teamId, | ||
| invalid: credentialFromDB.invalid, | ||
| appId: credentialFromDB.appId, | ||
| }; | ||
| } | ||
| } | ||
| result.push(updateEvent(credential, event, bookingRefUid, calenderExternalId)); | ||
| } else { | ||
| const credentials = this.calendarCredentials.filter( | ||
| (credential) => credential.type === reference?.type | ||
| ); | ||
| for (const credential of credentials) { | ||
| result.push(updateEvent(credential, event, bookingRefUid, calenderExternalId)); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing guard: credential may be undefined when passed to updateEvent.
At Line 542, updateEvent(credential, ...) is called unconditionally, but credential can be undefined if:
this.calendarCredentials.filter(...)found no match (Line 512–514), and- the DB lookup at Lines 517–528 returned nothing or lacked
app.slug(Line 529).
In createAllCalendarEvents, this is guarded by if (credential) at Line 369. The same guard should be applied here.
Proposed fix
}
}
- result.push(updateEvent(credential, event, bookingRefUid, calenderExternalId));
+ if (credential) {
+ result.push(updateEvent(credential, event, bookingRefUid, calenderExternalId));
+ }📝 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.
| for (const reference of calendarReference) { | |
| const { uid: bookingRefUid, externalCalendarId: bookingExternalCalendarId } = reference; | |
| let calenderExternalId: string | null = null; | |
| if (bookingExternalCalendarId) { | |
| calenderExternalId = bookingExternalCalendarId; | |
| } | |
| if (reference.credentialId) { | |
| credential = this.calendarCredentials.filter( | |
| (credential) => credential.id === reference?.credentialId | |
| )[0]; | |
| if (!credential) { | |
| // Fetch credential from DB | |
| const credentialFromDB = await prisma.credential.findUnique({ | |
| include: { | |
| app: { | |
| select: { | |
| slug: true, | |
| }, | |
| }, | |
| }, | |
| where: { | |
| id: reference.credentialId, | |
| }, | |
| }); | |
| if (credentialFromDB && credentialFromDB.app?.slug) { | |
| credential = { | |
| appName: credentialFromDB?.app.slug ?? "", | |
| id: credentialFromDB.id, | |
| type: credentialFromDB.type, | |
| key: credentialFromDB.key, | |
| userId: credentialFromDB.userId, | |
| teamId: credentialFromDB.teamId, | |
| invalid: credentialFromDB.invalid, | |
| appId: credentialFromDB.appId, | |
| }; | |
| } | |
| } | |
| result.push(updateEvent(credential, event, bookingRefUid, calenderExternalId)); | |
| } else { | |
| const credentials = this.calendarCredentials.filter( | |
| (credential) => credential.type === reference?.type | |
| ); | |
| for (const credential of credentials) { | |
| result.push(updateEvent(credential, event, bookingRefUid, calenderExternalId)); | |
| } | |
| } | |
| } | |
| for (const reference of calendarReference) { | |
| const { uid: bookingRefUid, externalCalendarId: bookingExternalCalendarId } = reference; | |
| let calenderExternalId: string | null = null; | |
| if (bookingExternalCalendarId) { | |
| calenderExternalId = bookingExternalCalendarId; | |
| } | |
| if (reference.credentialId) { | |
| credential = this.calendarCredentials.filter( | |
| (credential) => credential.id === reference?.credentialId | |
| )[0]; | |
| if (!credential) { | |
| // Fetch credential from DB | |
| const credentialFromDB = await prisma.credential.findUnique({ | |
| include: { | |
| app: { | |
| select: { | |
| slug: true, | |
| }, | |
| }, | |
| }, | |
| where: { | |
| id: reference.credentialId, | |
| }, | |
| }); | |
| if (credentialFromDB && credentialFromDB.app?.slug) { | |
| credential = { | |
| appName: credentialFromDB?.app.slug ?? "", | |
| id: credentialFromDB.id, | |
| type: credentialFromDB.type, | |
| key: credentialFromDB.key, | |
| userId: credentialFromDB.userId, | |
| teamId: credentialFromDB.teamId, | |
| invalid: credentialFromDB.invalid, | |
| appId: credentialFromDB.appId, | |
| }; | |
| } | |
| } | |
| if (credential) { | |
| result.push(updateEvent(credential, event, bookingRefUid, calenderExternalId)); | |
| } | |
| } else { | |
| const credentials = this.calendarCredentials.filter( | |
| (credential) => credential.type === reference?.type | |
| ); | |
| for (const credential of credentials) { | |
| result.push(updateEvent(credential, event, bookingRefUid, calenderExternalId)); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@packages/core/EventManager.ts` around lines 504 - 551, The code calls
updateEvent(credential, ...) without ensuring credential is defined; change the
logic in the EventManager loop over calendarReference so you only call
result.push(updateEvent(...)) when credential is truthy: after resolving
credential from this.calendarCredentials or from the DB (credentialFromDB), add
an if (credential) guard around the result.push(updateEvent(credential, event,
bookingRefUid, calenderExternalId)) call (leave the alternative loop over
credentials by type unchanged), so undefined credentials are not passed to
updateEvent; ensure the same presence check used in createAllCalendarEvents is
applied here.
| if ( | ||
| bookingToDelete.eventType?.recurringEvent && | ||
| bookingToDelete.recurringEventId && | ||
| allRemainingBookings | ||
| ) { | ||
| const promises = bookingToDelete.user.credentials | ||
| .filter((credential) => credential.type.endsWith("_calendar")) | ||
| .map(async (credential) => { | ||
| const calendar = await getCalendar(credential); | ||
| for (const updBooking of updatedBookings) { | ||
| const bookingRef = updBooking.references.find((ref) => ref.type.includes("_calendar")); | ||
| if (bookingRef) { | ||
| const { uid, externalCalendarId } = bookingRef; | ||
| const deletedEvent = await calendar?.deleteEvent(uid, evt, externalCalendarId); | ||
| apiDeletes.push(deletedEvent); | ||
| } | ||
| } | ||
| }); | ||
| try { | ||
| await Promise.all(promises); | ||
| } catch (error) { | ||
| if (error instanceof Error) { | ||
| logger.error(error.message); | ||
| } | ||
| }); | ||
| try { | ||
| await Promise.all(promises); | ||
| } catch (error) { | ||
| if (error instanceof Error) { | ||
| logger.error(error.message); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recurring-event deletion is multiplied by the number of calendar references.
This block runs inside for (const reference of bookingCalendarReference) (line 423). When there are multiple calendar references — the whole point of this PR — the recurring-event branch re-iterates all user calendar credentials and all updated bookings for each outer reference, producing N × M × B deletion attempts instead of M × B (where N = calendar references, M = credentials, B = updated bookings).
Additionally, the credential resolved at lines 428-441 is never used here — line 449 falls back to iterating all _calendar credentials from the user, defeating the per-reference credential resolution above it.
Consider either:
- Using only the resolved
calendarCredential+calendar(line 443) to delete only the current reference's event for each updated booking, or - Handling the recurring-event path once outside the
forloop rather than per-reference.
🤖 Prompt for AI Agents
In `@packages/features/bookings/lib/handleCancelBooking.ts` around lines 444 -
468, The recurring-event deletion currently runs inside the outer for loop over
bookingCalendarReference and re-iterates all user credentials, causing N×M×B
deletions; fix it by either (A) using the already-resolved calendarCredential
and its calendar (from the getCalendar call) to delete only the current
reference's event for each updBooking (call calendar.deleteEvent with the
specific calendarCredential/external id and push the result to apiDeletes), or
(B) move the entire recurring-event branch that iterates updatedBookings and
calls deleteEvent outside the for loop so it runs once per bookingToDelete (use
bookingToDelete.recurringEventId and updatedBookings there) — ensure you remove
the fallback iteration over bookingToDelete.user.credentials and only call
getCalendar once per intended credential.
| for (const updBooking of updatedBookings) { | ||
| const bookingRef = updBooking.references.find((ref) => ref.type.includes("_calendar")); | ||
| if (bookingRef) { | ||
| const { uid, externalCalendarId } = bookingRef; | ||
| const deletedEvent = await calendar?.deleteEvent(uid, evt, externalCalendarId); | ||
| apiDeletes.push(deletedEvent); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
find retrieves only the first calendar reference per updated booking.
With the move to multiple destination calendars, each updated booking may have multiple _calendar references. Using .find() only deletes the event for the first one; the rest are left behind. Consider using .filter() and iterating all matching references.
🐛 Suggested fix
for (const updBooking of updatedBookings) {
- const bookingRef = updBooking.references.find((ref) => ref.type.includes("_calendar"));
- if (bookingRef) {
- const { uid, externalCalendarId } = bookingRef;
+ const bookingRefs = updBooking.references.filter((ref) => ref.type.includes("_calendar"));
+ for (const bookingRef of bookingRefs) {
+ const { uid, externalCalendarId } = bookingRef;
const deletedEvent = await calendar?.deleteEvent(uid, evt, externalCalendarId);
apiDeletes.push(deletedEvent);
}🤖 Prompt for AI Agents
In `@packages/features/bookings/lib/handleCancelBooking.ts` around lines 453 -
459, The loop currently uses updatedBookings and calls references.find(ref =>
ref.type.includes("_calendar")) which only returns the first calendar reference
per booking; change this to collect all matching refs (e.g.,
references.filter(ref => ref.type.includes("_calendar"))) and iterate each
matching bookingRef, extracting uid and externalCalendarId and calling
calendar.deleteEvent(uid, evt, externalCalendarId) for every ref, pushing each
result into apiDeletes (or map to promises and concat) so events on all
destination calendars are deleted.
| destinationCalendar: eventType.destinationCalendar | ||
| ? [eventType.destinationCalendar] | ||
| : organizerUser.destinationCalendar | ||
| ? [organizerUser.destinationCalendar] | ||
| : null, | ||
| hideCalendarNotes: eventType.hideCalendarNotes, | ||
| requiresConfirmation: requiresConfirmation ?? false, | ||
| eventTypeId: eventType.id, | ||
| // if seats are not enabled we should default true | ||
| seatsShowAttendees: eventType.seatsPerTimeSlot ? eventType.seatsShowAttendees : true, | ||
| seatsPerTimeSlot: eventType.seatsPerTimeSlot, | ||
| schedulingType: eventType.schedulingType, | ||
| }; | ||
|
|
||
| if (isTeamEventType && eventType.schedulingType === "COLLECTIVE") { | ||
| evt.destinationCalendar?.push(...teamDestinationCalendars); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Team calendars silently dropped when no primary destination calendar exists.
If neither eventType.destinationCalendar nor organizerUser.destinationCalendar is set, evt.destinationCalendar is null (Line 1067). The optional chaining at Line 1078 (evt.destinationCalendar?.push(...)) then silently skips appending teamDestinationCalendars. For a COLLECTIVE event where the organizer lacks a configured destination calendar but team members have theirs, those calendars will be lost.
Consider initializing to an empty array instead of null when in a COLLECTIVE context, or guarding this block:
Proposed fix
if (isTeamEventType && eventType.schedulingType === "COLLECTIVE") {
+ if (!evt.destinationCalendar) {
+ evt.destinationCalendar = [];
+ }
evt.destinationCalendar?.push(...teamDestinationCalendars);
}🤖 Prompt for AI Agents
In `@packages/features/bookings/lib/handleNewBooking.ts` around lines 1063 - 1079,
The destination calendars for team events are being dropped because
evt.destinationCalendar is set to null when neither
eventType.destinationCalendar nor organizerUser.destinationCalendar exist, so
the subsequent evt.destinationCalendar?.push(...) is skipped; update the
initialization for evt.destinationCalendar (or just before the push) to ensure
it's an array in COLLECTIVE team events—either set destinationCalendar to []
instead of null when isTeamEventType and eventType.schedulingType ===
"COLLECTIVE", or add a guard like evt.destinationCalendar =
evt.destinationCalendar ?? [] before calling
evt.destinationCalendar.push(...teamDestinationCalendars) so
teamDestinationCalendars are always appended.
| destinationCalendar: bookingToReschedule?.destinationCalendar | ||
| ? [bookingToReschedule?.destinationCalendar] | ||
| : [], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Find TrpcSessionUser type definition
rg -n "type TrpcSessionUser|interface TrpcSessionUser" --type=tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 168
🏁 Script executed:
# Check destinationCalendar in trpc.ts
rg -n "destinationCalendar" --type=ts packages/trpc/ | head -20Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 2219
🏁 Script executed:
# Check confirm.handler.ts around the destinationCalendar lines mentioned
fd "confirm.handler.ts" --type f -exec grep -n "destinationCalendar" {} +Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 320
🏁 Script executed:
# Check deleteCredential.handler.ts
fd "deleteCredential.handler.ts" --type f -exec grep -n "destinationCalendar" {} +Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 320
🏁 Script executed:
# Check paypal-webhook.ts
fd "paypal-webhook.ts" --type f -exec grep -n "destinationCalendar" {} +Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 320
🏁 Script executed:
# Get the full context around lines 240-242 in requestReschedule.handler.ts
fd "requestReschedule.handler.ts" --type f -exec sed -n '235,250p' {} +Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 320
🏁 Script executed:
# Find UserFromSession type definition
rg -n "type UserFromSession|interface UserFromSession" --type=ts -A 10Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1217
🏁 Script executed:
# Get confirm.handler.ts destinationCalendar context
rg -n "destinationCalendar" packages/trpc/server/routers/viewer/bookings/confirm.handler.ts -B 5 -A 5Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 900
🏁 Script executed:
# Check if deleteCredential.handler.ts has destinationCalendar
rg -n "destinationCalendar" packages/trpc/server/routers/viewer/deleteCredential.handler.ts -B 5 -A 5Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 193
🏁 Script executed:
# Check paypal-webhook.ts
rg -n "destinationCalendar" --type=ts -l | xargs grep -l "paypal"Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 141
🏁 Script executed:
# Get requestReschedule.handler.ts context around line 240-242
rg -n "destinationCalendar" packages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts -B 5 -A 5Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 900
🏁 Script executed:
# Get the beginning of requestReschedule.handler.ts to see how user is obtained
head -100 packages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts | grep -n "ctx.user\|const user"Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 221
🏁 Script executed:
# Get the getUserFromSession function to see what it returns
rg -n "function getUserFromSession|const getUserFromSession" --type=ts -A 20Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1768
🏁 Script executed:
# Check paypal-webhook.ts for destinationCalendar usage
rg -n "destinationCalendar" packages/features/ee/payments/api/paypal-webhook.ts -B 5 -A 5Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1100
🏁 Script executed:
# Check if user is available in requestReschedule.handler.ts
rg -n "user\." packages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts | head -20Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 550
🏁 Script executed:
# Get the full context of where bookingToReschedule and user are defined
head -150 packages/trpc/server/routers/viewer/bookings/requestReschedule.handler.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 5469
🏁 Script executed:
# Get the complete getUserFromSession function with its select statement
rg -n "function getUserFromSession" --type=ts -A 60 packages/trpc/server/middlewares/sessionMiddleware.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1781
🏁 Script executed:
# Also check if there are any other files mentioned in the review comment
find . -name "deleteCredential.handler.ts" -o -name "*deleteCredential*" 2>/dev/null | head -5Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 236
🏁 Script executed:
# Search for other handlers that use destinationCalendar fallback pattern
rg -B 5 -A 5 "user\.destinationCalendar" --type=ts packages/trpc/server/routers/viewer/bookings/Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1242
🏁 Script executed:
# Check deleteCredential.handler.ts for destinationCalendar usage
rg -n "destinationCalendar" packages/trpc/server/routers/loggedInViewer/deleteCredential.handler.ts -B 5 -A 5Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 2502
Missing user.destinationCalendar fallback — inconsistent with other handlers.
Other handlers in this PR (e.g., confirm.handler.ts, deleteCredential.handler.ts, paypal-webhook.ts) fall back to user.destinationCalendar when the booking's destinationCalendar is absent. This handler only uses the booking-level calendar with no fallback, which could result in an empty destinationCalendar array in the webhook payload even when the user has a configured destination calendar.
Suggested fix
- destinationCalendar: bookingToReschedule?.destinationCalendar
- ? [bookingToReschedule?.destinationCalendar]
- : [],
+ destinationCalendar: bookingToReschedule?.destinationCalendar
+ ? [bookingToReschedule.destinationCalendar]
+ : user.destinationCalendar
+ ? [user.destinationCalendar]
+ : [],📝 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.
| destinationCalendar: bookingToReschedule?.destinationCalendar | |
| ? [bookingToReschedule?.destinationCalendar] | |
| : [], | |
| destinationCalendar: bookingToReschedule?.destinationCalendar | |
| ? [bookingToReschedule.destinationCalendar] | |
| : user.destinationCalendar | |
| ? [user.destinationCalendar] | |
| : [], |
🤖 Prompt for AI Agents
In `@packages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts`
around lines 240 - 242, The destinationCalendar for the webhook payload is only
using bookingToReschedule?.destinationCalendar and can become empty; change the
construction to fall back to user.destinationCalendar when
bookingToReschedule?.destinationCalendar is absent (i.e., set
destinationCalendar to [bookingToReschedule.destinationCalendar] if present else
[user.destinationCalendar] if present else []), updating the requestReschedule
handler where destinationCalendar is built so it matches the pattern used in
confirm.handler.ts and deleteCredential.handler.ts.
| @@ -216,7 +218,7 @@ export interface IntegrationCalendar extends Ensure<Partial<SelectedCalendar>, " | |||
| } | |||
|
|
|||
| export interface Calendar { | |||
| createEvent(event: CalendarEvent): Promise<NewCalendarEventType>; | |||
| createEvent(event: CalendarEvent, credentialId: number): Promise<NewCalendarEventType>; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all createEvent implementations and call sites
echo "=== createEvent implementations ==="
rg -n 'createEvent\(' --type=ts -C3 -g '!node_modules' -g '!*.d.ts' | head -100
echo ""
echo "=== Classes implementing Calendar interface ==="
rg -n 'implements Calendar' --type=ts -g '!node_modules'
echo ""
echo "=== Calendar interface definition ==="
rg -n 'interface Calendar' --type=ts -g '!node_modules' -A10Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 12528
🏁 Script executed:
#!/bin/bash
# Find the createEvent wrapper function
echo "=== createEvent function definition in EventManager ==="
rg -n 'createEvent.*=' packages/core/EventManager.ts -A5 | head -40
echo ""
echo "=== Search for createEvent import in EventManager ==="
rg -n 'import.*createEvent' packages/core/EventManager.ts
echo ""
echo "=== BaseCalendarService createEvent signature ==="
sed -n '120,130p' packages/lib/CalendarService.ts
echo ""
echo "=== Check GoogleCalendarService createEvent ==="
rg -n 'createEvent' packages/app-store/googlecalendar/lib/CalendarService.ts -A8 | head -30
echo ""
echo "=== Check if any implementation passes credentialId ==="
fd --extension ts --path '*/app-store/*/lib/CalendarService.ts' -x grep -n 'credentialId' {} +Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1429
🏁 Script executed:
#!/bin/bash
# Find CalendarManager.ts
echo "=== CalendarManager.ts createEvent function ==="
rg -n 'export.*createEvent' packages/core/CalendarManager.ts -A15 | head -50
echo ""
echo "=== Check all Calendar implementations for createEvent signatures ==="
for file in $(find packages/app-store -name 'CalendarService.ts' -not -path '*/node_modules/*'); do
echo "=== $file ==="
rg -n 'createEvent' "$file" -A2 | head -8
doneRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 4128
🏁 Script executed:
#!/bin/bash
# Full CalendarManager.ts createEvent implementation
echo "=== CalendarManager.ts createEvent full implementation ==="
sed -n '218,280p' packages/core/CalendarManager.ts
echo ""
echo "=== Check if calendar.createEvent is being called with credentialId ==="
rg -n 'calendar\.createEvent' packages/core/CalendarManager.ts -A2 -B2Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 2379
Critical: Update all Calendar implementations to accept the new credentialId parameter.
The interface change is a breaking change that has not been applied consistently across implementations. The CalendarManager wrapper is already calling calendar.createEvent(calEvent, credential.id) with two parameters, but only GoogleCalendarService has been updated to accept credentialId. The following implementations still only accept (event: CalendarEvent) and will fail when invoked:
BaseCalendarService(base class)BasecampCalendarServiceCloseComCalendarServiceExchangeCalendarService(all three versions: 2013, 2016, generic)HubspotCalendarServiceLarkCalendarServiceOffice365CalendarServiceSalesforceCalendarServiceSendgridCalendarServiceZohoCrmCalendarServiceZohoBiginCalendarService
Update all implementations to match the interface: async createEvent(event: CalendarEvent, credentialId: number): Promise<NewCalendarEventType>. Alternatively, if credentialId is not needed by all integrations (particularly CalDAV-based calendars), consider making it optional in the interface: credentialId?: number.
🤖 Prompt for AI Agents
In `@packages/types/Calendar.d.ts` at line 221, The createEvent signature changed
to include credentialId but many implementations still accept only (event:
CalendarEvent); update BaseCalendarService.createEvent and all concrete classes
(BasecampCalendarService, CloseComCalendarService, ExchangeCalendarService
(2013/2016/generic), HubspotCalendarService, LarkCalendarService,
Office365CalendarService, SalesforceCalendarService, SendgridCalendarService,
ZohoCrmCalendarService, ZohoBiginCalendarService) to match async
createEvent(event: CalendarEvent, credentialId: number):
Promise<NewCalendarEventType> (or make credentialId optional in the interface
and implementations if some integrations don’t need it), and ensure any callers
(CalendarManager already passes credential.id) continue to compile and that
unused credentialId is simply accepted/ignored where not needed.
This pull request was automatically created by
@coderabbitai/e2e-reviewer.Batch created pull request.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor