-
-
src/Lunr/Vortex/FCM/FCMDispatcher.php (Diff revision 1) This payload object need to be in a message object
[Vortex] FCM: Update general payload for new firebase v1 API
Review Request #1089 — Created Jan. 11, 2024 and submitted
Information | |
---|---|
b.stoop | |
Lunr.Vortex | |
fcm/updatepayload | |
|
|
1116, 1090 | |
f58793b... | |
Reviewers | |
lunr | |
Based on master
FCM: Update general payload for new firebase v1 API
https://firebase.google.com/docs/cloud-messaging/migrate-v1?hl=en&authuser=0#update-the-payload-of-send-requests, The payload needed some updates for this to work with the new firebase API.
Also referenced https://firebase.google.com/docs/reference/fcm/rest/v1/projects.messages.
Github Actions run: https://github.com/brianstoop/lunr.vortex/actions/runs/7843990001
Testing Done: |
|
||||||
---|---|---|---|---|---|---|---|
Commit: |
|
||||||
Diff: |
Revision 2 (+423 -80) |
Change Summary:
Rebased on master
Description: |
|
|||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
|||||||||||||||||||||
Commit: |
|
|||||||||||||||||||||
Diff: |
Revision 3 (+423 -80) |
-
-
src/Lunr/Vortex/FCM/FCMPayload.php (Diff revision 3) I'm not sure if we should be setting this here, since that is also reported as the default
-
src/Lunr/Vortex/FCM/FCMAndroidPriority.php (Diff revision 3) Let's make it a proper enum too if we rename it
Change Summary:
Fix issues
Testing Done: |
|
||||||
---|---|---|---|---|---|---|---|
Commit: |
|
||||||
Diff: |
Revision 4 (+418 -84) |
-
-
I'm against introducing platform specific overrides in the first version of the review. They can come in later as separate reviews, but the initial review should just contain the generic stuff.
Platform specific overrides would need to go into platform specific payloads. We would then move away from
fcm-data
andfcm-notification
to, I guess,fcm-android
,fcm-apns
andfcm-webpush
. -
-
src/Lunr/Vortex/FCM/FCMPayload.php (Diff revision 4) If you allow a
false
return, you need to add a test that checks that scenario.
Summary: |
|
|||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Description: |
|
|||||||||||||||||||||
Testing Done: |
|
|||||||||||||||||||||
Commit: |
|
|||||||||||||||||||||
Diff: |
Revision 5 (+125 -301) |