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

Mark iOS push notifications with push_type = alert to allow for on device silencing #305

Closed
wants to merge 1 commit into from
Closed

Mark iOS push notifications with push_type = alert to allow for on device silencing #305

wants to merge 1 commit into from

Conversation

stefanceriu
Copy link
Member

We were recently granted the com.apple.developer.usernotifications.filtering entitlement which allows push notifications to be completely discarded once received on the device. This is useful for hiding notifications from encrypted events we don't care about like poll answers (as opposed to showing an empty notification).

One caveat is that it only works for alert apns-push-type notifications which is an optional field we're not currently setting.

This PR does just that through the use of aioapns's PushType header.

@stefanceriu stefanceriu requested a review from a team as a code owner June 9, 2022 14:03
@clokep
Copy link
Member

clokep commented Jun 9, 2022

What happens if an app isn't approved for this and we set this push type?

@stefanceriu
Copy link
Member Author

Nothing changes as far as I can tell, alert is the correct one for notifications that trigger a user interaction—for example, an alert, badge, or sound.

https://developer.apple.com/documentation/usernotifications/setting_up_a_remote_notification_server/sending_notification_requests_to_apns/

@clokep
Copy link
Member

clokep commented Jun 9, 2022

Thanks! Mostly wanted to make sure it wouldn't reject the push completely.

@@ -230,6 +230,7 @@ async def _dispatch_request(
message=shaved_payload,
priority=prio,
notification_id=notif_id,
push_type=PushType.ALERT,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The apple docs imply that if we use the alert push type, we have to use the non-voip topic:
https://developer.apple.com/documentation/usernotifications/setting_up_a_remote_notification_server/sending_notification_requests_to_apns/

alert
Use the alert push type for notifications that trigger a user interaction—for example, an alert, badge, or sound. If you set this push type, the apns-topic header field must use your app’s bundle ID as the topic. For more information, see Generating a remote notification.

voip
[...] If you set this push type, the apns-topic header field must use your app’s bundle ID with .voip appended to the end.

However it seems that we configure sygnal to use both voip and non-voip topics:

    - name: im.vector.app.ios.prod
      [...]
      topic: im.vector.app

    - name: im.vector.app.ios.dev
      platform: sandbox
      [...]
      topic: im.vector.app

    - name: im.vector.app.ios.voip.prod
      [...]
      topic: im.vector.app.voip

    - name: im.vector.app.ios.voip.dev
      platform: sandbox
      [...]
      topic: im.vector.app.voip

I think it's best that we make a push_type config option that defaults to None.
We could then roll it out to dev builds first, before turning it on for production builds.

@stefanceriu
Copy link
Member Author

This PR is incomplete and would break voip UX. Raised separate ticket here #308

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for on device notification filtering
3 participants