Add backend support for push notification sounds with custom extensions

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
 

engine/apps/mobile_app/demo_push.py

@@ -6,7 +6,14 @@
 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 (
+    FCMMessageData,
+    MessageType,
+    Platform,
+    _construct_fcm_message,
+    _send_push_notification,
+    logger,
+)
 from apps.user_management.models import User
 
 if typing.TYPE_CHECKING:
@@ -38,27 +45,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 +86,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)
 
 

engine/apps/mobile_app/models.py

@@ -10,6 +10,7 @@
 
 from apps.auth_token import constants, crypto
 from apps.auth_token.models import BaseAuthToken
+from apps.mobile_app.tasks import MessageType, Platform
 
 if typing.TYPE_CHECKING:
     from apps.user_management.models import Organization, User
@@ -175,3 +176,22 @@ class VolumeType(models.TextChoices):
 
     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}"

engine/apps/mobile_app/tasks.py

@@ -3,7 +3,7 @@
 import logging
 import math
 import typing
-from enum import Enum
+from enum import StrEnum
 
 import humanize
 import pytz
@@ -36,12 +36,17 @@
 logger.setLevel(logging.DEBUG)
 
 
-class MessageType(str, Enum):
-    NORMAL = "oncall.message"
-    CRITICAL = "oncall.critical_message"
+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]
@@ -168,11 +173,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 +185,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 +222,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 +266,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 +274,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 +288,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 +639,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 +651,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 +665,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",

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

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

engine/apps/mobile_app/tests/test_shift_swap_request.py

@@ -9,7 +9,6 @@
 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 (

engine/apps/mobile_app/tests/test_user_settings.py

@@ -3,6 +3,9 @@
 from rest_framework import status
 from rest_framework.test import APIClient
 
+from apps.mobile_app.models import MobileAppUserSettings
+from apps.mobile_app.tasks 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

engine/apps/mobile_app/tests/test_your_going_oncall_notification.py

@@ -8,6 +8,7 @@
 
 from apps.mobile_app import tasks
 from apps.mobile_app.models import FCMDevice, MobileAppUserSettings
+from apps.mobile_app.tasks 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,