Skip to content

Commit

Permalink
Fix Booking View with daylight savings (#306)
Browse files Browse the repository at this point in the history
* Save the schedule based on the current date which should resolve the shifting schedule time issue.

* Type hint models.Schedule

* Add some virtual properties to get start/end time correctly from UTC.

* Return available slots from schedule localized in the subscribers timezone (based on day to account for daylight savings), and fix the frontend from assuming it's utc time.

* Adjust test_public_availability to cover a daylight savings example, 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.)

* Ensure our naive datetimes have utc timezone info before conversion.

* Remove stray git conflict marker

* Apply suggestions from code review

Co-authored-by: Andreas <mail@devmount.de>

* Quote-ify Availability as it's not defined at the point of reference.

---------

Co-authored-by: Andreas <mail@devmount.de>
  • Loading branch information
MelissaAutumn and devmount authored Mar 13, 2024
1 parent 3d9288e commit cfa8f6c
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 50 deletions.
37 changes: 23 additions & 14 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 os

import caldav.lib.error
Expand Down Expand Up @@ -413,46 +414,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")
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: list["Availability"] = relationship("Availability", cascade="all,delete", back_populates="schedule")
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, tzinfo=datetime.timezone.utc)
return time_of_save.astimezone(zoneinfo.ZoneInfo(self.calendar.owner.timezone)).time()

@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, tzinfo=datetime.timezone.utc)
return time_of_save.astimezone(zoneinfo.ZoneInfo(self.calendar.owner.timezone)).time()


class Availability(Base):
Expand Down
23 changes: 13 additions & 10 deletions backend/test/integration/test_schedule.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,9 +232,10 @@ 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_time = time(9)
end_time = time(17)
start_date = date(2024, 3, 1)
start_time = time(16)
# Next day
end_time = time(0)

subscriber = make_pro_subscriber()
generated_calendar = make_caldav_calendar(subscriber.id, connected=True)
Expand Down Expand Up @@ -263,9 +264,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'

# Check availability over a year from now
with freeze_time(date(2025, 6, 1)):
Expand All @@ -278,8 +281,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 @@ -292,8 +295,9 @@ 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'


def test_request_schedule_availability_slot(self, monkeypatch, with_client, make_pro_subscriber, make_caldav_calendar, make_schedule):
start_date = date(2024, 4, 1)
Expand Down Expand Up @@ -387,4 +391,3 @@ def bust_cached_events(self, all_calendars = False):
assert response.status_code == 200, response.text
data = response.json()
assert data is True

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 @@ -222,7 +222,7 @@ const getAppointment = async () => {
// convert start dates from UTC back to users timezone
data.value.slots.forEach((s) => {
s.start = dj.utc(s.start).tz(dj.tz.guess());
s.start = dj(s.start).tz(dj.tz.guess());
});
return data.value;
Expand Down

0 comments on commit cfa8f6c

Please sign in to comment.