-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New Components - rinkel #18175
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
New Components - rinkel #18175
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughAdds Rinkel integration features: new webhook source for call-ended, base webhook, and multiple actions (get call details, get call recording, get voicemail, add note, update note). Updates app module with HTTP client, propDefinitions, and API methods. Bumps package version and adds @pipedream/platform dependency. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant R as Rinkel API
participant S as Base Webhook Source
participant U as User Source (Call Ended)
rect rgba(230,240,255,0.5)
note right of S: Activation
U->>S: activate()
S->>R: POST /webhooks {event: getEvent(), url, active:true}
R-->>S: 201 Created
end
rect rgba(240,230,255,0.5)
note right of S: Incoming Event
R-->>S: POST callEnd payload
S->>S: generateMeta(event)
S-->>U: $emit(body, meta)
end
rect rgba(255,240,230,0.5)
note right of S: Deactivation
U->>S: deactivate()
S->>R: DELETE /webhooks {event: getEvent()}
R-->>S: 204 No Content
end
sequenceDiagram
autonumber
participant A as Action (Add/Update Note)
participant App as Rinkel App Client
participant API as Rinkel API
A->>App: addNote / updateNote(params)
App->>API: PATCH /calls/{id}/notes (or /calls/{callId}/notes/{noteId})
API-->>App: 200 OK (note data)
App-->>A: response
A-->>A: $.export("$summary", ...)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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: 0
🧹 Nitpick comments (12)
components/rinkel/sources/common/base-webhook.mjs (1)
10-24: Webhook subscribe/unsubscribe flow aligns with Rinkel APIPOST /webhooks/:event with url/contentType/active, and DELETE /webhooks/:event are the documented endpoints and event names (incomingCall, outgoingCall, callStart, callEnd, callInsights). This matches your activate/deactivate logic. Consider optionally sending a description to help users identify the subscription in Rinkel. (developers.rinkel.com)
Proposed tweak (optional):
await this.rinkel.createWebhook({ event: this.getEvent(), data: { url: this.http.endpoint, contentType: "application/json", active: true, + description: "Pipedream webhook", }, });components/rinkel/rinkel.app.mjs (5)
7-21: callId options: pagination params OK; consider larger page sizeSorting by date DESC matches the API defaults. You can reduce UI pagination by requesting more items per page. (developers.rinkel.com)
const response = await this.listCallDetailRecords({ params: { page: page + 1, + perPage: 50, sort: "date", sortOrder: "DESC", }, });
22-36: recordingId options: OK for “Recordings” (not “Call Recordings”)This lists reusable “recordings” (greetings, MOH, etc.), which is consistent with GET /recordings. If the intent is “call recordings,” this is a different resource and selection flow. See comments on the action file. (developers.rinkel.com)
Also consider the same perPage optimization:
const response = await this.listCallRecordings({ params: { page: page + 1, + perPage: 50, sort: "date", sortOrder: "DESC", }, });
37-51: voicemailId options: undocumented “sort” paramThe List voicemail records endpoint documents sortOrder but not sort. Recommend dropping sort to avoid potential 400s on stricter validation. (developers.rinkel.com)
const response = await this.listVoicemails({ params: { page: page + 1, - sort: "date", sortOrder: "DESC", }, });
52-66: noteId options: guard for calls without notesSome CDRs may have no notes; add a safe default before mapping. (developers.rinkel.com)
- const notes = response.data.notes; - return notes.map((note) => ({ + const notes = response.data?.notes ?? []; + return notes.map((note) => ({ label: note.content, value: note.id, }));
109-116: Method name suggests “call recording” but hits the “recordings” resourceGET /recordings/:id retrieves a reusable Recording (greetings/MOH), not a call recording. Either rename to getRecording (and keep a compatibility alias), or change the implementation and the prop to target call recordings. Right now the naming may confuse users and action authors. (developers.rinkel.com)
Non-breaking improvement:
getCallRecording({ id, ...opts }) { return this._makeRequest({ path: `/recordings/${id}`, ...opts, }); }, + // Alias for clarity: "Recording" refers to reusable assets (not call recordings) + getRecording({ id, ...opts }) { + return this._makeRequest({ + path: `/recordings/${id}`, + ...opts, + }); + },components/rinkel/actions/get-call-recording/get-call-recording.mjs (1)
4-16: Action name/docs don’t match the resource you’re callingThis action is titled “Get Call Recording” but calls GET /recordings/:id (a non-call “Recording”). Either:
- Rename the action to “Get Recording” (and keep the same implementation), or
- Switch to the Call Recording resource (requires a different prop and endpoint such as /call-recordings/:id/stream or using playUrl from a Call Detail Record). (developers.rinkel.com)
Minimal rename option:
- key: "rinkel-get-call-recording", - name: "Get Call Recording", - description: "Get a call recording. [See the documentation](https://developers.rinkel.com/docs/api/get-a-recording)", + key: "rinkel-get-recording", + name: "Get Recording", + description: "Get a recording. [See the documentation](https://developers.rinkel.com/docs/api/get-a-recording)",If you actually want call recordings, I can propose a revised action that selects a Call Detail Record, extracts callRecording.id, and fetches the audio URL.
components/rinkel/actions/get-voicemail/get-voicemail.mjs (2)
23-23: Nit: clarify the summary to reflect an expiring URLSmall UX win: communicate that the URL is temporary (3 hours), mirroring the docs. (developers.rinkel.com)
- $.export("$summary", `Voicemail ${this.voicemailId} retrieved`); + $.export("$summary", `Temporary URL (3h) generated for voicemail ${this.voicemailId}`);
9-17: Optional: expose thelanguagequery paramVerified that
getVoicemailincomponents/rinkel/rinkel.app.mjsdestructures...opts, so it forwards arbitrary options (includingparams) to the HTTP client. To let users request a localized stream/download URL, you can add alanguageprop incomponents/rinkel/actions/get-voicemail/get-voicemail.mjs:props: { rinkel, voicemailId: { propDefinition: [ rinkel, "voicemailId", ], }, + language: { + type: "string", + label: "Language", + description: "Optional language code for the returned URL (defaults to user language).", + optional: true, + }, }, async run({ $ }) { const response = await this.rinkel.getVoicemail({ $, id: this.voicemailId, + params: this.language ? { language: this.language } : undefined, }); $.export("$summary", `Voicemail ${this.voicemailId} retrieved`); return response; },This enhancement is fully compatible with the existing method signature and is purely optional.
components/rinkel/actions/add-note/add-note.mjs (1)
17-21: Optional: trim content before sendingAvoids storing accidental leading/trailing whitespace.
async run({ $ }) { const response = await this.rinkel.addNote({ $, id: this.callId, data: { - content: this.content, + content: this.content?.trim(), }, });Also applies to: 24-30
components/rinkel/actions/update-note/update-note.mjs (1)
26-30: Optional: trim content before updateSame rationale as “Add Note”; avoids noisy diffs in downstream systems.
content: { type: "string", label: "Content", description: "The content of the note", }, ... data: { - content: this.content, + content: this.content?.trim(), },Also applies to: 37-39
components/rinkel/sources/call-ended/call-ended.mjs (1)
17-23: Optional: guard against invaliddatetimeHighly unlikely here, but if
datetimeis ever missing or malformed,Date.parsereturnsNaN. You can fall back toDate.now()to keep emission robust.generateMeta(event) { - return { - id: event.id, - summary: `Call ended: ${event.cause}`, - ts: Date.parse(event.datetime), - }; + const tsParsed = Date.parse(event.datetime); + return { + id: event.id, + summary: `Call ended: ${event.cause}`, + ts: Number.isFinite(tsParsed) ? tsParsed : Date.now(), + }; },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
components/rinkel/actions/add-note/add-note.mjs(1 hunks)components/rinkel/actions/get-call-details/get-call-details.mjs(1 hunks)components/rinkel/actions/get-call-recording/get-call-recording.mjs(1 hunks)components/rinkel/actions/get-voicemail/get-voicemail.mjs(1 hunks)components/rinkel/actions/update-note/update-note.mjs(1 hunks)components/rinkel/package.json(2 hunks)components/rinkel/rinkel.app.mjs(1 hunks)components/rinkel/sources/call-ended/call-ended.mjs(1 hunks)components/rinkel/sources/call-ended/test-event.mjs(1 hunks)components/rinkel/sources/common/base-webhook.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 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/rinkel/sources/common/base-webhook.mjs
📚 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/rinkel/package.json
🧬 Code graph analysis (7)
components/rinkel/actions/get-call-details/get-call-details.mjs (3)
components/rinkel/actions/get-call-recording/get-call-recording.mjs (1)
response(19-22)components/rinkel/actions/get-voicemail/get-voicemail.mjs (1)
response(19-22)components/rinkel/rinkel.app.mjs (4)
response(12-18)response(27-33)response(42-48)response(57-59)
components/rinkel/sources/call-ended/call-ended.mjs (1)
components/rinkel/sources/common/base-webhook.mjs (1)
event(35-35)
components/rinkel/actions/add-note/add-note.mjs (1)
components/rinkel/actions/update-note/update-note.mjs (1)
response(33-40)
components/rinkel/actions/get-call-recording/get-call-recording.mjs (4)
components/rinkel/actions/add-note/add-note.mjs (1)
response(24-30)components/rinkel/actions/get-call-details/get-call-details.mjs (1)
response(19-22)components/rinkel/actions/get-voicemail/get-voicemail.mjs (1)
response(19-22)components/rinkel/rinkel.app.mjs (4)
response(12-18)response(27-33)response(42-48)response(57-59)
components/rinkel/actions/update-note/update-note.mjs (5)
components/rinkel/actions/add-note/add-note.mjs (1)
response(24-30)components/rinkel/actions/get-call-details/get-call-details.mjs (1)
response(19-22)components/rinkel/actions/get-call-recording/get-call-recording.mjs (1)
response(19-22)components/rinkel/actions/get-voicemail/get-voicemail.mjs (1)
response(19-22)components/rinkel/rinkel.app.mjs (4)
response(12-18)response(27-33)response(42-48)response(57-59)
components/rinkel/actions/get-voicemail/get-voicemail.mjs (5)
components/rinkel/actions/add-note/add-note.mjs (1)
response(24-30)components/rinkel/actions/get-call-details/get-call-details.mjs (1)
response(19-22)components/rinkel/actions/get-call-recording/get-call-recording.mjs (1)
response(19-22)components/rinkel/actions/update-note/update-note.mjs (1)
response(33-40)components/rinkel/rinkel.app.mjs (4)
response(12-18)response(27-33)response(42-48)response(57-59)
components/rinkel/rinkel.app.mjs (5)
components/rinkel/actions/add-note/add-note.mjs (1)
response(24-30)components/rinkel/actions/get-call-details/get-call-details.mjs (1)
response(19-22)components/rinkel/actions/get-call-recording/get-call-recording.mjs (1)
response(19-22)components/rinkel/actions/get-voicemail/get-voicemail.mjs (1)
response(19-22)components/rinkel/actions/update-note/update-note.mjs (1)
response(33-40)
⏰ 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: Verify TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
🔇 Additional comments (15)
components/rinkel/package.json (2)
15-17: Dependency addition looks right for axios usage via the Platform SDKAdding @pipedream/platform is the correct way to use axios in components. No built-in Node modules are added, so we’re good w.r.t. prior guidance.
3-3: Version bump matches a feature additionMinor bump to 0.1.0 makes sense given the new app methods, actions, and sources.
components/rinkel/sources/common/base-webhook.mjs (2)
27-33: Abstract-method pattern is appropriateThrowing ConfigurationError for unimplemented methods is the right contract for subclasses.
34-41: Run handler is minimal and correctEarly return on missing body and emitting with meta is fine for Pipedream HTTP sources. No changes needed.
components/rinkel/rinkel.app.mjs (3)
72-82: Auth header is correctx-rinkel-api-key is the documented authentication header. Nice. (developers.rinkel.com)
143-160: Notes endpoints look correctPUT /call-detail-records/:id/note and PATCH /call-detail-records/:id/note/:noteId match the docs. (developers.rinkel.com)
117-124: Voicemail endpoint likely incorrect (returns audio stream, not JSON)You’re calling /voicemails/:id/stream, which streams audio. Typical actions want a temporary URL (JSON) from GET /voicemails/:id. Also, if you really want to stream, axios should set responseType: "stream"/"arraybuffer". As written, this may fail or return binary unexpectedly. Recommend switching to GET /voicemails/:id. (developers.rinkel.com)
- getVoicemail({ - id, ...opts - }) { - return this._makeRequest({ - path: `/voicemails/${id}/stream`, - ...opts, - }); - }, + getVoicemail({ id, ...opts }) { + // Returns a temporary URL to stream/download the voicemail (JSON) + return this._makeRequest({ + path: `/voicemails/${id}`, + method: "GET", + ...opts, + }); + },Likely an incorrect or invalid review comment.
components/rinkel/actions/get-call-recording/get-call-recording.mjs (1)
18-25: Run logic is fine given the current implementationGiven the endpoint returns JSON metadata for recordings, exporting a concise summary and returning the response is sufficient.
components/rinkel/actions/get-call-details/get-call-details.mjs (2)
3-16: Props are correctly wired to the app’s callId selectorUsing the shared propDefinition keeps things consistent across actions.
18-25: Endpoint and summary align with docsGET /call-detail-records/:id is the correct endpoint; the user-facing summary is clear. (developers.rinkel.com)
components/rinkel/actions/get-voicemail/get-voicemail.mjs (1)
18-24: Good: action correctly proxies the Voicemail “temporary URL” endpointThe param shape and behavior match the Rinkel docs (path param id of the voicemail, returns a temporary URL that expires in ~3 hours). Looks consistent with the app wrapper pattern used elsewhere. (developers.rinkel.com)
components/rinkel/actions/add-note/add-note.mjs (1)
23-32: Good: conforms to “Add note to CDR” APIUsing
idfor the Call Detail Record andcontentin the body matches the API. This action follows the same pattern as the other Rinkel actions in this PR. (developers.rinkel.com)components/rinkel/actions/update-note/update-note.mjs (1)
32-41: Good: uses correct identifiers for update — callId + noteIdMatches the API route semantics for updating a note on a CDR. The cross-field dependency in the
noteIdpropDefinition is a nice touch for DX. (developers.rinkel.com)components/rinkel/sources/call-ended/test-event.mjs (1)
1-5: LGTM: sample payload matches docs for the “call end” webhookFields
id,datetime, andcausealign with the documented payload for call end events. (developers.rinkel.com)components/rinkel/sources/call-ended/call-ended.mjs (1)
12-16: Correct event name:callEndThe event identifier matches the official list of webhook events used when subscribing via API. (developers.rinkel.com)
GTFalcao
left a 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.
LGTM!
Resolves #17891
Summary by CodeRabbit