Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 27 additions & 24 deletions src/Utopia/Messaging/Adapter/SMS/Telnyx.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,55 +11,58 @@ class Telnyx extends SMSAdapter
protected const NAME = 'Telnyx';

/**
* @param string $apiKey Telnyx APIv2 Key
* @param string $apiKey Telnyx API Key
* @param string $from Telnyx phone number or profile ID
*/
public function __construct(
private string $apiKey,
private ?string $from = null
private string $from,
) {
}

/**
* Get adapter name.
*/
public function getName(): string
{
return static::NAME;
}

/**
* Get max messages per request.
*/
public function getMaxMessagesPerRequest(): int
{
return 1;
}

/**
* {@inheritdoc}
*
* @throws \Exception
*/
protected function process(SMSMessage $message): array
{
$response = new Response($this->getType());

$result = $this->request(
method: 'POST',
url: 'https://api.telnyx.com/v2/messages',
headers: [
'Content-Type: application/json',
'Authorization: Bearer '.$this->apiKey,
],
body: [
'text' => $message->getContent(),
'from' => $this->from ?? $message->getFrom(),
'to' => $message->getTo()[0],
],
);
foreach ($message->getTo() as $to) {
$result = $this->request(
method: 'POST',
url: 'https://api.telnyx.com/v2/messages',
headers: [
'Content-Type: application/json',
"Authorization: Bearer {$this->apiKey}",
],
body: [
'from' => $this->from,
'to' => $to,
'text' => $message->getContent(),
]
);

if ($result['statusCode'] >= 200 && $result['statusCode'] < 300) {
$response->setDeliveredTo(\count($message->getTo()));
foreach ($message->getTo() as $to) {
if ($result['statusCode'] >= 200 && $result['statusCode'] < 300) {
$response->incrementDeliveredTo();
$response->addResult($to);
}
} else {
foreach ($message->getTo() as $to) {
$response->addResult($to, 'Unknown error.');
} else {
$response->addResult($to, $result['response']['errors'][0]['detail'] ?? 'Unknown error');
}
}
Comment on lines +46 to 67
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Since getMaxMessagesPerRequest() returns 1, the parent send() method enforces that process() is always called with exactly one recipient and throws an exception otherwise. The foreach loop will therefore always iterate exactly once, making it misleading and inconsistent with the Twilio adapter (which uses $message->getTo()[0] directly). Accessing the single recipient directly is clearer and aligns with how other single-recipient adapters in this codebase are implemented.

Suggested change
foreach ($message->getTo() as $to) {
$result = $this->request(
method: 'POST',
url: 'https://api.telnyx.com/v2/messages',
headers: [
'Content-Type: application/json',
"Authorization: Bearer {$this->apiKey}",
],
body: [
'from' => $this->from,
'to' => $to,
'text' => $message->getContent(),
]
);
if ($result['statusCode'] >= 200 && $result['statusCode'] < 300) {
$response->setDeliveredTo(\count($message->getTo()));
foreach ($message->getTo() as $to) {
if ($result['statusCode'] >= 200 && $result['statusCode'] < 300) {
$response->incrementDeliveredTo();
$response->addResult($to);
}
} else {
foreach ($message->getTo() as $to) {
$response->addResult($to, 'Unknown error.');
} else {
$response->addResult($to, $result['response']['errors'][0]['detail'] ?? 'Unknown error');
}
}
$to = $message->getTo()[0];
$result = $this->request(
method: 'POST',
url: 'https://api.telnyx.com/v2/messages',
headers: [
'Content-Type: application/json',
"Authorization: Bearer {$this->apiKey}",
],
body: [
'from' => $this->from,
'to' => $to,
'text' => $message->getContent(),
]
);
if ($result['statusCode'] >= 200 && $result['statusCode'] < 300) {
$response->setDeliveredTo(1);
$response->addResult($to);
} else {
$response->addResult($to, $result['response']['errors'][0]['detail'] ?? 'Unknown error');
}

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


Expand Down
20 changes: 10 additions & 10 deletions tests/Messaging/Adapter/SMS/TelnyxTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

namespace Utopia\Tests\Adapter\SMS;

use Utopia\Messaging\Adapter\SMS\Telnyx;
use Utopia\Messaging\Messages\SMS;
use Utopia\Tests\Adapter\Base;

class TelnyxTest extends Base
Expand All @@ -11,18 +13,16 @@ class TelnyxTest extends Base
*/
public function testSendSMS(): void
{
// $sender = new Telnyx(\getenv('TELNYX_API_KEY'));
$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')
);
Comment on lines +16 to +22
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

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.

Suggested change
$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.

Comment on lines +16 to +22
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0 The Telnyx constructor now requires two arguments ($apiKey and $from), but only one is passed here. This will throw a PHP fatal error — "Too few arguments to function Telnyx::__construct(), 1 passed and exactly 2 expected" — making the entire test suite fail immediately. The TELNYX_FROM env var is already used for the SMS message, but needs to be passed to the adapter constructor as well.

Suggested change
$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')
);
$sender = new Telnyx(\getenv('TELNYX_API_KEY'), \getenv('TELNYX_FROM'));
$message = new SMS(
to: [\getenv('TELNYX_TO')],
content: 'Test Content',
);


// $result = $sender->send($message);
$result = $sender->send($message);

// $this->assertEquals('success', $result["type"]);

$this->markTestSkipped('Telnyx had no testing numbers available at this time.');
$this->assertResponse($result);
}
}