-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Zoom & Zoom Admin - new transcript components #18036
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
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
WalkthroughAdds Get Meeting Transcript actions for Zoom and Zoom Admin, plus New Recording Transcript Completed (Instant) sources for both that handle the recording.transcript_completed webhook and perform deploy-time backfill over the past month. Package versions and @pipedream/platform dependencies are bumped in both components. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Action as Get Meeting Transcript Action
participant ZoomAPI as Zoom API
User->>Action: Provide meetingId
Action->>ZoomAPI: GET /meetings/{meetingId}/transcript
ZoomAPI-->>Action: Transcript JSON
Action-->>User: Return transcript + $summary
sequenceDiagram
participant Zoom as Zoom Webhooks
participant Apphook as Apphook (recording.transcript_completed)
participant Source as Source
participant Emitter as Event Emitter
Zoom-->>Apphook: recording.transcript_completed
Apphook-->>Source: Event payload
Source->>Source: Filter recording_files for file_type=TRANSCRIPT and status=completed
Source-->>Emitter: Emit event per matching transcript file
Note over Source: On deploy: list meetings (last month) and emit completed transcript files
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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)
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 10
🧹 Nitpick comments (5)
components/zoom_admin/sources/new-recording-transcript-completed/new-recording-transcript-completed.mjs (1)
66-71
: Guard against missing recording_end when computing ts.
Some transcript files may not include recording_end; current code would set NaN.Apply:
generateMeta(file, meeting) { return { id: file.id, summary: `Transcript completed for ${meeting.topic}`, - ts: +new Date(file.recording_end), + ts: file.recording_end ? Date.parse(file.recording_end) : Date.now(), }; },components/zoom/actions/get-meeting-transcript/get-meeting-transcript.mjs (1)
21-28
: Optional: Add basic error context.
Wrap in try/catch to surface clearer errors (e.g., transcript not available/404).Proposed pattern:
methods: { getMeetingTranscript({ meetingId, ...opts }) { - return this.zoom._makeRequest({ + return this.zoom._makeRequest({ path: `/meetings/${meetingId}/transcript`, ...opts, }); }, },And in run:
- async run({ $ }) { - const { data: transcript } = await this.getMeetingTranscript({ - $, - meetingId: this.meetingId, - }); - $.export("$summary", `Retrieved transcript for meeting ${this.meetingId}`); - return transcript; - }, + async run({ $ }) { + try { + const { data: transcript } = await this.getMeetingTranscript({ + $, + meetingId: this.meetingId, + }); + $.export("$summary", `Retrieved transcript for meeting ${this.meetingId}`); + return transcript; + } catch (err) { + throw new Error(`Failed to retrieve transcript for meeting "${this.meetingId}": ${err.message || err}`); + } + },components/zoom_admin/actions/get-meeting-transcript/get-meeting-transcript.mjs (2)
12-17
: Make meetingId required for consistency with Zoom action.
Explicitly mark the prop as required.Apply:
meetingId: { propDefinition: [ zoomAdmin, "meeting", ], description: "The meeting ID to get the transcript for", + optional: false, },
20-27
: Optional: Add a method param for GET and forward $.
Small clarity improvement; ensures $ reaches _makeRequest even if not passed in run (defense in depth).Apply:
getMeetingTranscript({ meetingId, ...opts }) { - return this.zoomAdmin._makeRequest({ - path: `/meetings/${meetingId}/transcript`, - ...opts, - }); + return this.zoomAdmin._makeRequest({ + method: "GET", + path: `/meetings/${meetingId}/transcript`, + ...opts, + }); },components/zoom/sources/new-recording-transcript-completed/new-recording-transcript-completed.mjs (1)
73-77
: Align log message with event constantThe log uses “recording.transcript.completed” (dot) while the constant likely represents “recording.transcript_completed” (underscore). Use the constant in the message to avoid confusion.
Apply this diff:
- if (event.event !== constants.CUSTOM_EVENT_TYPES.RECORDING_TRANSCRIPT_COMPLETED) { - console.log("Not a recording.transcript.completed event. Exiting"); - return; - } + if (event.event !== constants.CUSTOM_EVENT_TYPES.RECORDING_TRANSCRIPT_COMPLETED) { + console.log(`Not a ${constants.CUSTOM_EVENT_TYPES.RECORDING_TRANSCRIPT_COMPLETED} event. Exiting`); + return; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
-
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
-
components/zoom/actions/get-meeting-transcript/get-meeting-transcript.mjs
(1 hunks) -
components/zoom/package.json
(2 hunks) -
components/zoom/sources/new-recording-transcript-completed/new-recording-transcript-completed.mjs
(1 hunks) -
components/zoom_admin/actions/get-meeting-transcript/get-meeting-transcript.mjs
(1 hunks) -
components/zoom_admin/package.json
(2 hunks) -
components/zoom_admin/sources/new-recording-transcript-completed/new-recording-transcript-completed.mjs
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-12T19:23:09.039Z
Learnt from: jcortes
PR: PipedreamHQ/pipedream#14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like `fs` to `package.json` dependencies, as they are native modules provided by the Node.js runtime.
Applied to files:
components/zoom_admin/package.json
🧬 Code Graph Analysis (1)
components/zoom/sources/new-recording-transcript-completed/new-recording-transcript-completed.mjs (2)
components/hubspot/actions/get-associated-meetings/get-associated-meetings.mjs (1)
meetings
(294-301)components/zoom_admin/sources/new-recording-transcript-completed/new-recording-transcript-completed.mjs (3)
event
(78-78)payload
(79-79)recordingFiles
(80-80)
⏰ 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: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (6)
components/zoom/package.json (2)
3-3
: Version bump looks good.
No issues with semver here given new features were added.
17-18
: Confirm @pipedream/platform v3 compatibility for components/zoomRan the grep you suggested — summary:
- components/zoom/package.json sets "@pipedream/platform": "^3.1.0".
- Apphook sources under components/zoom/sources/* use type: "$.interface.apphook" (e.g. meeting-, webinar-, recording-completed, new-recording-transcript-completed, custom-event, phone-event).
- Core files that import axios / call _makeRequest in this component: components/zoom/zoom.app.mjs and multiple actions under components/zoom/actions/* (e.g. get-meeting-transcript.mjs, list-past-meeting-participants.mjs, list-meetings.mjs, list-call-recordings.mjs, list-webinar-participants.mjs).
- No matches for the deprecated event name strings recording.transcript.completed or recording.transcript_completed.
Action (manual verification required):
- Exercise all Zoom apphook sources and key actions that call _makeRequest / axios.
- Verify $.export usage and payload handling in runtime (webhook payload shape, apphook registration, any HTTP helper behavior).
- Run integration tests / smoke tests for Zoom sources/actions after upgrading.
I can't prove runtime compatibility automatically — please run the checks above and report any failing call sites or apphook handlers to address.
components/zoom_admin/package.json (2)
3-3
: Version bump acknowledged.
0.10.0 → 0.11.0 aligns with added surface area.
17-20
: Validate @pipedream/platform v3.1.0 upgrade across zoom_admin componentsQuick summary: I scanned components/zoom_admin and found @pipedream/platform usage in package.json, the app wrapper, many actions, and many sources — these locations need a compatibility check for the v3.1.0 upgrade.
Files/locations to verify:
- components/zoom_admin/package.json — "@pipedream/platform": "^3.1.0"
- components/zoom_admin/zoom_admin.app.mjs — imports axios and implements/uses _makeRequest (multiple call sites)
- Sources (use $.interface.apphook):
- components/zoom_admin/sources/new-recording-transcript-completed/new-recording-transcript-completed.mjs
- components/zoom_admin/sources/user-deleted/user-deleted.mjs
- components/zoom_admin/sources/webinar-updated/webinar-updated.mjs
- components/zoom_admin/sources/webinar-started/meeting-started.mjs
- components/zoom_admin/sources/webinar-ended/webinar-ended.mjs
- components/zoom_admin/sources/webinar-deleted/webinar-deleted.mjs
- components/zoom_admin/sources/webinar-created/webinar-created.mjs
- components/zoom_admin/sources/user-invitation-accepted/user-invitation-accepted.mjs
- components/zoom_admin/sources/webinar-changes-to-panelists/webinar-changes-to-panelists.mjs (imports DEFAULT_POLLING_SOURCE_TIMER_INTERVAL)
- components/zoom_admin/sources/user-updated/user-updated.mjs
- components/zoom_admin/sources/user-deactivated/user-deactivated.mjs
- components/zoom_admin/sources/user-created/user-created.mjs
- components/zoom_admin/sources/user-activated/user-activated.mjs
- components/zoom_admin/sources/meeting-updated/meeting-updated.mjs
- components/zoom_admin/sources/meeting-ended/meeting-ended.mjs
- components/zoom_admin/sources/meeting-started/meeting-started.mjs
- components/zoom_admin/sources/recording-completed/recording-completed.mjs
- components/zoom_admin/sources/meeting-deleted/meeting-deleted.mjs
- components/zoom_admin/sources/meeting-created/meeting-created.mjs
- components/zoom_admin/sources/custom-events/custom-events.mjs
- components/zoom_admin/sources/account-created/account-created.mjs
- components/zoom_admin/sources/account-updated/account-updated.mjs
- components/zoom_admin/sources/account-settings-updated/account-settings-updated.mjs
- Actions (examples that import axios or call _makeRequest):
- components/zoom_admin/actions/get-meeting-transcript/get-meeting-transcript.mjs (this.zoomAdmin._makeRequest)
- components/zoom_admin/actions/update-webinar-registrant-status/update-webinar-registrant-status.mjs
- components/zoom_admin/actions/list-cloud-recordings/list-cloud-recordings.mjs
- components/zoom_admin/actions/update-webinar/update-webinar.mjs
- components/zoom_admin/actions/update-meeting/update-meeting.mjs
- components/zoom_admin/actions/list-webinars/list-webinars.mjs
- components/zoom_admin/actions/list-webinar-participants/list-webinar-participants.mjs
- components/zoom_admin/actions/list-users/list-users.mjs
- components/zoom_admin/actions/list-past-meeting-participants/list-past-meeting-participants.mjs
- components/zoom_admin/actions/list-webinar-registrants/list-webinar-registrants.mjs
- components/zoom_admin/actions/list-user-cloud-recordings/list-user-cloud-recordings.mjs
- components/zoom_admin/actions/list-meetings/list-meetings.mjs
- components/zoom_admin/actions/get-meeting/get-meeting.mjs
- components/zoom_admin/actions/list-account-call-logs/list-account-call-logs.mjs
- components/zoom_admin/actions/list-meeting-registrants/list-meeting-registrants.mjs
- components/zoom_admin/actions/get-webinar/get-webinar.mjs
- components/zoom_admin/actions/get-meeting-recordings/get-meeting-recordings.mjs
- components/zoom_admin/actions/end-meeting/end-meeting.mjs
- components/zoom_admin/actions/delete-webinar-panelist/delete-webinar-panelist.mjs
- components/zoom_admin/actions/delete-meeting/delete-meeting.mjs
- components/zoom_admin/actions/delete-webinar/delete-webinar.mjs
- components/zoom_admin/actions/delete-cloud-recording/delete-cloud-recording.mjs
- components/zoom_admin/actions/create-webinar/create-webinar.mjs
- components/zoom_admin/actions/add-webinar-registrant/add-webinar-registrant.mjs
- components/zoom_admin/actions/create-meeting/create-meeting.mjs
- components/zoom_admin/actions/add-webinar-panelist/add-webinar-panelist.mjs
- components/zoom_admin/actions/add-meeting-registrant/add-meeting-registrant.mjs
Suggested quick checks:
- Confirm import { axios } from "@pipedream/platform" still works and its behavior/signature is unchanged in v3.1.0.
- Confirm DEFAULT_POLLING_SOURCE_TIMER_INTERVAL is still exported where used.
- Confirm
$.interface.apphook and $ .export semantics haven’t changed.- Verify zoom_admin.app.mjs _makeRequest remains compatible with the platform axios wrapper and that call sites still supply expected args/response shapes.
- Run unit/integration/smoke tests and scan the @pipedream/platform 3.1.0 changelog for breaking changes.
Original package.json snippet (unchanged):
"@pipedream/platform": "^3.1.0",
"lodash": "^4.17.21",
"uuid": "^8.3.2"components/zoom/sources/new-recording-transcript-completed/new-recording-transcript-completed.mjs (2)
59-64
: LGTM on event emission shape and dedupe idEmitting
{ meeting, file }
withid: file.id
aligns withdedupe: "unique"
and should prevent duplicates across deploy-time backfill and subsequent webhooks.
15-23
: Confirm mapping is correct — please verify Zoom webhook subscriptionconstants.CUSTOM_EVENT_TYPES.RECORDING_TRANSCRIPT_COMPLETED is defined as "recording.transcript_completed" and is used by the apphook and the run() guard, which matches Zoom’s official webhook name. Code is correct; however, confirm your Zoom app’s webhook subscription includes recording.transcript_completed (external config cannot be checked from the repo).
Files to note:
- components/zoom/sources/common/constants.mjs — RECORDING_TRANSCRIPT_COMPLETED: "recording.transcript_completed" (around line 37)
- components/zoom/sources/new-recording-transcript-completed/new-recording-transcript-completed.mjs — apphook.eventNames() includes the constant (lines ~18–22)
- components/zoom/sources/new-recording-transcript-completed/new-recording-transcript-completed.mjs — run() checks event.event against the same constant (line ~74)
components/zoom_admin/actions/get-meeting-transcript/get-meeting-transcript.mjs
Outdated
Show resolved
Hide resolved
...zoom_admin/sources/new-recording-transcript-completed/new-recording-transcript-completed.mjs
Outdated
Show resolved
Hide resolved
...zoom_admin/sources/new-recording-transcript-completed/new-recording-transcript-completed.mjs
Show resolved
Hide resolved
...zoom_admin/sources/new-recording-transcript-completed/new-recording-transcript-completed.mjs
Outdated
Show resolved
Hide resolved
...zoom_admin/sources/new-recording-transcript-completed/new-recording-transcript-completed.mjs
Show resolved
Hide resolved
components/zoom/actions/get-meeting-transcript/get-meeting-transcript.mjs
Show resolved
Hide resolved
...nents/zoom/sources/new-recording-transcript-completed/new-recording-transcript-completed.mjs
Show resolved
Hide resolved
...nents/zoom/sources/new-recording-transcript-completed/new-recording-transcript-completed.mjs
Show resolved
Hide resolved
...nents/zoom/sources/new-recording-transcript-completed/new-recording-transcript-completed.mjs
Show resolved
Hide resolved
...nents/zoom/sources/new-recording-transcript-completed/new-recording-transcript-completed.mjs
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.
Hi @michelle0927, LGTM! Ready for QA!
Resolves #18032
Summary by CodeRabbit
New Features
Chores