-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Calendly - List Events - support listing events by organization #14410
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
WalkthroughThe pull request includes updates to multiple files within the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
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: 0
🧹 Outside diff range and nitpick comments (2)
components/calendly_v2/actions/list-events/list-events.mjs (1)
25-25
: Enhance the user property description for clarity.The current description could be more explicit about the relationship with organization-level events.
Consider updating the description to:
- description: "Returns events for a specified user", + description: "Optional. When specified, returns events for this user. When omitted with organization set, returns events for all users in the organization",components/calendly_v2/calendly_v2.app.mjs (1)
244-246
: Consider adding JSDoc documentation for the organization parameter behavior.While the implementation is correct, it would be helpful to document this behavior in the method's JSDoc comments to make it clear that:
- The user parameter becomes optional when an organization is provided
- Organization-wide event listing is supported
Add documentation above the method:
+/** + * List scheduled events + * @param {Object} params - Query parameters + * @param {string} params.organization - Optional organization URI. When provided, + * events for the entire organization will be returned + * @param {string} uuid - Optional user UUID + * @param {Object} $ - Runtime object + * @returns {Promise<Object>} Response containing events collection and pagination info + */ async listEvents(params, uuid, $) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
- components/calendly_v2/actions/create-invitee-no-show/create-invitee-no-show.mjs (1 hunks)
- components/calendly_v2/actions/create-scheduling-link/create-scheduling-link.mjs (1 hunks)
- components/calendly_v2/actions/get-event/get-event.mjs (1 hunks)
- components/calendly_v2/actions/list-event-invitees/list-event-invitees.mjs (1 hunks)
- components/calendly_v2/actions/list-events/list-events.mjs (3 hunks)
- components/calendly_v2/actions/list-webhook-subscriptions/list-webhook-subscriptions.mjs (1 hunks)
- components/calendly_v2/calendly_v2.app.mjs (1 hunks)
- components/calendly_v2/package.json (2 hunks)
- components/calendly_v2/sources/invitee-canceled/invitee-canceled.mjs (1 hunks)
- components/calendly_v2/sources/invitee-created/invitee-created.mjs (1 hunks)
- components/calendly_v2/sources/new-event-scheduled/new-event-scheduled.mjs (1 hunks)
- components/calendly_v2/sources/routing-form-submission-created/routing-form-submission-created.mjs (1 hunks)
✅ Files skipped from review due to trivial changes (8)
- components/calendly_v2/actions/create-invitee-no-show/create-invitee-no-show.mjs
- components/calendly_v2/actions/create-scheduling-link/create-scheduling-link.mjs
- components/calendly_v2/actions/get-event/get-event.mjs
- components/calendly_v2/actions/list-event-invitees/list-event-invitees.mjs
- components/calendly_v2/actions/list-webhook-subscriptions/list-webhook-subscriptions.mjs
- components/calendly_v2/sources/invitee-canceled/invitee-canceled.mjs
- components/calendly_v2/sources/new-event-scheduled/new-event-scheduled.mjs
- components/calendly_v2/sources/routing-form-submission-created/routing-form-submission-created.mjs
🧰 Additional context used
🔇 Additional comments (7)
components/calendly_v2/package.json (2)
3-3
: LGTM: Version bump follows semantic versioning.The increment from 1.2.0 to 1.2.1 is appropriate for a bug fix that maintains backward compatibility.
16-16
: Verify compatibility with @pipedream/platform v3.x.xThe major version update from ^1.5.1 to ^3.0.3 could introduce breaking changes. Please ensure all platform API usage in the component is compatible with v3.
components/calendly_v2/sources/invitee-created/invitee-created.mjs (2)
8-8
: Version bump needs clarification.The version has been incremented from 0.0.2 to 0.0.3, but there are no functional changes in this file. Could you please clarify why this version bump is necessary?
Line range hint
1-42
: Verify if this file should be included in the PR.This file handles webhook events for new invitees and doesn't contain any changes related to the organization-based event listing functionality described in the PR objectives. Consider removing this file from the PR if the version bump isn't strictly necessary for this feature.
components/calendly_v2/actions/list-events/list-events.mjs (2)
6-7
: LGTM: Version bump and documentation updates are appropriate.The version increment and documentation link updates align with the feature enhancement.
55-57
: Verify integration with listEvents method.The addition of organization to params aligns with the PR objectives to support organization-wide event listing.
Let's verify the integration with the app file's listEvents method:
✅ Verification successful
Let me try a different approach to verify the integration by searching for the listEvents method with a more flexible pattern and also checking the app file directly.
Based on the partial output we received, I can see the
listEvents
method implementation incalendly_v2.app.mjs
. Let me try one more search to get the complete context of this method.
Integration with listEvents method is properly implemented
The verification shows that the integration is working correctly:
In
calendly_v2.app.mjs
, thelistEvents
method properly handles the organization parameter:
- If
params.organization
is present, theuser
parameter becomesundefined
- The method correctly spreads all params (including organization) into the request parameters
In the action file
list-events.mjs
, the organization parameter is correctly passed through tolistEvents
method in the params object.This confirms that the changes align with the PR objectives to support organization-wide event listing, and the integration between the action and app file is properly implemented.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the calendly_v2.app.mjs file correctly handles the organization parameter # Expected: The listEvents method should handle the organization parameter and make user optional # Search for the listEvents method implementation ast-grep --pattern 'listEvents($params, $user, $_) { $$$ }' # Also check for any related test files fd -g '*test*.{js,mjs}' -x grep -l "listEvents"Length of output: 111
Script:
#!/bin/bash # Search for listEvents method with a more flexible pattern ast-grep --pattern 'listEvents($_) { $$$ }' # Also directly check the app file content rg -A 10 "listEvents" components/calendly_v2/calendly_v2.app.mjs # Check for any usage of listEvents in test files rg "listEvents" --type js --type mjsLength of output: 778
Script:
#!/bin/bash # Get full context of listEvents method rg -B 5 -A 15 "async listEvents" components/calendly_v2/calendly_v2.app.mjs # Also check the action file to see how it's being used rg -B 5 -A 15 "listEvents" components/calendly_v2/actions/list-events/list-events.mjsLength of output: 1036
components/calendly_v2/calendly_v2.app.mjs (1)
244-246
: LGTM! The changes correctly implement organization-based event listing.The modified logic properly handles the user parameter based on the presence of an organization, aligning with the Calendly API's behavior where the user parameter is optional when an organization URI is provided.
Let's verify the implementation matches the Calendly API documentation:
✅ Verification successful
Let me generate more specific scripts to verify the implementation.
The previous results showed matches in other components but we need to focus specifically on the calendly_v2 implementation to verify:
- The complete context of the listEvents method
- Test coverage for this functionality
- Documentation of this behavior
This will help us confirm if the implementation is properly documented and tested, and if it aligns with the Calendly API's expected behavior.
Let me try one more time with corrected commands to verify the implementation.
I've adjusted the commands to:
- Use ripgrep with multiline support to find the complete listEvents method
- Look for test files specifically in the calendly_v2 directory
- Directly read the README from the correct path
This should give us the context we need to verify the implementation's correctness and documentation.
Let me check the specific list-events action implementation since it was found in the previous results.
This will help us verify:
- How the list-events action uses the organization parameter
- The documentation of the organization parameter
- The complete context around organization usage in the app
Based on the gathered information, I can now provide a final response.
The organization-aware implementation is correct and well-documented
The implementation is verified to be correct because:
- The list-events action properly defines the organization parameter as a prop and passes it to the listEvents method
- The user parameter is correctly marked as optional and depends on the organization context
- The app's implementation aligns with Calendly's API behavior where:
- When organization is provided, user becomes optional (undefined)
- When no organization is provided, it falls back to the default user
- When UUID is explicitly provided, it takes precedence
The changes are properly integrated into the component's architecture and the implementation matches the expected behavior for organization-based event listing. The conditional logic in the original review comment is implementing the correct precedence order for the parameters.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the implementation aligns with Calendly API documentation # and that the changes don't introduce regressions in other files. # Test 1: Check for any other files that might need similar organization-aware changes rg -l "listEvents.*user.*organization" # Test 2: Verify the component's documentation reflects this change fd -e md -e mjs . -x grep -l "organization.*event"Length of output: 802
Script:
#!/bin/bash # Let's check the specific implementation details in calendly_v2 # Check the complete listEvents method implementation ast-grep --pattern 'async listEvents($_) { $$$ }' # Check if there are any tests validating this behavior fd -e test.mjs calendly_v2 -x cat {} # Check the component's documentation fd README.md -p "*calendly_v2*" -x cat {}Length of output: 241
Script:
#!/bin/bash # Let's verify the implementation with corrected commands # Check the complete listEvents method in calendly_v2 rg -U "async listEvents.*?\{.*?\}" -A 30 components/calendly_v2/calendly_v2.app.mjs # Check for tests rg -l "test|spec" components/calendly_v2/ # Check documentation cat components/calendly_v2/README.mdLength of output: 1526
Script:
#!/bin/bash # Check the list-events action implementation and its documentation # Check the list-events action implementation cat components/calendly_v2/actions/list-events/list-events.mjs # Check for any props or parameters documentation rg -A 10 "props: {" components/calendly_v2/actions/list-events/list-events.mjs # Check for organization parameter usage in the app rg -B 2 -A 2 "organization" components/calendly_v2/calendly_v2.app.mjsLength of output: 3474
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.
Hi @michelle0927 lgtm! Ready for QA!
Resolves #14149
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Chores