WalletConnect / a2

An Asynchronous Apple Push Notification (apns2) Client for Rust
MIT License
136 stars 47 forks source link

Default apns_priority doesn't match expectation #40

Closed pheki closed 4 years ago

pheki commented 4 years ago

First of all, thanks for this awesome library!

The default value for apns_priority in NotificationOptions is Normal (5), which is usually not delivered if the app is on background (I was actually unable to see it being delivered). Apple specifies in https://developer.apple.com/documentation/usernotifications/setting_up_a_remote_notification_server/sending_notification_requests_to_apns that the default value for the header is 10, if omitted, which makes it the expected behaviour, also making most notification tutorials omit the priority header.

I propose to make a breaking change and make apns_priority an Option inside NotificationOptions, just like every other header there.

Alternatives:

I'm willing send a PR to help.

What do you think?

(unrelated: I've got access to apple devices if you need any help)

pimeys commented 4 years ago

Hey. I could make you an admin if you own apple devices and can actually test these!

pheki commented 4 years ago

I can test, but I'm kind of limited to iOS 9 (and rarely a borrowed iOS 11) as I'm stuck on MacOS High Sierra (and XCode) for hardware support reasons. I don't think that's a problem though.

What do you think about making apns_priority in NotificationOptions an Option in 0.5?

pimeys commented 4 years ago

Well we anyhow break the interfaces when going async/await. Want to do a pr?

pheki commented 4 years ago

Did it :)