From 572131b921b0a22664048ab6cdd7167bec2c5b52 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Wed, 14 Jun 2023 18:19:58 +0200 Subject: [PATCH] add user locale field to mobile app user settings table + change going on call push notification text (#2131) # What this PR does - add user locale field to mobile app user settings table + add a test that sends `PATCH` requests to this endpoint - change "you're going on call" push notification text to include localized shift time. The general format is now: ```python f"You're going on call in {time_until_going_oncall} for schedule {schedule.name}, {formatted_shift}" ``` - `time_until_going_oncall` is a "human-readable" format of the time until the start) - `schedule.name` is self-explanatory - `formatted_shift` this depends on the shift. If the shift starts and ends on the same day, the format will be "HH:mm - HH:mm". Otherwise, if the shift starts and ends on different days, the format will be "YYYY-MM-DD HH:mm - YYYY-MM-DD HH:mm". **Note** that all datetime related formatting will use the new `locale` field that we are now storing in the mobile app user settings table. If no locale is yet present we will fallback to "en" ## Which issue(s) this PR fixes closes https://github.com/grafana/oncall/issues/2024 https://github.com/grafana/oncall-mobile-app/issues/187 ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [ ] 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 | 8 + .../0008_mobileappusersettings_locale.py | 18 ++ engine/apps/mobile_app/models.py | 2 + engine/apps/mobile_app/serializers.py | 1 + engine/apps/mobile_app/tasks.py | 34 ++- .../mobile_app/tests/test_user_settings.py | 41 ++++ .../test_your_going_oncall_notification.py | 199 ++++++++++++++++-- engine/common/l10n.py | 34 +++ engine/common/tests/test_l10n.py | 27 +++ engine/requirements.txt | 1 + 10 files changed, 346 insertions(+), 19 deletions(-) create mode 100644 engine/apps/mobile_app/migrations/0008_mobileappusersettings_locale.py create mode 100644 engine/common/l10n.py create mode 100644 engine/common/tests/test_l10n.py diff --git a/CHANGELOG.md b/CHANGELOG.md index ee689151e9..9cc744d0c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,14 @@ 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 `locale` column to mobile app user settings table by @joeyorlando [#2131](https://github.com/grafana/oncall/pull/2131) +- Update notification text for "You're going on call" push notifications to include information about the shift start + and end times by @joeyorlando ([#2131](https://github.com/grafana/oncall/pull/2131)) + ## v1.2.44 (2023-06-14) ### Added diff --git a/engine/apps/mobile_app/migrations/0008_mobileappusersettings_locale.py b/engine/apps/mobile_app/migrations/0008_mobileappusersettings_locale.py new file mode 100644 index 0000000000..f3fefa89fd --- /dev/null +++ b/engine/apps/mobile_app/migrations/0008_mobileappusersettings_locale.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.19 on 2023-06-08 10:51 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('mobile_app', '0007_alter_mobileappusersettings_info_notifications_enabled'), + ] + + operations = [ + migrations.AddField( + model_name='mobileappusersettings', + name='locale', + field=models.CharField(max_length=50, null=True), + ), + ] diff --git a/engine/apps/mobile_app/models.py b/engine/apps/mobile_app/models.py index 4fb7342fa4..7d45692e17 100644 --- a/engine/apps/mobile_app/models.py +++ b/engine/apps/mobile_app/models.py @@ -141,3 +141,5 @@ class VolumeType(models.TextChoices): going_oncall_notification_timing = models.IntegerField( choices=NOTIFICATION_TIMING_CHOICES, default=TWELVE_HOURS_IN_SECONDS ) + + locale = models.CharField(max_length=50, null=True) diff --git a/engine/apps/mobile_app/serializers.py b/engine/apps/mobile_app/serializers.py index d10dcff966..93b72f62a4 100644 --- a/engine/apps/mobile_app/serializers.py +++ b/engine/apps/mobile_app/serializers.py @@ -22,4 +22,5 @@ class Meta: "important_notification_override_dnd", "info_notifications_enabled", "going_oncall_notification_timing", + "locale", ) diff --git a/engine/apps/mobile_app/tasks.py b/engine/apps/mobile_app/tasks.py index 29e2528dd1..60be966f46 100644 --- a/engine/apps/mobile_app/tasks.py +++ b/engine/apps/mobile_app/tasks.py @@ -24,6 +24,7 @@ from apps.user_management.models import User from common.api_helpers.utils import create_engine_url from common.custom_celery_tasks import shared_dedicated_queue_retry_task +from common.l10n import format_localized_datetime, format_localized_time if typing.TYPE_CHECKING: from apps.mobile_app.models import MobileAppUserSettings @@ -225,8 +226,33 @@ def _get_alert_group_escalation_fcm_message( return _construct_fcm_message(message_type, device_to_notify, thread_id, fcm_message_data, apns_payload) +def _get_youre_going_oncall_notification_title( + schedule: OnCallSchedule, + seconds_until_going_oncall: int, + schedule_event: ScheduleEvent, + mobile_app_user_settings: "MobileAppUserSettings", +) -> str: + time_until_going_oncall = humanize.naturaldelta(seconds_until_going_oncall) + + shift_start = schedule_event["start"] + shift_end = schedule_event["end"] + shift_starts_and_ends_on_same_day = shift_start.date() == shift_end.date() + dt_formatter_func = format_localized_time if shift_starts_and_ends_on_same_day else format_localized_datetime + + def _format_datetime(dt): + return dt_formatter_func(dt, mobile_app_user_settings.locale) + + formatted_shift = f"{_format_datetime(shift_start)} - {_format_datetime(shift_end)}" + + return f"You're going on call in {time_until_going_oncall} for schedule {schedule.name}, {formatted_shift}" + + def _get_youre_going_oncall_fcm_message( - user: User, schedule: OnCallSchedule, device_to_notify: FCMDevice, seconds_until_going_oncall: int + user: User, + schedule: OnCallSchedule, + device_to_notify: FCMDevice, + seconds_until_going_oncall: int, + schedule_event: ScheduleEvent, ) -> Message: # avoid circular import from apps.mobile_app.models import MobileAppUserSettings @@ -235,8 +261,8 @@ def _get_youre_going_oncall_fcm_message( mobile_app_user_settings, _ = MobileAppUserSettings.objects.get_or_create(user=user) - notification_title = ( - f"You are going on call in {humanize.naturaldelta(seconds_until_going_oncall)} for schedule {schedule.name}" + notification_title = _get_youre_going_oncall_notification_title( + schedule, seconds_until_going_oncall, schedule_event, mobile_app_user_settings ) data: FCMMessageData = { @@ -446,7 +472,7 @@ def conditionally_send_going_oncall_push_notifications_for_schedule(schedule_pk) if seconds_until_going_oncall is not None and not already_sent_this_push_notification: message = _get_youre_going_oncall_fcm_message( - user, schedule, device_to_notify, seconds_until_going_oncall + user, schedule, device_to_notify, seconds_until_going_oncall, schedule_event ) _send_push_notification(device_to_notify, message) cache.set(cache_key, True, PUSH_NOTIFICATION_TRACKING_CACHE_KEY_TTL) diff --git a/engine/apps/mobile_app/tests/test_user_settings.py b/engine/apps/mobile_app/tests/test_user_settings.py index 3f15fefaf9..043b052123 100644 --- a/engine/apps/mobile_app/tests/test_user_settings.py +++ b/engine/apps/mobile_app/tests/test_user_settings.py @@ -31,6 +31,7 @@ def test_user_settings_get(make_organization_and_user_with_mobile_app_auth_token "important_notification_override_dnd": True, "info_notifications_enabled": False, "going_oncall_notification_timing": 43200, + "locale": None, } @@ -67,6 +68,7 @@ def test_user_settings_put( "important_notification_override_dnd": False, "info_notifications_enabled": True, "going_oncall_notification_timing": going_oncall_notification_timing, + "locale": "ca_FR", } response = client.put(url, data=data, format="json", HTTP_AUTHORIZATION=auth_token) @@ -75,3 +77,42 @@ def test_user_settings_put( if expected_status_code == status.HTTP_200_OK: # Check the values are updated correctly assert response.json() == data + + +@pytest.mark.django_db +def test_user_settings_patch(make_organization_and_user_with_mobile_app_auth_token): + _, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() + + original_default_notification_sound_name = "test_default" + patch_default_notification_sound_name = "test_default_patched" + + client = APIClient() + url = reverse("mobile_app:user_settings") + data = { + "default_notification_sound_name": original_default_notification_sound_name, + "default_notification_volume_type": "intensifying", + "default_notification_volume": 1, + "default_notification_volume_override": True, + "info_notification_sound_name": "default_sound", + "info_notification_volume_type": "constant", + "info_notification_volume": 0.8, + "info_notification_volume_override": False, + "important_notification_sound_name": "test_important", + "important_notification_volume_type": "intensifying", + "important_notification_volume": 1, + "important_notification_volume_override": False, + "important_notification_override_dnd": False, + "info_notifications_enabled": True, + } + + response = client.put(url, data=data, format="json", HTTP_AUTHORIZATION=auth_token) + original_settings = response.json() + + assert response.status_code == status.HTTP_200_OK + + patch_data = {"default_notification_sound_name": patch_default_notification_sound_name} + response = client.patch(url, data=patch_data, format="json", HTTP_AUTHORIZATION=auth_token) + + assert response.status_code == status.HTTP_200_OK + # all original settings should stay the same, only data set in PATCH call should get updated + assert response.json() == {**original_settings, **patch_data} diff --git a/engine/apps/mobile_app/tests/test_your_going_oncall_notification.py b/engine/apps/mobile_app/tests/test_your_going_oncall_notification.py index 9cd99a0337..586cc913d8 100644 --- a/engine/apps/mobile_app/tests/test_your_going_oncall_notification.py +++ b/engine/apps/mobile_app/tests/test_your_going_oncall_notification.py @@ -1,3 +1,4 @@ +import json import typing from unittest import mock @@ -25,9 +26,9 @@ def clear_cache(): def _create_schedule_event( - start_time: timezone.datetime, shift_pk: str, users: typing.List[ScheduleEventUser] + start_time: timezone.datetime, end_time: timezone.datetime, shift_pk: str, users: typing.List[ScheduleEventUser] ) -> ScheduleEvent: - return {"start": start_time, "shift": {"pk": shift_pk}, "users": users} + return {"start": start_time, "end": end_time, "shift": {"pk": shift_pk}, "users": users} @pytest.mark.parametrize( @@ -47,6 +48,171 @@ def test_shift_starts_within_range(timing_window_lower, timing_window_upper, sec ) +@pytest.mark.django_db +def test_get_youre_going_oncall_notification_title(make_organization_and_user, make_user, make_schedule): + schedule_name = "asdfasdfasdfasdf" + + organization, user = make_organization_and_user() + user2 = make_user(organization=organization) + schedule = make_schedule(organization, name=schedule_name, schedule_class=OnCallScheduleWeb) + shift_pk = "mncvmnvc" + user_pk = user.public_primary_key + user_locale = "fr_CA" + seconds_until_going_oncall = 600 + humanized_time_until_going_oncall = "10 minutes" + + same_day_shift_start = timezone.datetime(2023, 7, 8, 9, 0, 0) + same_day_shift_end = timezone.datetime(2023, 7, 8, 17, 0, 0) + + multiple_day_shift_start = timezone.datetime(2023, 7, 8, 9, 0, 0) + multiple_day_shift_end = timezone.datetime(2023, 7, 12, 17, 0, 0) + + same_day_shift = _create_schedule_event( + same_day_shift_start, + same_day_shift_end, + shift_pk, + [ + { + "pk": user_pk, + }, + ], + ) + + multiple_day_shift = _create_schedule_event( + multiple_day_shift_start, + multiple_day_shift_end, + shift_pk, + [ + { + "pk": user_pk, + }, + ], + ) + + maus = MobileAppUserSettings.objects.create(user=user, locale=user_locale) + maus_no_locale = MobileAppUserSettings.objects.create(user=user2) + + ################## + # same day shift + ################## + same_day_shift_title = tasks._get_youre_going_oncall_notification_title( + schedule, seconds_until_going_oncall, same_day_shift, maus + ) + same_day_shift_no_locale_title = tasks._get_youre_going_oncall_notification_title( + schedule, seconds_until_going_oncall, same_day_shift, maus_no_locale + ) + + assert ( + same_day_shift_title + == f"You're going on call in {humanized_time_until_going_oncall} for schedule {schedule_name}, 09 h 00 - 17 h 00" + ) + assert ( + same_day_shift_no_locale_title + == f"You're going on call in {humanized_time_until_going_oncall} for schedule {schedule_name}, 9:00\u202fAM - 5:00\u202fPM" + ) + + ################## + # multiple day shift + ################## + multiple_day_shift_title = tasks._get_youre_going_oncall_notification_title( + schedule, seconds_until_going_oncall, multiple_day_shift, maus + ) + multiple_day_shift_no_locale_title = tasks._get_youre_going_oncall_notification_title( + schedule, seconds_until_going_oncall, multiple_day_shift, maus_no_locale + ) + + assert ( + multiple_day_shift_title + == f"You're going on call in {humanized_time_until_going_oncall} for schedule {schedule_name}, 2023-07-08 09 h 00 - 2023-07-12 17 h 00" + ) + assert ( + multiple_day_shift_no_locale_title + == f"You're going on call in {humanized_time_until_going_oncall} for schedule {schedule_name}, 7/8/23, 9:00\u202fAM - 7/12/23, 5:00\u202fPM" + ) + + +@mock.patch("apps.mobile_app.tasks._get_youre_going_oncall_notification_title") +@mock.patch("apps.mobile_app.tasks._construct_fcm_message") +@mock.patch("apps.mobile_app.tasks.APNSPayload") +@mock.patch("apps.mobile_app.tasks.Aps") +@mock.patch("apps.mobile_app.tasks.ApsAlert") +@mock.patch("apps.mobile_app.tasks.CriticalSound") +@pytest.mark.django_db +def test_get_youre_going_oncall_fcm_message( + mock_critical_sound, + mock_aps_alert, + mock_aps, + mock_apns_payload, + mock_construct_fcm_message, + mock_get_youre_going_oncall_notification_title, + make_organization_and_user, + make_schedule, +): + mock_fcm_message = "mncvmnvcmnvcnmvcmncvmn" + mock_notification_title = "asdfasdf" + shift_pk = "mncvmnvc" + seconds_until_going_oncall = 600 + + mock_construct_fcm_message.return_value = mock_fcm_message + mock_get_youre_going_oncall_notification_title.return_value = mock_notification_title + + organization, user = make_organization_and_user() + 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(), + shift_pk, + [ + { + "pk": user_pk, + }, + ], + ) + + device = FCMDevice.objects.create(user=user) + maus = MobileAppUserSettings.objects.create(user=user) + + data = { + "title": mock_notification_title, + "info_notification_sound_name": ( + maus.info_notification_sound_name + MobileAppUserSettings.ANDROID_SOUND_NAME_EXTENSION + ), + "info_notification_volume_type": maus.info_notification_volume_type, + "info_notification_volume": str(maus.info_notification_volume), + "info_notification_volume_override": json.dumps(maus.info_notification_volume_override), + } + + fcm_message = tasks._get_youre_going_oncall_fcm_message( + user, schedule, device, seconds_until_going_oncall, schedule_event + ) + + assert fcm_message == mock_fcm_message + + mock_aps_alert.assert_called_once_with(title=mock_notification_title) + mock_critical_sound.assert_called_once_with( + critical=False, name=maus.info_notification_sound_name + MobileAppUserSettings.IOS_SOUND_NAME_EXTENSION + ) + mock_aps.assert_called_once_with( + thread_id=notification_thread_id, + alert=mock_aps_alert.return_value, + sound=mock_critical_sound.return_value, + custom_data={ + "interruption-level": "time-sensitive", + }, + ) + mock_apns_payload.assert_called_once_with(aps=mock_aps.return_value) + + mock_get_youre_going_oncall_notification_title.assert_called_once_with( + schedule, seconds_until_going_oncall, schedule_event, maus + ) + mock_construct_fcm_message.assert_called_once_with( + tasks.MessageType.INFO, device, notification_thread_id, data, mock_apns_payload.return_value + ) + + @pytest.mark.parametrize( "info_notifications_enabled,now,going_oncall_notification_timing,schedule_start,expected", [ @@ -162,7 +328,7 @@ def test_should_we_send_going_oncall_push_notification( assert ( tasks.should_we_send_going_oncall_push_notification( - now, user_mobile_settings, _create_schedule_event(schedule_start, "12345", []) + now, user_mobile_settings, _create_schedule_event(schedule_start, schedule_start, "12345", []) ) == expected ) @@ -205,17 +371,18 @@ def test_conditionally_send_going_oncall_push_notifications_for_schedule( shift_pk = "mncvmnvc" user_pk = user.public_primary_key mock_fcm_message = {"foo": "bar"} - final_events = [ - _create_schedule_event( - timezone.now(), - shift_pk, - [ - { - "pk": user_pk, - }, - ], - ), - ] + + schedule_event = _create_schedule_event( + timezone.now(), + timezone.now(), + shift_pk, + [ + { + "pk": user_pk, + }, + ], + ) + final_events = [schedule_event] seconds_until_shift_starts = 58989 mock_get_youre_going_oncall_fcm_message.return_value = mock_fcm_message @@ -237,7 +404,9 @@ def test_conditionally_send_going_oncall_push_notifications_for_schedule( tasks.conditionally_send_going_oncall_push_notifications_for_schedule(schedule.pk) - mock_get_youre_going_oncall_fcm_message.assert_called_once_with(user, schedule, device, seconds_until_shift_starts) + mock_get_youre_going_oncall_fcm_message.assert_called_once_with( + user, schedule, device, seconds_until_shift_starts, schedule_event + ) mock_send_push_notification.assert_called_once_with(device, mock_fcm_message) assert cache.get(cache_key) is True diff --git a/engine/common/l10n.py b/engine/common/l10n.py new file mode 100644 index 0000000000..998529d5a6 --- /dev/null +++ b/engine/common/l10n.py @@ -0,0 +1,34 @@ +import logging +import typing + +from babel.core import UnknownLocaleError +from babel.dates import format_datetime, format_time +from django.utils import timezone + +logger = logging.getLogger(__name__) + +FALLBACK_LOCALE = "en" + + +def _format_dt( + func: typing.Callable[[timezone.datetime, typing.Optional[str]], str], + dt: timezone.datetime, + locale: typing.Optional[str], +) -> str: + format = "short" + try: + # can't pass in locale of None otherwise TypeError is raised + return func(dt, format=format, locale=locale if locale else FALLBACK_LOCALE) + except UnknownLocaleError: + logger.warning( + f"babel.core.UnknownLocaleError encountered, locale={locale}. Will retry with fallback locale of {FALLBACK_LOCALE}" + ) + return func(dt, format=format, locale=FALLBACK_LOCALE) + + +def format_localized_datetime(dt: timezone.datetime, locale: typing.Optional[str]) -> str: + return _format_dt(format_datetime, dt, locale) + + +def format_localized_time(dt: timezone.datetime, locale: typing.Optional[str]) -> str: + return _format_dt(format_time, dt, locale) diff --git a/engine/common/tests/test_l10n.py b/engine/common/tests/test_l10n.py new file mode 100644 index 0000000000..7f4e05f7b8 --- /dev/null +++ b/engine/common/tests/test_l10n.py @@ -0,0 +1,27 @@ +from django.utils import timezone + +from common import l10n + +REAL_LOCALE = "fr_CA" +FAKE_LOCALE = "potato" +dt = timezone.datetime(2022, 5, 4, 15, 14, 13, 12) + + +def test_format_localized_datetime(): + assert l10n.format_localized_datetime(dt, REAL_LOCALE) == "2022-05-04 15 h 14" + + # test that it catches the exception and falls back to some configured default + assert l10n.format_localized_datetime(dt, FAKE_LOCALE) == "5/4/22, 3:14\u202fPM" + + # test that it properly handles None and falls back to some configured default + assert l10n.format_localized_datetime(dt, None) == "5/4/22, 3:14\u202fPM" + + +def test_format_localized_time(): + assert l10n.format_localized_time(dt, REAL_LOCALE) == "15 h 14" + + # test that it catches the exception and falls back to some configured default + assert l10n.format_localized_time(dt, FAKE_LOCALE) == "3:14\u202fPM" + + # test that it properly handles None and falls back to some configured default + assert l10n.format_localized_time(dt, None) == "3:14\u202fPM" diff --git a/engine/requirements.txt b/engine/requirements.txt index a6d595b4fe..3e4e4f2226 100644 --- a/engine/requirements.txt +++ b/engine/requirements.txt @@ -53,3 +53,4 @@ requests==2.31.0 urllib3==1.26.15 prometheus_client==0.16.0 lxml==4.9.2 +babel==2.12.1