diff --git a/CHANGELOG.md b/CHANGELOG.md index 88143ff9..7aeb2127 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Change permissions used during setup to better represent actions being taken by @mderynck ([#2242](https://github.com/grafana/oncall/pull/2242)) - Display 100000+ in stats when there are more than 100000 alert groups in the result ([#1901](https://github.com/grafana/oncall/pull/1901)) +### Fixed + +- For "You're Going OnCall" push notifications, show shift times in the user's configured timezone, otherwise UTC + by @joeyorlando ([#2351](https://github.com/grafana/oncall/pull/2351)) + ## v1.3.1 (2023-06-26) ### Fixed diff --git a/engine/apps/mobile_app/tasks.py b/engine/apps/mobile_app/tasks.py index aede4858..1dd8c833 100644 --- a/engine/apps/mobile_app/tasks.py +++ b/engine/apps/mobile_app/tasks.py @@ -6,6 +6,7 @@ import typing from enum import Enum import humanize +import pytz import requests from celery.utils.log import get_task_logger from django.conf import settings @@ -25,6 +26,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 +from common.timezones import is_valid_timezone if typing.TYPE_CHECKING: from apps.mobile_app.models import MobileAppUserSettings @@ -227,23 +229,33 @@ def _get_alert_group_escalation_fcm_message( def _get_youre_going_oncall_notification_title(seconds_until_going_oncall: int) -> str: - time_until_going_oncall = humanize.naturaldelta(seconds_until_going_oncall) - - return f"Your on-call shift starts in {time_until_going_oncall}" + return f"Your on-call shift starts in {humanize.naturaldelta(seconds_until_going_oncall)}" 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"] 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) + def _format_datetime(dt: datetime.datetime) -> str: + """ + 1. Convert the shift datetime to the user's configured timezone, if set, otherwise fallback to UTC + 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)) + return dt_formatter_func(localized_dt, mobile_app_user_settings.locale) formatted_shift = f"{_format_datetime(shift_start)} - {_format_datetime(shift_end)}" @@ -266,7 +278,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 + schedule, schedule_event, mobile_app_user_settings, user.timezone ) data: FCMMessageData = { 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 5b4a4527..ebf438c0 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 @@ -96,9 +96,11 @@ 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) + same_day_shift_subtitle = tasks._get_youre_going_oncall_notification_subtitle( + schedule, same_day_shift, maus, user.timezone + ) same_day_shift_no_locale_subtitle = tasks._get_youre_going_oncall_notification_subtitle( - schedule, same_day_shift, maus_no_locale + schedule, same_day_shift, maus_no_locale, user2.timezone ) assert same_day_shift_title == f"Your on-call shift starts in {humanized_time_until_going_oncall}" @@ -110,10 +112,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 + schedule, multiple_day_shift, maus, user.timezone ) multiple_day_shift_no_locale_subtitle = tasks._get_youre_going_oncall_notification_subtitle( - schedule, multiple_day_shift, maus_no_locale + schedule, multiple_day_shift, maus_no_locale, user2.timezone ) assert multiple_day_shift_title == f"Your on-call shift starts in {humanized_time_until_going_oncall}" @@ -124,6 +126,47 @@ 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"), + ("Europe/Amsterdam", "11:00 AM - 7:00 PM"), + ("asdfasdfasdf", "9:00 AM - 5:00 PM"), + ], +) +@pytest.mark.django_db +def test_get_youre_going_oncall_notification_subtitle( + make_organization, make_user_for_organization, make_schedule, user_timezone, expected_shift_times +): + schedule_name = "asdfasdfasdfasdf" + + organization = make_organization() + user = make_user_for_organization(organization, _timezone=user_timezone) + user_pk = user.public_primary_key + maus = MobileAppUserSettings.objects.create(user=user) + + schedule = make_schedule(organization, name=schedule_name, schedule_class=OnCallScheduleWeb) + + shift_start = timezone.datetime(2023, 7, 8, 9, 0, 0) + shift_end = timezone.datetime(2023, 7, 8, 17, 0, 0) + + shift = _create_schedule_event( + shift_start, + shift_end, + "asdfasdfasdf", + [ + { + "pk": user_pk, + }, + ], + ) + + assert ( + tasks._get_youre_going_oncall_notification_subtitle(schedule, shift, maus, user.timezone) + == f"{expected_shift_times}\nSchedule {schedule_name}" + ) + + @mock.patch("apps.mobile_app.tasks._get_youre_going_oncall_notification_subtitle") @mock.patch("apps.mobile_app.tasks._get_youre_going_oncall_notification_title") @mock.patch("apps.mobile_app.tasks._construct_fcm_message") @@ -140,7 +183,8 @@ def test_get_youre_going_oncall_fcm_message( mock_construct_fcm_message, mock_get_youre_going_oncall_notification_title, mock_get_youre_going_oncall_notification_subtitle, - make_organization_and_user, + make_organization, + make_user_for_organization, make_schedule, ): mock_fcm_message = "mncvmnvcmnvcnmvcmncvmn" @@ -153,7 +197,9 @@ def test_get_youre_going_oncall_fcm_message( mock_get_youre_going_oncall_notification_title.return_value = mock_notification_title mock_get_youre_going_oncall_notification_subtitle.return_value = mock_notification_subtitle - organization, user = make_organization_and_user() + organization = make_organization() + user_tz = "Europe/Amsterdam" + user = make_user_for_organization(organization, _timezone=user_tz) 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" @@ -203,7 +249,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) + mock_get_youre_going_oncall_notification_subtitle.assert_called_once_with(schedule, schedule_event, maus, user_tz) mock_get_youre_going_oncall_notification_title.assert_called_once_with(seconds_until_going_oncall) mock_construct_fcm_message.assert_called_once_with( diff --git a/engine/apps/user_management/models/user.py b/engine/apps/user_management/models/user.py index 3ff977e8..fbcdb96c 100644 --- a/engine/apps/user_management/models/user.py +++ b/engine/apps/user_management/models/user.py @@ -244,7 +244,7 @@ class User(models.Model): return self.username @property - def timezone(self): + def timezone(self) -> typing.Optional[str]: if self._timezone: return self._timezone