[Notifier] Add Expo bridge #42414
[Notifier] Add Expo bridge #42414
Conversation
|
Hey! To help keep things organized, we don't allow "Draft" pull requests. Could you please click the "ready for review" button or close this PR and open a new one when you are done? Note that a pull request does not have to be "perfect" or "ready for merge" when you first open it. We just want it to be ready for a first review. Cheers! Carsonbot |
854617f to
383b6da
Compare
|
Maybe push channel #39402 should be merged before? :) |
d3edebf to
71f895b
Compare
71f895b to
6f68c0c
Compare
0eab128 to
fbf4bd2
Compare
| $jsonContents = 0 === strpos($contentType, 'application/json') ? $response->toArray(false) : null; | ||
|
|
||
| if (200 !== $statusCode) { | ||
| $errorMessage = $jsonContents ? $jsonContents['error']['message'] : $response->getContent(false); |
There was a problem hiding this comment.
what if $jsonContents['error']['message'] does not exist?
There was a problem hiding this comment.
if i understand well the official documentation https://docs.expo.dev/accounts/programmatic-access/#personal-account-personal-access-tokens
normally when the status code is not OK we are in error mode for expo and we should have the error payload as explained in the article.
There was a problem hiding this comment.
Done, you are right I spent some time testing the possibles cases, I found out that when it's 401 the response is different, I updated this part of handling the exception.
src/Symfony/Component/Notifier/Bridge/Expo/ExpoTransportFactory.php
Outdated
Show resolved
Hide resolved
addb8fa to
a15efa8
Compare
a15efa8 to
391e241
Compare
|
|
||
| throw new TransportException('Unable to post the Expo message: '.$errorMessage, $response); | ||
| } | ||
| if (isset($jsonContents['error']['message'])) { |
There was a problem hiding this comment.
Is it possible to get a 200 with errors in the response? If not, the 1st check is enough.
There was a problem hiding this comment.
you are right I did revisit the API documentation again, I will delete this check the first one is enough. good catch
cc39c71 to
1ba96a7
Compare
|
Now that the push channel feature has been merged (see #39402), I think this PR can be finished. @zairigimad Do you have time to update the code? |
Yes, I will take some time today to update the code. |
cd64196 to
ed8f2fa
Compare
|
@fabpot I updated the code with the new PushChannel. |
| @@ -0,0 +1,22 @@ | |||
| Expo Notifier | |||
| ================= | |||
There was a problem hiding this comment.
| ================= | |
| ============= |
|
Thank you @zairigimad. |
|
@zairigimad Can you create a PR on symfony/recipes like done for other notifiers? |
Yes i will check how it’s done and do a PR asap |
Expo makes implementing push notifications almost too easy. All the hassle with native device information and communicating with APNs (Apple Push Notification service) or FCM (Firebase Cloud Messaging) is taken care of behind the scenes, so that you can treat iOS and Android notifications the same, saving you time on the front-end, and back-end!
this PR will add the support of Expo Notification as a bridge to Symfony
screenshot from a real application build with expo
✏️ this is a work in progress I would love to hear your feedbacks to improve it.