Skip to content

feat: add Vonage Messages API adapter - #127

Open
deepshekhardas wants to merge 2 commits into
utopia-php:mainfrom
deepshekhardas:fix/111-vonage-messages-api
Open

feat: add Vonage Messages API adapter#127
deepshekhardas wants to merge 2 commits into
utopia-php:mainfrom
deepshekhardas:fix/111-vonage-messages-api

Conversation

@deepshekhardas

Copy link
Copy Markdown

Port of PR #111 by bhardwajparth51.

Renames the existing Vonage adapter to VonageLegacy and adds a new VonageMessages adapter using the modern Vonage Messages API (V1) which is more cost-effective and versatile.

Changes:

  • Rename Vonage -> VonageLegacy (backward compat note)
  • Add VonageMessages adapter using Messages API v1
  • Add corresponding tests

@greptile-apps

greptile-apps Bot commented Jun 14, 2026

Copy link
Copy Markdown

Greptile Summary

This PR renames the existing Vonage SMS adapter to VonageLegacy and introduces a new VonageMessages adapter targeting the modern Vonage Messages API v1 with Basic auth. The request body shape and HTTP response handling look correct against the API spec.

  • Both VonageMessages and the updated VonageLegacy are missing a parent::__construct() call. Adapter::__construct() initializes the typed Counter $sendCounter property; skipping it leaves the property uninitialized, causing a fatal Error on every send() via recordResponse()recordSend(). Every other adapter in the codebase calls parent::__construct() — this must be fixed in both files before merging.
  • The VonageMessagesTest unconditionally calls markTestSkipped, with the real assertions buried in a block comment rather than behind an env-var guard, so the test never executes even when credentials are available.

Confidence Score: 3/5

Not safe to merge — both new and updated adapters will throw a fatal PHP error on every send call due to an uninitialized typed property.

Both VonageMessages and VonageLegacy are missing parent::__construct(), which means Adapter::$sendCounter is never initialized. Since send() unconditionally calls recordResponse()recordSend()$this->sendCounter->add(...) after every message, every real use of either adapter will crash with a fatal Error.

Files Needing Attention: Both src/Utopia/Messaging/Adapter/SMS/VonageMessages.php and src/Utopia/Messaging/Adapter/SMS/VonageLegacy.php need parent::__construct() added back to their constructors.

Important Files Changed

Filename Overview
src/Utopia/Messaging/Adapter/SMS/VonageMessages.php New Vonage Messages API adapter — missing parent::__construct() causes a fatal Error on every send() call due to uninitialized $sendCounter
src/Utopia/Messaging/Adapter/SMS/VonageLegacy.php Renamed from Vonage.php; parent::__construct() was inadvertently removed, breaking telemetry counter initialization and causing fatal errors on send
tests/Messaging/Adapter/SMS/VonageLegacyTest.php Renamed from VonageTest.php; test body is still fully skipped via markTestSkipped with real assertions in a block comment (pre-existing pattern)
tests/Messaging/Adapter/SMS/VonageMessagesTest.php New test file; entire test body is dead code — markTestSkipped fires unconditionally with real assertions trapped in a block comment

Reviews (5): Last reviewed commit: "chore: remove old Vonage files after ren..." | Re-trigger Greptile

use Utopia\Messaging\Response;

class Vonage extends SMSAdapter
class VonageLegacy extends SMSAdapter

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Breaking rename with no backward-compatibility alias

Renaming VonageVonageLegacy drops the original class name entirely. Any downstream code using new \Utopia\Messaging\Adapter\SMS\Vonage(...) or use Utopia\Messaging\Adapter\SMS\Vonage will throw a fatal Class not found error after upgrading this library. The PR description mentions a "backward compat note" but no alias or deprecation shim is present in the diff. Consider adding class_alias(VonageLegacy::class, 'Utopia\\Messaging\\Adapter\\SMS\\Vonage'); at the bottom of this file (or in a dedicated compat file), or coordinate a semver-major release.

Comment on lines +14 to +35
public function testSendSMS(): void
{
$this->markTestSkipped('Vonage Messages credentials are not available.');

/*
$apiKey = \getenv('VONAGE_MESSAGES_API_KEY');
$apiSecret = \getenv('VONAGE_MESSAGES_API_SECRET');

$sender = new VonageMessages($apiKey, $apiSecret);

$message = new SMS(
to: [\getenv('VONAGE_MESSAGES_TO')],
content: 'Test Content',
from: \getenv('VONAGE_MESSAGES_FROM'),
);

$response = $sender->send($message);

$this->assertNotEmpty($response['results']);
$this->assertNotEmpty($response['results'][0]['success']);
*/
}

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 Entire test body is dead code

The test immediately calls markTestSkipped and the real assertions are in a block comment rather than an if (! $creds) { markTestSkipped(); } guard pattern. Tests for other adapters (e.g., VonageLegacyTest) use live credentials gated by env vars. If VONAGE_MESSAGES_API_KEY etc. are simply not available in CI, the existing skip-unless-env-var pattern should be used here too, so the test actually executes when credentials are configured.

@deepshekhardas

Copy link
Copy Markdown
Author

Following up - this PR has been waiting for review. Happy to address any feedback or make changes.

deepshekhardas added 2 commits August 4, 2026 09:31
Renames the existing Vonage adapter to VonageLegacy and adds a new VonageMessages adapter using the modern Vonage Messages API (V1). Based on PR utopia-php#111 by bhardwajparth51.
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.

1 participant