feat: add Telnyx SMS adapter (#7811)#108
feat: add Telnyx SMS adapter (#7811)#108deepshekhardas wants to merge 1 commit intoutopia-php:mainfrom
Conversation
WalkthroughThe Telnyx SMS adapter's constructor parameter Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/Messaging/Adapter/SMS/TelnyxTest.php`:
- Around line 16-22: The test fails because Telnyx is instantiated with too few
arguments and doesn't guard missing env vars; update the test in TelnyxTest to
first read and validate getenv('TELNYX_API_KEY'), getenv('TELNYX_TO') and
getenv('TELNYX_FROM') and call markTestSkipped (or return) if any are empty,
then construct the Telnyx sender passing the required from argument (e.g.,
include the from value when calling new Telnyx(...)) and create the SMS using
the validated env values before calling send.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ea64e157-ee53-4fa5-baaf-f297eaae6463
📒 Files selected for processing (2)
src/Utopia/Messaging/Adapter/SMS/Telnyx.phptests/Messaging/Adapter/SMS/TelnyxTest.php
| $sender = new Telnyx(\getenv('TELNYX_API_KEY')); | ||
|
|
||
| // $message = new SMS( | ||
| // to: ['+18034041123'], | ||
| // content: 'Test Content', | ||
| // from: '+15005550006' | ||
| // ); | ||
| $message = new SMS( | ||
| to: [\getenv('TELNYX_TO')], | ||
| content: 'Test Content', | ||
| from: \getenv('TELNYX_FROM') | ||
| ); |
There was a problem hiding this comment.
Pass the required Telnyx from argument and guard missing env vars.
This test currently instantiates Telnyx with too few arguments, which will fail before the send call runs. It also should skip cleanly when Telnyx env vars are not configured.
Proposed patch
public function testSendSMS(): void
{
- $sender = new Telnyx(\getenv('TELNYX_API_KEY'));
+ $apiKey = \getenv('TELNYX_API_KEY') ?: null;
+ $from = \getenv('TELNYX_FROM') ?: null;
+ $to = \getenv('TELNYX_TO') ?: null;
+
+ if (!$apiKey || !$from || !$to) {
+ $this->markTestSkipped('TELNYX_API_KEY, TELNYX_FROM, and TELNYX_TO are required for this integration test.');
+ }
+
+ $sender = new Telnyx($apiKey, $from);
$message = new SMS(
- to: [\getenv('TELNYX_TO')],
+ to: [$to],
content: 'Test Content',
- from: \getenv('TELNYX_FROM')
+ from: $from
);📝 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.
| $sender = new Telnyx(\getenv('TELNYX_API_KEY')); | |
| // $message = new SMS( | |
| // to: ['+18034041123'], | |
| // content: 'Test Content', | |
| // from: '+15005550006' | |
| // ); | |
| $message = new SMS( | |
| to: [\getenv('TELNYX_TO')], | |
| content: 'Test Content', | |
| from: \getenv('TELNYX_FROM') | |
| ); | |
| $apiKey = \getenv('TELNYX_API_KEY') ?: null; | |
| $from = \getenv('TELNYX_FROM') ?: null; | |
| $to = \getenv('TELNYX_TO') ?: null; | |
| if (!$apiKey || !$from || !$to) { | |
| $this->markTestSkipped('TELNYX_API_KEY, TELNYX_FROM, and TELNYX_TO are required for this integration test.'); | |
| } | |
| $sender = new Telnyx($apiKey, $from); | |
| $message = new SMS( | |
| to: [$to], | |
| content: 'Test Content', | |
| from: $from | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/Messaging/Adapter/SMS/TelnyxTest.php` around lines 16 - 22, The test
fails because Telnyx is instantiated with too few arguments and doesn't guard
missing env vars; update the test in TelnyxTest to first read and validate
getenv('TELNYX_API_KEY'), getenv('TELNYX_TO') and getenv('TELNYX_FROM') and call
markTestSkipped (or return) if any are empty, then construct the Telnyx sender
passing the required from argument (e.g., include the from value when calling
new Telnyx(...)) and create the SMS using the validated env values before
calling send.
1 participant
What does this PR do?
(Provide a description of what this PR does.)
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)
Summary by CodeRabbit
Bug Fixes
Tests