Skip to content

Conversation

@michelle0927
Copy link
Collaborator

@michelle0927 michelle0927 commented Nov 20, 2025

Resolves #17800

Summary by CodeRabbit

  • New Features
    • Added three new SMS sending actions: Send SMS, Send Registered SMS, and Send Registered SMS Contract
    • Support for sending messages to multiple phone numbers in international format
    • Email delivery receipt notifications for registered SMS actions to track message state changes

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 20, 2025

Walkthrough

This PR introduces three new SMS action modules (send-sms, send-registered-sms, send-registered-sms-contract) to the sms_messages component. The app module is enhanced with propDefinitions for message, numbers, and email inputs; a getUser() method; and a sendSMS() method for API communication. Package version is bumped to 0.1.0 with a new @pipedream/platform dependency.

Changes

Cohort / File(s) Summary
New SMS Action Modules
components/sms_messages/actions/send-sms/send-sms.mjs, components/sms_messages/actions/send-registered-sms/send-registered-sms.mjs, components/sms_messages/actions/send-registered-sms-contract/send-registered-sms-contract.mjs
Three new action definitions added with consistent structure. Each exports a default action object with metadata (key, name, description, version, type, annotations) and props schema referencing smsMessages, message, numbers, and optionally email. The run method invokes this.smsMessages.sendSMS with the user, destination numbers, message text, and delivery receipt configuration (cert_type differs: "T" for contract, "D" for registered, omitted for basic). All export a success summary.
App Configuration
components/sms_messages/sms_messages.app.mjs
Added propDefinitions for message (string), numbers (string[]), and email (string) fields with labels and descriptions. Replaced authKeys() with getUser() method returning this.$auth.username. Introduced sendSMS({ $ = this, data = {} }) method that sends POST requests to https://api.lleida.net/sms/v2/ with structured JSON payload and x-api-key authorization header. Added axios import from @pipedream/platform.
Package Metadata
components/sms_messages/package.json
Version bumped from 0.0.1 to 0.1.0. Added dependencies section with "@pipedream/platform": "^3.1.1" under publishConfig.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

  • The three action modules follow a highly repetitive pattern with minimal variations (only cert_type configuration and optional email parameter differ), reducing per-file complexity.
  • App modifications are straightforward additions of propDefinitions, method implementations, and a dependency import without intricate logic.
  • No complex control flow or architectural changes; primarily configuration and boilerplate action definitions.
  • Areas to note during review:
    • Verify that the cert_type values ("T", "D", and absent) are correct for each action variant
    • Confirm the sendSMS payload structure aligns with the SMS Messages API specification at https://api.lleida.net/dtd/sms/v2/en/
    • Ensure propDefinitions are correctly wired in all three action modules

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'SMS Messages - new components' clearly summarizes the main change: introducing new SMS Messages components.
Description check ✅ Passed The description is minimal but directly references issue #17800, which provides necessary context and objectives for the pull request.
Linked Issues check ✅ Passed All three required actions from issue #17800 are implemented: send-sms, send-registered-sms, and send-registered-sms-contract with proper SMS API integration.
Out of Scope Changes check ✅ Passed All changes directly support the linked issue objectives: new action modules, supporting app methods, prop definitions, and version bump for SMS Messages integration.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch issue-17800

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6f9a2f5 and 3c1e0d7.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (5)
  • components/sms_messages/actions/send-registered-sms-contract/send-registered-sms-contract.mjs (1 hunks)
  • components/sms_messages/actions/send-registered-sms/send-registered-sms.mjs (1 hunks)
  • components/sms_messages/actions/send-sms/send-sms.mjs (1 hunks)
  • components/sms_messages/package.json (2 hunks)
  • components/sms_messages/sms_messages.app.mjs (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-08T15:33:38.240Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 12731
File: components/hackerone/actions/get-members/get-members.mjs:3-28
Timestamp: 2024-10-08T15:33:38.240Z
Learning: When exporting a summary message in the `run` method of an action, ensure the message is correctly formatted. For example, in the `hackerone-get-members` action, the correct format is `Successfully retrieved ${response.data.length} members`.

Applied to files:

  • components/sms_messages/actions/send-registered-sms/send-registered-sms.mjs
  • components/sms_messages/actions/send-sms/send-sms.mjs
  • components/sms_messages/actions/send-registered-sms-contract/send-registered-sms-contract.mjs
📚 Learning: 2024-12-12T19:23:09.039Z
Learnt from: jcortes
Repo: PipedreamHQ/pipedream PR: 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/sms_messages/package.json
🧬 Code graph analysis (3)
components/sms_messages/actions/send-registered-sms/send-registered-sms.mjs (2)
components/sms_messages/actions/send-registered-sms-contract/send-registered-sms-contract.mjs (1)
  • response (36-51)
components/sms_messages/actions/send-sms/send-sms.mjs (1)
  • response (30-41)
components/sms_messages/actions/send-sms/send-sms.mjs (2)
components/sms_messages/actions/send-registered-sms-contract/send-registered-sms-contract.mjs (1)
  • response (36-51)
components/sms_messages/actions/send-registered-sms/send-registered-sms.mjs (1)
  • response (36-51)
components/sms_messages/actions/send-registered-sms-contract/send-registered-sms-contract.mjs (2)
components/sms_messages/actions/send-registered-sms/send-registered-sms.mjs (1)
  • response (36-51)
components/sms_messages/actions/send-sms/send-sms.mjs (1)
  • response (30-41)
⏰ 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 (6)
components/sms_messages/package.json (1)

3-3: LGTM! Version bump and dependency addition are appropriate.

The version bump from 0.0.1 to 0.1.0 correctly reflects the addition of new features (three new SMS actions), and the @pipedream/platform dependency is properly declared to support the axios import in the app module.

Also applies to: 15-17

components/sms_messages/actions/send-registered-sms-contract/send-registered-sms-contract.mjs (1)

45-48: Verify cert_type "T" is correct for SMS contracts.

This action uses cert_type: "T" for registered SMS contracts, while the send-registered-sms action uses "D". Ensure this distinction is correct per the API documentation—the difference between these certificate types and when to use each should be clearly understood.

(Note: This can be verified with the same web search query from the send-registered-sms review regarding valid cert_type values)

components/sms_messages/sms_messages.app.mjs (2)

33-36: The Authorization header format is correct—no action needed.

The Lleida SMS API v2 requires the HTTP Authorization header with the literal scheme "x-api-key" followed by a space and the key: Authorization: x-api-key YOUR_API_KEY. The code already implements this exact format correctly:

"Authorization": `x-api-key ${this.$auth.api_key}`

When combined with the header name, this produces the required format. The original review comment incorrectly flagged this as problematic.

Likely an incorrect or invalid review comment.


12-15: Fix payload transformation for multiple recipients in send-sms action.

The Lleida SMS API v2 dst.num field accepts one phone number per element and requires repeating the dst.num field for multiple recipients. However, the action at components/sms_messages/actions/send-sms/send-sms.mjs line 33 passes the entire numbers array directly to dst.num:

dst: {
  num: this.numbers,
},

This will fail the API request. Instead, transform the numbers array into the proper format the Lleida API expects—either as an array of objects with individual num fields or by structuring dst appropriately for multiple recipients.

⛔ Skipped due to learnings
Learnt from: js07
Repo: PipedreamHQ/pipedream PR: 18744
File: components/slack_v2/actions/send-large-message/send-large-message.mjs:49-64
Timestamp: 2025-10-20T01:01:02.970Z
Learning: In components/slack_v2/actions/send-large-message/send-large-message.mjs, the metadata_event_payload prop is typed as string, so the code only needs to handle string-to-JSON parsing and does not need to handle object inputs.
components/sms_messages/actions/send-sms/send-sms.mjs (1)

29-41: The implementation is correct—no changes needed.

The authorization header format x-api-key YOUR_API_KEY and the JSON payload structure with the outer "sms" object, user, dst.num, and txt fields are both correct per the Lleida API v2 specification. The code snippet at lines 29-41 properly constructs the payload matching the documented structure.

components/sms_messages/actions/send-registered-sms/send-registered-sms.mjs (1)

45-48: Unable to verify cert_type values without access to Lleida API documentation.

The codebase shows a consistent pattern where registered SMS uses cert_type: "D" and registered SMS contract uses cert_type: "T", but public API documentation and internal code comments do not explain what these certificate type values mean or whether they are correct. Manual verification against official Lleida SMS API v2 documentation is required to confirm the cert_type values match the API specification.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@vercel
Copy link

vercel bot commented Nov 20, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
pipedream-docs Ignored Ignored Nov 20, 2025 10:44pm
pipedream-docs-redirect-do-not-edit Ignored Ignored Nov 20, 2025 10:44pm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Components] sms_messages

2 participants