[Notifier] [Smsapi] fixed checking whether message is sent #42053
Conversation
|
Hey! I see that this is your first PR. That is great! Welcome! Symfony has a contribution guide which I suggest you to read. In short:
Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change. When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor! I am going to sit back now and wait for the reviews. Cheers! Carsonbot |
src/Symfony/Component/Notifier/Bridge/Smsapi/SmsapiTransport.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Notifier/Bridge/Smsapi/Tests/SmsapiTransportTest.php
Show resolved
Hide resolved
|
|
||
| throw new TransportException(sprintf('Unable to send the SMS: "%s".', $error['message']), $response); | ||
| if ((isset($responseContent['error']) && null !== $responseContent['error']) || (200 !== $response->getStatusCode())) { |
There was a problem hiding this comment.
isset($foo['bar']) returns also true when the key exists but it's value is null.
You can remove the second assertion
There was a problem hiding this comment.
I need to confirm if the value is not null. Checking only if the key exists is not sufficient
src/Symfony/Component/Notifier/Bridge/Smsapi/Tests/SmsapiTransportTest.php
Show resolved
Hide resolved
|
I would really appreciate if you finish this PR and apply the comments. Thanks |
I'll finish the PR this weekend |
|
Thank you @damlox. |
SmsAPI always return 200 status code even if we enter incorrect token.
The content is:
{"error":101,"message":"Authorization failed"}.