For "You're Going OnCall" push notifications, show shift times in the…

… user's configured timezone, otherwise UTC (#2351) # Which issue(s) this PR fixes closes #2350 ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [ ] Documentation added (or `pr:no public docs` PR label added if not required) (N/A) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required)
grafana · Jun 27, 2023 · f75747a · f75747a
1 parent 065cc93
commit f75747a
Show file tree

Hide file tree

Showing 4 changed files with 77 additions and 14 deletions.
diff --git 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
@@ -6,6 +6,7 @@
 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 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
@@ -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
@@ -244,7 +244,7 @@ def get_username_with_slack_verbal(self, mention=False):
         return self.username
 
     @property
-    def timezone(self):
+    def timezone(self) -> typing.Optional[str]:
         if self._timezone:
             return self._timezone