From 208db9cdb7a45a35867949aa3e97a8fbd59bb02a Mon Sep 17 00:00:00 2001 From: Salvatore Giordano Date: Fri, 15 Nov 2024 11:29:00 +0100 Subject: [PATCH] remove add_stack_slug_to_message_title utility from push notification titles (#5258) # What this PR does We noticed that the backend was adding the stack name to the notification title only on Android. We thought it makes sense to add the stack name only if the user has more than 1 stack connected, but that's not doable right now since the backend doesn't know how many stacks are connected in the app. Also we took a look at the analytics for the app and basically 95% of the users have only 1 stack connected. This pr removes the stack name from the notifications title. If in the future we think it makes sense to add it conditionally based on the number of stacks we can open another pr, but given the very little amount of users with more than 1 stack I think this is not needed. ## 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] Added the relevant release notes label (see labels prefixed w/ `release:`). These labels dictate how your PR will show up in the autogenerated release notes. --- engine/apps/mobile_app/demo_push.py | 4 ++-- .../apps/mobile_app/tasks/going_oncall_notification.py | 9 ++------- engine/apps/mobile_app/tasks/new_alert_group.py | 9 ++------- engine/apps/mobile_app/tasks/new_shift_swap_request.py | 9 ++------- .../tests/tasks/test_going_oncall_notification.py | 3 +-- .../tests/tasks/test_new_shift_swap_request.py | 7 ++----- engine/apps/mobile_app/tests/test_demo_push.py | 7 +++---- 7 files changed, 14 insertions(+), 34 deletions(-) diff --git a/engine/apps/mobile_app/demo_push.py b/engine/apps/mobile_app/demo_push.py index 19daca5b..01194c14 100644 --- a/engine/apps/mobile_app/demo_push.py +++ b/engine/apps/mobile_app/demo_push.py @@ -8,7 +8,7 @@ from firebase_admin.messaging import APNSPayload, Aps, ApsAlert, CriticalSound, from apps.mobile_app.exceptions import DeviceNotSet from apps.mobile_app.types import FCMMessageData, MessageType, Platform -from apps.mobile_app.utils import add_stack_slug_to_message_title, construct_fcm_message, send_push_notification +from apps.mobile_app.utils import construct_fcm_message, send_push_notification from apps.user_management.models import User if typing.TYPE_CHECKING: @@ -47,7 +47,7 @@ def _get_test_escalation_fcm_message(user: User, device_to_notify: "FCMDevice", apns_sound_name = mobile_app_user_settings.get_notification_sound_name(message_type, Platform.IOS) fcm_message_data: FCMMessageData = { - "title": add_stack_slug_to_message_title(get_test_push_title(critical), user.organization), + "title": get_test_push_title(critical), "orgName": user.organization.stack_slug, # 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.get_notification_sound_name( diff --git a/engine/apps/mobile_app/tasks/going_oncall_notification.py b/engine/apps/mobile_app/tasks/going_oncall_notification.py index 214fa19d..34fd4160 100644 --- a/engine/apps/mobile_app/tasks/going_oncall_notification.py +++ b/engine/apps/mobile_app/tasks/going_oncall_notification.py @@ -12,12 +12,7 @@ from django.utils import timezone from firebase_admin.messaging import APNSPayload, Aps, ApsAlert, CriticalSound, Message from apps.mobile_app.types import FCMMessageData, MessageType, Platform -from apps.mobile_app.utils import ( - MAX_RETRIES, - add_stack_slug_to_message_title, - construct_fcm_message, - send_push_notification, -) +from apps.mobile_app.utils import MAX_RETRIES, construct_fcm_message, send_push_notification from apps.schedules.models.on_call_schedule import OnCallSchedule, ScheduleEvent from apps.user_management.models import User from common.cache import ensure_cache_key_allocates_to_the_same_hash_slot @@ -82,7 +77,7 @@ def _get_fcm_message( notification_subtitle = _get_notification_subtitle(schedule, schedule_event, mobile_app_user_settings) data: FCMMessageData = { - "title": add_stack_slug_to_message_title(notification_title, user.organization), + "title": notification_title, "subtitle": notification_subtitle, "orgName": user.organization.stack_slug, "info_notification_sound_name": mobile_app_user_settings.get_notification_sound_name( diff --git a/engine/apps/mobile_app/tasks/new_alert_group.py b/engine/apps/mobile_app/tasks/new_alert_group.py index e33e9111..2b759f5f 100644 --- a/engine/apps/mobile_app/tasks/new_alert_group.py +++ b/engine/apps/mobile_app/tasks/new_alert_group.py @@ -8,12 +8,7 @@ from firebase_admin.messaging import APNSPayload, Aps, ApsAlert, CriticalSound, from apps.alerts.models import AlertGroup from apps.mobile_app.alert_rendering import get_push_notification_subtitle, get_push_notification_title from apps.mobile_app.types import FCMMessageData, MessageType, Platform -from apps.mobile_app.utils import ( - MAX_RETRIES, - add_stack_slug_to_message_title, - construct_fcm_message, - send_push_notification, -) +from apps.mobile_app.utils import MAX_RETRIES, construct_fcm_message, send_push_notification from apps.user_management.models import User from common.custom_celery_tasks import shared_dedicated_queue_retry_task @@ -46,7 +41,7 @@ def _get_fcm_message(alert_group: AlertGroup, user: User, device_to_notify: "FCM apns_sound_name = mobile_app_user_settings.get_notification_sound_name(message_type, Platform.IOS) fcm_message_data: FCMMessageData = { - "title": add_stack_slug_to_message_title(alert_title, alert_group.channel.organization), + "title": alert_title, "subtitle": alert_subtitle, "orgId": alert_group.channel.organization.public_primary_key, "orgName": alert_group.channel.organization.stack_slug, diff --git a/engine/apps/mobile_app/tasks/new_shift_swap_request.py b/engine/apps/mobile_app/tasks/new_shift_swap_request.py index a6d49c8b..3ab71674 100644 --- a/engine/apps/mobile_app/tasks/new_shift_swap_request.py +++ b/engine/apps/mobile_app/tasks/new_shift_swap_request.py @@ -10,12 +10,7 @@ from django.utils import timezone from firebase_admin.messaging import APNSPayload, Aps, ApsAlert, CriticalSound, Message from apps.mobile_app.types import FCMMessageData, MessageType, Platform -from apps.mobile_app.utils import ( - MAX_RETRIES, - add_stack_slug_to_message_title, - construct_fcm_message, - send_push_notification, -) +from apps.mobile_app.utils import MAX_RETRIES, construct_fcm_message, send_push_notification from apps.schedules.models import ShiftSwapRequest from apps.user_management.models import User from common.custom_celery_tasks import shared_dedicated_queue_retry_task @@ -121,7 +116,7 @@ def _get_fcm_message( route = f"/schedules/{shift_swap_request.schedule.public_primary_key}/ssrs/{shift_swap_request.public_primary_key}" data: FCMMessageData = { - "title": add_stack_slug_to_message_title(notification_title, user.organization), + "title": notification_title, "subtitle": notification_subtitle, "orgName": user.organization.stack_slug, "route": route, diff --git a/engine/apps/mobile_app/tests/tasks/test_going_oncall_notification.py b/engine/apps/mobile_app/tests/tasks/test_going_oncall_notification.py index 2541d507..051e4ffb 100644 --- a/engine/apps/mobile_app/tests/tasks/test_going_oncall_notification.py +++ b/engine/apps/mobile_app/tests/tasks/test_going_oncall_notification.py @@ -18,7 +18,6 @@ from apps.mobile_app.tasks.going_oncall_notification import ( conditionally_send_going_oncall_push_notifications_for_schedule, ) from apps.mobile_app.types import MessageType, Platform -from apps.mobile_app.utils import add_stack_slug_to_message_title from apps.schedules.models import OnCallScheduleCalendar, OnCallScheduleICal, OnCallScheduleWeb from apps.schedules.models.on_call_schedule import ScheduleEvent @@ -228,7 +227,7 @@ def test_get_fcm_message( maus = MobileAppUserSettings.objects.create(user=user, time_zone=user_tz) data = { - "title": add_stack_slug_to_message_title(mock_notification_title, organization), + "title": mock_notification_title, "subtitle": mock_notification_subtitle, "orgName": organization.stack_slug, "info_notification_sound_name": maus.get_notification_sound_name(MessageType.INFO, Platform.ANDROID), diff --git a/engine/apps/mobile_app/tests/tasks/test_new_shift_swap_request.py b/engine/apps/mobile_app/tests/tasks/test_new_shift_swap_request.py index 452b9895..f77674f8 100644 --- a/engine/apps/mobile_app/tests/tasks/test_new_shift_swap_request.py +++ b/engine/apps/mobile_app/tests/tasks/test_new_shift_swap_request.py @@ -19,7 +19,6 @@ from apps.mobile_app.tasks.new_shift_swap_request import ( notify_shift_swap_requests, notify_user_about_shift_swap_request, ) -from apps.mobile_app.utils import add_stack_slug_to_message_title from apps.schedules.models import CustomOnCallShift, OnCallScheduleWeb, ShiftSwapRequest from apps.user_management.models import User from apps.user_management.models.user import default_working_hours @@ -288,7 +287,7 @@ def test_notify_user_about_shift_swap_request( message: Message = mock_send_push_notification.call_args.args[1] assert message.data["type"] == "oncall.info" - assert message.data["title"] == add_stack_slug_to_message_title("New shift swap request", organization) + assert message.data["title"] == "New shift swap request" assert message.data["subtitle"] == "John Doe, Test Schedule" assert ( message.data["route"] @@ -487,9 +486,7 @@ def test_notify_beneficiary_about_taken_shift_swap_request( message: Message = mock_send_push_notification.call_args.args[1] assert message.data["type"] == "oncall.info" - assert message.data["title"] == add_stack_slug_to_message_title( - "Your shift swap request has been taken", organization - ) + assert message.data["title"] == "Your shift swap request has been taken" assert message.data["subtitle"] == schedule_name assert ( message.data["route"] diff --git a/engine/apps/mobile_app/tests/test_demo_push.py b/engine/apps/mobile_app/tests/test_demo_push.py index 769691f7..abf5f6eb 100644 --- a/engine/apps/mobile_app/tests/test_demo_push.py +++ b/engine/apps/mobile_app/tests/test_demo_push.py @@ -2,7 +2,6 @@ import pytest from apps.mobile_app.demo_push import _get_test_escalation_fcm_message, get_test_push_title from apps.mobile_app.models import FCMDevice, MobileAppUserSettings -from apps.mobile_app.utils import add_stack_slug_to_message_title @pytest.mark.django_db @@ -34,7 +33,7 @@ def test_test_escalation_fcm_message_user_settings( # Check expected test push content 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"] == add_stack_slug_to_message_title(get_test_push_title(critical=False), organization) + assert message.data["title"] == get_test_push_title(critical=False) assert message.data["type"] == "oncall.message" @@ -68,7 +67,7 @@ def test_escalation_fcm_message_user_settings_critical( # Check expected test push content 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"] == add_stack_slug_to_message_title(get_test_push_title(critical=True), organization) + assert message.data["title"] == get_test_push_title(critical=True) assert message.data["type"] == "oncall.critical_message" @@ -94,4 +93,4 @@ def test_escalation_fcm_message_user_settings_critical_override_dnd_disabled( # Check expected test push content 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"] == add_stack_slug_to_message_title(get_test_push_title(critical=True), organization) + assert message.data["title"] == get_test_push_title(critical=True)