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)
This commit is contained in:
parent
c8669ce94c
commit
572131b921
10 changed files with 346 additions and 19 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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),
|
||||
),
|
||||
]
|
||||
|
|
@ -141,3 +141,5 @@ class MobileAppUserSettings(models.Model):
|
|||
going_oncall_notification_timing = models.IntegerField(
|
||||
choices=NOTIFICATION_TIMING_CHOICES, default=TWELVE_HOURS_IN_SECONDS
|
||||
)
|
||||
|
||||
locale = models.CharField(max_length=50, null=True)
|
||||
|
|
|
|||
|
|
@ -22,4 +22,5 @@ class MobileAppUserSettingsSerializer(serializers.ModelSerializer):
|
|||
"important_notification_override_dnd",
|
||||
"info_notifications_enabled",
|
||||
"going_oncall_notification_timing",
|
||||
"locale",
|
||||
)
|
||||
|
|
|
|||
|
|
@ -24,6 +24,7 @@ from apps.schedules.models.on_call_schedule import OnCallSchedule, ScheduleEvent
|
|||
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)
|
||||
|
|
|
|||
|
|
@ -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}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
34
engine/common/l10n.py
Normal file
34
engine/common/l10n.py
Normal file
|
|
@ -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)
|
||||
27
engine/common/tests/test_l10n.py
Normal file
27
engine/common/tests/test_l10n.py
Normal file
|
|
@ -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"
|
||||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue