fix review

adamspd · adamspd · commit 11ba4b97f8cb · 2026-04-06T13:12:17.000+02:00
diff --git a/appointment/forms.py b/appointment/forms.py
@@ -25,15 +25,14 @@ class SlotForm(forms.Form):
             error_messages={'invalid_choice': _('Staff member does not exist')}
     )
     service_id = forms.ModelChoiceField(
-            queryset=None,
+            queryset=Service.objects.none(),
             required=False,
             error_messages={'invalid_choice': _('Service does not exist')}
     )
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
-        from appointment.models import Service as ServiceModel
-        self.fields['service_id'].queryset = ServiceModel.objects.all()
+        self.fields['service_id'].queryset = Service.objects.all()
 
 
 class AppointmentRequestForm(forms.ModelForm):
diff --git a/appointment/tests/utils/test_db_helpers.py b/appointment/tests/utils/test_db_helpers.py
@@ -629,12 +629,27 @@ def test_gap_time_blocks_slot_immediately_after_appointment(self):
     def test_gap_time_slot_after_gap_window_is_available(self):
         """A slot starting after appointment_end + gap_time is available."""
         appt = self._make_appointment(datetime.time(9, 0), datetime.time(10, 0), service=self.service1)
-        # 30-min gap → effective blocked range is [9:00, 10:30)
-        # slot 10:30 is at the boundary: slot < (10:00 + 30min) is False → available
+        # 30-min gap applied on both sides:
+        # gap-after: slot 10:00-10:30 is blocked (10:00 < 10:00+30min)
+        # slot 10:30: slot(10:30) < 10:30 is False → available
         result = exclude_booked_slots([appt], self.slots, self.slot_duration, gap_time=30)
         slot_10_30 = datetime.datetime.combine(self.today, datetime.time(10, 30))
         self.assertIn(slot_10_30, result)
 
+    def test_gap_time_blocks_slot_ending_within_gap_before_appointment(self):
+        """A slot whose end falls within the gap window before an appointment starts is blocked.
+        Covers the pre-appointment side of the gap (reviewer comment).
+        """
+        # Appointment starts at 10:30. slot_duration=30min, so slot 10:00→10:30 ends exactly at 10:30.
+        # With a 30-min gap: slot_end(10:30) + gap(30min) = 11:00 > appointment_start(10:30) → blocked.
+        appt = self._make_appointment(datetime.time(10, 30), datetime.time(11, 30), service=self.service1)
+        result = exclude_booked_slots([appt], self.slots, self.slot_duration, gap_time=30)
+        slot_10_00 = datetime.datetime.combine(self.today, datetime.time(10, 0))
+        self.assertNotIn(slot_10_00, result)
+        # slot 9:30 → slot_end 10:00 + gap 30min = 10:30; 10:30 < 10:30 is False → available
+        slot_9_30 = datetime.datetime.combine(self.today, datetime.time(9, 30))
+        self.assertIn(slot_9_30, result)
+
     def test_gap_time_zero_no_effect(self):
         """gap_time=0 has no effect compared to gap_time=None."""
         appt = self._make_appointment(datetime.time(9, 0), datetime.time(10, 0), service=self.service1)
@@ -644,21 +659,23 @@ def test_gap_time_zero_no_effect(self):
 
     def test_combined_service_duration_and_gap_time(self):
         """service_duration and gap_time can be used together."""
-        # 1-hr appointment at 10:00-11:00, 1-hr service_duration, 30-min gap
+        # 1-hr appointment at 10:00-11:00, 1-hr service_duration, 30-min gap (both sides)
         appt = self._make_appointment(datetime.time(10, 0), datetime.time(11, 0), service=self.service1)
         service_duration = datetime.timedelta(hours=1)
-        # Effective: check_duration=1hr, gap=30min, so slot < 11:30 is blocked if slot+1hr > 10:00
-        # slot 9:30 → [9:30, 10:30] > 10:00 → blocked
-        # slot 10:00 → blocked (inside appointment)
-        # slot 10:30 → slot(10:30) < 11:30 and [10:30,11:30] > 10:00 → blocked
-        # slot 11:00 → slot(11:00) < 11:30 → blocked
-        # slot 11:30 → slot(11:30) not < 11:30 → available
+        # check_duration = max(30min, 1hr) = 1hr; gap = 30min
+        # blocked if: appointment_start(10:00) < slot_end+gap AND slot < appointment_end(11:00)+gap(11:30)
+        # slot 9:00 → slot_end=10:00; 10:00 < 10:30=True; 9:00 < 11:30=True → blocked (pre-gap)
+        # slot 9:30 → slot_end=10:30; 10:00 < 11:00=True; 9:30 < 11:30=True → blocked
+        # slot 10:00, 10:30, 11:00 → blocked (inside/near appointment)
+        # slot 11:30 → slot(11:30) < 11:30 is False → available
         result = exclude_booked_slots([appt], self.slots, self.slot_duration,
                                       service_duration=service_duration, gap_time=30)
         slot_11_30 = datetime.datetime.combine(self.today, datetime.time(11, 30))
         self.assertIn(slot_11_30, result)
         slot_10_30 = datetime.datetime.combine(self.today, datetime.time(10, 30))
         self.assertNotIn(slot_10_30, result)
+        slot_9_00 = datetime.datetime.combine(self.today, datetime.time(9, 0))
+        self.assertNotIn(slot_9_00, result)
 
 
 class TestDayOffExistsForDateRange(BaseTest):
diff --git a/appointment/utils/db_helpers.py b/appointment/utils/db_helpers.py
@@ -357,8 +357,9 @@ def exclude_booked_slots(appointments, slots, slot_duration=None, service_durati
     :param service_duration: The actual service duration (timedelta). When provided, the effective check
         window is max(slot_duration, service_duration), preventing overlaps for services longer than the
         configured slot step.
-    :param gap_time: Required rest time in minutes between appointments. When provided, a slot is
-        considered unavailable if it falls within gap_time minutes after an existing appointment ends.
+    :param gap_time: Required rest time in minutes between appointments. Applied on both sides: a slot is
+        unavailable if it ends within gap_time minutes before an appointment starts, or if it starts
+        within gap_time minutes after an appointment ends.
     :return: The slots with the booked slots excluded.
     """
     if service_duration is not None:
@@ -374,7 +375,7 @@ def exclude_booked_slots(appointments, slots, slot_duration=None, service_durati
         for appointment in appointments:
             appointment_start_time = appointment.get_start_time()
             appointment_end_time = appointment.get_end_time()
-            if appointment_start_time < slot_end and slot < appointment_end_time + gap_delta:
+            if appointment_start_time < slot_end + gap_delta and slot < appointment_end_time + gap_delta:
                 is_available = False
                 break
         if is_available:
