Outlook Calendar - use listCalendarView #18822
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughAdds optional recurring-event support to the list-events action by introducing an Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ListEventsAction as ListEvents
participant App
participant MSGraph as MicrosoftGraph
User->>ListEvents: invoke list-events
alt includeRecurring == false
ListEvents->>App: listCalendarEvents(params)
App->>MSGraph: GET /me/events
MSGraph-->>App: events (non-expanded recurring)
else includeRecurring == true
ListEvents->>App: listCalendarView({...params, startDateTime, endDateTime})
App->>MSGraph: GET /me/calendar/calendarView
MSGraph-->>App: expanded event instances (recurring expanded)
end
App-->>ListEvents: events
ListEvents-->>User: results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/microsoft_outlook_calendar/actions/get-schedule/get-schedule.mjs (1)
58-63: Validate after parsing to avoid empty/invalid inputs slipping through.The emptiness check runs before
utils.parseArray(...). Inputs like"[]","", or" , "could pass the pre-check but parse to an empty array. Perform the check on the parsed array instead.Apply this diff:
- if (this.schedules === null || this.schedules === undefined || this.schedules?.length === 0) { - throw new ConfigurationError("The **Schedules** property is required"); - } - - const schedules = utils.parseArray(this.schedules); + const schedules = utils.parseArray(this.schedules); + if (!Array.isArray(schedules) || schedules.length === 0) { + throw new ConfigurationError("The **Schedules** property is required and must contain at least one email."); + }
🧹 Nitpick comments (3)
components/microsoft_outlook_calendar/actions/list-events/list-events.mjs (1)
60-79: Consider adding validation for required date parameters.While marking
startDateTimeandendDateTimeas required inadditionalProps()handles UI validation, adding defensive validation in therun()method would provide better error handling and clearer error messages if the parameters are missing.Apply this diff to add validation:
async run({ $ }) { const params = { "$orderby": this.orderBy, "$filter": this.filter, "$top": this.maxResults, }; + if (this.includeRecurring && (!this.startDateTime || !this.endDateTime)) { + throw new Error("startDateTime and endDateTime are required when includeRecurring is enabled"); + } + const { value = [] } = !this.includeRecurring ? await this.microsoftOutlook.listCalendarEvents({ $, params, }) : await this.microsoftOutlook.listCalendarView({ $, params: { ...params, startDateTime: this.startDateTime, endDateTime: this.endDateTime, }, });components/microsoft_outlook_calendar/actions/get-schedule/get-schedule.mjs (2)
57-78: Add lightweight guards for date ordering and interval bounds.Prevent avoidable API errors with simple local validation.
Apply this diff:
async run({ $ }) { - const schedules = utils.parseArray(this.schedules); + const schedules = utils.parseArray(this.schedules); + if (!Array.isArray(schedules) || schedules.length === 0) { + throw new ConfigurationError("The **Schedules** property is required and must contain at least one email."); + } + + // Validate time range + const startDt = new Date(this.start); + const endDt = new Date(this.end); + if (Number.isNaN(startDt.valueOf()) || Number.isNaN(endDt.valueOf())) { + throw new ConfigurationError("Invalid start or end date/time."); + } + if (startDt >= endDt) { + throw new ConfigurationError("`Start` must be earlier than `End`."); + } + + // Validate interval within documented bounds (min 5, max 1440 minutes) + if (this.availabilityViewInterval != null) { + const iv = Number(this.availabilityViewInterval); + if (!Number.isFinite(iv) || iv < 5 || iv > 1440) { + throw new ConfigurationError("Availability View Interval must be between 5 and 1440 minutes."); + } + } const { value } = await this.getSchedule({ $, data: { schedules, startTime: { dateTime: this.start, timeZone: this.timeZone, }, endTime: { dateTime: this.end, timeZone: this.timeZone, }, availabilityViewInterval: this.availabilityViewInterval, }, });
80-80: Avoid exposing raw email addresses in step summaries.Summaries are user-visible; consider masking or summarizing counts to reduce PII exposure.
Apply this diff:
- $.export("$summary", `Successfully retrieved schedules for \`${schedules.join("`, `")}\``); + $.export("$summary", `Successfully retrieved schedules for ${schedules.length} recipient(s).`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
components/microsoft_outlook_calendar/actions/create-calendar-event/create-calendar-event.mjs(1 hunks)components/microsoft_outlook_calendar/actions/delete-calendar-event/delete-calendar-event.mjs(1 hunks)components/microsoft_outlook_calendar/actions/get-schedule/get-schedule.mjs(1 hunks)components/microsoft_outlook_calendar/actions/list-events/list-events.mjs(2 hunks)components/microsoft_outlook_calendar/actions/update-calendar-event/update-calendar-event.mjs(1 hunks)components/microsoft_outlook_calendar/microsoft_outlook_calendar.app.mjs(1 hunks)components/microsoft_outlook_calendar/package.json(2 hunks)components/microsoft_outlook_calendar/sources/common.mjs(0 hunks)components/microsoft_outlook_calendar/sources/common/common.mjs(1 hunks)components/microsoft_outlook_calendar/sources/new-calendar-event/new-calendar-event.mjs(1 hunks)components/microsoft_outlook_calendar/sources/new-upcoming-event/new-upcoming-event.mjs(1 hunks)components/microsoft_outlook_calendar/sources/updated-calendar-event/updated-calendar-event.mjs(1 hunks)
💤 Files with no reviewable changes (1)
- components/microsoft_outlook_calendar/sources/common.mjs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (11)
components/microsoft_outlook_calendar/actions/delete-calendar-event/delete-calendar-event.mjs (1)
6-6: Version bump is appropriate and consistent with PR changes.The version change from 0.0.2 to 0.0.3 aligns with the coordinated version updates across related actions in this PR (e.g., create-calendar-event, update-calendar-event, list-events, and get-schedule also receive increments). Since the app module and supporting infrastructure receive updates for recurring event support, incrementing dependent action versions is standard practice.
components/microsoft_outlook_calendar/actions/update-calendar-event/update-calendar-event.mjs (1)
1-146: Existing code is solid; confirm version bump rationale.The action implementation is well-structured with proper optional parameter handling and conditional data construction. However, the version bump from 0.0.2 to 0.0.3 on Line 6 appears to be cosmetic—there are no functional or interface changes in this file itself.
Per the AI summary, other actions were also versioned (create-calendar-event 0.0.7→0.0.8, delete-calendar-event 0.0.2→0.0.3, get-schedule 0.0.4→0.0.5), suggesting a coordinated versioning effort. This is likely due to the new
listCalendarView()method added to the app module or updated dependencies in package.json (now 0.3.3).Please verify and clarify:
- Is this version bump intentional, signaling a change in the app interface or breaking compatibility?
- Should consumers of this action expect any behavior or interface changes, or is this purely an internal dependency/app-level update?
Clarifying this will help justify the semver bump and ensure proper consumer communication.
components/microsoft_outlook_calendar/sources/new-calendar-event/new-calendar-event.mjs (1)
1-1: LGTM! Import path and version updated correctly.The import path change aligns with the new common module structure, and the version bump is appropriate for this non-breaking refactoring.
Also applies to: 8-8
components/microsoft_outlook_calendar/sources/updated-calendar-event/updated-calendar-event.mjs (1)
1-1: LGTM! Import path and version updated correctly.Consistent with the common module restructuring across the codebase.
Also applies to: 8-8
components/microsoft_outlook_calendar/actions/create-calendar-event/create-calendar-event.mjs (1)
6-6: LGTM! Version bump for package consistency.components/microsoft_outlook_calendar/microsoft_outlook_calendar.app.mjs (1)
183-189: LGTM! New method follows existing patterns.The
listCalendarView()method correctly implements the calendarView endpoint following the same pattern aslistCalendarEvents(). This provides the necessary infrastructure for retrieving recurring event instances.components/microsoft_outlook_calendar/sources/common/common.mjs (1)
1-10: LGTM! Common module wrapper correctly structured.The module properly extends the shared common source from
@pipedream/microsoft_outlookwhile adding the localmicrosoftOutlookapp prop for calendar-specific functionality.components/microsoft_outlook_calendar/sources/new-upcoming-event/new-upcoming-event.mjs (1)
1-2: LGTM! Import paths and version updated correctly.The changes properly reflect the module restructuring and new package dependencies added in package.json.
Also applies to: 9-9
components/microsoft_outlook_calendar/package.json (1)
3-3: Version and dependencies updated appropriately.The version bump and new dependencies support the module restructuring and shared common source integration.
Also applies to: 18-19
components/microsoft_outlook_calendar/actions/list-events/list-events.mjs (1)
35-41: LGTM! Prop enables backwards-compatible recurring events support.The
includeRecurringprop withreloadProps: truecorrectly implements the backwards-compatible approach requested in the issue, defaulting to false to preserve existing behavior.components/microsoft_outlook_calendar/actions/get-schedule/get-schedule.mjs (1)
9-9: Patch version bump only — OK to publish.No functional changes in this file; patch bump is appropriate.
components/microsoft_outlook_calendar/actions/list-events/list-events.mjs
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/microsoft_outlook_calendar/actions/list-events/list-events.mjs (1)
48-57: MarkstartDateTimeandendDateTimeas required.The Microsoft Graph calendarView endpoint requires both
startDateTimeandendDateTimeas mandatory query parameters. Without marking these props as required, users could enable recurring events without providing dates, resulting in runtime API errors.Apply this diff to mark the props as required:
return { startDateTime: { type: "string", label: "Start Date Time", description: "The start date and time of the time range, represented in ISO 8601 format. For example, `2019-11-08T19:00:00-08:00`", + optional: false, }, endDateTime: { type: "string", label: "End Date Time", description: "The end date and time of the time range, represented in ISO 8601 format. For example, `2019-11-08T20:00:00-08:00`", + optional: false, }, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
components/microsoft_outlook_calendar/actions/list-events/list-events.mjs(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/microsoft_outlook_calendar/actions/list-events/list-events.mjs (1)
components/microsoft_outlook_calendar/microsoft_outlook_calendar.app.mjs (1)
events(12-12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (2)
components/microsoft_outlook_calendar/actions/list-events/list-events.mjs (2)
7-7: LGTM!The version bump is appropriate for this feature addition.
35-41: LGTM!The
includeRecurringprop is correctly structured withreloadProps: trueto conditionally expose the date range props.
| const events = !this.includeRecurring | ||
| ? value.filter((event) => !event.recurrence) | ||
| : value; |
There was a problem hiding this comment.
Breaking change: default behavior now filters out recurring events.
The filtering logic changes the default behavior when includeRecurring is false or undefined. Previously, listCalendarEvents() would return recurring event definitions in the results. Now, line 82 filters them out by default, which breaks backwards compatibility.
The issue (#18789) requires this to be "backwards-compatible," meaning the default behavior should remain unchanged. Consider one of these approaches:
Option 1: Remove the filter when includeRecurring is false/undefined
- const events = !this.includeRecurring
- ? value.filter((event) => !event.recurrence)
- : value;
+ const events = value;Option 2: Use three states instead of two
- includeRecurring: {
- type: "boolean",
- label: "Include Recurring",
- description: "Whether to include recurring events",
+ includeRecurring: {
+ type: "string",
+ label: "Include Recurring",
+ description: "How to handle recurring events",
+ options: [
+ { label: "Default (recurring definitions only)", value: "default" },
+ { label: "Exclude recurring events", value: "exclude" },
+ { label: "Include recurring instances", value: "instances" },
+ ],
+ default: "default",📝 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 events = !this.includeRecurring | |
| ? value.filter((event) => !event.recurrence) | |
| : value; | |
| const events = value; |
🤖 Prompt for AI Agents
components/microsoft_outlook_calendar/actions/list-events/list-events.mjs around
lines 81 to 83: the current conditional treats undefined as falsy and therefore
filters out recurring events by default, breaking backwards compatibility;
change the condition so recurring events are only filtered when includeRecurring
is explicitly false (e.g., replace the check with this.includeRecurring ===
false), so the default (undefined) continues to include recurring event
definitions.
2 participants
Resolves #18789
Summary by CodeRabbit
New Features
Chores