feat: add Vonage multi-channel Messages adapter (#8139)#110
feat: add Vonage multi-channel Messages adapter (#8139)#110deepshekhardas wants to merge 1 commit into
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✨ 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 Tip You can generate walkthrough in a markdown collapsible section to save space.Enable the |
deepshekhardas
force-pushed
the
feat/8139-vonage-messages-adapter
branch
from
ba22934 to
d623ac7
Compare
|
Rebased onto latest main. Could you please review? 🙏 |
Greptile SummaryThis PR introduces Vonage multi-channel messaging support (SMS via the Messages API, WhatsApp, Viber, and MMS) by extracting shared JWT authentication and HTTP request logic into a
Confidence Score: 2/5Not safe to merge — the primary new adapter ( The missing trait import in
Important Files Changed
Reviews (1): Last reviewed commit: "feat: add Vonage multi-channel Messages ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| use Utopia\Messaging\Adapter\SMS as SMSAdapter; | ||
| use Utopia\Messaging\Helpers\JWT; | ||
| use Utopia\Messaging\Messages\SMS as SMSMessage; | ||
| use Utopia\Messaging\Response; |
There was a problem hiding this comment.
VonageTrait is never imported in this file. Because VonageMessages lives in Utopia\Messaging\Adapter\SMS, PHP will look for Utopia\Messaging\Adapter\SMS\VonageTrait when it encounters use VonageTrait; — a class that doesn't exist — causing a fatal "Class not found" error at runtime. Additionally, JWT and Response are imported here but never used directly in this class (they are consumed by the trait), making those imports both redundant and misleading.
| use Utopia\Messaging\Adapter\SMS as SMSAdapter; | |
| use Utopia\Messaging\Helpers\JWT; | |
| use Utopia\Messaging\Messages\SMS as SMSMessage; | |
| use Utopia\Messaging\Response; | |
| use Utopia\Messaging\Adapter\SMS as SMSAdapter; | |
| use Utopia\Messaging\Adapter\VonageTrait; | |
| use Utopia\Messaging\Messages\SMS as SMSMessage; |
|
|
||
| // This assumes the mock server returns a success response | ||
| // In a real environment, we'd check the request-catcher data | ||
| $this->assertNotEmpty($result); | ||
| } | ||
|
|
||
| public function testSendWhatsApp(): void | ||
| { | ||
| $sender = new VonageWhatsApp($this->applicationId, $this->privateKey); |
There was a problem hiding this comment.
setEndpoint() is undefined on these adapters. None of VonageMessages, VonageWhatsApp, VonageViber, or VonageMMS define a setEndpoint() method — it only exists on Mock. Every test method in this class will crash with "Call to undefined method" before reaching any assertions. The same issue occurs in all four test methods (testSendSMS, testSendWhatsApp, testSendViber, testSendMMS). Either add setEndpoint() / getEndpoint() to VonageTrait, or use an environment variable / constructor parameter to override the base URL.
| public function testSendWhatsApp(): void | ||
| { | ||
| $sender = new VonageWhatsApp($this->applicationId, $this->privateKey); |
There was a problem hiding this comment.
json_decode receives an array, not a string. send() returns array, so \json_decode($response, true) is called with an array argument. In PHP 8 this causes a TypeError; in earlier versions it silently returns null, making assertNotEmpty($result) fail. The same pattern is repeated in all four test methods. Either call send() directly and use assertResponse() (already defined in Base), or cast to JSON before decoding.
| $jwt = JWT::encode( | ||
| [ | ||
| 'application_id' => $this->applicationId, | ||
| 'iat' => \time(), | ||
| 'jti' => \bin2hex(\random_bytes(16)), | ||
| 'exp' => \time() + 3600, | ||
| ], |
There was a problem hiding this comment.
iat and exp call ime() separately. In the unlikely but possible case where the system clock advances between the two calls, exp will differ from iat + 3600 by a second. Capture the timestamp once to guarantee consistency.
| $jwt = JWT::encode( | |
| [ | |
| 'application_id' => $this->applicationId, | |
| 'iat' => \time(), | |
| 'jti' => \bin2hex(\random_bytes(16)), | |
| 'exp' => \time() + 3600, | |
| ], | |
| $now = \time(); | |
| $jwt = JWT::encode( | |
| [ | |
| 'application_id' => $this->applicationId, | |
| 'iat' => $now, | |
| 'jti' => \bin2hex(\random_bytes(16)), | |
| 'exp' => $now + 3600, | |
| ], |
1 participant
Adds a Vonage multi-channel Messages adapter supporting SMS, WhatsApp, Viber, and MMS.
Changes
Testing
Closes #8139