From 06790b9085e9cb14e53871ed5a49d5f26cbf9c2f Mon Sep 17 00:00:00 2001 From: Salvatore Giordano Date: Tue, 28 Mar 2023 10:12:39 +0200 Subject: [PATCH 1/5] Update apns payload with corrected `critical` field --- engine/apps/mobile_app/tasks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/apps/mobile_app/tasks.py b/engine/apps/mobile_app/tasks.py index 76d3d32b29..73d3164050 100644 --- a/engine/apps/mobile_app/tasks.py +++ b/engine/apps/mobile_app/tasks.py @@ -208,7 +208,7 @@ def _get_fcm_message(alert_group, user, registration_id, critical): alert=ApsAlert(title=alert_title, subtitle=alert_subtitle, body=alert_body), sound=CriticalSound( # The notification shouldn't be critical if the user has disabled "override DND" setting - critical=(critical and mobile_app_user_settings.important_notification_override_dnd), + critical=1 if (critical and mobile_app_user_settings.important_notification_override_dnd) else 0, name=apns_sound_name, volume=apns_volume, ), From 06535f8c6e20f611803ea968d73d32ffc857085e Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Tue, 28 Mar 2023 10:34:11 +0200 Subject: [PATCH 2/5] update unit tests and changelog --- CHANGELOG.md | 6 ++++++ engine/apps/mobile_app/tests/test_notify_user.py | 6 +++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e8508787b..0d93e1f416 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +### Fixed + +- Addressed bug with iOS mobile push notifications always being set to critical by @imtoori and @joeyorlando ([#1646](https://github.com/grafana/oncall/pull/1646)) + ## v1.2.2 (2023-03-27) ### Changed diff --git a/engine/apps/mobile_app/tests/test_notify_user.py b/engine/apps/mobile_app/tests/test_notify_user.py index 8e3d548758..73404156d8 100644 --- a/engine/apps/mobile_app/tests/test_notify_user.py +++ b/engine/apps/mobile_app/tests/test_notify_user.py @@ -237,7 +237,7 @@ def test_fcm_message_user_settings( # Check APNS notification sound is set correctly apns_sound = message.apns.payload.aps.sound - assert apns_sound.critical is False + assert apns_sound.critical == 0 assert apns_sound.name == "default_sound.aiff" assert apns_sound.volume is None # APNS doesn't allow to specify volume for non-critical notifications @@ -267,7 +267,7 @@ def test_fcm_message_user_settings_critical( # Check APNS notification sound is set correctly apns_sound = message.apns.payload.aps.sound - assert apns_sound.critical is True + assert apns_sound.critical == 1 assert apns_sound.name == "default_sound_important.aiff" assert apns_sound.volume == 0.8 @@ -292,4 +292,4 @@ def test_fcm_message_user_settings_critical_override_dnd_disabled( # Check APNS notification sound is set correctly apns_sound = message.apns.payload.aps.sound - assert apns_sound.critical is False + assert apns_sound.critical == 0 From 4114f83fe981e164b25534673fd55c0d319b555e Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Tue, 28 Mar 2023 10:36:36 +0200 Subject: [PATCH 3/5] linting fix --- engine/apps/mobile_app/tasks.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/engine/apps/mobile_app/tasks.py b/engine/apps/mobile_app/tasks.py index 73d3164050..e97a5909ea 100644 --- a/engine/apps/mobile_app/tasks.py +++ b/engine/apps/mobile_app/tasks.py @@ -208,7 +208,9 @@ def _get_fcm_message(alert_group, user, registration_id, critical): alert=ApsAlert(title=alert_title, subtitle=alert_subtitle, body=alert_body), sound=CriticalSound( # The notification shouldn't be critical if the user has disabled "override DND" setting - critical=1 if (critical and mobile_app_user_settings.important_notification_override_dnd) else 0, + critical=1 + if (critical and mobile_app_user_settings.important_notification_override_dnd) + else 0, name=apns_sound_name, volume=apns_volume, ), From 7d5fe431a68822121a512e527b4f2f547d4a1243 Mon Sep 17 00:00:00 2001 From: Salvatore Giordano Date: Tue, 28 Mar 2023 11:54:15 +0200 Subject: [PATCH 4/5] revert and set interruption-level correctly --- engine/apps/mobile_app/tasks.py | 10 ++++++---- engine/apps/mobile_app/tests/test_notify_user.py | 2 ++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/engine/apps/mobile_app/tasks.py b/engine/apps/mobile_app/tasks.py index e97a5909ea..972b34a27b 100644 --- a/engine/apps/mobile_app/tasks.py +++ b/engine/apps/mobile_app/tasks.py @@ -141,6 +141,10 @@ def _get_fcm_message(alert_group, user, registration_id, critical): mobile_app_user_settings, _ = MobileAppUserSettings.objects.get_or_create(user=user) + # critical defines the type of notification. + # we use overrideDND to establish if the notification should sound even if DND is on + overrideDND = critical and mobile_app_user_settings.important_notification_override_dnd + # APNS only allows to specify volume for critical notifications apns_volume = mobile_app_user_settings.important_notification_volume if critical else None apns_sound_name = ( @@ -208,14 +212,12 @@ def _get_fcm_message(alert_group, user, registration_id, critical): alert=ApsAlert(title=alert_title, subtitle=alert_subtitle, body=alert_body), sound=CriticalSound( # The notification shouldn't be critical if the user has disabled "override DND" setting - critical=1 - if (critical and mobile_app_user_settings.important_notification_override_dnd) - else 0, + critical=overrideDND, name=apns_sound_name, volume=apns_volume, ), custom_data={ - "interruption-level": "critical" if critical else "time-sensitive", + "interruption-level": "critical" if overrideDND else "time-sensitive", }, ), ), diff --git a/engine/apps/mobile_app/tests/test_notify_user.py b/engine/apps/mobile_app/tests/test_notify_user.py index 73404156d8..ffe661c028 100644 --- a/engine/apps/mobile_app/tests/test_notify_user.py +++ b/engine/apps/mobile_app/tests/test_notify_user.py @@ -270,6 +270,7 @@ def test_fcm_message_user_settings_critical( assert apns_sound.critical == 1 assert apns_sound.name == "default_sound_important.aiff" assert apns_sound.volume == 0.8 + assert message.apns.payload.aps.custom_data["interruption-level"] == "critical" @pytest.mark.django_db @@ -293,3 +294,4 @@ def test_fcm_message_user_settings_critical_override_dnd_disabled( # Check APNS notification sound is set correctly apns_sound = message.apns.payload.aps.sound assert apns_sound.critical == 0 + assert message.apns.payload.aps.custom_data["interruption-level"] == "time-sensitive" From 1fbd5af5228592935c3bf76d3714136b86438505 Mon Sep 17 00:00:00 2001 From: Salvatore Giordano Date: Tue, 28 Mar 2023 11:57:17 +0200 Subject: [PATCH 5/5] revert tests --- engine/apps/mobile_app/tests/test_notify_user.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/engine/apps/mobile_app/tests/test_notify_user.py b/engine/apps/mobile_app/tests/test_notify_user.py index ffe661c028..9f76d2f76c 100644 --- a/engine/apps/mobile_app/tests/test_notify_user.py +++ b/engine/apps/mobile_app/tests/test_notify_user.py @@ -237,7 +237,7 @@ def test_fcm_message_user_settings( # Check APNS notification sound is set correctly apns_sound = message.apns.payload.aps.sound - assert apns_sound.critical == 0 + assert apns_sound.critical is False assert apns_sound.name == "default_sound.aiff" assert apns_sound.volume is None # APNS doesn't allow to specify volume for non-critical notifications @@ -267,7 +267,7 @@ def test_fcm_message_user_settings_critical( # Check APNS notification sound is set correctly apns_sound = message.apns.payload.aps.sound - assert apns_sound.critical == 1 + assert apns_sound.critical is True assert apns_sound.name == "default_sound_important.aiff" assert apns_sound.volume == 0.8 assert message.apns.payload.aps.custom_data["interruption-level"] == "critical" @@ -293,5 +293,5 @@ def test_fcm_message_user_settings_critical_override_dnd_disabled( # Check APNS notification sound is set correctly apns_sound = message.apns.payload.aps.sound - assert apns_sound.critical == 0 + assert apns_sound.critical is False assert message.apns.payload.aps.custom_data["interruption-level"] == "time-sensitive"