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

Fix Booking View with daylight savings #306

Merged
merged 10 commits into from
Mar 13, 2024
39 changes: 24 additions & 15 deletions backend/src/appointment/controller/calendar.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
Handle connection to a CalDAV server.
"""
import json
import zoneinfo

import caldav.lib.error
import requests
Expand All @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had ruff set to run on save. Hence the unused import removals, and re-appearence of the zoneinfo.

from dateutil.parser import parse

from .apis.google_client import GoogleClient
from ..database import schemas, models
from ..database.models import CalendarProvider
from ..controller.mailer import Attachment, InvitationMail
from ..controller.mailer import Attachment
from ..l10n import l10n
from ..tasks.emails import send_invite_email

Expand Down Expand Up @@ -316,44 +315,54 @@ def send_vevent(
background_tasks.add_task(send_invite_email, to=attendee.email, attachment=invite)

@staticmethod
def available_slots_from_schedule(s: models.Schedule) -> list[schemas.SlotBase]:
def available_slots_from_schedule(schedule: models.Schedule) -> list[schemas.SlotBase]:
"""This helper calculates a list of slots according to the given schedule configuration."""
slots = []

now = datetime.now()

subscriber = schedule.calendar.owner
timezone = zoneinfo.ZoneInfo(subscriber.timezone)

# Start and end time in the subscriber's timezone
start_time_local = schedule.start_time_local
end_time_local = schedule.end_time_local

# FIXME: Currently the earliest booking acts in normal days, not within the scheduled days.
# So if they have the schedule setup for weekdays, it will count weekends too.
earliest_booking = now + timedelta(minutes=s.earliest_booking)
earliest_booking = now + timedelta(minutes=schedule.earliest_booking)
# We add a day here because it should be inclusive of the final day.
farthest_booking = now + timedelta(days=1, minutes=s.farthest_booking)
farthest_booking = now + timedelta(days=1, minutes=schedule.farthest_booking)

schedule_start = max([datetime.combine(s.start_date, s.start_time), earliest_booking])
schedule_end = min([datetime.combine(s.end_date, s.end_time), farthest_booking]) if s.end_date else farthest_booking
schedule_start = max([datetime.combine(schedule.start_date, start_time_local), earliest_booking])
schedule_end = min([datetime.combine(schedule.end_date, end_time_local), farthest_booking]) if schedule.end_date else farthest_booking

start_time = datetime.combine(now.min, s.start_time) - datetime.min
end_time = datetime.combine(now.min, s.end_time) - datetime.min
start_time = datetime.combine(now.min, start_time_local) - datetime.min
end_time = datetime.combine(now.min, end_time_local) - datetime.min

# Thanks to timezone conversion end_time can wrap around to the next day
if start_time > end_time:
end_time += timedelta(days=1)

# All user defined weekdays, falls back to working week if invalid
weekdays = s.weekdays if type(s.weekdays) == list else json.loads(s.weekdays)
weekdays = schedule.weekdays if type(schedule.weekdays) is list else json.loads(schedule.weekdays)
if not weekdays or len(weekdays) == 0:
weekdays = [1, 2, 3, 4, 5]

# Difference of the start and end time. Since our times are localized we start at 0, and go until we hit the diff.
total_time = int(end_time.total_seconds()) - int(start_time.total_seconds())

# Between the available booking time
for ordinal in range(schedule_start.toordinal(), schedule_end.toordinal()):
date = datetime.fromordinal(ordinal)
current_datetime = datetime(year=date.year, month=date.month, day=date.day)
current_datetime = datetime(year=date.year, month=date.month, day=date.day, hour=start_time_local.hour, minute=start_time_local.minute, tzinfo=timezone)
# Check if this weekday is within our schedule
if current_datetime.isoweekday() in weekdays:
# Generate each timeslot based on the selected duration
# We just loop through the start and end time and step by slot duration in seconds.
# We just loop through the difference of the start and end time and step by slot duration in seconds.
slots += [
schemas.SlotBase(start=current_datetime + timedelta(seconds=time), duration=s.slot_duration)
for time in range(int(start_time.total_seconds()), int(end_time.total_seconds()), s.slot_duration * 60)
schemas.SlotBase(start=current_datetime + timedelta(seconds=time), duration=schedule.slot_duration)
for time in range(0, total_time, schedule.slot_duration * 60)
]

return slots
Expand Down
61 changes: 38 additions & 23 deletions backend/src/appointment/database/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@

Definitions of database tables and their relationships.
"""
import datetime
import enum
import os
import uuid
import zoneinfo

