[Components] zoom_admin - fix to get-meeting-recordings #16940
Conversation
|
Someone is attempting to deploy a commit to the Pipedreamers Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
WalkthroughThe code updates the Zoom Admin "get-meeting-recordings" action by adding an optional Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Action
participant ZoomAdmin
User->>Action: Invoke get-meeting-recordings (with optional ttl)
Action->>ZoomAdmin: listMeetingRecordings({ include_fields, ttl })
ZoomAdmin-->>Action: Return meeting recordings (with optional download token)
Action-->>User: Return recordings data
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/zoom_admin/actions/get-meeting-recordings/get-meeting-recordings.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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 (
|
|
Thank you so much for submitting this! We've added it to our backlog to review, and our team has been notified. |
|
Thanks for submitting this PR! When we review PRs, we follow the Pipedream component guidelines. If you're not familiar, here's a quick checklist:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
components/zoom_admin/actions/get-meeting-recordings/get-meeting-recordings.mjs (1)
2-2: Remove unused import.The
paginateimport is no longer used after the refactoring that removes pagination in favor of direct API calls.-import { paginate } from "../../common/pagination.mjs";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/zoom_admin/actions/get-meeting-recordings/get-meeting-recordings.mjs(2 hunks)
🔇 Additional comments (3)
components/zoom_admin/actions/get-meeting-recordings/get-meeting-recordings.mjs (3)
10-10: LGTM!Appropriate version bump for the changes made.
26-33: LGTM!The TTL property is well-defined with appropriate constraints and clear documentation. The 7-day maximum aligns with typical token expiration practices.
36-45:❓ Verification inconclusive
Verify API parameter requirements.
The parameter building logic is well-structured and correctly implements the conditional inclusion of
include_fieldsandttl. The refactoring from pagination to direct API calls aligns with the PR objectives.Please verify that the Zoom API supports the
ttlparameter wheninclude_fields=download_access_tokenis used:
🏁 Script executed:
#!/bin/bash # Search for API documentation or usage examples of the ttl parameter with Zoom recordings API rg -i "ttl.*download.*token|download.*token.*ttl" --type js rg -i "listMeetingRecordings.*ttl|ttl.*listMeetingRecordings" --type jsLength of output: 135
Verify TTL parameter support in Zoom API.
The refactored logic correctly adds
include_fields = "download_access_token"andttlonly whendownloadAccessTokenis enabled. We didn’t find any in-repo examples of TTL usage—please confirm that Zoom’s Cloud Recording API supports attlparameter on thelistMeetingRecordingsendpoint wheninclude_fields=download_access_token(see https://marketplace.zoom.us/docs/api-reference/zoom-api/cloud-recording/recordingget).
jocarino
changed the title
andrewjschuang
moved this to Ready for PR Review
in Component (Source and Action) Backlog
andrewjschuang
requested review from
GTFalcao
and removed request for
luancazarine
GTFalcao
moved this from Ready for PR Review
to Ready for QA
in Component (Source and Action) Backlog
vunguyenhung
moved this from Ready for QA
to Ready for Release
in Component (Source and Action) Backlog
|
Hi everyone, all test cases are passed! Ready for release! |
WHY
Correct downloadAccessToken -> string not a boolean
Add TTL for the token
Removed pagination on recordings -> wrong prop and don't need this or won't show other important props
Tested custom version on workflow to check it's working as intended
Summary by CodeRabbit
New Features
Refactor