Skip to content

Commit

Permalink
Add multi-stack support for mobile app (#3500)
Browse files Browse the repository at this point in the history
# What this PR does
Allow creating multiple mobile devices with same `registration_id` for
different users (multi-stack support)

## Which issue(s) this PR fixes
#3452

## 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 13, 2023
1 parent 1ac39c2 commit 088414c
Show file tree
Hide file tree
Showing 15 changed files with 356 additions and 24 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## Unreleased

### Added

- Add backend for multi-stack support for mobile-app @Ferril ([#3500](https://github.com/grafana/oncall/pull/3500))

## v1.3.78 (2023-12-12)

### Changed
Expand Down
5 changes: 3 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 construct_fcm_message, send_push_notification
from apps.mobile_app.utils import add_stack_slug_to_message_title, construct_fcm_message, send_push_notification
from apps.user_management.models import User

if typing.TYPE_CHECKING:
Expand Down Expand Up @@ -47,7 +47,8 @@ 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": get_test_push_title(critical),
"title": add_stack_slug_to_message_title(get_test_push_title(critical), user.organization),
"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(
MessageType.DEFAULT, Platform.ANDROID
Expand Down
37 changes: 37 additions & 0 deletions engine/apps/mobile_app/serializers.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import typing

from fcm_django.api.rest_framework import FCMDeviceSerializer as BaseFCMDeviceSerializer
from rest_framework import serializers
from rest_framework.serializers import ValidationError

from apps.mobile_app.models import MobileAppUserSettings
from common.api_helpers.custom_fields import TimeZoneField
Expand Down Expand Up @@ -43,3 +45,38 @@ def validate_going_oncall_notification_timing(
if option not in notification_timing_options:
raise serializers.ValidationError(detail="invalid timing options")
return going_oncall_notification_timing


class FCMDeviceSerializer(BaseFCMDeviceSerializer):
def validate(self, attrs):
"""
Overrides `validate` method from BaseFCMDeviceSerializer to allow different users have same device
`registration_id` (multi-stack support).
Removed deactivating devices with the same `registration_id` during validation.
"""
devices = None
request_method = None
request = self.context["request"]

if self.initial_data.get("registration_id", None):
request_method = "update" if self.instance else "create"
else:
if request.method in ["PUT", "PATCH"]:
request_method = "update"
elif request.method == "POST":
request_method = "create"

Device = self.Meta.model
# unique together with registration_id and user
user = request.user
registration_id = attrs.get("registration_id")

if request_method == "update":
if registration_id:
devices = Device.objects.filter(registration_id=registration_id, user=user).exclude(id=self.instance.id)
elif request_method == "create":
devices = Device.objects.filter(user=user, registration_id=registration_id)

if devices:
raise ValidationError({"registration_id": "This field must be unique per us."})
return attrs
10 changes: 8 additions & 2 deletions engine/apps/mobile_app/tasks/going_oncall_notification.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@
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, construct_fcm_message, send_push_notification
from apps.mobile_app.utils import (
MAX_RETRIES,
add_stack_slug_to_message_title,
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 @@ -72,8 +77,9 @@ def _get_fcm_message(
notification_subtitle = _get_notification_subtitle(schedule, schedule_event, mobile_app_user_settings)

data: FCMMessageData = {
"title": notification_title,
"title": add_stack_slug_to_message_title(notification_title, user.organization),
"subtitle": notification_subtitle,
"orgName": user.organization.stack_slug,
"info_notification_sound_name": mobile_app_user_settings.get_notification_sound_name(
MessageType.INFO, Platform.ANDROID
),
Expand Down
9 changes: 7 additions & 2 deletions engine/apps/mobile_app/tasks/new_alert_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@
from apps.alerts.models import AlertGroup
from apps.mobile_app.alert_rendering import get_push_notification_subtitle
from apps.mobile_app.types import FCMMessageData, MessageType, Platform
from apps.mobile_app.utils import MAX_RETRIES, construct_fcm_message, send_push_notification
from apps.mobile_app.utils import (
MAX_RETRIES,
add_stack_slug_to_message_title,
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 @@ -41,7 +46,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": alert_title,
"title": add_stack_slug_to_message_title(alert_title, alert_group.channel.organization),
"subtitle": alert_subtitle,
"orgId": alert_group.channel.organization.public_primary_key,
"orgName": alert_group.channel.organization.stack_slug,
Expand Down
10 changes: 8 additions & 2 deletions engine/apps/mobile_app/tasks/new_shift_swap_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,12 @@
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, construct_fcm_message, send_push_notification
from apps.mobile_app.utils import (
MAX_RETRIES,
add_stack_slug_to_message_title,
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 @@ -116,8 +121,9 @@ def _get_fcm_message(
route = f"/schedules/{shift_swap_request.schedule.public_primary_key}/ssrs/{shift_swap_request.public_primary_key}"

data: FCMMessageData = {
"title": notification_title,
"title": add_stack_slug_to_message_title(notification_title, user.organization),
"subtitle": notification_subtitle,
"orgName": user.organization.stack_slug,
"route": route,
"info_notification_sound_name": mobile_app_user_settings.get_notification_sound_name(
MessageType.INFO, Platform.ANDROID
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
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 @@ -182,6 +183,13 @@ def test_get_fcm_message(
make_user_for_organization,
make_schedule,
):
organization = make_organization()
user_tz = "Europe/Amsterdam"
user = make_user_for_organization(organization)
user_pk = user.public_primary_key
schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb)
notification_thread_id = f"{schedule.public_primary_key}:{user_pk}:going-oncall"

mock_fcm_message = "mncvmnvcmnvcnmvcmncvmn"
mock_notification_title = "asdfasdf"
mock_notification_subtitle = "9:06\u202fAM - 9:06\u202fAM\nSchedule XYZ"
Expand All @@ -192,13 +200,6 @@ def test_get_fcm_message(
mock_get_notification_title.return_value = mock_notification_title
mock_get_notification_subtitle.return_value = mock_notification_subtitle

organization = make_organization()
user_tz = "Europe/Amsterdam"
user = make_user_for_organization(organization)
user_pk = user.public_primary_key
schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb)
notification_thread_id = f"{schedule.public_primary_key}:{user_pk}:going-oncall"

schedule_event = _create_schedule_event(
timezone.now(),
timezone.now(),
Expand All @@ -214,8 +215,9 @@ def test_get_fcm_message(
maus = MobileAppUserSettings.objects.create(user=user, time_zone=user_tz)

data = {
"title": mock_notification_title,
"title": add_stack_slug_to_message_title(mock_notification_title, organization),
"subtitle": mock_notification_subtitle,
"orgName": organization.stack_slug,
"info_notification_sound_name": maus.get_notification_sound_name(MessageType.INFO, Platform.ANDROID),
"info_notification_volume_type": maus.info_notification_volume_type,
"info_notification_volume": str(maus.info_notification_volume),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
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 @@ -268,7 +269,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"] == "New shift swap request"
assert message.data["title"] == add_stack_slug_to_message_title("New shift swap request", organization)
assert message.data["subtitle"] == "John Doe, Test Schedule"
assert (
message.data["route"]
Expand Down Expand Up @@ -467,7 +468,9 @@ 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"] == "Your shift swap request has been taken"
assert message.data["title"] == add_stack_slug_to_message_title(
"Your shift swap request has been taken", organization
)
assert message.data["subtitle"] == schedule_name
assert (
message.data["route"]
Expand Down
7 changes: 4 additions & 3 deletions engine/apps/mobile_app/tests/test_demo_push.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

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 @@ -33,7 +34,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"] == 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["type"] == "oncall.message"


Expand Down Expand Up @@ -67,7 +68,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"] == 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["type"] == "oncall.critical_message"


Expand All @@ -93,4 +94,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"] == get_test_push_title(critical=True)
assert message.data["title"] == add_stack_slug_to_message_title(get_test_push_title(critical=True), organization)
Loading

0 comments on commit 088414c

Please sign in to comment.