From e003e8a0b86cdb96d7c6fb682b2bdfcb26157870 Mon Sep 17 00:00:00 2001 From: Yulya Artyukhina Date: Tue, 12 Dec 2023 17:46:08 +0100 Subject: [PATCH] Fix `message is too big` exception for mobile push notification (#3556) # What this PR does Adds limit for alert title length in mobile app push notifications ## Which issue(s) this PR fixes https://github.com/grafana/oncall-private/issues/2375 ## 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) --- CHANGELOG.md | 4 +++ engine/apps/mobile_app/alert_rendering.py | 5 +++ .../tests/tasks/test_new_alert_group.py | 36 +++++++++++++++++++ 3 files changed, 45 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 915fa4c739..5c17a65095 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add error code for mobile push notification logs when device is not set up @Ferril ([#3554](https://github.com/grafana/oncall/pull/3554)) +### Fixed + +- Fix issue when mobile push notification message is too big @Ferril ([#3556](https://github.com/grafana/oncall/pull/3556) + ## v1.3.77 (2023-12-11) ### Fixed diff --git a/engine/apps/mobile_app/alert_rendering.py b/engine/apps/mobile_app/alert_rendering.py index ef5082db39..37453c15c0 100644 --- a/engine/apps/mobile_app/alert_rendering.py +++ b/engine/apps/mobile_app/alert_rendering.py @@ -10,9 +10,14 @@ def _render_for(self): def get_push_notification_subtitle(alert_group): + MAX_ALERT_TITLE_LENGTH = 200 alert = alert_group.alerts.first() templated_alert = AlertMobileAppTemplater(alert).render() alert_title = str_or_backup(templated_alert.title, "Alert Group") + # limit alert title length to prevent FCM `message is too big` exception + # https://firebase.google.com/docs/cloud-messaging/concept-options#notifications_and_data_messages + if len(alert_title) > MAX_ALERT_TITLE_LENGTH: + alert_title = f"{alert_title[:MAX_ALERT_TITLE_LENGTH]}..." status_verbose = "Firing" # TODO: we should probably de-duplicate this text if alert_group.resolved: diff --git a/engine/apps/mobile_app/tests/tasks/test_new_alert_group.py b/engine/apps/mobile_app/tests/tasks/test_new_alert_group.py index 1800753f71..fec871e11e 100644 --- a/engine/apps/mobile_app/tests/tasks/test_new_alert_group.py +++ b/engine/apps/mobile_app/tests/tasks/test_new_alert_group.py @@ -2,7 +2,9 @@ import pytest +from apps.alerts.incident_appearance.templaters.alert_templater import TemplatedAlert from apps.base.models import UserNotificationPolicy, UserNotificationPolicyLogRecord +from apps.mobile_app.alert_rendering import AlertMobileAppTemplater, get_push_notification_subtitle from apps.mobile_app.models import FCMDevice, MobileAppUserSettings from apps.mobile_app.tasks.new_alert_group import _get_fcm_message, notify_user_about_new_alert_group @@ -175,3 +177,37 @@ def test_fcm_message_user_settings_critical_override_dnd_disabled( apns_sound = message.apns.payload.aps.sound assert apns_sound.critical is False assert message.apns.payload.aps.custom_data["interruption-level"] == "time-sensitive" + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "alert_title", + [ + "Some short title", + "Some long title" * 100, + ], +) +def test_get_push_notification_subtitle( + alert_title, + make_organization_and_user, + make_alert_receive_channel, + make_alert_group, + make_alert, +): + MAX_ALERT_TITLE_LENGTH = 200 + organization, user = make_organization_and_user() + alert_receive_channel = make_alert_receive_channel(organization=organization) + alert_group = make_alert_group(alert_receive_channel) + make_alert(alert_group=alert_group, title=alert_title, raw_request_data={"title": alert_title}) + expected_alert_title = ( + f"{alert_title[:MAX_ALERT_TITLE_LENGTH]}..." if len(alert_title) > MAX_ALERT_TITLE_LENGTH else alert_title + ) + expected_result = ( + f"#1 {expected_alert_title}\n" + f"via {alert_group.channel.short_name}" + "\nStatus: Firing, alerts: 1" + ) + templated_alert = TemplatedAlert() + templated_alert.title = alert_title + with patch.object(AlertMobileAppTemplater, "render", return_value=templated_alert): + result = get_push_notification_subtitle(alert_group) + assert len(expected_alert_title) <= MAX_ALERT_TITLE_LENGTH + 3 + assert result == expected_result