-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Calendly - Improve List Events action and New Event Scheduled source #17687
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis update introduces a new "scope" property to the Calendly "List Events" action and "New Event Scheduled" source, allowing users to specify whether to fetch events for the authenticated user, organization, user, or group. Supporting code for group selection, dynamic UI property visibility, and conditional parameter handling was added. Several components and the package version were incremented. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Action/Source
participant CalendlyApp
participant CalendlyAPI
User->>Action/Source: Select scope (authenticatedUser/org/user/group)
Action/Source->>CalendlyApp: listEvents({scope, organization, user, group, ...})
CalendlyApp->>CalendlyAPI: GET /events with params based on scope
CalendlyAPI-->>CalendlyApp: Events data
CalendlyApp-->>Action/Source: Events data
Action/Source-->>User: Display or emit events
Assessment against linked issues
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
components/calendly_v2/actions/list-events/list-events.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/calendly_v2/calendly_v2.app.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/calendly_v2/common/constants.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
components/calendly_v2/sources/new-event-scheduled/new-event-scheduled.mjs
Outdated
Show resolved
Hide resolved
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: 1
📜 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 (4)
components/calendly_v2/actions/list-events/list-events.mjs(3 hunks)components/calendly_v2/calendly_v2.app.mjs(7 hunks)components/calendly_v2/common/constants.mjs(1 hunks)components/calendly_v2/sources/new-event-scheduled/new-event-scheduled.mjs(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/calendly_v2/common/constants.mjs
🚧 Files skipped from review as they are similar to previous changes (2)
- components/calendly_v2/actions/list-events/list-events.mjs
- components/calendly_v2/sources/new-event-scheduled/new-event-scheduled.mjs
🧰 Additional context used
🧠 Learnings (1)
components/calendly_v2/calendly_v2.app.mjs (3)
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#12697
File: components/salesforce_rest_api/sources/common-webhook-methods.mjs:1-71
Timestamp: 2024-10-08T15:33:38.240Z
Learning: The `common-webhook-methods.mjs` object is designed to be extended, similar to an abstract class, and intentionally does not implement certain methods like `generateWebhookMeta` and `getEventType` to enforce implementation in subclasses.
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#12697
File: components/salesforce_rest_api/sources/common-webhook-methods.mjs:1-71
Timestamp: 2024-07-24T02:06:47.016Z
Learning: The `common-webhook-methods.mjs` object is designed to be extended, similar to an abstract class, and intentionally does not implement certain methods like `generateWebhookMeta` and `getEventType` to enforce implementation in subclasses.
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#16954
File: components/salesloft/salesloft.app.mjs:14-23
Timestamp: 2025-06-04T17:52:05.780Z
Learning: In the Salesloft API integration (components/salesloft/salesloft.app.mjs), the _makeRequest method returns response.data which directly contains arrays for list endpoints like listPeople, listCadences, listUsers, and listAccounts. The propDefinitions correctly call .map() directly on these responses without needing to destructure a nested data property.
⏰ 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: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (6)
components/calendly_v2/calendly_v2.app.mjs (6)
59-73: LGTM: Well-structured prop definition following established patterns.The
groupIdprop definition correctly follows the same pattern as other entity prop definitions in the codebase, with proper async options handling and context passing.
135-137: LGTM: Consistent helper method following established patterns.The
_buildGroupUrimethod correctly mirrors the existing_buildUserUripattern and maintains consistency in URI construction.
340-347: LGTM: Consistent implementation following established patterns.The
listGroupsmethod correctly follows the same pattern as other list methods in the codebase, using_makeRequestfor proper pagination handling.
111-126: LGTM: Well-designed props with helpful user guidance.The
listEventsScopeandlistEventsAlertprops provide excellent user experience with clear explanations of scope options and their privilege requirements.
394-405: LGTM: Clean implementation of dynamic UI logic.The
listEventsAdditionalPropsmethod correctly implements scope-based property visibility, providing a good user experience by showing only relevant fields based on the selected scope.
279-295: Clarify scope parameter precedence inlistEventsCurrently, if both
uuid(user) andparams.groupare provided, they’ll both be set—potentially conflicting or unintended. We need to confirm whether the Calendly API allows multiple scopes simultaneously or if only one filter (organization, group, or user) should be active at a time.If exclusivity is required, consider refactoring to an
if … else if … elsechain:async listEvents(params, uuid, $) { - if (uuid) { + if (uuid) { params.user = this._buildUserUri(uuid); - } - if (params.group) { + } else if (params.group) { params.group = this._buildGroupUri(params.group); - } - if (!params.organization && !params.group && !params.user) { + } else if (!params.organization) { params.user = await this.defaultUser($); } const opts = { path: "/scheduled_events", params, }; return this._makeRequest(opts, $);• Confirm intended API behavior: can multiple scope parameters be used together?
• If not, enforce mutual exclusivity as shown above.
|
/approve |
Resolves #17679
Summary by CodeRabbit
New Features
Bug Fixes
Chores