diff --git a/CHANGELOG.md b/CHANGELOG.md index f3a0e133..02dd2683 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Shift Swap Requests Web UI ([#2593](https://github.com/grafana/oncall/issues/2593)) -- Final schedule shifts should lay in one line [1665](https://github.com/grafana/oncall/issues/1665) +- Final schedule shifts should lay in one line ([#1665](https://github.com/grafana/oncall/issues/1665)) +- Add backend support for push notification sounds with custom extensions by @vadimkerr ([#2759](https://github.com/grafana/oncall/pull/2759)) ### Changed diff --git a/engine/apps/mobile_app/demo_push.py b/engine/apps/mobile_app/demo_push.py index 557c34e3..4ac67969 100644 --- a/engine/apps/mobile_app/demo_push.py +++ b/engine/apps/mobile_app/demo_push.py @@ -6,7 +6,8 @@ import typing from firebase_admin.messaging import APNSPayload, Aps, ApsAlert, CriticalSound, Message from apps.mobile_app.exceptions import DeviceNotSet -from apps.mobile_app.tasks import FCMMessageData, MessageType, _construct_fcm_message, _send_push_notification, logger +from apps.mobile_app.tasks import _construct_fcm_message, _send_push_notification, logger +from apps.mobile_app.types import FCMMessageData, MessageType, Platform from apps.user_management.models import User if typing.TYPE_CHECKING: @@ -38,27 +39,22 @@ def _get_test_escalation_fcm_message(user: User, device_to_notify: "FCMDevice", # 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 = ( - mobile_app_user_settings.important_notification_sound_name - if critical - else mobile_app_user_settings.default_notification_sound_name - ) + MobileAppUserSettings.IOS_SOUND_NAME_EXTENSION # iOS app expects the filename to have an extension + message_type = MessageType.IMPORTANT if critical else MessageType.DEFAULT + apns_sound_name = mobile_app_user_settings.get_notification_sound_name(message_type, Platform.IOS) fcm_message_data: FCMMessageData = { "title": get_test_push_title(critical), # Pass user settings, so the Android app can use them to play the correct sound and volume - "default_notification_sound_name": ( - mobile_app_user_settings.default_notification_sound_name - + MobileAppUserSettings.ANDROID_SOUND_NAME_EXTENSION + "default_notification_sound_name": mobile_app_user_settings.get_notification_sound_name( + MessageType.DEFAULT, Platform.ANDROID ), "default_notification_volume_type": mobile_app_user_settings.default_notification_volume_type, "default_notification_volume": str(mobile_app_user_settings.default_notification_volume), "default_notification_volume_override": json.dumps( mobile_app_user_settings.default_notification_volume_override ), - "important_notification_sound_name": ( - mobile_app_user_settings.important_notification_sound_name - + MobileAppUserSettings.ANDROID_SOUND_NAME_EXTENSION + "important_notification_sound_name": mobile_app_user_settings.get_notification_sound_name( + MessageType.IMPORTANT, Platform.ANDROID ), "important_notification_volume_type": mobile_app_user_settings.important_notification_volume_type, "important_notification_volume": str(mobile_app_user_settings.important_notification_volume), @@ -84,8 +80,6 @@ def _get_test_escalation_fcm_message(user: User, device_to_notify: "FCMDevice", ), ) - message_type = MessageType.CRITICAL if critical else MessageType.NORMAL - return _construct_fcm_message(message_type, device_to_notify, thread_id, fcm_message_data, apns_payload) diff --git a/engine/apps/mobile_app/models.py b/engine/apps/mobile_app/models.py index a4a79abd..46680ae1 100644 --- a/engine/apps/mobile_app/models.py +++ b/engine/apps/mobile_app/models.py @@ -10,6 +10,7 @@ from fcm_django.models import FCMDevice as BaseFCMDevice from apps.auth_token import constants, crypto from apps.auth_token.models import BaseAuthToken +from apps.mobile_app.types import MessageType, Platform if typing.TYPE_CHECKING: from apps.user_management.models import Organization, User @@ -175,3 +176,22 @@ class MobileAppUserSettings(models.Model): locale = models.CharField(max_length=50, null=True) time_zone = models.CharField(max_length=100, default="UTC") + + def get_notification_sound_name(self, message_type: MessageType, platform: Platform) -> str: + sound_name = { + MessageType.DEFAULT: self.default_notification_sound_name, + MessageType.IMPORTANT: self.important_notification_sound_name, + MessageType.INFO: self.info_notification_sound_name, + }[message_type] + + # If sound name already contains an extension, return it as is + if "." in sound_name: + return sound_name + + # Add appropriate extension based on platform, for cases when no extension is specified in the sound name + extension = { + Platform.IOS: self.IOS_SOUND_NAME_EXTENSION, + Platform.ANDROID: self.ANDROID_SOUND_NAME_EXTENSION, + }[platform] + + return f"{sound_name}{extension}" diff --git a/engine/apps/mobile_app/tasks.py b/engine/apps/mobile_app/tasks.py index 456cbe45..d4eb354e 100644 --- a/engine/apps/mobile_app/tasks.py +++ b/engine/apps/mobile_app/tasks.py @@ -3,7 +3,6 @@ import json import logging import math import typing -from enum import Enum import humanize import pytz @@ -20,6 +19,7 @@ from rest_framework import status from apps.alerts.models import AlertGroup from apps.base.utils import live_settings from apps.mobile_app.alert_rendering import get_push_notification_subtitle +from apps.mobile_app.types import FCMMessageData, MessageType, Platform from apps.schedules.models import ShiftSwapRequest from apps.schedules.models.on_call_schedule import OnCallSchedule, ScheduleEvent from apps.user_management.models import User @@ -36,18 +36,6 @@ logger = get_task_logger(__name__) logger.setLevel(logging.DEBUG) -class MessageType(str, Enum): - NORMAL = "oncall.message" - CRITICAL = "oncall.critical_message" - INFO = "oncall.info" - - -class FCMMessageData(typing.TypedDict): - title: str - subtitle: typing.Optional[str] - body: typing.Optional[str] - - def send_push_notification_to_fcm_relay(message: Message) -> requests.Response: """ Send push notification to FCM relay on cloud instance: apps.mobile_app.fcm_relay.FCMRelayView @@ -168,11 +156,8 @@ def _get_alert_group_escalation_fcm_message( # 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 = ( - mobile_app_user_settings.important_notification_sound_name - if critical - else mobile_app_user_settings.default_notification_sound_name - ) + MobileAppUserSettings.IOS_SOUND_NAME_EXTENSION # iOS app expects the filename to have an extension + message_type = MessageType.IMPORTANT if critical else MessageType.DEFAULT + apns_sound_name = mobile_app_user_settings.get_notification_sound_name(message_type, Platform.IOS) fcm_message_data: FCMMessageData = { "title": alert_title, @@ -183,18 +168,16 @@ def _get_alert_group_escalation_fcm_message( # alert_group.status is an int so it must be casted... "status": str(alert_group.status), # Pass user settings, so the Android app can use them to play the correct sound and volume - "default_notification_sound_name": ( - mobile_app_user_settings.default_notification_sound_name - + MobileAppUserSettings.ANDROID_SOUND_NAME_EXTENSION + "default_notification_sound_name": mobile_app_user_settings.get_notification_sound_name( + MessageType.DEFAULT, Platform.ANDROID ), "default_notification_volume_type": mobile_app_user_settings.default_notification_volume_type, "default_notification_volume": str(mobile_app_user_settings.default_notification_volume), "default_notification_volume_override": json.dumps( mobile_app_user_settings.default_notification_volume_override ), - "important_notification_sound_name": ( - mobile_app_user_settings.important_notification_sound_name - + MobileAppUserSettings.ANDROID_SOUND_NAME_EXTENSION + "important_notification_sound_name": mobile_app_user_settings.get_notification_sound_name( + MessageType.IMPORTANT, Platform.ANDROID ), "important_notification_volume_type": mobile_app_user_settings.important_notification_volume_type, "important_notification_volume": str(mobile_app_user_settings.important_notification_volume), @@ -222,8 +205,6 @@ def _get_alert_group_escalation_fcm_message( ), ) - message_type = MessageType.CRITICAL if critical else MessageType.NORMAL - return _construct_fcm_message(message_type, device_to_notify, thread_id, fcm_message_data, apns_payload) @@ -268,8 +249,6 @@ def _get_youre_going_oncall_fcm_message( thread_id = f"{schedule.public_primary_key}:{user.public_primary_key}:going-oncall" mobile_app_user_settings, _ = MobileAppUserSettings.objects.get_or_create(user=user) - info_notification_sound_name = mobile_app_user_settings.info_notification_sound_name - notification_title = _get_youre_going_oncall_notification_title(seconds_until_going_oncall) notification_subtitle = _get_youre_going_oncall_notification_subtitle( schedule, schedule_event, mobile_app_user_settings @@ -278,7 +257,9 @@ def _get_youre_going_oncall_fcm_message( data: FCMMessageData = { "title": notification_title, "subtitle": notification_subtitle, - "info_notification_sound_name": f"{info_notification_sound_name}{MobileAppUserSettings.ANDROID_SOUND_NAME_EXTENSION}", + "info_notification_sound_name": mobile_app_user_settings.get_notification_sound_name( + MessageType.INFO, Platform.ANDROID + ), "info_notification_volume_type": mobile_app_user_settings.info_notification_volume_type, "info_notification_volume": str(mobile_app_user_settings.info_notification_volume), "info_notification_volume_override": json.dumps(mobile_app_user_settings.info_notification_volume_override), @@ -290,7 +271,7 @@ def _get_youre_going_oncall_fcm_message( alert=ApsAlert(title=notification_title, subtitle=notification_subtitle), sound=CriticalSound( critical=False, - name=f"{info_notification_sound_name}{MobileAppUserSettings.IOS_SOUND_NAME_EXTENSION}", + name=mobile_app_user_settings.get_notification_sound_name(MessageType.INFO, Platform.IOS), ), custom_data={ "interruption-level": "time-sensitive", @@ -641,8 +622,6 @@ def _shift_swap_request_fcm_message( device_to_notify: "FCMDevice", mobile_app_user_settings: "MobileAppUserSettings", ) -> Message: - from apps.mobile_app.models import MobileAppUserSettings - thread_id = f"{shift_swap_request.public_primary_key}:{user.public_primary_key}:ssr" notification_title = "New shift swap request" beneficiary_name = shift_swap_request.beneficiary.name or shift_swap_request.beneficiary.username @@ -655,8 +634,8 @@ def _shift_swap_request_fcm_message( "title": notification_title, "subtitle": notification_subtitle, "route": route, - "info_notification_sound_name": ( - mobile_app_user_settings.info_notification_sound_name + MobileAppUserSettings.ANDROID_SOUND_NAME_EXTENSION + "info_notification_sound_name": mobile_app_user_settings.get_notification_sound_name( + MessageType.INFO, Platform.ANDROID ), "info_notification_volume_type": mobile_app_user_settings.info_notification_volume_type, "info_notification_volume": str(mobile_app_user_settings.info_notification_volume), @@ -669,8 +648,7 @@ def _shift_swap_request_fcm_message( alert=ApsAlert(title=notification_title, subtitle=notification_subtitle), sound=CriticalSound( critical=False, - name=mobile_app_user_settings.info_notification_sound_name - + MobileAppUserSettings.IOS_SOUND_NAME_EXTENSION, + name=mobile_app_user_settings.get_notification_sound_name(MessageType.INFO, Platform.IOS), ), custom_data={ "interruption-level": "time-sensitive", diff --git a/engine/apps/mobile_app/tests/test_demo_push.py b/engine/apps/mobile_app/tests/test_demo_push.py index 73041ac3..abf5f6eb 100644 --- a/engine/apps/mobile_app/tests/test_demo_push.py +++ b/engine/apps/mobile_app/tests/test_demo_push.py @@ -34,6 +34,7 @@ def test_test_escalation_fcm_message_user_settings( assert message.apns.payload.aps.badge is None assert message.apns.payload.aps.alert.title == get_test_push_title(critical=False) assert message.data["title"] == get_test_push_title(critical=False) + assert message.data["type"] == "oncall.message" @pytest.mark.django_db @@ -67,6 +68,7 @@ def test_escalation_fcm_message_user_settings_critical( assert message.apns.payload.aps.badge is None assert message.apns.payload.aps.alert.title == get_test_push_title(critical=True) assert message.data["title"] == get_test_push_title(critical=True) + assert message.data["type"] == "oncall.critical_message" @pytest.mark.django_db diff --git a/engine/apps/mobile_app/tests/test_notify_user.py b/engine/apps/mobile_app/tests/test_notify_user.py index 17e28d12..b7ba5046 100644 --- a/engine/apps/mobile_app/tests/test_notify_user.py +++ b/engine/apps/mobile_app/tests/test_notify_user.py @@ -234,6 +234,7 @@ def test_fcm_message_user_settings( assert message.data["important_notification_volume"] == "0.8" assert message.data["important_notification_volume_override"] == "true" assert message.data["important_notification_override_dnd"] == "true" + assert message.data["type"] == "oncall.message" # Check APNS notification sound is set correctly apns_sound = message.apns.payload.aps.sound @@ -265,6 +266,7 @@ def test_fcm_message_user_settings_critical( assert message.data["important_notification_volume"] == "0.8" assert message.data["important_notification_volume_override"] == "true" assert message.data["important_notification_override_dnd"] == "true" + assert message.data["type"] == "oncall.critical_message" # Check APNS notification sound is set correctly apns_sound = message.apns.payload.aps.sound diff --git a/engine/apps/mobile_app/tests/test_shift_swap_request.py b/engine/apps/mobile_app/tests/test_shift_swap_request.py index 9d763713..afd216a7 100644 --- a/engine/apps/mobile_app/tests/test_shift_swap_request.py +++ b/engine/apps/mobile_app/tests/test_shift_swap_request.py @@ -9,7 +9,6 @@ from apps.mobile_app.models import FCMDevice, MobileAppUserSettings from apps.mobile_app.tasks import ( SSR_EARLIEST_NOTIFICATION_OFFSET, SSR_NOTIFICATION_WINDOW, - MessageType, _get_shift_swap_requests_to_notify, _has_user_been_notified_for_shift_swap_request, _mark_shift_swap_request_notified_for_user, @@ -261,7 +260,7 @@ def test_notify_user_about_shift_swap_request(make_organization, make_user, make assert mock_send_push_notification.call_args.args[0] == device_to_notify message: Message = mock_send_push_notification.call_args.args[1] - assert message.data["type"] == MessageType.INFO + assert message.data["type"] == "oncall.info" assert message.data["title"] == "New shift swap request" assert message.data["subtitle"] == "John Doe, Test Schedule" assert ( diff --git a/engine/apps/mobile_app/tests/test_user_settings.py b/engine/apps/mobile_app/tests/test_user_settings.py index 063a142c..43ceede9 100644 --- a/engine/apps/mobile_app/tests/test_user_settings.py +++ b/engine/apps/mobile_app/tests/test_user_settings.py @@ -3,6 +3,9 @@ from django.urls import reverse from rest_framework import status from rest_framework.test import APIClient +from apps.mobile_app.models import MobileAppUserSettings +from apps.mobile_app.types import MessageType, Platform + @pytest.mark.django_db def test_user_settings_get(make_organization_and_user_with_mobile_app_auth_token): @@ -140,3 +143,35 @@ def test_user_settings_time_zone_must_be_valid(make_organization_and_user_with_m response = client.put(url, data=null_timezone, format="json", HTTP_AUTHORIZATION=auth_token) assert response.status_code == status.HTTP_400_BAD_REQUEST + + +@pytest.mark.parametrize( + "message_type,platform,sound_names,expected_sound_name", + [ + (MessageType.DEFAULT, Platform.ANDROID, ["default", "empty", "empty"], "default.mp3"), + (MessageType.DEFAULT, Platform.ANDROID, ["default.extension", "empty", "empty"], "default.extension"), + (MessageType.DEFAULT, Platform.IOS, ["default", "empty", "empty"], "default.aiff"), + (MessageType.DEFAULT, Platform.IOS, ["default.extension", "empty", "empty"], "default.extension"), + (MessageType.IMPORTANT, Platform.ANDROID, ["empty", "important", "empty"], "important.mp3"), + (MessageType.IMPORTANT, Platform.ANDROID, ["empty", "important.extension", "empty"], "important.extension"), + (MessageType.IMPORTANT, Platform.IOS, ["empty", "important", "empty"], "important.aiff"), + (MessageType.IMPORTANT, Platform.IOS, ["empty", "important.extension", "empty"], "important.extension"), + (MessageType.INFO, Platform.ANDROID, ["empty", "empty", "info"], "info.mp3"), + (MessageType.INFO, Platform.ANDROID, ["empty", "empty", "info.extension"], "info.extension"), + (MessageType.INFO, Platform.IOS, ["empty", "empty", "info"], "info.aiff"), + (MessageType.INFO, Platform.IOS, ["empty", "empty", "info.extension"], "info.extension"), + ], +) +@pytest.mark.django_db +def test_get_notification_sound_name( + make_organization_and_user, message_type, platform, sound_names, expected_sound_name +): + organization, user = make_organization_and_user() + mobile_app_user_settings = MobileAppUserSettings.objects.create( + user=user, + default_notification_sound_name=sound_names[0], + important_notification_sound_name=sound_names[1], + info_notification_sound_name=sound_names[2], + ) + + assert mobile_app_user_settings.get_notification_sound_name(message_type, platform) == expected_sound_name diff --git a/engine/apps/mobile_app/tests/test_your_going_oncall_notification.py b/engine/apps/mobile_app/tests/test_your_going_oncall_notification.py index a218fd6f..3cb90061 100644 --- a/engine/apps/mobile_app/tests/test_your_going_oncall_notification.py +++ b/engine/apps/mobile_app/tests/test_your_going_oncall_notification.py @@ -8,6 +8,7 @@ from django.utils import timezone from apps.mobile_app import tasks from apps.mobile_app.models import FCMDevice, MobileAppUserSettings +from apps.mobile_app.types import MessageType, Platform from apps.schedules.models import OnCallScheduleCalendar, OnCallScheduleICal, OnCallScheduleWeb from apps.schedules.models.on_call_schedule import ScheduleEvent @@ -217,9 +218,7 @@ def test_get_youre_going_oncall_fcm_message( data = { "title": mock_notification_title, "subtitle": mock_notification_subtitle, - "info_notification_sound_name": ( - maus.info_notification_sound_name + MobileAppUserSettings.ANDROID_SOUND_NAME_EXTENSION - ), + "info_notification_sound_name": maus.get_notification_sound_name(MessageType.INFO, Platform.ANDROID), "info_notification_volume_type": maus.info_notification_volume_type, "info_notification_volume": str(maus.info_notification_volume), "info_notification_volume_override": json.dumps(maus.info_notification_volume_override), @@ -233,7 +232,7 @@ def test_get_youre_going_oncall_fcm_message( mock_aps_alert.assert_called_once_with(title=mock_notification_title, subtitle=mock_notification_subtitle) mock_critical_sound.assert_called_once_with( - critical=False, name=maus.info_notification_sound_name + MobileAppUserSettings.IOS_SOUND_NAME_EXTENSION + critical=False, name=maus.get_notification_sound_name(MessageType.INFO, Platform.IOS) ) mock_aps.assert_called_once_with( thread_id=notification_thread_id, @@ -249,7 +248,7 @@ def test_get_youre_going_oncall_fcm_message( mock_get_youre_going_oncall_notification_title.assert_called_once_with(seconds_until_going_oncall) mock_construct_fcm_message.assert_called_once_with( - tasks.MessageType.INFO, device, notification_thread_id, data, mock_apns_payload.return_value + MessageType.INFO, device, notification_thread_id, data, mock_apns_payload.return_value ) diff --git a/engine/apps/mobile_app/types.py b/engine/apps/mobile_app/types.py new file mode 100644 index 00000000..210a4696 --- /dev/null +++ b/engine/apps/mobile_app/types.py @@ -0,0 +1,19 @@ +import typing +from enum import StrEnum + + +class MessageType(StrEnum): + DEFAULT = "oncall.message" + IMPORTANT = "oncall.critical_message" + INFO = "oncall.info" + + +class Platform(StrEnum): + ANDROID = "android" + IOS = "ios" + + +class FCMMessageData(typing.TypedDict): + title: str + subtitle: typing.Optional[str] + body: typing.Optional[str]