Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Priority settings missing? #48

Closed
lukasza67 opened this issue Apr 14, 2021 · 4 comments
Closed

Priority settings missing? #48

lukasza67 opened this issue Apr 14, 2021 · 4 comments
Labels
documentation enhancement New feature or request

Comments

@lukasza67
Copy link

Hello and thanks for a great work!

I have been going through the documentation and it seams that there is no way of setting the message priority for the notification. As it is now it sends with no settings and it gets "normal" priority by default at FCM-level. Or did I miss to set it manually somewhere?

I think this is a very important feature as on battery saving mode or locked screen the notification is now delayed until the user becomes active with the display.

@lukasza67
Copy link
Author

lukasza67 commented Apr 14, 2021

So I've solved the issue by doing the following quick fix:

In the end of Push/Notification->getBody() I simply added:

$request['priority'] = "high";

Now; you might want to consider extend this within your management getters/settes to make this a real feature.

This made my notifications go in warp speed no matter of the devices state (I use cordova).

image

Also for housekeeping purposes 😃 the naming of function getBody() in this class is contradictory as it clashes with setBody(..) and it is not exactly the same type of "body" these two are playing around with (I managed to figure this out when I tried manually to build the full payload array body but the setter function just manages the message body).

My Regards to all for the nice work with the API once again!

@AndyGaskell
Copy link
Collaborator

Hi @lukasza67
That's really useful to know, thanks, I didn't realised normal priority push messages were treated that way. I'll have a look at writing a getter and setter for this.
Re getBody v setBody, I see what you mean, if we can't fix this in a non breaking way we can adjust the docs to better explain it.

@AndyGaskell AndyGaskell added documentation enhancement New feature or request labels Apr 26, 2021
@Philipp91
Copy link
Contributor

Re getBody v setBody, I see what you mean, if we can't fix this in a non breaking way we can adjust the docs to better explain it.

How about renaming getBody() to buildJsonRequest() and leaving behind a deprecated getBody() that just redirects and will be removed in the next major release?

@Philipp91
Copy link
Contributor

I'd also need the priority. I believe I could put together a PR if that helps.

Philipp91 added a commit to Philipp91/php-fcm that referenced this issue Nov 27, 2022
As discussed on EdwinHoksberg#48. This is a user-visible breaking API change, though 
most users probably won't care, as they use FcmClient::send() instead.

Also document the NotificationException thrown from there, which existed 
before but just wasn't documented (not a breaking change).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants