-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Fathom - new components #18766
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
Fathom - new components #18766
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughAdds a Fathom app with HTTP helpers and propDefinitions (recordingId, include flags), three actions (list meetings, get recording summary, get recording transcript), a reusable webhook base, a New Recording (Instant) source with sample/test event, and package dependency/version updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User Action
participant Action as Action Module
participant App as Fathom App
participant API as Fathom API
rect rgb(240,245,255)
note over User,API: Actions: listMeetings / getRecordingSummary / getRecordingTranscript
User->>Action: invoke action with props
Action->>App: call corresponding method (listMeetings|getRecordingSummary|getRecordingTranscript)
App->>App: _makeRequest(path, opts)
App->>API: HTTPS GET request
API-->>App: Response JSON
App-->>Action: return data
Action-->>User: $.export("$summary") and return response
end
sequenceDiagram
autonumber
participant Src as New Recording Source
participant Base as Base Webhook
participant App as Fathom App
participant API as Fathom API
participant PD as Pipedream Runtime
rect rgb(242,255,242)
note over Src,API: Activation
Src->>Base: activate()
Base->>Src: getWebhookData()
Base->>App: createWebhook({ destination_url, ... })
App->>API: POST /webhooks
API-->>App: { id }
App-->>Base: id
Base->>Base: _setWebhookId(id)
end
rect rgb(255,248,240)
note over API,PD: Event delivery
API-->>Src: POST webhook payload
Src->>Src: generateMeta(event)
Src->>PD: $emit(event, meta)
end
rect rgb(255,240,240)
note over Src,API: Deactivation
Src->>Base: deactivate()
Base->>Base: _getWebhookId()
Base->>App: deleteWebhook({ webhookId })
App->>API: DELETE /webhooks/{id}
API-->>App: 204
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ 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 |
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 (8)
components/fathom/actions/get-recording-summary/get-recording-summary.mjs(1 hunks)components/fathom/actions/get-recording-transcript/get-recording-transcript.mjs(1 hunks)components/fathom/actions/list-meetings/list-meetings.mjs(1 hunks)components/fathom/fathom.app.mjs(1 hunks)components/fathom/package.json(2 hunks)components/fathom/sources/common/base-webhook.mjs(1 hunks)components/fathom/sources/new-recording/new-recording.mjs(1 hunks)components/fathom/sources/new-recording/test-event.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-07-24T02:06:47.016Z
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.
Applied to files:
components/fathom/sources/common/base-webhook.mjs
🧬 Code graph analysis (5)
components/fathom/actions/get-recording-summary/get-recording-summary.mjs (2)
components/fathom/actions/get-recording-transcript/get-recording-transcript.mjs (1)
response(24-27)components/fathom/actions/list-meetings/list-meetings.mjs (1)
response(48-59)
components/fathom/sources/new-recording/new-recording.mjs (1)
components/fathom/sources/common/base-webhook.mjs (1)
event(78-78)
components/fathom/actions/get-recording-transcript/get-recording-transcript.mjs (2)
components/fathom/actions/get-recording-summary/get-recording-summary.mjs (1)
response(24-27)components/fathom/actions/list-meetings/list-meetings.mjs (1)
response(48-59)
components/fathom/actions/list-meetings/list-meetings.mjs (2)
components/fathom/actions/get-recording-summary/get-recording-summary.mjs (1)
response(24-27)components/fathom/actions/get-recording-transcript/get-recording-transcript.mjs (1)
response(24-27)
components/fathom/fathom.app.mjs (1)
components/fathom/sources/common/base-webhook.mjs (1)
webhookId(55-55)
⏰ 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 (16)
components/fathom/package.json (2)
3-3: LGTM! Version bump appropriate for new features.The version bump from 0.0.1 to 0.1.0 correctly reflects the addition of new actions and sources in this PR.
15-16: LGTM! Using the latest stable platform SDK.The addition of
@pipedream/platform^3.1.0 aligns with the latest stable release and enables the new HTTP helpers and typings used throughout the integration.components/fathom/actions/list-meetings/list-meetings.mjs (1)
1-46: LGTM! Props and metadata are well-structured.The action metadata, annotations, and defined props follow Pipedream conventions and align with the Fathom API requirements.
components/fathom/sources/new-recording/test-event.mjs (1)
1-55: LGTM! Representative test fixture.The test event provides a comprehensive sample payload with all key fields (action_items, calendar_invitees, summary, transcript, etc.), which will help developers understand the webhook payload structure.
components/fathom/actions/get-recording-summary/get-recording-summary.mjs (1)
1-31: LGTM! Action correctly implements recording summary retrieval.The action properly uses the
recordingIdprop definition and calls the corresponding app method. The metadata and annotations are appropriate for a read-only operation.components/fathom/fathom.app.mjs (4)
1-1: LGTM! Correct platform import.The axios import from
@pipedream/platformprovides typed HTTP capabilities consistent with the SDK.
6-56: LGTM! Well-structured prop definitions.The prop definitions follow Pipedream conventions:
recordingIduses async options with cursor-based pagination from thelistMeetingsAPI- Include flags are properly typed as optional booleans with clear descriptions
58-73: LGTM! Clean API request abstraction.The
_baseUrland_makeRequestmethods properly centralize HTTP logic and authorization header management, reducing duplication across API methods.
74-111: LGTM! API methods correctly implement Fathom endpoints.All API methods properly delegate to
_makeRequestwith correct paths and HTTP methods. The webhook and recording methods align with the Fathom API documentation.components/fathom/actions/get-recording-transcript/get-recording-transcript.mjs (1)
1-31: LGTM! Action correctly implements transcript retrieval.The action follows the same clean pattern as the recording summary action, properly using the
recordingIdprop definition and calling the corresponding app method.components/fathom/sources/common/base-webhook.mjs (4)
1-33: LGTM! Props correctly reference app definitions.The props properly import the include flags from the Fathom app's
propDefinitions, ensuring consistency across actions and sources.
34-62: LGTM! Webhook lifecycle properly managed.The activation hook validates configuration (at least one include flag) and creates the webhook, while deactivation safely cleans up. The webhook ID persistence using
dbfollows Pipedream patterns.
63-76: LGTM! Abstract base pattern correctly implemented.The abstract methods (
getWebhookData,generateMeta) properly throwConfigurationErrorto enforce implementation in subclasses, which aligns with established Pipedream patterns for base sources.Based on learnings.
77-87: LGTM! Event emission safely handles missing body.The run method checks for body existence before processing, preventing errors from malformed webhook requests.
components/fathom/sources/new-recording/new-recording.mjs (2)
1-25: LGTM! Source properly extends base webhook.The source correctly extends the common base and adds the
triggeredForprop with appropriate options that align with Fathom's webhook API.
26-46: LGTM! Abstract methods correctly implemented.Both
getWebhookDataandgenerateMetaproperly implement the abstract base methods:
getWebhookDatamaps UI props to the Fathom API webhook payload formatgenerateMetagenerates appropriate event metadata with unique ID and timestampThe inclusion of
sampleEmitprovides a test fixture for developers.
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.
I left comments that are optional suggestions. Moving this to QA
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
🧹 Nitpick comments (1)
components/fathom/fathom.app.mjs (1)
11-30: Consider adding defensive checks for API response.The async options function destructures
itemsandnext_cursordirectly from the API response. If the API returns an unexpected structure or encounters an error, this could fail at runtime.Apply this diff to add defensive handling:
async options({ prevContext }) { - const { - items, next_cursor: next, - } = await this.listMeetings({ + const response = await this.listMeetings({ params: { cursor: prevContext?.cursor, }, }); + const items = response?.items || []; + const next = response?.next_cursor; + return { options: items.map(({ recording_id: value, title: label, }) => ({ label, value, })), context: { cursor: next, }, }; },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
components/fathom/actions/list-meetings/list-meetings.mjs(1 hunks)components/fathom/fathom.app.mjs(1 hunks)components/fathom/sources/common/base-webhook.mjs(1 hunks)components/fathom/sources/new-recording/new-recording.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- components/fathom/sources/common/base-webhook.mjs
- components/fathom/sources/new-recording/new-recording.mjs
- components/fathom/actions/list-meetings/list-meetings.mjs
🧰 Additional context used
🧬 Code graph analysis (1)
components/fathom/fathom.app.mjs (1)
components/fathom/sources/common/base-webhook.mjs (1)
webhookId(33-33)
⏰ 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: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (1)
components/fathom/fathom.app.mjs (1)
1-101: LGTM! Clean implementation following Pipedream conventions.The app component is well-structured with:
- Proper authentication via OAuth access token
- Reusable HTTP helper pattern (
_makeRequest)- Comprehensive propDefinitions for UI components
- Clean API method wrappers for webhooks, meetings, and recordings
|
/approve |
Resolves #13473
Summary by CodeRabbit
New Features
Chores