Feat 7714 adding semaphore messaging adapter - #144
Conversation
|
Thanks for contributing! This repository is a read-only mirror; development for this library happens in |
Greptile SummaryAdds a Semaphore SMS provider integration.
Confidence Score: 2/5This PR is not safe to merge until Semaphore correctly handles all accepted recipients, reports successful delivery counts, preserves request failures, and safely parses validation errors. The new adapter silently drops recipients after the first, returns deliveredTo as zero on success, produces an empty response for common HTTP and transport failures, and reads a missing numeric index from documented validation-error responses. Files Needing Attention: src/Utopia/Messaging/Adapter/SMS/Semaphore.php Important Files Changed
Prompt To Fix All With AIFix the following 4 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 4
src/Utopia/Messaging/Adapter/SMS/Semaphore.php:51
**Bulk sends drop recipients**
When a message contains 2–1000 recipients, the adapter accepts the message but submits and records only `getTo()[0]`, causing every remaining recipient to be silently omitted from delivery and results.
### Issue 2 of 4
src/Utopia/Messaging/Adapter/SMS/Semaphore.php:59
**Successful delivery count stays zero**
When Semaphore returns a successful response, this branch adds a result without updating `deliveredTo`, causing successful sends to report zero deliveries and fail the shared adapter response assertion.
### Issue 3 of 4
src/Utopia/Messaging/Adapter/SMS/Semaphore.php:70-73
**Non-500 failures lose results**
When Semaphore returns an authentication, validation, rate-limit, service, or transport failure with a status other than 500, neither branch adds a result, causing the adapter to return an empty result set with no recipient or error details.
### Issue 4 of 4
src/Utopia/Messaging/Adapter/SMS/Semaphore.php:58
**Validation errors access missing index**
When Semaphore returns its documented 200 validation-error object keyed by fields such as `sendername`, this condition reads the nonexistent numeric key `0`, causing an undefined-array-key warning before error handling continues.
Reviews (1): Last reviewed commit: "Added: Semaphore SMS adapter" | Re-trigger Greptile |
| ], | ||
| body: [ | ||
| 'apikey' => $this->apikey, | ||
| 'number' => $message->getTo()[0], |
There was a problem hiding this comment.
When a message contains 2–1000 recipients, the adapter accepts the message but submits and records only getTo()[0], causing every remaining recipient to be silently omitted from delivery and results.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/Utopia/Messaging/Adapter/SMS/Semaphore.php
Line: 51
Comment:
**Bulk sends drop recipients**
When a message contains 2–1000 recipients, the adapter accepts the message but submits and records only `getTo()[0]`, causing every remaining recipient to be silently omitted from delivery and results.
How can I resolve this? If you propose a fix, please make it concise.
|
|
||
| if ($result['statusCode'] === 200) { | ||
| if ($result['response'][0] && count($result['response'][0]) > 1) { | ||
| $response->addResult($message->getTo()[0]); |
There was a problem hiding this comment.
When Semaphore returns a successful response, this branch adds a result without updating deliveredTo, causing successful sends to report zero deliveries and fail the shared adapter response assertion.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/Utopia/Messaging/Adapter/SMS/Semaphore.php
Line: 59
Comment:
**Successful delivery count stays zero**
When Semaphore returns a successful response, this branch adds a result without updating `deliveredTo`, causing successful sends to report zero deliveries and fail the shared adapter response assertion.
How can I resolve this? If you propose a fix, please make it concise.| } | ||
| if ($result['statusCode'] === 500) { | ||
| $response->addResult($message->getTo()[0], $result['response'][0]); | ||
| } |
There was a problem hiding this comment.
When Semaphore returns an authentication, validation, rate-limit, service, or transport failure with a status other than 500, neither branch adds a result, causing the adapter to return an empty result set with no recipient or error details.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/Utopia/Messaging/Adapter/SMS/Semaphore.php
Line: 70-73
Comment:
**Non-500 failures lose results**
When Semaphore returns an authentication, validation, rate-limit, service, or transport failure with a status other than 500, neither branch adds a result, causing the adapter to return an empty result set with no recipient or error details.
How can I resolve this? If you propose a fix, please make it concise.| ); | ||
|
|
||
| if ($result['statusCode'] === 200) { | ||
| if ($result['response'][0] && count($result['response'][0]) > 1) { |
There was a problem hiding this comment.
When Semaphore returns its documented 200 validation-error object keyed by fields such as sendername, this condition reads the nonexistent numeric key 0, causing an undefined-array-key warning before error handling continues.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/Utopia/Messaging/Adapter/SMS/Semaphore.php
Line: 58
Comment:
**Validation errors access missing index**
When Semaphore returns its documented 200 validation-error object keyed by fields such as `sendername`, this condition reads the nonexistent numeric key `0`, causing an undefined-array-key warning before error handling continues.
How can I resolve this? If you propose a fix, please make it concise.
Rebased on latest main with conflict resolution.
Changes:
Conflicts resolved: