Fix Booking View with daylight savings #306
Conversation
…shifting schedule time issue.
…ezone (based on day to account for daylight savings), and fix the frontend from assuming it's utc time.
…where the start of the month is PST and the end is PDT both with the proper times. (And fix the missing timezone string check.)
| @@ -11,13 +12,11 @@ | |||
| from google.oauth2.credentials import Credentials | |||
| from icalendar import Calendar, Event, vCalAddress, vText | |||
| from datetime import datetime, timedelta, timezone, UTC | |||
| from zoneinfo import ZoneInfo | |||
There was a problem hiding this comment.
I had ruff set to run on save. Hence the unused import removals, and re-appearence of the zoneinfo.
| def start_time_local(self) -> datetime.time: | ||
| """Start Time in the Schedule's Calendar's Owner's timezone""" | ||
| time_of_save = self.time_updated.replace(hour=self.start_time.hour, minute=self.start_time.minute, second=0) | ||
| return time_of_save.astimezone(zoneinfo.ZoneInfo(self.calendar.owner.timezone)).time() |
There was a problem hiding this comment.
When we save start_time we convert it and store it as UTC. To get back their original intended start time we simply grab the date the user saved their schedule, append the UTC start time and convert that back to their timezone.
| owner = relationship("Subscriber", back_populates="calendars") | ||
| appointments = relationship("Appointment", cascade="all,delete", back_populates="calendar") | ||
| schedules = relationship("Schedule", cascade="all,delete", back_populates="calendar") | ||
| owner: Subscriber = relationship("Subscriber", back_populates="calendars") |
There was a problem hiding this comment.
I got annoyed at the lack of type hinting.
There was a problem hiding this comment.
Oh nice, thanks for the types. And sorry for not doing it right from the start 😬
| # Based off the farthest_booking our latest slot is 4:30pm | ||
| assert slots[-1]['start'] == '2024-04-15T16:30:00' | ||
| # Note: This should be in PDT (Pacific Daylight Time) | ||
| assert slots[-1]['start'] == '2024-03-15T16:30:00-07:00' |
There was a problem hiding this comment.
Let me know if we need a more exhaustive test here. make_pro_subscriber creates a subscriber (using faker data) and is hardcoded to America/Vancouver for the timezone. Since daylight savings did happen this year this shouldn't break if we remove daylight savings from BC going forward.
There was a problem hiding this comment.
Fine with me. Since we (you) just had a perfect example for DST, makes totally sense to just take that as testing example 👍🏻
|
...this test should actually not be passing on my local machine. Looking into it. |
|
datetimes without any timezone information will automatically take the system's timezone rather than utc. Duh. So I've fixed it for the conversion code. |
| owner = relationship("Subscriber", back_populates="calendars") | ||
| appointments = relationship("Appointment", cascade="all,delete", back_populates="calendar") | ||
| schedules = relationship("Schedule", cascade="all,delete", back_populates="calendar") | ||
| owner: Subscriber = relationship("Subscriber", back_populates="calendars") |
There was a problem hiding this comment.
Oh nice, thanks for the types. And sorry for not doing it right from the start 😬
| # Based off the farthest_booking our latest slot is 4:30pm | ||
| assert slots[-1]['start'] == '2024-04-15T16:30:00' | ||
| # Note: This should be in PDT (Pacific Daylight Time) | ||
| assert slots[-1]['start'] == '2024-03-15T16:30:00-07:00' |
There was a problem hiding this comment.
Fine with me. Since we (you) just had a perfect example for DST, makes totally sense to just take that as testing example 👍🏻
Co-authored-by: Andreas <mail@devmount.de>
Fixes #289
We should no longer have varying schedule times based on when daylight savings hits
The real fix here is adjusting the start/end times from UTC to the subscriber's timezone based on the date the schedule object was changed. I believe we only ever update the schedule in a scenario when the schedule also updates so this should work. If that changes we'll have to add a
schedule_changed_atdatetime.cc @malini