2 Commits 248590d6c8 ... 736c6dcd26

Author SHA1 Message Date
  ZiB 736c6dcd26 Fix test_guest_name_three_words_last_is_final to match correct first_name behavior 3 months ago
  ZiB e301e9488a Fix natural checkout not deactivating guest mode 3 months ago

+ 1 - 1
custom_components/room_cal_ha/coordinator.py

@@ -550,7 +550,7 @@ class RoomCalCoordinator(DataUpdateCoordinator[CoordinatorData]):
     from .models import GuestStateDiff
     from .wifi import get_wlan_state
 
-    diff = reservation_diff(old, new, now)
+    diff = reservation_diff(old, new)
 
     if not self._is_use_unifi_on():
       return diff

+ 20 - 9
custom_components/room_cal_ha/models.py

@@ -94,29 +94,40 @@ class ReservationData:
     return ReservationData.from_dict(current)
 
 
+def _is_occupied(res: ReservationData | None) -> bool:
+  """Whether `res` represents an assigned (checked-in) reservation, not an empty slot."""
+  return res is not None and res.check_in_date is not None and res.check_in_time is not None
+
+
 def reservation_diff(
   old: ReservationData | None,
   new: ReservationData | None,
-  now: datetime,
 ) -> GuestStateDiff:
   """Compute what guest-mode actions are needed when moving from old to new reservation state.
 
-  `now` must be a naive local datetime. `old` is the reservation that drove the current
-  guest-mode state; `new` is what should be reflected after the state change.
+  `old` is the reservation that drove the current guest-mode state; `new` is what should
+  be reflected after the state change. Both are Current-slot values, so by the caller's
+  invariant either is only ever populated with an already-active/checked-in reservation
+  (never a not-yet-arrived one) — occupancy of the slot is therefore what determines
+  whether check-in/check-out actions are needed, not a fresh wall-clock check against
+  `old`'s window. Using wall-clock time here was the bug: by the time a scheduled
+  natural-checkout transition runs, `now` is always already past `old`'s check-out, so
+  re-deriving activeness from `now` made `old` look inactive and silently dropped the
+  checkout action.
   """
-  old_active = is_window_active(old, now)
-  new_active = is_window_active(new, now)
+  old_occupied = _is_occupied(old)
+  new_occupied = _is_occupied(new)
 
-  if not old_active and not new_active:
+  if not old_occupied and not new_occupied:
     return GuestStateDiff()
 
-  if old_active and not new_active:
+  if old_occupied and not new_occupied:
     return GuestStateDiff(checkout_needed=True, departing=old)
 
-  if not old_active and new_active:
+  if not old_occupied and new_occupied:
     return GuestStateDiff(checkin_needed=True)
 
-  # Both active — same reservation window if check-in datetime matches.
+  # Both occupied — same reservation window if check-in datetime matches.
   assert old is not None and new is not None
   same_window = old.check_in_date == new.check_in_date and old.check_in_time == new.check_in_time
   if not same_window:

+ 12 - 3
tests/test_coordinator.py

