From 7cf0c4f693e095389d8ef7aba9d917ef082a61d0 Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Wed, 3 May 2023 17:24:10 -0300 Subject: [PATCH] Update ical comparison to check only for event components (#1870) May be related to https://github.com/grafana/oncall/issues/1553. We got feedback about that happening for Google Calendar imported icals. Google Calendar exported ics URL was returning different VTIMEZONE components on different requests, triggering differences in the imported ical. Updated the comparison to only consider events (while keep assuming the sequence will reflect if there are any particular event change). --- CHANGELOG.md | 6 + engine/apps/schedules/ical_utils.py | 24 ++- .../apps/schedules/tests/test_ical_utils.py | 138 ++++++++++++++++++ 3 files changed, 153 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ad31ac6..8afe958d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +### Changed + +- Improve ical comparison when checking for imported ical updates ([1870](https://github.com/grafana/oncall/pull/1870)) + ## v1.2.18 (2023-05-03) ### Added diff --git a/engine/apps/schedules/ical_utils.py b/engine/apps/schedules/ical_utils.py index 4c9a3821..a7bce8b5 100644 --- a/engine/apps/schedules/ical_utils.py +++ b/engine/apps/schedules/ical_utils.py @@ -509,21 +509,15 @@ def is_icals_equal(first, second): second_cal = Calendar.from_ical(second) first_subcomponents = first_cal.subcomponents second_subcomponents = second_cal.subcomponents - if len(first_subcomponents) != len(second_subcomponents): - return False - first_cal_events = {} - second_cal_events = {} - for cmp in first_cal.subcomponents: - first_cal_events[cmp.get("UID", None)] = cmp.get("SEQUENCE", None) - for cmp in second_cal.subcomponents: - second_cal_events[cmp.get("UID", None)] = cmp.get("SEQUENCE", None) - for first_uid, first_seq in first_cal_events.items(): - if first_uid not in second_cal_events: - return False - second_seq = second_cal_events.get(first_uid, None) - if first_seq != second_seq: - return False - return True + # only consider VEVENT components + first_cal_events = { + cmp.get("UID", None): cmp.get("SEQUENCE", None) for cmp in first_subcomponents if cmp.name == "VEVENT" + } + second_cal_events = { + cmp.get("UID", None): cmp.get("SEQUENCE", None) for cmp in second_subcomponents if cmp.name == "VEVENT" + } + # check events and their respective sequences are equal + return first_cal_events == second_cal_events def ical_date_to_datetime(date, tz, start): diff --git a/engine/apps/schedules/tests/test_ical_utils.py b/engine/apps/schedules/tests/test_ical_utils.py index cce70ccf..c2521d3d 100644 --- a/engine/apps/schedules/tests/test_ical_utils.py +++ b/engine/apps/schedules/tests/test_ical_utils.py @@ -1,4 +1,5 @@ import datetime +import textwrap from uuid import uuid4 import pytest @@ -7,6 +8,7 @@ from django.utils import timezone from apps.api.permissions import LegacyAccessControlRole from apps.schedules.ical_utils import ( + is_icals_equal, list_of_oncall_shifts_from_ical, list_users_to_notify_from_ical, parse_event_uid, @@ -153,3 +155,139 @@ def test_parse_event_uid_fallback(): pk, source = parse_event_uid(event_uid) assert pk == event_uid assert source is None + + +def test_is_icals_equal_compare_events(): + with_vtimezone = textwrap.dedent( + """ + BEGIN:VCALENDAR + PRODID:-//Google Inc//Google Calendar 70.9054//EN + VERSION:2.0 + CALSCALE:GREGORIAN + METHOD:PUBLISH + X-WR-TIMEZONE:Europe/Amsterdam + BEGIN:VTIMEZONE + TZID:Europe/Amsterdam + X-LIC-LOCATION:Europe/Amsterdam + BEGIN:DAYLIGHT + TZOFFSETFROM:+0100 + TZOFFSETTO:+0200 + TZNAME:CEST + DTSTART:19700329T020000 + RRULE:FREQ=YEARLY;BYMONTH=3;BYDAY=-1SU + END:DAYLIGHT + BEGIN:STANDARD + TZOFFSETFROM:+0200 + TZOFFSETTO:+0100 + TZNAME:CET + DTSTART:19701025T030000 + RRULE:FREQ=YEARLY;BYMONTH=10;BYDAY=-1SU + END:STANDARD + END:VTIMEZONE + BEGIN:VEVENT + DTSTART;VALUE=DATE:20230515 + DTEND;VALUE=DATE:20230522 + DTSTAMP:20230503T152557Z + UID:something@google.com + RECURRENCE-ID;VALUE=DATE:20230501 + CREATED:20230403T073117Z + LAST-MODIFIED:20230424T123617Z + SEQUENCE:2 + STATUS:CONFIRMED + SUMMARY:some@user.com + END:VEVENT + END:VCALENDAR + """ + ) + without_vtimezone = textwrap.dedent( + """ + BEGIN:VCALENDAR + PRODID:-//Google Inc//Google Calendar 70.9054//EN + VERSION:2.0 + CALSCALE:GREGORIAN + METHOD:PUBLISH + X-WR-TIMEZONE:Europe/Amsterdam + BEGIN:VEVENT + DTSTART;VALUE=DATE:20230515 + DTEND;VALUE=DATE:20230522 + DTSTAMP:20230503T162103Z + UID:something@google.com + RECURRENCE-ID;VALUE=DATE:20230501 + CREATED:20230403T073117Z + LAST-MODIFIED:20230424T123617Z + SEQUENCE:2 + STATUS:CONFIRMED + SUMMARY:some@user.com + END:VEVENT + END:VCALENDAR + """ + ) + assert is_icals_equal(with_vtimezone, without_vtimezone) + + +def test_is_icals_equal_compare_events_not_equal(): + with_vtimezone = textwrap.dedent( + """ + BEGIN:VCALENDAR + PRODID:-//Google Inc//Google Calendar 70.9054//EN + VERSION:2.0 + CALSCALE:GREGORIAN + METHOD:PUBLISH + X-WR-TIMEZONE:Europe/Amsterdam + BEGIN:VTIMEZONE + TZID:Europe/Amsterdam + X-LIC-LOCATION:Europe/Amsterdam + BEGIN:DAYLIGHT + TZOFFSETFROM:+0100 + TZOFFSETTO:+0200 + TZNAME:CEST + DTSTART:19700329T020000 + RRULE:FREQ=YEARLY;BYMONTH=3;BYDAY=-1SU + END:DAYLIGHT + BEGIN:STANDARD + TZOFFSETFROM:+0200 + TZOFFSETTO:+0100 + TZNAME:CET + DTSTART:19701025T030000 + RRULE:FREQ=YEARLY;BYMONTH=10;BYDAY=-1SU + END:STANDARD + END:VTIMEZONE + BEGIN:VEVENT + DTSTART;VALUE=DATE:20230515 + DTEND;VALUE=DATE:20230522 + DTSTAMP:20230503T152557Z + UID:something@google.com + RECURRENCE-ID;VALUE=DATE:20230501 + CREATED:20230403T073117Z + LAST-MODIFIED:20230424T123617Z + SEQUENCE:2 + STATUS:CONFIRMED + SUMMARY:some@user.com + END:VEVENT + END:VCALENDAR + """ + ) + without_vtimezone = textwrap.dedent( + """ + BEGIN:VCALENDAR + PRODID:-//Google Inc//Google Calendar 70.9054//EN + VERSION:2.0 + CALSCALE:GREGORIAN + METHOD:PUBLISH + X-WR-TIMEZONE:Europe/Amsterdam + BEGIN:VEVENT + DTSTART;VALUE=DATE:20230515 + DTEND;VALUE=DATE:20230522 + DTSTAMP:20230503T162103Z + UID:something@google.com + RECURRENCE-ID;VALUE=DATE:20230501 + CREATED:20230403T073117Z + LAST-MODIFIED:20230424T123617Z + SEQUENCE:3 + STATUS:CONFIRMED + SUMMARY:some@user.com + END:VEVENT + END:VCALENDAR + """ + ) + assert not is_icals_equal(with_vtimezone, without_vtimezone)