from sqlalchemy import Column, ForeignKey, Integer, String, DateTime, Enum, Boolean, JSON, Date, Time
from sqlalchemy_utils import StringEncryptedType, ChoiceType
from sqlalchemy_utils.types.encrypted.encrypted_type import AesEngine
Expand Down Expand Up @@ -128,9 +131,9 @@ class Calendar(Base):
connected = Column(Boolean, index=True, default=False)
connected_at = Column(DateTime)

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")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got annoyed at the lack of type hinting.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh nice, thanks for the types. And sorry for not doing it right from the start 😬

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no worries! 😄

appointments: list["Appointment"] = relationship("Appointment", cascade="all,delete", back_populates="calendar")
schedules: list["Schedule"] = relationship("Schedule", cascade="all,delete", back_populates="calendar")


class Appointment(Base):
Expand Down Expand Up @@ -200,28 +203,40 @@ class Slot(Base):
class Schedule(Base):
__tablename__ = "schedules"

id = Column(Integer, primary_key=True, index=True)
calendar_id = Column(Integer, ForeignKey("calendars.id"))
active = Column(Boolean, index=True, default=True)
name = Column(StringEncryptedType(String, secret, AesEngine, "pkcs5", length=255), index=True)
location_type = Column(Enum(LocationType), default=LocationType.inperson)
location_url = Column(StringEncryptedType(String, secret, AesEngine, "pkcs5", length=2048))
details = Column(StringEncryptedType(String, secret, AesEngine, "pkcs5", length=255))
start_date = Column(StringEncryptedType(Date, secret, AesEngine, "pkcs5", length=255), index=True)
end_date = Column(StringEncryptedType(Date, secret, AesEngine, "pkcs5", length=255), index=True)
start_time = Column(StringEncryptedType(Time, secret, AesEngine, "pkcs5", length=255), index=True)
end_time = Column(StringEncryptedType(Time, secret, AesEngine, "pkcs5", length=255), index=True)
earliest_booking = Column(Integer, default=1440) # in minutes, defaults to 24 hours
farthest_booking = Column(Integer, default=20160) # in minutes, defaults to 2 weeks
weekdays = Column(JSON, default="[1,2,3,4,5]") # list of ISO weekdays, Mo-Su => 1-7
slot_duration = Column(Integer, default=30) # defaults to 30 minutes
id: int = Column(Integer, primary_key=True, index=True)
calendar_id: int = Column(Integer, ForeignKey("calendars.id"))
active: bool = Column(Boolean, index=True, default=True)
name: str = Column(StringEncryptedType(String, secret, AesEngine, "pkcs5", length=255), index=True)
location_type: LocationType = Column(Enum(LocationType), default=LocationType.inperson)
location_url: str = Column(StringEncryptedType(String, secret, AesEngine, "pkcs5", length=2048))
details: str = Column(StringEncryptedType(String, secret, AesEngine, "pkcs5", length=255))
start_date: datetime.date = Column(StringEncryptedType(Date, secret, AesEngine, "pkcs5", length=255), index=True)
end_date: datetime.date = Column(StringEncryptedType(Date, secret, AesEngine, "pkcs5", length=255), index=True)
start_time: datetime.time = Column(StringEncryptedType(Time, secret, AesEngine, "pkcs5", length=255), index=True)
end_time: datetime.time = Column(StringEncryptedType(Time, secret, AesEngine, "pkcs5", length=255), index=True)
earliest_booking: int = Column(Integer, default=1440) # in minutes, defaults to 24 hours
farthest_booking: int = Column(Integer, default=20160) # in minutes, defaults to 2 weeks
weekdays: str | dict = Column(JSON, default="[1,2,3,4,5]") # list of ISO weekdays, Mo-Su => 1-7
slot_duration: int = Column(Integer, default=30) # defaults to 30 minutes

# What (if any) meeting link will we generate once the meeting is booked
meeting_link_provider = Column(StringEncryptedType(ChoiceType(MeetingLinkProviderType), secret, AesEngine, "pkcs5", length=255), default=MeetingLinkProviderType.none, index=False)

