Fix processing buffers when finding the first reservable time

Author: ranta

opening_hours/models.py

@@ -1,4 +1,4 @@
-from datetime import datetime, timedelta
+from datetime import datetime
 
 from django.db import models
 from django.db.models import F, Q
@@ -64,15 +64,9 @@ def _get_datetime_str(self) -> str:
         else:
             return f"{start.strftime(strformat)}-{end.strftime(strformat)}"
 
-    def as_time_span_element(
-        self,
-        buffer_time_before: timedelta | None = None,
-        buffer_time_after: timedelta | None = None,
-    ) -> TimeSpanElement:
+    def as_time_span_element(self) -> TimeSpanElement:
         return TimeSpanElement(
             start_datetime=self.start_datetime.astimezone(DEFAULT_TIMEZONE),
             end_datetime=self.end_datetime.astimezone(DEFAULT_TIMEZONE),
-            buffer_time_before=buffer_time_before,
-            buffer_time_after=buffer_time_after,
             is_reservable=True,
         )

opening_hours/utils/reservable_time_span_client.py

@@ -256,12 +256,8 @@ def override_reservable_with_closed_time_spans(
                 new_reservable_time_span = copy(reservable_time_span)
                 reservable_time_spans.append(new_reservable_time_span)
                 # Split the reservable time span in two
-                # Save the buffers on the reservable times, so that we can check if another buffered timespan
-                # (i.e. a reservation's timespan) would fit between this and the next/previous time span
                 reservable_time_span.end_datetime = closed_time_span.buffered_start_datetime
                 new_reservable_time_span.start_datetime = closed_time_span.buffered_end_datetime
-                reservable_time_span.buffer_time_after = closed_time_span.buffer_time_before
-                new_reservable_time_span.buffer_time_before = closed_time_span.buffer_time_after
 
             # Reservable time span starts inside the closed time span.
             # Shorten the reservable time span from the beginning
@@ -276,10 +272,7 @@ def override_reservable_with_closed_time_spans(
             # │       xxxx ->       xxxx │ Handled later in the next step    │
             # └──────────────────────────┴───────────────────────────────────┘
             elif reservable_time_span.starts_inside_of(closed_time_span):
-                # Save the buffer on the reservable time, so that we can check if another buffered timespan
-                # (i.e. a reservation's timespan) would fit between this and the previous time span
                 reservable_time_span.start_datetime = closed_time_span.buffered_end_datetime
-                reservable_time_span.buffer_time_before = closed_time_span.buffer_time_after
 
             # Reservable time span ends inside the closed time span.
             # Shorten the reservable time span from the end
@@ -294,10 +287,7 @@ def override_reservable_with_closed_time_spans(
             # │ xxxx       -> xxxx       │ Already handled in last step      │
             # └──────────────────────────┴───────────────────────────────────┘
             elif reservable_time_span.ends_inside_of(closed_time_span):
-                # Save the buffer on the reservable time, so that we can check if another buffered timespan
-                # (i.e. a reservation's timespan) would fit between this and the next time span
                 reservable_time_span.end_datetime = closed_time_span.buffered_start_datetime
-                reservable_time_span.buffer_time_after = closed_time_span.buffer_time_before
 
             # If the duration of the reservable time span is negative or zero after adjustments
             # (buffered time is ignored here), remove it.

reservation_units/querysets.py

@@ -115,15 +115,74 @@ def _get_soft_closed_time_spans_for_reservation_unit(reservation_unit: "Reservat
 def _find_first_reservable_time_span_for_reservation_unit(
     reservation_unit: "ReservationUnit",
     normalised_reservable_time_spans: list[TimeSpanElement],
+    reservations: list[TimeSpanElement],
     minimum_duration_minutes: int,
 ) -> datetime | None:
     """
-    Find the first reservable time span for the ReservationUnit that is long enough to fit the minimum duration
-
-    Loop through the normalised time spans to find the first one that is long enough to fit the minimum duration
+    Find the first reservable time span for the ReservationUnit.
+
+    In this function we know that normalised_reservable_time_spans (without buffers) and reservations (with buffers)
+    never overlap due to earlier normalisation.
+    However, normalised_reservable_time_spans (with buffers from reservation_unit) and
+    reservations (without buffers) can overlap, so we need to process the normalised_reservable_time_spans one-by-one
+    and check if they overlap with reservations.
+    If they do, we need to shorten the reservable_time_span so that its buffer doesn't overlap with the reservation,
+    while leaving the buffer size unchanged.
+    After processing, we can check if the reservable_time_span is still long enough to fit the minimum duration.
     """
     for reservable_time_span in normalised_reservable_time_spans:
-        # Move time span start time to the next valid start time
+        if reservable_time_span is None:
+            continue
+
+        reservable_time_span.buffer_time_before = reservation_unit.buffer_time_before
+        reservable_time_span.buffer_time_after = reservation_unit.buffer_time_after
+
+        for reservation in reservations:
+            # Reservation (with buffers) can not overlap with the reservable time span (without buffers).
+            if reservable_time_span.overlaps_with(reservation):
+                raise ValueError("Reservable Time Span overlaps with Reservations buffer, this should never happen.")
+
+            # Only continue forward if a buffered time span overlaps with a reservation (without buffers)
+            if not reservation.overlaps_with(reservable_time_span):
+                # No overlapping, skip
+                continue
+
+            # Reservation is inside the Before-buffer, shorten the reservable time span from the start
+            # ┌──────────────────────────┬─────────────────────────────────────┐
+            # │ ----oooooo ->     ----oo │ Reservation starts in Before-buffer │
+            # │ xxxx       -> xxxx       │                                     │
+            # ├──────────────────────────┼─────────────────────────────────────┤
+            # │     --oooo ->       --oo │ Reservation ends in Before-buffer   │
+            # │   xxxx     ->   xxxx     │                                     │
+            # └──────────────────────────┴─────────────────────────────────────┘
+            elif (
+                reservable_time_span.buffered_start_datetime
+                <= reservation.end_datetime
+                <= reservable_time_span.start_datetime
+            ):
+                overlap = reservation.end_datetime - reservable_time_span.buffered_start_datetime
+                reservable_time_span.start_datetime += overlap
+
+            # Reservation is inside the After-buffer, shorten the reservable time span from the end
+            # ┌──────────────────────────┬─────────────────────────────────────┐
+            # │ oooo----   -> oo----     │ Reservation starts in After-buffer  │
+            # │       xxxx ->       xxxx │                                     │
+            # ├──────────────────────────┼─────────────────────────────────────┤
+            # │ oooooo---- -> oo----     │ Reservation ends in After-buffer    │
+            # │       xxxx ->       xxxx │                                     │
+            # └──────────────────────────┴─────────────────────────────────────┘
+            elif (
+                reservable_time_span.end_datetime
+                <= reservation.start_datetime
+                <= reservable_time_span.buffered_end_datetime
+            ):
+                overlap = reservable_time_span.buffered_end_datetime - reservation.start_datetime
+                reservable_time_span.end_datetime -= overlap
+
+            if reservable_time_span.duration_minutes <= 0:
+                break
+
+        # In case of an invalid start time due to normalisation, move to the next valid start time
         reservable_time_span.move_to_next_valid_start_time(reservation_unit)
 
         if reservable_time_span.can_fit_reservation_for_reservation_unit(
@@ -305,15 +364,15 @@ def with_first_reservable_time(
             #  Affect closed status
             #  Can overlap with buffers
             reservation_unit_hard_closed_time_spans: list[TimeSpanElement] = (
-                _get_hard_closed_time_spans_for_reservation_unit(reservation_unit)  #
+                _get_hard_closed_time_spans_for_reservation_unit(reservation_unit)  # Formatting :)
                 + shared_hard_closed_time_spans
             )
 
             # Reservation Unit Soft Closed Time Spans
             #  Don't affect closed status
             #  Can overlap with buffers
             reservation_unit_soft_closed_time_spans: list[TimeSpanElement] = (
-                _get_soft_closed_time_spans_for_reservation_unit(reservation_unit)  #
+                _get_soft_closed_time_spans_for_reservation_unit(reservation_unit)  # Formatting :)
             )
 
             # Reservation Closed Time Spans
@@ -326,7 +385,7 @@ def with_first_reservable_time(
             # These time spans have no effect on the closed status of the ReservationUnit,
             # meaning that the ReservationUnit can be shown as open, even if there are no reservable time spans.
             soft_closed_time_spans = (
-                reservation_unit_soft_closed_time_spans  #
+                reservation_unit_soft_closed_time_spans  # Formatting :)
                 + reservation_unit_reservation_closed_time_spans
             )
 
@@ -339,10 +398,7 @@ def with_first_reservable_time(
 
             # Go through each ReservableTimeSpan individually one-by-one
             for reservable_time_span in reservation_unit.origin_hauki_resource.reservable_time_spans.all():
-                current_time_span = reservable_time_span.as_time_span_element(
-                    buffer_time_before=reservation_unit.buffer_time_before,
-                    buffer_time_after=reservation_unit.buffer_time_after,
-                )
+                current_time_span = reservable_time_span.as_time_span_element()
 
                 # Combine all Hard-Closed time spans into one list
                 hard_closed_time_spans: list[TimeSpanElement] = (
@@ -377,6 +433,7 @@ def with_first_reservable_time(
                 first_reservable_times[reservation_unit.pk] = _find_first_reservable_time_span_for_reservation_unit(
                     reservation_unit=reservation_unit,
                     normalised_reservable_time_spans=soft_normalised_reservable_time_spans,
+                    reservations=reservation_unit_reservation_closed_time_spans,
                     minimum_duration_minutes=minimum_duration_minutes,
                 )
                 # Suitable timespan was found, select and continue to next reservation unit