Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use X-WR-TIMEZONE as calendar tz when available, fallback to UTC #2091

Merged
merged 2 commits into from
Jun 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
127 changes: 127 additions & 0 deletions engine/apps/alerts/tests/test_notify_ical_schedule_shift.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 1 addition & 4 deletions engine/apps/schedules/ical_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
55 changes: 55 additions & 0 deletions engine/apps/schedules/tests/test_ical_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@
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,
Expand All @@ -17,6 +19,59 @@
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()
Expand Down