[TRIGGER] New SMS in Aircall #17538
[TRIGGER] New SMS in Aircall #17538
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
|
Warning Rate limit exceeded@jcortes has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 5 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
WalkthroughA new "New SMS" trigger source was added for Aircall, emitting events when an SMS is received. The webhook ID assignment in the common webhook logic was updated, and version numbers were incremented in several Aircall-related files. No other functional changes were introduced. Changes
Sequence Diagram(s)sequenceDiagram
participant Aircall API
participant CommonWebhookSource
participant NewSMSSource
Aircall API-->>CommonWebhookSource: Sends "message.received" webhook
CommonWebhookSource->>NewSMSSource: Passes event data
NewSMSSource->>NewSMSSource: generateMeta(event)
NewSMSSource-->>CommonWebhookSource: Emits event with metadata
Assessment against linked issues
Poem
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
components/aircall/sources/new-sms/new-sms.mjs (2)
16-18: Confirm Aircall event type constant
message.receivedis correct for inbound SMS, but Aircall’s docs also emitmessage.sent. If you plan to cover both directions later, consider parameterising or renaming tosms-receivedfor clarity.
3-10: Minor metadata alignmentThe component’s
descriptionshould start with a verb per Pipedream guidelines (e.g., “Emit an event when…”).
Also bumping the package to0.2.0while the source stays0.0.1is fine, but remember to increment this source when behaviour changes.
📜 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 (6)
components/aircall/package.json(1 hunks)components/aircall/sources/common/common-webhook.mjs(1 hunks)components/aircall/sources/new-call-ended/new-call-ended.mjs(1 hunks)components/aircall/sources/new-note-added/new-note-added.mjs(1 hunks)components/aircall/sources/new-number-created/new-number-created.mjs(1 hunks)components/aircall/sources/new-sms/new-sms.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
components/aircall/sources/new-note-added/new-note-added.mjs (1)
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#15376
File: components/monday/sources/name-updated/name-updated.mjs:6-6
Timestamp: 2025-01-23T03:55:15.166Z
Learning: Source names in Monday.com components don't need to start with "New" if they emit events for updated items (e.g., "Name Updated", "Column Value Updated") rather than new items. This follows the component guidelines exception where the "New" prefix is only required when emits are limited to new items.
components/aircall/package.json (1)
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.
components/aircall/sources/new-number-created/new-number-created.mjs (1)
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#15376
File: components/monday/sources/name-updated/name-updated.mjs:6-6
Timestamp: 2025-01-23T03:55:15.166Z
Learning: Source names in Monday.com components don't need to start with "New" if they emit events for updated items (e.g., "Name Updated", "Column Value Updated") rather than new items. This follows the component guidelines exception where the "New" prefix is only required when emits are limited to new items.
components/aircall/sources/common/common-webhook.mjs (2)
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#12697
File: components/salesforce_rest_api/sources/common-webhook-methods.mjs:1-71
Timestamp: 2024-10-08T15:33:38.240Z
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.
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.
components/aircall/sources/new-sms/new-sms.mjs (3)
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#12697
File: components/salesforce_rest_api/sources/common-webhook-methods.mjs:1-71
Timestamp: 2024-10-08T15:33:38.240Z
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.
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.
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#15376
File: components/monday/sources/name-updated/name-updated.mjs:6-6
Timestamp: 2025-01-23T03:55:15.166Z
Learning: Source names in Monday.com components don't need to start with "New" if they emit events for updated items (e.g., "Name Updated", "Column Value Updated") rather than new items. This follows the component guidelines exception where the "New" prefix is only required when emits are limited to new items.
⏰ 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: Lint Code Base
- GitHub Check: pnpm publish
🔇 Additional comments (5)
components/aircall/package.json (1)
3-3: LGTM! Appropriate version bump for new features.The minor version bump from 0.1.1 to 0.2.0 correctly follows semantic versioning for the addition of new SMS trigger functionality.
components/aircall/sources/new-call-ended/new-call-ended.mjs (1)
8-8: LGTM! Appropriate patch version bump.The patch version increment from 0.0.2 to 0.0.3 is correct since no functional changes were made to this component.
components/aircall/sources/new-number-created/new-number-created.mjs (1)
8-8: LGTM! Appropriate patch version bump.The patch version increment from 0.0.2 to 0.0.3 is correct since no functional changes were made to this component.
components/aircall/sources/new-note-added/new-note-added.mjs (1)
8-8: LGTM! Appropriate patch version bump.The patch version increment from 0.0.2 to 0.0.3 is correct since no functional changes were made to this component.
components/aircall/sources/common/common-webhook.mjs (1)
24-24: LGTM – Webhook ID property updated consistently
- All instances of the old
webhook.idreference have been replaced withwebhook.webhook_id.- Verified in:
•components/aircall/sources/common/common-webhook.mjs(activate/deactivate flows)
•components/aircall/aircall.app.mjs(createWebhook/deleteWebhook)- No remaining mismatches detected; the change correctly aligns with the API wrapper’s response handling.
| generateMeta(data) { | ||
| return { | ||
| id: data.id, | ||
| summary: `New SMS received from ${data.raw_digits} to ${data.number.digits}`, | ||
| ts: data.created_at, | ||
| }; | ||
| }, |
There was a problem hiding this comment.
Harden generateMeta to avoid runtime crashes + ensure correct timestamp type
data.number can be null for ported numbers, and created_at is an ISO string.
Pipedream expects ts in milliseconds since epoch, so passing the raw ISO string may break sorting / dedupe. Defensive access & conversion fixes both issues:
- return {
- id: data.id,
- summary: `New SMS received from ${data.raw_digits} to ${data.number.digits}`,
- ts: data.created_at,
- };
+ return {
+ id: data.id,
+ summary: `New SMS from ${data.raw_digits}` +
+ (data.number?.digits ? ` to ${data.number.digits}` : ""),
+ ts: Date.parse(data.created_at),
+ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| generateMeta(data) { | |
| return { | |
| id: data.id, | |
| summary: `New SMS received from ${data.raw_digits} to ${data.number.digits}`, | |
| ts: data.created_at, | |
| }; | |
| }, | |
| generateMeta(data) { | |
| return { | |
| id: data.id, | |
| summary: `New SMS from ${data.raw_digits}` + | |
| (data.number?.digits ? ` to ${data.number.digits}` : ""), | |
| ts: Date.parse(data.created_at), | |
| }; | |
| }, |
🤖 Prompt for AI Agents
In components/aircall/sources/new-sms/new-sms.mjs around lines 19 to 25, the
generateMeta function assumes data.number is always an object and
data.created_at is already a timestamp, which can cause runtime errors and
incorrect timestamp handling. Update the code to safely access
data.number.digits only if data.number is not null, and convert data.created_at
from an ISO string to milliseconds since epoch before assigning it to ts. This
will prevent crashes and ensure the timestamp is in the expected format.
There was a problem hiding this comment.
I believe CodeRabbit's questioning here is correct - ts should be in milliseconds , please double check if the API returns created_at in milliseconds, or convert it otherwise
There was a problem hiding this comment.
🧩 Analysis chain
@GTFalcao, you're absolutely right to question this. Let me check the Aircall API documentation and examine how other Aircall components handle timestamps.
🏁 Script executed:
#!/bin/bash
# Check other Aircall components to see how they handle timestamps in generateMeta
find components/aircall -name "*.mjs" -type f | xargs grep -l "generateMeta" | head -5
Length of output: 389
🏁 Script executed:
#!/bin/bash
# Look at how other Aircall components handle timestamps
rg "generateMeta" components/aircall/ -A 10 -B 2
Length of output: 7038
🏁 Script executed:
#!/bin/bash
# Search for any references to created_at in Aircall components
rg "created_at" components/aircall/ -A 3 -B 3
Length of output: 1321
🌐 Web query:
Aircall API webhook payload format created_at timestamp format
💡 Result:
In Aircall's API, the created_at timestamp in webhook payloads is formatted as an ISO 8601 string, which includes the date and time in UTC. For example, a created_at value might look like "2020-02-18T20:52:22.000Z". (developer.aircall.io)
This format is consistent across various resources in Aircall's API, such as Users, Teams, Calls, and Numbers. For instance, when creating a new User, the response includes a created_at field with this timestamp format:
{
"user": {
"id": 458,
"direct_link": "https://api.aircall.io/v1/users/458",
"name": "Jeffrey Curtis",
"email": "jeffrey.curtis@aircall.io",
"available": false,
"availability_status": "available",
"created_at": "2020-02-18T20:52:22.000Z",
"time_zone": "Etc/UTC",
"language": "en-US",
"numbers": [],
"wrap_up_time": 0
}
}Similarly, when retrieving a Team, the created_at field is also in the ISO 8601 format:
{
"team": {
"id": 123,
"direct_link": "https://api.aircall.io/v1/teams/123",
"name": "Sales Team",
"created_at": "2020-02-18T20:52:22.000Z",
"users": [
{
"id": 456,
"direct_link": "https://api.aircall.io/v1/users/456",
"name": "John Doe",
"email": "john.doe@aircall.io",
"available": true,
"availability_status": "available",
"created_at": "2019-12-29T10:03:18.000Z",
"time_zone": "America/New_York",
"language": "en-US"
}
]
}
}This consistent timestamp format ensures uniformity across Aircall's API responses and webhook payloads.
Citations:
Perfect! I've confirmed that the conversion is absolutely needed. Here's what I found:
From the Aircall API documentation: The created_at field is returned as an ISO 8601 string (e.g., "2020-02-18T20:52:22.000Z"), not milliseconds.
From the existing codebase: I found that other Aircall components have the same issue:
new-number-created.mjshas a comment saying "Convert created_at timestamp to a Date object" but doesn't actually convert itnew-call-ended.mjsalso uses the raw timestamp without conversion- Only
new-note-added.mjsandnew-tag-added.mjsuseDate.now()(which is already in milliseconds)
The fix: CodeRabbit's suggestion to use Date.parse(data.created_at) is correct. This will convert the ISO string to milliseconds since epoch, which is what Pipedream expects for proper sorting and deduplication.
You're absolutely right to question this - the conversion is essential for the component to work correctly. The existing components (new-number-created and new-call-ended) likely have the same bug and should be fixed in a follow-up.
✏️ Learnings added
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#17538
File: components/aircall/sources/new-sms/new-sms.mjs:19-25
Timestamp: 2025-07-09T18:07:12.388Z
Learning: In Aircall API webhook payloads, the `created_at` field is returned as an ISO 8601 string format (e.g., "2020-02-18T20:52:22.000Z"), not as milliseconds since epoch. For Pipedream components, this needs to be converted to milliseconds using `Date.parse()` before assigning to the `ts` field in `generateMeta()`.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#12697
File: components/salesforce_rest_api/sources/common-webhook-methods.mjs:1-71
Timestamp: 2024-10-08T15:33:38.240Z
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.
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.
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#14265
File: components/the_magic_drip/sources/common.mjs:35-43
Timestamp: 2024-10-10T19:18:27.998Z
Learning: In `components/the_magic_drip/sources/common.mjs`, when processing items in `getAndProcessData`, `savedIds` is intentionally updated with IDs of both emitted and non-emitted items to avoid emitting retroactive events upon first deployment and ensure only new events are emitted as they occur.
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#15376
File: components/monday/sources/name-updated/name-updated.mjs:6-6
Timestamp: 2025-01-23T03:55:15.166Z
Learning: Source names in Monday.com components don't need to start with "New" if they emit events for updated items (e.g., "Name Updated", "Column Value Updated") rather than new items. This follows the component guidelines exception where the "New" prefix is only required when emits are limited to new items.
There was a problem hiding this comment.
Hi @GTFalcao I've already checked this and it is in miliseconds
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| generateMeta(data) { | ||
| return { | ||
| id: data.id, | ||
| summary: `New SMS received from ${data.raw_digits} to ${data.number.digits}`, | ||
| ts: data.created_at, | ||
| }; | ||
| }, |
There was a problem hiding this comment.
I believe CodeRabbit's questioning here is correct - ts should be in milliseconds , please double check if the API returns created_at in milliseconds, or convert it otherwise
jcortes
force-pushed
the
aircall-new-source-component
branch
2 times, most recently
from
6088a93 to
72fa924
Compare
jcortes
force-pushed
the
aircall-new-source-component
branch
from
72fa924 to
9d6b420
Compare
|
/approve |
github-project-automation
bot
moved this to Done
in Component (Source and Action) Backlog
WHY
Resolves #17285
Summary by CodeRabbit
New Features
Bug Fixes
Chores