@@ -377,6 +377,13 @@ _WIFI_NEXT = ReservationData(
 _AT_CHECK_IN = MagicMock()
 _AT_CHECK_IN.replace.return_value = datetime(2026, 7, 1, 14, 0, 0)
 
+# Freeze time at (i.e. slightly after) the scheduled check-out moment — this is what
+# `dt_util.now()` actually reads when a natural-checkout handler runs in production, and
+# is the case that exposed the bug where checkout actions require re-deriving "was old
+# active" from wall-clock time (see reservation_diff in models.py).
+_AT_CHECK_OUT = MagicMock()
+_AT_CHECK_OUT.replace.return_value = datetime(2026, 7, 5, 12, 0, 0)
+
 
 async def test_check_in_calls_wifi_key(hass: HomeAssistant, coordinator: RoomCalCoordinator):
   await coordinator.store.async_set(STORE_NEXT, _WIFI_NEXT)
@@ -469,7 +476,7 @@ async def test_check_out_disables_ssid(hass: HomeAssistant, coordinator: RoomCal
 
   with (
     patch.object(coordinator, "_is_mode_sync_on", return_value=True),
-    patch("custom_components.room_cal_ha.coordinator.dt_util.now", return_value=_AT_CHECK_IN),
+    patch("custom_components.room_cal_ha.coordinator.dt_util.now", return_value=_AT_CHECK_OUT),
     patch("custom_components.room_cal_ha.wifi.async_set_ssid_enabled", new_callable=AsyncMock) as mock_ssid,
   ):
     await coordinator._async_do_check_out()
@@ -510,7 +517,7 @@ async def test_check_out_ssid_error_still_deactivates_guest_mode(coordinator: Ro
 
   with (
     patch.object(coordinator, "_is_mode_sync_on", return_value=True),
-    patch("custom_components.room_cal_ha.coordinator.dt_util.now", return_value=_AT_CHECK_IN),
+    patch("custom_components.room_cal_ha.coordinator.dt_util.now", return_value=_AT_CHECK_OUT),
     patch("custom_components.room_cal_ha.wifi.async_set_ssid_enabled", side_effect=HomeAssistantError("hub down")),
   ):
     await coordinator._async_do_check_out()
@@ -1084,12 +1091,14 @@ async def test_unifi_diff_skipped_when_use_unifi_off(coordinator):
 
 
 async def test_unifi_diff_skipped_when_window_not_active(coordinator):
+  # Unchanged record (no occupancy transition) isolates the UniFi drift-check gate itself:
+  # even with use_unifi on, no wall-clock-active window means no drift check is made.
   before_window = datetime(2026, 6, 30, 12, 0, 0)
   with (
     patch.object(coordinator, "_is_use_unifi_on", return_value=True),
     patch("custom_components.room_cal_ha.wifi.get_wlan_state") as mock_get,
   ):
-    diff = coordinator._diff_with_unifi_check(None, _ACTIVE_WIFI_RES, before_window)
+    diff = coordinator._diff_with_unifi_check(_ACTIVE_WIFI_RES, _ACTIVE_WIFI_RES, before_window)
   mock_get.assert_not_called()
   assert not diff.has_changes
 

+ 17 - 30
tests/test_models.py

@@ -191,19 +191,14 @@ def _res(checkin_h: int, checkout_h: int, *, wifi_key="key", wifi_ssid="ssid", d
   )
 
 
-_DURING = datetime(2026, 7, 1, 15, 0)   # 15:00 — inside a 14:00–12:00 window
-_BEFORE = datetime(2026, 7, 1, 12, 0)   # 12:00 — before 14:00 check-in
-_AFTER  = datetime(2026, 7, 1, 22, 0)   # 22:00 — after typical check-out
-
-
 def test_diff_both_inactive_returns_no_changes():
-  diff = reservation_diff(None, None, _DURING)
+  diff = reservation_diff(None, None)
   assert not diff.has_changes
 
 
 def test_diff_old_none_new_active_returns_checkin():
   res = _res(14, 20)
-  diff = reservation_diff(None, res, _DURING)
+  diff = reservation_diff(None, res)
   assert diff.checkin_needed
   assert not diff.checkout_needed
   assert diff.departing is None
@@ -211,24 +206,24 @@ def test_diff_old_none_new_active_returns_checkin():
 
 def test_diff_old_active_new_none_returns_checkout():
   res = _res(14, 20)
-  diff = reservation_diff(res, None, _DURING)
+  diff = reservation_diff(res, None)
   assert diff.checkout_needed
   assert not diff.checkin_needed
   assert diff.departing is res
 
 
-def test_diff_both_elapsed_returns_no_changes():
-  # Window already passed for both — time-based handler owns this transition, not diff.
+def test_diff_unchanged_record_returns_no_changes():
   old = _res(14, 20)
-  diff = reservation_diff(old, old, _AFTER)
+  diff = reservation_diff(old, old)
   assert not diff.has_changes
 
 
-def test_diff_backend_clears_active_reservation_returns_checkout():
-  # Backend returns None while we're still inside the window → checkout needed.
+def test_diff_checkout_needed_even_when_now_is_past_check_out():
+  # Regression test: a scheduled natural checkout runs at/after old's check-out time,
+  # so the diff must not re-derive "was old active" from wall-clock time — it must
+  # trigger checkout purely because old was occupied and new (cleared) is not.
   old = _res(14, 20)
-  now = datetime(2026, 7, 1, 16, 0)
-  diff = reservation_diff(old, None, now)
+  diff = reservation_diff(old, None)
   assert diff.checkout_needed
   assert not diff.checkin_needed
   assert diff.departing is old
@@ -236,14 +231,14 @@ def test_diff_backend_clears_active_reservation_returns_checkout():
 
 def test_diff_same_window_no_changes_returns_empty():
   res = _res(14, 20)
-  diff = reservation_diff(res, res, _DURING)
+  diff = reservation_diff(res, res)
   assert not diff.has_changes
 
 
 def test_diff_same_window_wifi_key_changed():
   old = _res(14, 20, wifi_key="old-key")
   new = _res(14, 20, wifi_key="new-key")
-  diff = reservation_diff(old, new, _DURING)
+  diff = reservation_diff(old, new)
   assert diff.wifi_key_changed
   assert not diff.door_code_changed
   assert not diff.checkin_needed
@@ -253,7 +248,7 @@ def test_diff_same_window_wifi_key_changed():
 def test_diff_same_window_door_code_changed():
   old = _res(14, 20, door_code="1234")
   new = _res(14, 20, door_code="5678")
-  diff = reservation_diff(old, new, _DURING)
+  diff = reservation_diff(old, new)
   assert diff.door_code_changed
   assert not diff.wifi_key_changed
   assert not diff.checkin_needed
@@ -263,7 +258,7 @@ def test_diff_same_window_door_code_changed():
 def test_diff_same_window_no_change():
   old = _res(14, 20, wifi_key="key", door_code="1234")
   new = _res(14, 20, wifi_key="key", door_code="1234")
-  diff = reservation_diff(old, new, _DURING)
+  diff = reservation_diff(old, new)
   assert not diff.has_changes
 
 
@@ -278,23 +273,15 @@ def test_diff_different_window_both_active_returns_checkout_and_checkin():
     check_out_date=date(2026, 7, 2), check_out_time=time(12, 0),
     wifi_ssid="s", wifi_key="k",
   )
-  now = datetime(2026, 7, 1, 15, 0)  # after both check-ins
-  diff = reservation_diff(old, new, now)
+  diff = reservation_diff(old, new)
   assert diff.checkout_needed
   assert diff.checkin_needed
   assert diff.departing is old
 
 
-def test_diff_inactive_old_becomes_active_new_returns_checkin():
-  res = _res(14, 20)
-  diff = reservation_diff(res, res, _BEFORE)   # same record, but we're before check-in
-  # old is inactive (before window), new is inactive too → no changes
-  assert not diff.has_changes
-
-
-def test_diff_no_checkin_time_treated_as_inactive():
+def test_diff_no_checkin_time_treated_as_unoccupied():
   res = ReservationData(check_in_date=date(2026, 7, 1))  # no time fields
-  diff = reservation_diff(None, res, _DURING)
+  diff = reservation_diff(None, res)
   assert not diff.has_changes
 
 

+ 2 - 2
tests/test_sensors.py

@@ -125,10 +125,10 @@ def test_guest_name_three_words_last_is_final():
   coord = _coordinator(current=ReservationData(guest_name="ZiB Middle Guest"))
   s = _sensor(GuestNameSensor, coord, field="guest_name")
   attrs = s.extra_state_attributes
-  assert attrs["first_name"] == "ZiB Middle"
+  assert attrs["first_name"] == "ZiB"
   assert attrs["last_name"] == "Guest"
   assert attrs["last_abbr"] == "G"
-  assert attrs["public"] == "ZiB Middle G."
+  assert attrs["public"] == "ZiB G."
 
 
 def test_guest_name_none_returns_no_attrs():