Page MenuHomePhabricator

overwrite background notification priority anbd pushType properties so that they match apple docs
ClosedPublic

Authored by marcin on Jul 18 2022, 7:06 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Dec 25, 11:01 PM
Unknown Object (File)
Wed, Dec 25, 11:01 PM
Unknown Object (File)
Sat, Dec 21, 12:15 PM
Unknown Object (File)
Mon, Dec 16, 12:25 AM
Unknown Object (File)
Mon, Dec 16, 12:25 AM
Unknown Object (File)
Mon, Dec 16, 12:25 AM
Unknown Object (File)
Mon, Dec 16, 12:24 AM
Unknown Object (File)
Nov 16 2024, 9:41 AM

Details

Summary

This differential updates headers of APNs notification push request so that they are consitent with apple docs regarding background notifications. We set notification priority to 5, so that decision upon its delivery is dependent on power consumption and push-type to 'background' so that we do not risk notification being discarded by the APNs.

Test Plan

This differential does not introduce new functionality. We can open two clients - web wnd mobile - to ensure correct rescinding mechansim.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Thanks for making these changes! One question for you – do you think we need to specify notification.priority in D4113 as well? And does notification.mutableContent = true require a specific notification.pushType, similar to notification.contentAvailable?

This revision is now accepted and ready to land.Jul 18 2022, 8:17 AM

Thanks for making these changes! One question for you – do you think we need to specify notification.priority in D4113 as well?

Apple docs are strict about background notifications: https://developer.apple.com/documentation/usernotifications/setting_up_a_remote_notification_server/sending_notification_requests_to_apns?language=objc#2947607. Priority other than 5 is an error. When it comes to other notification types, priority 10 is a good choice if we expect immediate reaction from user. Otherwise notification delivery will be based on power consumption. Personally I would not change priority here. Apple is not strict about it and we used to send notifications with priority 10 for some time without any issues.

And does notification.mutableContent = true require a specific notification.pushType, similar to notification.contentAvailable?

Setting notification.mutableContent = true means notification is of type alert and is expected to be processed by NotificationService before being shown to user (https://developer.apple.com/documentation/usernotifications/unnotificationserviceextension?language=objc). If you look at the code, we already specify correct push type: https://github.com/CommE2E/comm/blob/master/keyserver/src/push/send.js#L492