[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 |
|
Maybe push channel #39402 should be merged before? :) |
| $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
|
|
||
| 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
|
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. |
|
@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 |
8 participants
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.