Skip to content

Commit

Permalink
Fix message is too big exception for mobile push notification (#3556)
Browse files Browse the repository at this point in the history
# What this PR does
Adds limit for alert title length in mobile app push notifications
## Which issue(s) this PR fixes
grafana/oncall-private#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)
  • Loading branch information
Ferril authored Dec 12, 2023
1 parent 0861113 commit e003e8a
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 0 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions engine/apps/mobile_app/alert_rendering.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
36 changes: 36 additions & 0 deletions engine/apps/mobile_app/tests/tasks/test_new_alert_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

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

0 comments on commit e003e8a

Please sign in to comment.