From 4afba220ea2be36b1d5737fffc0f30b4f7457c57 Mon Sep 17 00:00:00 2001 From: Zachary Sistrunk Date: Wed, 20 Nov 2024 11:04:25 -0600 Subject: [PATCH] Fix issue with DST starting/ending causing overlaps/gaps (#5266) # What this PR does This older version of recurring_ical_events does not call the pytz .normalize() function, which can cause some invalid datetime objects to return when a DST swap happens. For example: Nov 3, 2024 9:00 AM CDT instead of the correct 8:00 AM CST). By calling tz.normalize on the end date and checking if the time zone information changed, we can detect when DST starts/stops and adjust the end date accordingly. | | DST stopping on November 3, 2024: | DST starting on March 9, 2024 | |-|-----------------------------------------|-----------------------------------| | Before | ![image](https://github.com/user-attachments/assets/933bce80-9b6a-475b-88f2-6356d0e3a6fd) | ![image](https://github.com/user-attachments/assets/264b816f-6f40-4f14-bbc0-1d03f7b74ac4) | After | ![image](https://github.com/user-attachments/assets/fbd71991-c4f8-4685-a527-6dbb147b2cb6) | ![image](https://github.com/user-attachments/assets/ccd932df-2ab4-4472-bc90-045372712f75) | ## Which issue(s) this PR closes Closes #5247 ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated - [ ] Documentation added (or `pr:no public docs` PR label added if not required) - [ ] Added the relevant release notes label (see labels prefixed w/ `release:`). These labels dictate how your PR will show up in the autogenerated release notes. --------- Co-authored-by: Matias Bordese --- .../amixr_recurring_ical_events_adapter.py | 13 +++++ .../schedules/tests/test_on_call_schedule.py | 57 +++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/engine/apps/schedules/ical_events/adapter/amixr_recurring_ical_events_adapter.py b/engine/apps/schedules/ical_events/adapter/amixr_recurring_ical_events_adapter.py index 155a6161..fa0dc3ff 100644 --- a/engine/apps/schedules/ical_events/adapter/amixr_recurring_ical_events_adapter.py +++ b/engine/apps/schedules/ical_events/adapter/amixr_recurring_ical_events_adapter.py @@ -20,6 +20,15 @@ EXTRA_LOOKUP_DAYS = 16 class AmixrRecurringIcalEventsAdapter(IcalService): + def _normalize(self, dt: datetime.datetime) -> datetime.datetime: + tz = getattr(dt, "tzinfo", None) + if tz: + normalized = tz.normalize(dt) + if normalized.tzinfo != tz: + diff = dt.dst() - normalized.dst() + dt = normalized + diff + return dt + def get_events_from_ical_between( self, calendar: Calendar, start_date: datetime.datetime, end_date: datetime.datetime ) -> typing.List[Event]: @@ -38,6 +47,10 @@ class AmixrRecurringIcalEventsAdapter(IcalService): end_date + datetime.timedelta(days=EXTRA_LOOKUP_DAYS), ) + for event in events: + # account for timezones not being properly calculated when DST changes. + event[ICAL_DATETIME_END].dt = self._normalize(event[ICAL_DATETIME_END].dt) + def filter_extra_days(event): event_start, event_end = self.get_start_and_end_with_respect_to_event_type(event) if event_start > event_end: diff --git a/engine/apps/schedules/tests/test_on_call_schedule.py b/engine/apps/schedules/tests/test_on_call_schedule.py index e63c96b2..71a029b4 100644 --- a/engine/apps/schedules/tests/test_on_call_schedule.py +++ b/engine/apps/schedules/tests/test_on_call_schedule.py @@ -1818,6 +1818,63 @@ def test_refresh_ical_final_schedule_ok( assert event in expected_events +@pytest.mark.django_db +def test_filter_events_during_dst_change( + make_organization, + make_user_for_organization, + make_schedule, + make_on_call_shift, +): + organization = make_organization() + u1 = make_user_for_organization(organization) + + schedule = make_schedule( + organization, + time_zone="America/Chicago", # UTC-6 or UTC-5 depending on DST + schedule_class=OnCallScheduleCalendar, + ) + start_datetime = timezone.datetime(2024, 10, 1, 0, 0, 0, tzinfo=timezone.utc) + duration = timezone.timedelta(seconds=60 * 60 * 24 * 7) # 1 week + data = { + "start": start_datetime, + "rotation_start": start_datetime, + "duration": duration, + "frequency": CustomOnCallShift.FREQUENCY_WEEKLY, + "week_start": CustomOnCallShift.MONDAY, + } + on_call_shift = make_on_call_shift( + organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data + ) + on_call_shift.add_rolling_users([[u1]]) + schedule.custom_on_call_shifts.add(on_call_shift) + + schedule.refresh_ical_file() + + # week with DST change + start_date = timezone.datetime(2024, 11, 4, 0, 0, 0, tzinfo=timezone.utc) + end_date = start_date + timezone.timedelta(days=1) + events = schedule.filter_events(start_date, end_date) + expected = { + "start": timezone.datetime(2024, 10, 29, 5, 0, 0, tzinfo=timezone.utc), + "end": timezone.datetime(2024, 11, 5, 6, 0, 0, tzinfo=timezone.utc), + } + assert len(events) == 1 + returned = {"start": events[0]["start"], "end": events[0]["end"]} + assert returned == expected + + # week with DST change back + start_date = timezone.datetime(2025, 3, 10, 0, 0, 0, tzinfo=timezone.utc) + end_date = start_date + timezone.timedelta(days=1) + events = schedule.filter_events(start_date, end_date) + expected = { + "start": timezone.datetime(2025, 3, 4, 6, 0, 0, tzinfo=timezone.utc), + "end": timezone.datetime(2025, 3, 11, 5, 0, 0, tzinfo=timezone.utc), + } + assert len(events) == 1 + returned = {"start": events[0]["start"], "end": events[0]["end"]} + assert returned == expected + + @pytest.mark.django_db def test_refresh_ical_final_schedule_cancel_deleted_events( make_organization,