calendar = relationship("Calendar", back_populates="schedules")
availabilities = relationship("Availability", cascade="all,delete", back_populates="schedule")
slots = relationship("Slot", cascade="all,delete", back_populates="schedule")
meeting_link_provider: MeetingLinkProviderType = Column(StringEncryptedType(ChoiceType(MeetingLinkProviderType), secret, AesEngine, "pkcs5", length=255), default=MeetingLinkProviderType.none, index=False)

calendar: Calendar = relationship("Calendar", back_populates="schedules")
availabilities: "Availability" = relationship("Availability", cascade="all,delete", back_populates="schedule")
MelissaAutumn marked this conversation as resolved.
Show resolved Hide resolved
slots: list[Slot] = relationship("Slot", cascade="all,delete", back_populates="schedule")

@property
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()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


@property
def end_time_local(self) -> datetime.time:
"""End Time in the Schedule's Calendar's Owner's timezone"""
time_of_save = self.time_updated.replace(hour=self.end_time.hour, minute=self.end_time.minute, second=0)
return time_of_save.astimezone(zoneinfo.ZoneInfo(self.calendar.owner.timezone)).time()


class Availability(Base):
Expand Down
16 changes: 9 additions & 7 deletions backend/test/integration/test_schedule.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ def list_events(self, start, end):
monkeypatch.setattr(CalDavConnector, "__init__", MockCaldavConnector.__init__)
monkeypatch.setattr(CalDavConnector, "list_events", MockCaldavConnector.list_events)

start_date = date(2024, 4, 1)
start_date = date(2024, 3, 1)
start_time = time(9)
end_time = time(17)

Expand Down Expand Up @@ -262,9 +262,11 @@ def list_events(self, start, end):
slots = data['slots']

# Based off the earliest_booking our earliest slot is tomorrow at 9:00am
assert slots[0]['start'] == '2024-04-02T09:00:00'
# Note: this should be in PST (Pacific Standard Time)
assert slots[0]['start'] == '2024-03-04T09:00:00-08:00'
# 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'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine with me. Since we (you) just had a perfect example for DST, makes totally sense to just take that as testing example 👍🏻


# Check availability over a year from now
with freeze_time(date(2025, 6, 1)):
Expand All @@ -277,8 +279,8 @@ def list_events(self, start, end):
data = response.json()
slots = data['slots']

assert slots[0]['start'] == '2025-06-02T09:00:00'
assert slots[-1]['start'] == '2025-06-13T16:30:00'
assert slots[0]['start'] == '2025-06-02T09:00:00-07:00'
assert slots[-1]['start'] == '2025-06-13T16:30:00-07:00'

# Check availability with a start date day greater than the farthest_booking day
with freeze_time(date(2025, 6, 27)):
Expand All @@ -291,5 +293,5 @@ def list_events(self, start, end):
data = response.json()
slots = data['slots']

assert slots[0]['start'] == '2025-06-30T09:00:00'
assert slots[-1]['start'] == '2025-07-11T16:30:00'
assert slots[0]['start'] == '2025-06-30T09:00:00-07:00'
assert slots[-1]['start'] == '2025-07-11T16:30:00-07:00'
5 changes: 3 additions & 2 deletions frontend/src/components/ScheduleCreation.vue
Original file line number Diff line number Diff line change
Expand Up @@ -538,11 +538,12 @@ const saveSchedule = async (withConfirmation = true) => {
// build data object for post request
const obj = { ...scheduleInput.value };
// convert local input times to utc times
obj.start_time = dj(`${dj(obj.start_date).format('YYYY-MM-DD')}T${obj.start_time}:00`)
obj.start_time = dj(`${dj().format('YYYY-MM-DD')}T${obj.start_time}:00`)
.tz(user.data.timezone ?? dj.tz.guess(), true)
.utc()
.format('HH:mm');
obj.end_time = dj(`${dj(obj.start_date).format('YYYY-MM-DD')}T${obj.end_time}:00`)
obj.end_time = dj(`${dj().format('YYYY-MM-DD')}T${obj.end_time}:00`)
.tz(user.data.timezone ?? dj.tz.guess(), true)
.utc()
.format('HH:mm');
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/views/BookingView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ onMounted(async () => {
if (appointment.value) {
// convert start dates from UTC back to users timezone
appointment.value.slots.forEach((s) => {
s.start = dj.utc(s.start).tz(dj.tz.guess());
s.start = dj(s.start).tz(dj.tz.guess());
});
activeDate.value = dj(appointment.value?.slots[0].start);
// check appointment slots for appropriate view
Expand Down
Loading