Skip to content

feat: add Vonage Messages API adapter#111

Open
bhardwajparth51 wants to merge 9 commits intoutopia-php:mainfrom
bhardwajparth51:feat/vonage-messages-adapter
Open

feat: add Vonage Messages API adapter#111
bhardwajparth51 wants to merge 9 commits intoutopia-php:mainfrom
bhardwajparth51:feat/vonage-messages-adapter

Conversation

@bhardwajparth51
Copy link
Copy Markdown

@bhardwajparth51 bhardwajparth51 commented Apr 1, 2026

What does this PR do?

Adds a new SMS adapter for the Vonage Messages API (v1). This adapter is preferred over the legacy SMS API as it is more cost-effective and built on Vonage's modern messaging infrastructure.

Key details of the implementation:

  • Uses Basic authentication (Base64 encoded API Key and Secret) for the Messages API, matching Vonage's requirements for this endpoint.
  • Strictly validates responses against the 202 Accepted status code, which is the exclusive success response for the Messages API.
  • Implements a shared VonageMessagesBase trait containing the common API logic, allowing future channel adapters (like WhatsApp or Viber) to easily reuse it.

Test Plan

  1. Static Analysis: Verified type safety and syntax using vendor/bin/phpstan analyse --level=6 src tests.
  2. Integration Test: Added tests/Messaging/Adapter/SMS/VonageMessagesTest.php, adhering to the repository's convention of environment-gated integration testing using the assertResponse() request-catcher.
  3. Operational Verification: Validated the request Authorization header generation, JSON payload composition, and fallbacks to guarantee the adapter correctly structures API requests.

Related PRs and Issues

Closes #82

Have you read the Contributing Guidelines on issues?

Yes.

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 1, 2026

Greptile Summary

This PR adds a new VonageMessages SMS adapter targeting the Vonage Messages API v1 with Basic auth, JSON payloads, and a 202-only success check. Both previously flagged concerns — the missing VONAGE_TO guard in tests and the null from producing an invalid JSON body — have been addressed: the test now skips cleanly when any of the three env vars are absent, and process() returns an early error result when $from is empty before the API call is made.

Confidence Score: 5/5

Safe to merge — all previously identified P1 issues have been resolved and no new defects were found.

Both critical issues (null from serialized as JSON null, missing VONAGE_TO skip guard) from prior review rounds are now handled correctly. The implementation is consistent with other SMS adapters in the repo, uses the correct Vonage Messages API endpoint and auth scheme, and the error-extraction logic covers detail, title, and raw-string error shapes.

No files require special attention.

Important Files Changed

Filename Overview
src/Utopia/Messaging/Adapter/SMS/VonageMessages.php New SMS adapter for Vonage Messages API v1 with Basic auth, JSON body, and explicit guard for empty from; logic is correct and consistent with other adapters in the repo.
tests/Messaging/Adapter/SMS/VonageMessagesTest.php Integration tests guard all three env vars (VONAGE_API_KEY, VONAGE_API_SECRET, VONAGE_TO) and handle optional VONAGE_FROM correctly in both test cases.

Reviews (8): Last reviewed commit: "fix: address review feedback and harden ..." | Re-trigger Greptile

@bhardwajparth51
Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 1, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 1, 2026

Walkthrough

Adds a new SMS adapter Utopia\Messaging\Adapter\SMS\VonageMessages that sends SMS via the Vonage Messages API (v1). The adapter accepts apiKey, apiSecret, and optional default from, limits requests to one message per call, normalizes destination numbers, and treats HTTP 202 as delivered; other responses are reported as failures with error extraction. A shared trait Utopia\Messaging\Adapter\VonageMessagesBase supplies the API endpoint and request/authorization header helpers. Two PHPUnit tests exercise send behavior with and without a fallback from.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and clearly summarizes the main change: adding a new Vonage Messages API adapter for SMS messaging.
Linked Issues check ✅ Passed The PR fully addresses all coding requirements from issue #82: creates VonageMessagesBase trait [#82], implements VonageMessages SMS adapter [#82], and includes proper integration tests.
Out of Scope Changes check ✅ Passed All changes are directly scoped to issue #82 requirements: new VonageMessagesBase trait, VonageMessages SMS adapter, and corresponding integration tests with no extraneous modifications.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description check ✅ Passed The pull request description clearly explains the motivation, implementation details, and test plan for adding a new Vonage Messages API SMS adapter.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
tests/Messaging/Adapter/SMS/VonageMessagesTest.php (1)

30-34: Inconsistent handling of VONAGE_FROM between constructor and message.

The constructor fallback (line 27) uses 'Vonage' when VONAGE_FROM is unset, but the message's from (line 33) receives false (from getenv() returning false). This creates an inconsistent test scenario where the adapter's from differs from the message's from.

If the intent is to test the adapter's constructor from taking precedence, the message's from could be explicitly set to null or omitted. If the intent is to pass matching values, both should use the same fallback.

♻️ Suggested clarification
         $message = new SMS(
             to: [$to],
             content: 'Test Content',
-            from: \getenv('VONAGE_FROM')
+            from: \getenv('VONAGE_FROM') ?: null
         );

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e6eed56d-cd95-4c22-9ff3-18c201ec0c9c

📥 Commits

Reviewing files that changed from the base of the PR and between fcb4c3c and 6710210.

📒 Files selected for processing (3)
  • src/Utopia/Messaging/Adapter/SMS/VonageMessages.php
  • src/Utopia/Messaging/Adapter/VonageMessagesBase.php
  • tests/Messaging/Adapter/SMS/VonageMessagesTest.php

@bhardwajparth51
Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 1, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@bhardwajparth51
Copy link
Copy Markdown
Author

@stnguyen90 Ready for review! let me know if it need any additional changes.

@bhardwajparth51
Copy link
Copy Markdown
Author

i think the CI failures are all pre-existing they're coming from expired or missing github secrets for Twilio, Resend, Sendgrid, Mailgun, Fast2SMS, Inforu, Discord, APNS, and FCM. My Vonage tests are correctly skipping since the VONAGE_* secrets aren't configured in CI yet.

@bhardwajparth51
Copy link
Copy Markdown
Author

@ChiragAgg5k can you review it

@bhardwajparth51
Copy link
Copy Markdown
Author

bhardwajparth51 commented Apr 8, 2026

I went ahead and set up a Vonage developer account to run the integration tests locally against the live API. Everything works perfectly!
image
Screenshot_20260409_012650.jpg

Copy link
Copy Markdown
Member

@ChiragAgg5k ChiragAgg5k left a comment

Choose a reason for hiding this comment

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

Image

lets avoid a base class, not sure why we would need one

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 10, 2026

Tip:

Greploop — Automatically fix all review issues by running /greploops in Claude Code. It iterates: fix, push, re-review, repeat until 5/5 confidence.

Use the Greptile plugin for Claude Code to query reviews, search comments, and manage custom context directly from your terminal.

@bhardwajparth51 bhardwajparth51 force-pushed the feat/vonage-messages-adapter branch from c5a14c0 to 815454f Compare April 10, 2026 16:09
@ChiragAgg5k
Copy link
Copy Markdown
Member

@bhardwajparth51 lets fix the static analysis. also i propose we can make this a breaking change and mark the older adapter as legacy, perhaps VonageLegacy? since this one is clearly better and recommended to use

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.

🚀 Feature: Vonage Messages Adapter

2 participants