Use X-WR-TIMEZONE as calendar tz when available, fallback to UTC (#2091)
When checking current shifts to trigger slack notifications, a stable value for timezone should be used (to avoid triggering multiple duplicated notifications; using the first `VTIMEZONE` value available may change on refresh, besides being potentially incorrect). Also, `VTIMEZONE` shouldn't be used as the calendar timezone, it just describes a [timezone definition](https://icalendar.org/iCalendar-RFC-5545/3-6-5-time-zone-component.html) which can later be [used when specifying a start/end date/datetime](https://icalendar.org/iCalendar-RFC-5545/3-2-19-time-zone-identifier.html) ("The parameter MUST be specified on properties with a DATE-TIME value if the DATE-TIME is not either a UTC or a "floating" time."). OTOH, Google uses the non-standard[ `X-WR-TIMEZONE` ](https://github.com/collective/icalendar/issues/343)field as fallback TZ for date-times without a `TZID`. Try to use this when available, otherwise fallback to `UTC`.
This commit is contained in:
Matias Bordese
GitHub
parent
8dfa929d5d
commit
0ab5312bf1
4 changed files with 184 additions and 4 deletions
|
|
@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||
### Fixed
|
||||
|
||||
- Fix demo alert for inbound email integration by @vadimkerr ([#2081](https://github.com/grafana/oncall/pull/2081))
|
||||
- Fix calendar TZ used when comparing current shifts triggering slack shift notifications ([#2091](https://github.com/grafana/oncall/pull/2091))
|
||||
|
||||
## v1.2.35 (2023-06-01)
|
||||
|
||||
|
|
|
|||
|
|
@ -304,3 +304,130 @@ def test_current_shift_changes_trigger_notification(
|
|||
notify_ical_schedule_shift(schedule.oncallschedule_ptr_id)
|
||||
|
||||
assert mock_slack_api_call.called
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_vtimezone_changes_no_triggering_notification(
|
||||
make_organization_and_user_with_slack_identities,
|
||||
make_user,
|
||||
make_schedule,
|
||||
):
|
||||
organization, _, _, _ = make_organization_and_user_with_slack_identities()
|
||||
make_user(organization=organization, username="user1")
|
||||
# clear users pks <-> organization cache (persisting between tests)
|
||||
memoized_users_in_ical.cache_clear()
|
||||
|
||||
ical_before = textwrap.dedent(
|
||||
"""
|
||||
BEGIN:VCALENDAR
|
||||
PRODID:-//Google Inc//Google Calendar 70.9054//EN
|
||||
VERSION:2.0
|
||||
CALSCALE:GREGORIAN
|
||||
X-WR-TIMEZONE:Europe/London
|
||||
METHOD:PUBLISH
|
||||
BEGIN:VTIMEZONE
|
||||
TZID:Europe/Rome
|
||||
BEGIN:STANDARD
|
||||
TZOFFSETFROM:0200
|
||||
TZOFFSETTO:0100
|
||||
TZNAME:CET
|
||||
DTSTART:19701025T030000
|
||||
END:STANDARD
|
||||
END:VTIMEZONE
|
||||
BEGIN:VTIMEZONE
|
||||
TZID:America/Argentina/Buenos_Aires
|
||||
X-LIC-LOCATION:America/Argentina/Buenos_Aires
|
||||
BEGIN:STANDARD
|
||||
TZOFFSETFROM:-0300
|
||||
TZOFFSETTO:-0300
|
||||
TZNAME:-03
|
||||
DTSTART:19700101T000000
|
||||
END:STANDARD
|
||||
END:VTIMEZONE
|
||||
BEGIN:VEVENT
|
||||
DTSTART;VALUE=DATE:20230101
|
||||
DTEND;VALUE=DATE:20230102
|
||||
RRULE:FREQ=DAILY
|
||||
DTSTAMP:20230101T000000
|
||||
UID:id1@google.com
|
||||
CREATED:20230101T000000
|
||||
DESCRIPTION:
|
||||
LAST-MODIFIED:20230101T000000
|
||||
LOCATION:
|
||||
SEQUENCE:1
|
||||
STATUS:CONFIRMED
|
||||
SUMMARY:user1
|
||||
TRANSP:TRANSPARENT
|
||||
END:VEVENT
|
||||
END:VCALENDAR"""
|
||||
)
|
||||
|
||||
# same data, timezones in different order (eg. google usually randomly reorders them)
|
||||
ical_after = textwrap.dedent(
|
||||
"""
|
||||
BEGIN:VCALENDAR
|
||||
PRODID:-//Google Inc//Google Calendar 70.9054//EN
|
||||
VERSION:2.0
|
||||
CALSCALE:GREGORIAN
|
||||
METHOD:PUBLISH
|
||||
X-WR-TIMEZONE:Europe/London
|
||||
BEGIN:VTIMEZONE
|
||||
TZID:America/Argentina/Buenos_Aires
|
||||
X-LIC-LOCATION:America/Argentina/Buenos_Aires
|
||||
BEGIN:STANDARD
|
||||
TZOFFSETFROM:-0300
|
||||
TZOFFSETTO:-0300
|
||||
TZNAME:-03
|
||||
DTSTART:19700101T000000
|
||||
END:STANDARD
|
||||
END:VTIMEZONE
|
||||
BEGIN:VTIMEZONE
|
||||
TZID:Europe/Rome
|
||||
BEGIN:STANDARD
|
||||
TZOFFSETFROM:0200
|
||||
TZOFFSETTO:0100
|
||||
TZNAME:CET
|
||||
DTSTART:19701025T030000
|
||||
END:STANDARD
|
||||
END:VTIMEZONE
|
||||
BEGIN:VEVENT
|
||||
DTSTART;VALUE=DATE:20230101
|
||||
DTEND;VALUE=DATE:20230102
|
||||
RRULE:FREQ=DAILY
|
||||
DTSTAMP:20230101T000000
|
||||
UID:id1@google.com
|
||||
CREATED:20230101T000000
|
||||
DESCRIPTION:
|
||||
LAST-MODIFIED:20230101T000000
|
||||
LOCATION:
|
||||
SEQUENCE:1
|
||||
STATUS:CONFIRMED
|
||||
SUMMARY:user1
|
||||
TRANSP:TRANSPARENT
|
||||
END:VEVENT
|
||||
END:VCALENDAR"""
|
||||
)
|
||||
|
||||
schedule = make_schedule(
|
||||
organization,
|
||||
schedule_class=OnCallScheduleICal,
|
||||
name="test_ical_schedule",
|
||||
channel="channel",
|
||||
ical_url_primary="url",
|
||||
prev_ical_file_primary=ical_before,
|
||||
cached_ical_file_primary=ical_after,
|
||||
prev_ical_file_overrides=None,
|
||||
cached_ical_file_overrides=None,
|
||||
)
|
||||
|
||||
# setup current shifts before checking/triggering for notifications
|
||||
calendar = icalendar.Calendar.from_ical(ical_before)
|
||||
current_shifts, _ = get_current_shifts_from_ical(calendar, schedule, 0)
|
||||
schedule.current_shifts = json.dumps(current_shifts, default=str)
|
||||
schedule.empty_oncall = False
|
||||
schedule.save()
|
||||
|
||||
with patch("apps.slack.slack_client.SlackClientWithErrorHandling.api_call") as mock_slack_api_call:
|
||||
notify_ical_schedule_shift(schedule.oncallschedule_ptr_id)
|
||||
|
||||
assert not mock_slack_api_call.called
|
||||
|
|
|
|||
|
|
@ -575,10 +575,7 @@ def calculate_shift_diff(first_shift, second_shift):
|
|||
|
||||
|
||||
def get_icalendar_tz_or_utc(icalendar):
|
||||
try:
|
||||
calendar_timezone = icalendar.walk("VTIMEZONE")[0]["TZID"]
|
||||
except (IndexError, KeyError):
|
||||
calendar_timezone = "UTC"
|
||||
calendar_timezone = icalendar.get("X-WR-TIMEZONE", "UTC")
|
||||
|
||||
if pytz_timezone := is_valid_timezone(calendar_timezone):
|
||||
return pytz_timezone
|
||||
|
|
|
|||
|
|
@ -2,12 +2,14 @@ import datetime
|
|||
import textwrap
|
||||
from uuid import uuid4
|
||||
|
||||
import icalendar
|
||||
import pytest
|
||||
import pytz
|
||||
from django.utils import timezone
|
||||
|
||||
from apps.api.permissions import LegacyAccessControlRole
|
||||
from apps.schedules.ical_utils import (
|
||||
get_icalendar_tz_or_utc,
|
||||
is_icals_equal,
|
||||
list_of_oncall_shifts_from_ical,
|
||||
list_users_to_notify_from_ical,
|
||||
|
|
@ -17,6 +19,59 @@ from apps.schedules.ical_utils import (
|
|||
from apps.schedules.models import CustomOnCallShift, OnCallSchedule, OnCallScheduleCalendar, OnCallScheduleWeb
|
||||
|
||||
|
||||
def test_get_icalendar_tz_or_utc():
|
||||
ical_data = textwrap.dedent(
|
||||
"""
|
||||
BEGIN:VCALENDAR
|
||||
PRODID:-//Google Inc//Google Calendar 70.9054//EN
|
||||
VERSION:2.0
|
||||
CALSCALE:GREGORIAN
|
||||
METHOD:PUBLISH
|
||||
X-WR-TIMEZONE:Europe/London
|
||||
BEGIN:VTIMEZONE
|
||||
TZID:America/Argentina/Buenos_Aires
|
||||
X-LIC-LOCATION:America/Argentina/Buenos_Aires
|
||||
BEGIN:STANDARD
|
||||
TZOFFSETFROM:-0300
|
||||
TZOFFSETTO:-0300
|
||||
TZNAME:-03
|
||||
DTSTART:19700101T000000
|
||||
END:STANDARD
|
||||
END:VTIMEZONE
|
||||
END:VCALENDAR
|
||||
"""
|
||||
)
|
||||
ical = icalendar.Calendar.from_ical(ical_data)
|
||||
tz = get_icalendar_tz_or_utc(ical)
|
||||
assert tz == pytz.timezone("Europe/London")
|
||||
|
||||
|
||||
def test_get_icalendar_tz_or_utc_fallback():
|
||||
ical_data = textwrap.dedent(
|
||||
"""
|
||||
BEGIN:VCALENDAR
|
||||
PRODID:-//Google Inc//Google Calendar 70.9054//EN
|
||||
VERSION:2.0
|
||||
CALSCALE:GREGORIAN
|
||||
METHOD:PUBLISH
|
||||
BEGIN:VTIMEZONE
|
||||
TZID:America/Argentina/Buenos_Aires
|
||||
X-LIC-LOCATION:America/Argentina/Buenos_Aires
|
||||
BEGIN:STANDARD
|
||||
TZOFFSETFROM:-0300
|
||||
TZOFFSETTO:-0300
|
||||
TZNAME:-03
|
||||
DTSTART:19700101T000000
|
||||
END:STANDARD
|
||||
END:VTIMEZONE
|
||||
END:VCALENDAR
|
||||
"""
|
||||
)
|
||||
ical = icalendar.Calendar.from_ical(ical_data)
|
||||
tz = get_icalendar_tz_or_utc(ical)
|
||||
assert tz == pytz.timezone("UTC")
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_users_in_ical_email_case_insensitive(make_organization_and_user, make_user_for_organization):
|
||||
organization, user = make_organization_and_user()
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue