Add backend support for push notification sounds with custom extensions (#2759)

# What this PR does

Instead of always adding `.aiff` or `.mp3` at the end of notification
sound names depending on the platform (iOS vs Android), add them only if
no extension is present already. This should make it possible to use
sounds with custom extensions.

## Checklist

- [x] Unit, integration, and e2e (if applicable) tests updated
- [x] Documentation added (or `pr:no public docs` PR label added if not
required)
- [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not
required)

Author: vstpme

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,8 @@
 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)
 
 

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.types 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,6 @@
 import logging
 import math
 import typing
-from enum import Enum
 
 import humanize
 import pytz
@@ -20,6 +19,7 @@
 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.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",

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.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

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.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
     )
 
 

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]