Skip to content

Commit

Permalink
remove add_stack_slug_to_message_title utility from push notification…
Browse files Browse the repository at this point in the history
… 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.
  • Loading branch information
imtoori authored Nov 15, 2024
1 parent 9338cff commit 208db9c
Show file tree
Hide file tree
Showing 7 changed files with 14 additions and 34 deletions.
4 changes: 2 additions & 2 deletions engine/apps/mobile_app/demo_push.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

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:
Expand Down Expand Up @@ -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(
Expand Down
9 changes: 2 additions & 7 deletions engine/apps/mobile_app/tasks/going_oncall_notification.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,7 @@
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
Expand Down Expand Up @@ -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(
Expand Down
9 changes: 2 additions & 7 deletions engine/apps/mobile_app/tasks/new_alert_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,7 @@
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

Expand Down Expand Up @@ -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,
Expand Down
9 changes: 2 additions & 7 deletions engine/apps/mobile_app/tasks/new_shift_swap_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,7 @@
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
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
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

Expand Down Expand Up @@ -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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
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
Expand Down Expand Up @@ -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"]
Expand Down Expand Up @@ -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"]
Expand Down
7 changes: 3 additions & 4 deletions engine/apps/mobile_app/tests/test_demo_push.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

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
Expand Down Expand Up @@ -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"


Expand Down Expand Up @@ -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"


Expand All @@ -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)

0 comments on commit 208db9c

Please sign in to comment.