Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make mobile app notification title and subtitle templatable #3845

Merged
merged 4 commits into from
Feb 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed

- Allow mobile app to access escalation options endpoints @imtoori ([#3847](https://github.com/grafana/oncall/pull/3847))
- Enable templating for alert escalation mobile app push notifications by @joeyorlando ([#3845](https://github.com/grafana/oncall/pull/3845))

## v1.3.102 (2024-02-06)

Expand Down
1 change: 1 addition & 0 deletions docs/sources/jinja2-templating/_index.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ How alerts are displayed in the UI, messengers, and notifications
- `Title` for SMS
- `Title` for Phone Call
- `Title`, `Message` for Email
- `Title`, `Message` for Mobile app push notifications

#### Behavioral templates

Expand Down
14 changes: 12 additions & 2 deletions docs/sources/mobile-app/push-notifications/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ There are four types of push notifications for the mobile app:
To receive push notifications from the Grafana OnCall mobile app, you must add them to your notification policy steps.
**Important notifications** should include **Mobile push important** and **Default notifications** should include **Mobile push**.

In the **Settings** tab of the mobile app, tap on **Notification policies** to review, reorder, remove, add or change steps.
In the **Settings** tab of the mobile app, tap on **Notification policies** to review, reorder, remove, add or change steps.
Alternatively, you can do the same on desktop. From Grafana OnCall, navigate to the **Users** page, click **View my profile** and navigate to the **User Info** tab.

<img src="/static/img/oncall/mobile-app-v1-android-notification-policies.png" width="300px">
Expand Down Expand Up @@ -87,13 +87,23 @@ To enable or disable on-call shift notifications, use the **info notifications**

### Shift swap notifications

Shift swap notifications are generated when a [shift swap ][shift-swaps] is requested,
Shift swap notifications are generated when a [shift swap][shift-swaps] is requested,
informing all users in the on-call schedule (except the initiator) about it.

To enable or disable shift swap notifications and their follow-ups, use the **info notifications** section
in the **Push notifications** settings.

## Templating of alert notifications

It is possible to modify the title and body (or subtitle), for push notifications related to alert escalations. For
more information on how to do this see the [docs on Appearance templates][templating].

<img src="/static/img/oncall/mobile-app-alert-notification-custom-template.png" width="400px">

{{% docs/reference %}}
[shift-swaps]: "/docs/oncall/ -> /docs/oncall/<ONCALL VERSION>/on-call-schedules/shift-swaps"
[shift-swaps]: "/docs/grafana-cloud/ -> /docs/grafana-cloud/alerting-and-irm/oncall/on-call-schedules/shift-swaps"

[templating]: "/docs/oncall/ -> /docs/oncall/<ONCALL VERSION>/jinja2-templating#appearance-templates"
[templating]: "/docs/grafana-cloud/ -> /docs/grafana-cloud/alerting-and-irm/oncall/jinja2-templating#appearance-templates"
{{% /docs/reference %}}
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,10 @@ def _preformat_request_data(self, request_data):
preformatted_data = request_data
return preformatted_data

def _preformat(self, data):
def _preformat(self, data: str) -> str:
return data

def _postformat(self, templated_alert):
def _postformat(self, templated_alert: TemplatedAlert) -> TemplatedAlert:
return templated_alert

def _apply_templates(self, data):
Expand Down
2 changes: 1 addition & 1 deletion engine/apps/api/serializers/alert_receive_channel.py
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,7 @@ def _get_messaging_backend_templates(self, obj: "AlertReceiveChannel"):
is_default = False
if obj.messaging_backends_templates:
value = obj.messaging_backends_templates.get(backend_id, {}).get(field)
if not value:
if not value and not backend.skip_default_template_fields:
value = obj.get_default_template_attribute(backend_id, field)
is_default = True
field_name = f"{backend.slug}_{field}_template"
Expand Down
1 change: 1 addition & 0 deletions engine/apps/base/messaging.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ class BaseMessagingBackend:

templater = None
template_fields = ("title", "message", "image_url")
skip_default_template_fields = False
Copy link
Contributor Author

@joeyorlando joeyorlando Feb 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this to avoid having these auto-added as the default template values (see usage here):
Screenshot 2024-02-08 at 15 44 48
(this is a screenshot of the Web templates but the same values were showing up by default for Mobile app templates (even though the templates for these fields in the alert_receive_channel.messaging_backends_templates object were not set))


def __init__(self, *args, **kwargs):
self.notification_channel_id = kwargs.get("notification_channel_id")
Expand Down
50 changes: 42 additions & 8 deletions engine/apps/mobile_app/alert_rendering.py
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with the latest commit, c071ce6, labels cannot be (easily w/o decent refactor/hackery) used in the templates here because I am using the MessagingBackend/AlertTemplater approach. AlertTemplater will need to be refactored to support the concept of labels.

Otherwise 27ecdd4 supports the usage of labels in the templates (but as a caveat adds two new columns to the AlertReceiveChannel table, as it does not use the MessagingBackend approach)

Original file line number Diff line number Diff line change
@@ -1,23 +1,57 @@
import typing

from emoji import emojize

from apps.alerts.incident_appearance.templaters.alert_templater import AlertTemplater
from apps.alerts.incident_appearance.templaters.alert_templater import AlertTemplater, TemplatedAlert
from apps.alerts.models import AlertGroup
from common.utils import str_or_backup


def _validate_fcm_length_limit(value: typing.Optional[str]) -> str:
"""
NOTE: technically FCM limits the data we send based on total # of bytes, not characters for title/subtitle. For now
lets simply limit the title and subtitle to 200 characters and see how that goes with avoiding the `message is too big`
FCM exception
https://firebase.google.com/docs/reference/fcm/rest/v1/ErrorCode
"""
MAX_ALERT_TITLE_LENGTH = 200

if value is None:
return ""
return f"{value[:MAX_ALERT_TITLE_LENGTH]}..." if len(value) > MAX_ALERT_TITLE_LENGTH else value


class AlertMobileAppTemplater(AlertTemplater):
def _render_for(self):
return "MOBILE_APP"

def _postformat(self, templated_alert: TemplatedAlert) -> TemplatedAlert:
templated_alert.title = _validate_fcm_length_limit(templated_alert.title)
templated_alert.message = _validate_fcm_length_limit(templated_alert.message)
return templated_alert


def _templatize_alert(alert_group: AlertGroup) -> TemplatedAlert:
alert = alert_group.alerts.first()
return AlertMobileAppTemplater(alert).render()


def get_push_notification_title(alert_group: AlertGroup, critical: bool) -> str:
return _templatize_alert(alert_group).title or ("New Important Alert" if critical else "New Alert")


def get_push_notification_subtitle(alert_group: AlertGroup) -> str:
templatized_subtitle = _templatize_alert(alert_group).message
if templatized_subtitle:
# only return the templatized subtitle if it resolves to something that is not None
# otherwise fallback to the default
return templatized_subtitle

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]}..."

alert_title = _validate_fcm_length_limit(str_or_backup(templated_alert.title, "Alert Group"))

status_verbose = "Firing" # TODO: we should probably de-duplicate this text
if alert_group.resolved:
Expand Down
12 changes: 4 additions & 8 deletions engine/apps/mobile_app/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ class MobileAppBackend(BaseMessagingBackend):
label = "Mobile push"
short_label = "Mobile push"
available_for_use = True
template_fields = ["title"]

templater = "apps.mobile_app.alert_rendering.AlertMobileAppTemplater"
template_fields = ("title", "message")
skip_default_template_fields = True

def generate_user_verification_code(self, user):
from apps.mobile_app.models import MobileAppVerificationToken
Expand Down Expand Up @@ -51,13 +54,6 @@ def notify_user(self, user, alert_group, notification_policy, critical=False):
critical=critical,
)

@property
def customizable_templates(self):
"""
Disable customization if templates for mobile app
"""
return False


class MobileAppCriticalBackend(MobileAppBackend):
"""
Expand Down
4 changes: 2 additions & 2 deletions engine/apps/mobile_app/tasks/new_alert_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from firebase_admin.messaging import APNSPayload, Aps, ApsAlert, CriticalSound, Message

from apps.alerts.models import AlertGroup
from apps.mobile_app.alert_rendering import get_push_notification_subtitle
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,
Expand All @@ -31,7 +31,7 @@ def _get_fcm_message(alert_group: AlertGroup, user: User, device_to_notify: "FCM

thread_id = f"{alert_group.channel.organization.public_primary_key}:{alert_group.public_primary_key}"

alert_title = "New Important Alert" if critical else "New Alert"
alert_title = get_push_notification_title(alert_group, critical)
alert_subtitle = get_push_notification_subtitle(alert_group)

mobile_app_user_settings, _ = MobileAppUserSettings.objects.get_or_create(user=user)
Expand Down
36 changes: 0 additions & 36 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,9 +2,7 @@

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 @@ -219,37 +217,3 @@ 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
Loading
Loading