Fixes reserved_slots_by_reservation deleting sibling slots on partly_…

src/libres/db/scheduler.py

@@ -1668,7 +1668,9 @@ def change_reservation(
         assert new_start and new_end
 
         new_start, new_end = self._prepare_range(new_start, new_end)
-        existing_reservation = self.reservations_by_token(token, id).one()
+        existing_reservation = self.reservations_by_token(token, id).first()
+        if existing_reservation is None:
+            raise errors.InvalidReservationToken()
 
         # if there's nothing to change, do not change
         if quota is None or existing_reservation.quota == quota:  # noqa: SIM102
@@ -1680,7 +1682,10 @@ def change_reservation(
 
         # will raise a MultipleResultsFound exception if this is a group
         if existing_reservation.status == 'approved':
-            allocation = self.allocations_by_reservation(token, id).one()
+            try:
+                allocation = self.allocations_by_reservation(token, id).one()
+            except exc.NoResultFound as ex:
+                raise errors.InvalidReservationError() from ex
         else:
             _allocation = existing_reservation._target_allocations().first()
             assert _allocation is not None
@@ -2299,7 +2304,20 @@ def reserved_slots_by_reservation(
         else:
             allocations = self.allocations_by_reservation(token, id)
             ids = allocations.with_entities(Allocation.id)
-            return query.filter(ReservedSlot.allocation_id.in_(ids))
+            query = query.filter(ReservedSlot.allocation_id.in_(ids))
+
+            # When multiple reservations share the same token on the same
+            # partly_available allocation, allocation_id alone doesn't
+            # distinguish their slots. Filter by the reservation's time range
+            # as a workaround until ReservedSlot has a reservation_id column.
+            reservation = self.reservations_by_token(token, id).first()
+            if reservation is not None and reservation.start is not None:
+                query = query.filter(
+                    ReservedSlot.start >= reservation.start,
+                    ReservedSlot.end <= reservation.end
+                )
+
+            return query
 
     def reserved_slots_by_blocker(
         self,

tests/test_scheduler.py

@@ -618,6 +618,83 @@ def test_change_reservation_assertions(scheduler: Scheduler) -> None:
         )
 
 
+def test_change_reservation_with_nonexistent_id(
+    scheduler: Scheduler,
+) -> None:
+    dates = (datetime(2014, 3, 7, 8, 0), datetime(2014, 3, 7, 17, 0))
+    scheduler.allocate(dates, partly_available=True)
+    token = scheduler.reserve('user@example.org', dates)
+    scheduler.commit()
+
+    reservation = scheduler.reservations_by_token(token).one()
+    scheduler.approve_reservations(token)
+    scheduler.commit()
+
+    with pytest.raises(errors.InvalidReservationToken):
+        scheduler.change_reservation(
+            token, reservation.id + 99999,
+            datetime(2014, 3, 7, 9, 0), datetime(2014, 3, 7, 16, 0)
+        )
+
+
+def test_remove_reservation_does_not_affect_sibling_reservations(
+    scheduler: Scheduler,
+) -> None:
+    """Removing one reservation on a partly_available allocation must not
+    delete the ReservedSlot of another reservation sharing the same token
+    on the same allocation (the root cause of a production NoResultFound)."""
+    dates_full = (datetime(2014, 3, 7, 8, 0), datetime(2014, 3, 7, 18, 0))
+    scheduler.allocate(dates_full, partly_available=True)
+
+    dates_a = (datetime(2014, 3, 7, 8, 0), datetime(2014, 3, 7, 10, 0))
+    dates_b = (datetime(2014, 3, 7, 10, 0), datetime(2014, 3, 7, 12, 0))
+
+    session = new_uuid()
+    token_a = scheduler.reserve(
+        'user@example.org', dates_a,
+        session_id=session, single_token_per_session=True
+    )
+    scheduler.commit()
+    # Both reservations share the same token because single_token_per_session
+    # is used — this is how onegov groups multiple dates into one booking.
+    token_b = scheduler.reserve(
+        'user@example.org', dates_b,
+        session_id=session, single_token_per_session=True
+    )
+    assert token_a == token_b
+    scheduler.commit()
+
+    scheduler.approve_reservations(token_a)
+    scheduler.commit()
+
+    reservations = sorted(
+        scheduler.reservations_by_token(token_a).all(),
+        key=lambda r: r.start
+    )
+    assert len(reservations) == 2
+    res_a, res_b = reservations  # res_a starts at 08:00, res_b at 10:00
+
+    # Each reservation on a partly_available allocation has its own slots.
+    slots_a_count = scheduler.reserved_slots_by_reservation(
+        token_a, res_a.id
+    ).count()
+    slots_b_count = scheduler.reserved_slots_by_reservation(
+        token_a, res_b.id
+    ).count()
+    assert slots_a_count > 0
+    assert slots_b_count > 0
+    # The two reservations cover different time ranges so their slots differ.
+    assert slots_a_count != 0
+    assert slots_b_count != 0
+
+    # Remove only res_b — res_a's slots must survive intact.
+    scheduler.remove_reservation(token_a, res_b.id)
+    scheduler.commit()
+
+    remaining_count = scheduler.reserved_slots_by_reservation(token_a).count()
+    assert remaining_count == slots_a_count
+
+
 def test_change_unapproved_reservation_quota(scheduler: Scheduler) -> None:
     dates = (datetime(2014, 8, 7, 8, 0), datetime(2014, 8, 7, 10, 0))
     scheduler.allocate(dates, quota=2)