fix: rebase PR #104 - Taqnyat SMS adapter - #139
Conversation
Original by @ali-alharthi. Fixed: - Added parent::__construct() call - Made properties private readonly - Aligned with v2.0.0 conventions
|
Thanks for contributing! This repository is a read-only mirror; development for this library happens in |
Greptile SummaryAdds a Taqnyat SMS provider adapter and a live-credential integration test.
Confidence Score: 4/5The PR appears safe to merge, with the non-blocking concern that normal CI does not exercise the new adapter. The adapter follows existing loading, request serialization, and response conventions, but its only test is skipped whenever live Taqnyat credentials are unavailable. tests/Messaging/Adapter/SMS/TaqnyatTest.php Important Files Changed
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
tests/Messaging/Adapter/SMS/TaqnyatTest.php:16-18
**Live credentials leave adapter untested**
The sole Taqnyat test skips when live credentials are unavailable, so normal CI never exercises recipient normalization, JSON payload construction, or success and error response handling. Add deterministic client-backed tests for these paths so adapter regressions are detected without contacting the provider.
Reviews (1): Last reviewed commit: "fix: rebase PR #104 - Taqnyat SMS adapte..." | Re-trigger Greptile |
|
|
||
| if (!$apiKey || !$senderId || !$to) { | ||
| $this->markTestSkipped('TAQNYAT credentials not configured'); |
There was a problem hiding this comment.
The sole Taqnyat test skips when live credentials are unavailable, so normal CI never exercises recipient normalization, JSON payload construction, or success and error response handling. Add deterministic client-backed tests for these paths so adapter regressions are detected without contacting the provider.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/Messaging/Adapter/SMS/TaqnyatTest.php
Line: 16-18
Comment:
**Live credentials leave adapter untested**
The sole Taqnyat test skips when live credentials are unavailable, so normal CI never exercises recipient normalization, JSON payload construction, or success and error response handling. Add deterministic client-backed tests for these paths so adapter regressions are detected without contacting the provider.
How can I resolve this? If you propose a fix, please make it concise.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!
Replacement for #104. Original by @ali-alharthi.
Rebased on latest main. Fixed: