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)
This commit is contained in:
Joey Orlando 2023-06-27 07:59:54 +02:00 committed by GitHub
parent 065cc9343a
commit f75747ac23
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 77 additions and 14 deletions

View file

@ -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

View file

@ -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 = {

View file

@ -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:00AM - 5:00PM"),
("Europe/Amsterdam", "11:00AM - 7:00PM"),
("asdfasdfasdf", "9:00AM - 5:00PM"),
],
)
@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(

View file

@ -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