diff --git a/CHANGELOG.md b/CHANGELOG.md index fa67c0e772..fd9a6481c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Added + +- Allow persisting mobile app's timezone, to allow for more accurate datetime related notifications by @joeyorlando + ([#2601](https://github.com/grafana/oncall/pull/2601)) + ### Changed - Update direct paging docs by @vadimkerr ([#2600](https://github.com/grafana/oncall/pull/2600)) diff --git a/engine/apps/mobile_app/migrations/0010_mobileappusersettings_time_zone.py b/engine/apps/mobile_app/migrations/0010_mobileappusersettings_time_zone.py new file mode 100644 index 0000000000..d1e283d43b --- /dev/null +++ b/engine/apps/mobile_app/migrations/0010_mobileappusersettings_time_zone.py @@ -0,0 +1,26 @@ +# Generated by Django 3.2.20 on 2023-07-20 14:53 + +from django.db import migrations, models +from django_add_default_value import AddDefaultValue + + +class Migration(migrations.Migration): + + dependencies = [ + ('mobile_app', '0009_fcmdevice'), + ] + + operations = [ + migrations.AddField( + model_name='mobileappusersettings', + name='time_zone', + field=models.CharField(default='UTC', max_length=100), + ), + # migrations.AddField enforces the default value on the app level, which leads to the issues during release + # adding same default value on the database level + AddDefaultValue( + model_name='mobileappusersettings', + name='time_zone', + value='UTC', + ), + ] diff --git a/engine/apps/mobile_app/models.py b/engine/apps/mobile_app/models.py index 2e5ec601cf..89c6703e07 100644 --- a/engine/apps/mobile_app/models.py +++ b/engine/apps/mobile_app/models.py @@ -174,3 +174,4 @@ class VolumeType(models.TextChoices): ) locale = models.CharField(max_length=50, null=True) + time_zone = models.CharField(max_length=100, default="UTC") diff --git a/engine/apps/mobile_app/serializers.py b/engine/apps/mobile_app/serializers.py index 93b72f62a4..72dd13f79c 100644 --- a/engine/apps/mobile_app/serializers.py +++ b/engine/apps/mobile_app/serializers.py @@ -1,9 +1,12 @@ from rest_framework import serializers from apps.mobile_app.models import MobileAppUserSettings +from common.timezones import TimeZoneField class MobileAppUserSettingsSerializer(serializers.ModelSerializer): + time_zone = TimeZoneField(required=False, allow_null=False) + class Meta: model = MobileAppUserSettings fields = ( @@ -23,4 +26,5 @@ class Meta: "info_notifications_enabled", "going_oncall_notification_timing", "locale", + "time_zone", ) diff --git a/engine/apps/mobile_app/tasks.py b/engine/apps/mobile_app/tasks.py index 6dbb71626d..9b16baae60 100644 --- a/engine/apps/mobile_app/tasks.py +++ b/engine/apps/mobile_app/tasks.py @@ -25,7 +25,6 @@ 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 -from common.timezones import is_valid_timezone if typing.TYPE_CHECKING: from apps.mobile_app.models import FCMDevice, MobileAppUserSettings @@ -235,7 +234,6 @@ def _get_youre_going_oncall_notification_subtitle( schedule: OnCallSchedule, schedule_event: ScheduleEvent, mobile_app_user_settings: "MobileAppUserSettings", - user_timezone: typing.Optional[str], ) -> str: shift_start = schedule_event["start"] shift_end = schedule_event["end"] @@ -244,16 +242,11 @@ def _get_youre_going_oncall_notification_subtitle( def _format_datetime(dt: datetime.datetime) -> str: """ - 1. Convert the shift datetime to the user's configured timezone, if set, otherwise fallback to UTC + 1. Convert the shift datetime to the user's mobile device's timezone 2. Display the timezone aware datetime as a formatted string that is based on the user's configured mobile app locale, otherwise fallback to "en" """ - if user_timezone is None or not is_valid_timezone(user_timezone): - user_tz = "UTC" - else: - user_tz = user_timezone - - localized_dt = dt.astimezone(pytz.timezone(user_tz)) + localized_dt = dt.astimezone(pytz.timezone(mobile_app_user_settings.time_zone)) return dt_formatter_func(localized_dt, mobile_app_user_settings.locale) formatted_shift = f"{_format_datetime(shift_start)} - {_format_datetime(shift_end)}" @@ -277,7 +270,7 @@ def _get_youre_going_oncall_fcm_message( notification_title = _get_youre_going_oncall_notification_title(seconds_until_going_oncall) notification_subtitle = _get_youre_going_oncall_notification_subtitle( - schedule, schedule_event, mobile_app_user_settings, user.timezone + schedule, schedule_event, mobile_app_user_settings ) data: FCMMessageData = { diff --git a/engine/apps/mobile_app/tests/test_user_settings.py b/engine/apps/mobile_app/tests/test_user_settings.py index 043b052123..063a142cf6 100644 --- a/engine/apps/mobile_app/tests/test_user_settings.py +++ b/engine/apps/mobile_app/tests/test_user_settings.py @@ -32,6 +32,7 @@ def test_user_settings_get(make_organization_and_user_with_mobile_app_auth_token "info_notifications_enabled": False, "going_oncall_notification_timing": 43200, "locale": None, + "time_zone": "UTC", } @@ -69,6 +70,7 @@ def test_user_settings_put( "info_notifications_enabled": True, "going_oncall_notification_timing": going_oncall_notification_timing, "locale": "ca_FR", + "time_zone": "Europe/Paris", } response = client.put(url, data=data, format="json", HTTP_AUTHORIZATION=auth_token) @@ -103,6 +105,7 @@ def test_user_settings_patch(make_organization_and_user_with_mobile_app_auth_tok "important_notification_volume_override": False, "important_notification_override_dnd": False, "info_notifications_enabled": True, + "time_zone": "Europe/Luxembourg", } response = client.put(url, data=data, format="json", HTTP_AUTHORIZATION=auth_token) @@ -116,3 +119,24 @@ def test_user_settings_patch(make_organization_and_user_with_mobile_app_auth_tok 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} + + +@pytest.mark.django_db +def test_user_settings_time_zone_must_be_valid(make_organization_and_user_with_mobile_app_auth_token): + _, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() + + valid_timezone = {"time_zone": "Europe/Luxembourg"} + invalid_timezone = {"time_zone": "asdflkjasdlkj"} + null_timezone = {"time_zone": None} + + client = APIClient() + url = reverse("mobile_app:user_settings") + + response = client.put(url, data=valid_timezone, format="json", HTTP_AUTHORIZATION=auth_token) + assert response.status_code == status.HTTP_200_OK + + response = client.put(url, data=invalid_timezone, format="json", HTTP_AUTHORIZATION=auth_token) + assert response.status_code == status.HTTP_400_BAD_REQUEST + + response = client.put(url, data=null_timezone, format="json", HTTP_AUTHORIZATION=auth_token) + assert response.status_code == status.HTTP_400_BAD_REQUEST 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 3d02384afd..6d713671c6 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 @@ -95,11 +95,9 @@ def test_get_youre_going_oncall_notification_title(make_organization_and_user, m # same day shift ################## same_day_shift_title = tasks._get_youre_going_oncall_notification_title(seconds_until_going_oncall) - same_day_shift_subtitle = tasks._get_youre_going_oncall_notification_subtitle( - schedule, same_day_shift, maus, user.timezone - ) + same_day_shift_subtitle = tasks._get_youre_going_oncall_notification_subtitle(schedule, same_day_shift, maus) same_day_shift_no_locale_subtitle = tasks._get_youre_going_oncall_notification_subtitle( - schedule, same_day_shift, maus_no_locale, user2.timezone + schedule, same_day_shift, maus_no_locale ) assert same_day_shift_title == f"Your on-call shift starts in {humanized_time_until_going_oncall}" @@ -111,10 +109,10 @@ def test_get_youre_going_oncall_notification_title(make_organization_and_user, m ################## multiple_day_shift_title = tasks._get_youre_going_oncall_notification_title(seconds_until_going_oncall) multiple_day_shift_subtitle = tasks._get_youre_going_oncall_notification_subtitle( - schedule, multiple_day_shift, maus, user.timezone + schedule, multiple_day_shift, maus ) multiple_day_shift_no_locale_subtitle = tasks._get_youre_going_oncall_notification_subtitle( - schedule, multiple_day_shift, maus_no_locale, user2.timezone + schedule, multiple_day_shift, maus_no_locale ) assert multiple_day_shift_title == f"Your on-call shift starts in {humanized_time_until_going_oncall}" @@ -128,9 +126,8 @@ def test_get_youre_going_oncall_notification_title(make_organization_and_user, m @pytest.mark.parametrize( "user_timezone,expected_shift_times", [ - (None, "9:00 AM - 5:00 PM"), + ("UTC", "9:00 AM - 5:00 PM"), ("Europe/Amsterdam", "11:00 AM - 7:00 PM"), - ("asdfasdfasdf", "9:00 AM - 5:00 PM"), ], ) @pytest.mark.django_db @@ -140,9 +137,9 @@ def test_get_youre_going_oncall_notification_subtitle( schedule_name = "asdfasdfasdfasdf" organization = make_organization() - user = make_user_for_organization(organization, _timezone=user_timezone) + user = make_user_for_organization(organization) user_pk = user.public_primary_key - maus = MobileAppUserSettings.objects.create(user=user) + maus = MobileAppUserSettings.objects.create(user=user, time_zone=user_timezone) schedule = make_schedule(organization, name=schedule_name, schedule_class=OnCallScheduleWeb) @@ -161,7 +158,7 @@ def test_get_youre_going_oncall_notification_subtitle( ) assert ( - tasks._get_youre_going_oncall_notification_subtitle(schedule, shift, maus, user.timezone) + tasks._get_youre_going_oncall_notification_subtitle(schedule, shift, maus) == f"{expected_shift_times}\nSchedule {schedule_name}" ) @@ -198,7 +195,7 @@ def test_get_youre_going_oncall_fcm_message( organization = make_organization() user_tz = "Europe/Amsterdam" - user = make_user_for_organization(organization, _timezone=user_tz) + 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" @@ -215,7 +212,7 @@ def test_get_youre_going_oncall_fcm_message( ) device = FCMDevice.objects.create(user=user) - maus = MobileAppUserSettings.objects.create(user=user) + maus = MobileAppUserSettings.objects.create(user=user, time_zone=user_tz) data = { "title": mock_notification_title, @@ -248,7 +245,7 @@ def test_get_youre_going_oncall_fcm_message( ) mock_apns_payload.assert_called_once_with(aps=mock_aps.return_value) - mock_get_youre_going_oncall_notification_subtitle.assert_called_once_with(schedule, schedule_event, maus, user_tz) + mock_get_youre_going_oncall_notification_subtitle.assert_called_once_with(schedule, schedule_event, maus) mock_get_youre_going_oncall_notification_title.assert_called_once_with(seconds_until_going_oncall) mock_construct_fcm_message.assert_called_once_with(