diff --git a/src/hatty/controllers/lists.py b/src/hatty/controllers/lists.py index 463bedf..7b48748 100644 --- a/src/hatty/controllers/lists.py +++ b/src/hatty/controllers/lists.py @@ -72,6 +72,7 @@ def _do_delete(confirmed, _name=list_name): app.persist("lists", "manual_lists", "notify_lists", "default_list") app.notify(f"List '{_name}' deleted.", title="List Deleted") app._update_entities_display() + app.refresh_table_log_scope() app.push_screen(ConfirmPopup(f"Delete list '{list_name}'?"), _do_delete) elif action == "set_default": @@ -82,6 +83,7 @@ def _do_delete(confirmed, _name=list_name): app.notify(f"'{list_name}' set as default list.", title="Default List Set") app.set_title_based_on_focused_ui() app._update_entities_display() + app.refresh_table_log_scope() elif action == "view_as_dashboard": if app.dash_ctl.preview_list_as_dashboard(list_name): app.push_screen(DashboardScreen(), lambda _: app.dash_ctl.cleanup_temp_dashboards()) @@ -120,6 +122,7 @@ def rename_list(self, old_name: str | None, new_name: str | None) -> None: app.persist("lists", "manual_lists", "notify_lists", "default_list") app.set_title_based_on_focused_ui() app._update_entities_display() + app.refresh_table_log_scope() app.notify(f"Renamed list '{old_name}' to '{new_name}'.", title="List Renamed") def select_or_create(self, list_name: str) -> None: @@ -141,6 +144,7 @@ def select_or_create(self, list_name: str) -> None: self.unlocked_list = None self._app.set_title_based_on_focused_ui() self._app._update_entities_display() + self._app.refresh_table_log_scope() def is_locked(self, list_name: str) -> bool: """Whether removals from `list_name` currently require an unlock @@ -164,6 +168,8 @@ def apply_membership(self, list_name: str, entity_id: str, action: str) -> None: current_list.remove(entity_id) self._app.persist("lists") self._app._update_entities_display() + if list_name == self.current_list_name: + self._app.refresh_table_log_scope() def _freeze_visual_order(self, list_name: str, ordered_ids: list[str]) -> None: """Overwrite the stored list order with `ordered_ids` (the order currently diff --git a/src/hatty/controllers/logbook.py b/src/hatty/controllers/logbook.py index 0073563..094746a 100644 --- a/src/hatty/controllers/logbook.py +++ b/src/hatty/controllers/logbook.py @@ -33,6 +33,12 @@ DataTable.CellHighlighted into follow_cursor, which quietly re-applies the active option when the resolved scope actually changed. A maximized panel opts out — the table isn't what's focused there. + +A base_option's entity set is fixed only for as long as the session's +current `options` list says so — when what it closed over changes (the +table switching lists, issue #48), the host rebuilds fresh options and +hands them to rebuild_options, which swaps them in and re-applies the +still-active option id. """ import asyncio @@ -194,8 +200,10 @@ def base_option( ) -> LogScopeOption: """A fixed base entity set — the table's active list/all-entities snapshot, or an i/graph-opened entity set. Captured by closure at - build time since the base doesn't change while the panel stays open - (only cursor_option below re-derives on every call).""" + build time since the base doesn't change while these options stay + current (only cursor_option below re-derives on every call); a host + whose base can change from under it (the table switching lists) + rebuilds fresh options and swaps them in via rebuild_options.""" def _resolve() -> "LogScope | None": if not entity_ids: @@ -278,6 +286,22 @@ def apply_option(self, host: LogHost, option_id: str, *, quiet: bool = False) -> session.panel().clear() self.reload(host) + def rebuild_options(self, host: LogHost, options: list[LogScopeOption]) -> None: + """Swap an open session's scope options for freshly-built ones and + re-apply the active one — for when what a base_option closed over + changed underneath it (the table switching lists, issue #48). Keeps + the paged window and maximized state: a scope change in place, not a + reopen. A follows_cursor option needs no re-apply — follow_cursor + already re-resolves it as the table's new selection lands.""" + session = self.session_for(host) + if session is None: + return + session.options = options + option = next((o for o in options if o.id == session.option_id), None) + if option is None or option.follows_cursor: + return + self.apply_option(host, session.option_id) + def handle_scope_popup_result(self, host: LogHost, result: "str | None") -> None: if result is not None: self.apply_option(host, result) diff --git a/src/hatty/main.py b/src/hatty/main.py index 46fc733..6585baa 100644 --- a/src/hatty/main.py +++ b/src/hatty/main.py @@ -42,7 +42,7 @@ from hatty.controllers.dashboards import DashboardController from hatty.controllers.graphs import GraphController, _trim_history # noqa: F401 (_trim_history re-exported for tests) from hatty.controllers.lists import ListController -from hatty.controllers.logbook import LogbookController +from hatty.controllers.logbook import LogbookController, LogScopeOption from hatty.controllers.notifications import NotificationController from hatty.service_calls import _CONTROL_SERVICE_BUILDERS from hatty.types import Entity @@ -882,6 +882,19 @@ def action_toggle_activity_log(self) -> None: self.log_ctl.open(self, options=options, option_id="entities", hint=self._LOG_HINT) return + options = self._table_log_options() + if options is None: + self.notify("No entities to log. Select a list or add entities.", severity="warning") + return + self.log_ctl.open(self, options=options, option_id="list", hint=self._LOG_HINT) + + def _table_log_options(self) -> "list[LogScopeOption] | None": + """The `list`/`list_devices`/`cursor`/`cursor_device` scope rows for + whatever the table currently shows — the active list, or all + entities. None when there's nothing to log. Shared by + action_toggle_activity_log (fresh open) and refresh_table_log_scope + (issue #48 — re-derived when the active list changes under an + already-open log).""" if self.current_list_name: entity_ids = list(self.entity_lists.get(self.current_list_name, [])) base_label = self.current_list_name @@ -896,16 +909,31 @@ def action_toggle_activity_log(self) -> None: base_label = "All Entities" if not entity_ids: - self.notify("No entities to log. Select a list or add entities.", severity="warning") - return + return None - options = [ + return [ self.log_ctl.base_option("list", base_label, entity_ids, with_devices=False), self.log_ctl.base_option("list_devices", base_label, entity_ids, with_devices=True), self.log_ctl.cursor_option("cursor", self._selected_entity_id, with_device=False), self.log_ctl.cursor_option("cursor_device", self._selected_entity_id, with_device=True), ] - self.log_ctl.open(self, options=options, option_id="list", hint=self._LOG_HINT) + + _TABLE_LOG_OPTION_IDS = frozenset({"list", "list_devices", "cursor", "cursor_device"}) + + def refresh_table_log_scope(self) -> None: + """Re-point an open list-scoped activity log at whatever the table + now shows (issue #48) — called wherever the active list changes out + from under an already-open log. No-op for a graph/`i`-scoped + session, whose entity set is a deliberate snapshot.""" + session = self.log_ctl.session_for(self) + if session is None or not any(o.id in self._TABLE_LOG_OPTION_IDS for o in session.options): + return + options = self._table_log_options() + if options is None: + self.log_ctl.close(self) + self.notify("No entities to log. Select a list or add entities.", severity="warning") + return + self.log_ctl.rebuild_options(self, options) def action_toggle_entity_log(self) -> None: if self.log_ctl.is_open(self): @@ -1203,6 +1231,7 @@ def _do_leave_list(confirmed: bool | None, _name: str = self.current_list_name) self.current_list_name = None self._update_entities_display() self.set_title_based_on_focused_ui() + self.refresh_table_log_scope() self.push_screen(ConfirmPopup(f"Leave list '{self.current_list_name}'?"), _do_leave_list) diff --git a/tests/test_activity_log.py b/tests/test_activity_log.py index 761ced8..52ec9fa 100644 --- a/tests/test_activity_log.py +++ b/tests/test_activity_log.py @@ -8,7 +8,7 @@ from hatty.ui.entity_table import EntitiesTable from hatty.ui.graph.duration_popup import GraphDurationPopup from hatty.ui.graph.entity_detail import EntityDetailPanel -from tests.conftest import NO_LIST_CONFIG, make_config +from tests.conftest import NO_LIST_CONFIG, make_config, notified async def test_a_with_an_empty_active_list_notifies_and_stays_hidden(make_app, sample_entities): @@ -729,4 +729,174 @@ async def test_logbook_stream_unsubscribes_on_close(make_app): await pilot.press("a") # close await pilot.pause() assert app.client.logbook_subscription_id is None + + +# ── list switch re-scopes an open list-scoped log (issue #48) ─────────────── + + +def _two_list_config(**overrides): + return { + **make_config(), + "default_list": "list_a", + "lists": {"list_a": ["light.living_room_lamp"], "list_b": ["switch.fan"]}, + **overrides, + } + + +async def _switch_to_list_b(pilot) -> None: + """'l' opens the list popup with rows [View All, list_a, list_b] (issue + #48 fixtures use exactly two lists) — the first `down` merely engages + highlighting (no row starts highlighted), so 3 downs lands on list_b, + the same overshoot-clamped convention test_list_management.py uses.""" + await pilot.press("l") + await pilot.pause() + await pilot.press("down", "down", "down") + await pilot.press("enter") + await pilot.pause() + + +async def test_switching_list_rescopes_an_open_list_scoped_log(make_app, sample_entities): + app = make_app(entities=sample_entities, config_data=_two_list_config()) + async with app.run_test() as pilot: + await pilot.pause() + await pilot.press("a") + await pilot.pause() + panel = app.query_one("#activity_log_panel", ActivityLogPanel) + title = str(panel.query_one("#log_title", Label).content) + assert "list_a" in title + assert app.log_ctl.session_for(app).entity_ids == {"light.living_room_lamp"} + + await _switch_to_list_b(pilot) + + assert app.current_list_name == "list_b" + session = app.log_ctl.session_for(app) + assert session is not None + assert session.entity_ids == {"switch.fan"} + title = str(panel.query_one("#log_title", Label).content) + assert "list_b" in title and "list_a" not in title + assert app.client.logbook_calls[-1][0] == ["switch.fan"] + assert app.client.subscribe_logbook_calls[-1] == (["switch.fan"], []) + + +async def test_switching_list_rederives_the_list_devices_scope(make_app, sample_entities): + registry = [ + {"entity_id": "light.living_room_lamp", "device_id": "dev_lamp"}, + {"entity_id": "switch.fan", "device_id": "dev_fan"}, + ] + app = make_app(entities=sample_entities, config_data=_two_list_config(), registry=registry) + async with app.run_test() as pilot: + await pilot.pause() + await pilot.press("a") + await pilot.pause() + app.log_ctl.apply_option(app, "list_devices") + await pilot.pause() + assert app.log_ctl.session_for(app).device_ids == ["dev_lamp"] + + await _switch_to_list_b(pilot) + + session = app.log_ctl.session_for(app) + assert session.option_id == "list_devices" + assert session.device_ids == ["dev_fan"] + assert app.client.subscribe_logbook_calls[-1] == (["switch.fan"], ["dev_fan"]) + + +async def test_switching_to_view_all_retitles_the_open_log(make_app, sample_entities): + app = make_app(entities=sample_entities, config_data=_two_list_config()) + async with app.run_test() as pilot: + await pilot.pause() + await pilot.press("a") + await pilot.pause() + + await pilot.press("l") + await pilot.pause() + await pilot.press("down") # highlight "View All" at index 0 + await pilot.press("enter") + await pilot.pause() + + assert app.current_list_name is None + panel = app.query_one("#activity_log_panel", ActivityLogPanel) + title = str(panel.query_one("#log_title", Label).content) + assert "All Entities" in title + + +async def test_leaving_the_list_via_escape_retitles_the_open_log(make_app, sample_entities): + app = make_app(entities=sample_entities, config_data=_two_list_config()) + async with app.run_test() as pilot: + await pilot.pause() + await pilot.press("a") + await pilot.pause() + + await pilot.press("escape") + await pilot.pause() + await pilot.press("y") # confirm "Leave list" + await pilot.pause() + + assert app.current_list_name is None + panel = app.query_one("#activity_log_panel", ActivityLogPanel) + title = str(panel.query_one("#log_title", Label).content) + assert "All Entities" in title + + +async def test_switching_into_an_empty_list_closes_the_open_log(make_app, sample_entities): + config = _two_list_config(lists={"list_a": ["light.living_room_lamp"], "list_b": []}) + app = make_app(entities=sample_entities, config_data=config) + async with app.run_test() as pilot: + await pilot.pause() + await pilot.press("a") + await pilot.pause() + panel = app.query_one("#activity_log_panel", ActivityLogPanel) + assert panel.has_class("-visible") + + await _switch_to_list_b(pilot) + + assert not panel.has_class("-visible") + assert app.log_ctl.session_for(app) is None + assert notified(app, message_contains="No entities to log") + + +async def test_i_scoped_log_is_unaffected_by_a_list_switch(make_app, sample_entities): + """The `i` single-entity log is a deliberate snapshot (base_option's + docstring) — a list switch must not touch it.""" + app = make_app(entities=sample_entities, config_data=_two_list_config()) + async with app.run_test() as pilot: + await pilot.pause() + table = app.query_one(EntitiesTable) + table.jump_cursor_to_row_key("light.living_room_lamp") + await pilot.pause() + await pilot.press("i") + await pilot.pause() + session_before = app.log_ctl.session_for(app) + assert session_before.entity_ids == {"light.living_room_lamp"} + + await _switch_to_list_b(pilot) + + session = app.log_ctl.session_for(app) + assert session is not None + assert session.entity_ids == {"light.living_room_lamp"} + assert session.option_id == "entities" + + +async def test_toggling_membership_of_the_active_list_refreshes_the_open_log(make_app, sample_entities): + config = _two_list_config(lists={"list_a": ["light.living_room_lamp"], "list_b": ["switch.fan"]}) + app = make_app(entities=sample_entities, config_data=config) + async with app.run_test() as pilot: + await pilot.pause() + await pilot.press("a") + await pilot.pause() + assert app.log_ctl.session_for(app).entity_ids == {"light.living_room_lamp"} + + # A list's own view hides non-members; search overrides that so the + # target row is on screen without leaving list_a (issue #211's rule). + app.search_term = "temperature" + app._update_entities_display() + await pilot.pause() + table = app.query_one(EntitiesTable) + table.jump_cursor_to_row_key("sensor.temperature") + await pilot.pause() + await pilot.press("space") # add sensor.temperature to list_a + await pilot.pause() + + session = app.log_ctl.session_for(app) + assert session.entity_ids == {"light.living_room_lamp", "sensor.temperature"} + assert set(app.client.logbook_calls[-1][0]) == session.entity_ids assert app.client.unsubscribe_logbook_calls == 1 diff --git a/tests/unit/test_lists_controller.py b/tests/unit/test_lists_controller.py index 6199d4c..509711e 100644 --- a/tests/unit/test_lists_controller.py +++ b/tests/unit/test_lists_controller.py @@ -24,6 +24,7 @@ def __init__(self): self.pushed = [] self.search_term = "" self.notify_ctl = _StubNotifyCtl() + self.log_scope_refreshes = 0 def persist(self, *keys): self.persist_calls.append(keys) @@ -37,6 +38,9 @@ def _update_entities_display(self): def set_title_based_on_focused_ui(self): self.title_updates += 1 + def refresh_table_log_scope(self): + self.log_scope_refreshes += 1 + def push_screen(self, screen, callback=None): self.pushed.append(screen) if callback is not None: diff --git a/tests/unit/test_logbook_controller.py b/tests/unit/test_logbook_controller.py index 3b1fa06..3b28e2e 100644 --- a/tests/unit/test_logbook_controller.py +++ b/tests/unit/test_logbook_controller.py @@ -378,6 +378,81 @@ async def test_apply_option_resubscribes_with_the_new_scope(): assert app.client.subscribe_calls[-1] == (["light.b"], []) +# ── rebuild_options ────────────────────────────────────────────────────────── + + +async def test_rebuild_options_reapplies_the_active_base_option_against_the_new_set(): + """Issue #48: the table switched lists, so the host hands the session a + fresh options list built from the new list — the still-active option id + should be re-resolved against the new entity set, not the stale one.""" + ctl, app = _controller() + host = _StubHost() + old_options = [ctl.base_option("list", "list_a", ["light.a"], with_devices=False)] + ctl.open(host, options=old_options, option_id="list", hint="") + await app.run_spawned() + assert app.client.subscribe_calls[-1] == (["light.a"], []) + + new_options = [ctl.base_option("list", "list_b", ["light.b"], with_devices=False)] + ctl.rebuild_options(host, new_options) + await app.run_spawned() + + session = ctl.session_for(host) + assert session.entity_ids == {"light.b"} + assert session.title_base == "Activity Log — list_b" + assert app.client.logbook_calls[-1][0] == ["light.b"] + assert app.client.subscribe_calls[-1] == (["light.b"], []) + assert session.options is new_options + + +async def test_rebuild_options_keeps_the_paged_window_and_maximized_state(): + ctl, app = _controller() + host = _StubHost() + old_options = [ctl.base_option("list", "list_a", ["light.a"], with_devices=False)] + ctl.open(host, options=old_options, option_id="list", hint="") + await app.run_spawned() + ctl.page(host, -1) + await app.run_spawned() + session = ctl.session_for(host) + paged_end = session.end + assert paged_end is not None + host.panel_widget.add_class("-maximized") + + new_options = [ctl.base_option("list", "list_b", ["light.b"], with_devices=False)] + ctl.rebuild_options(host, new_options) + await app.run_spawned() + + assert session.end == paged_end + assert host.panel_widget.has_class("-maximized") + + +async def test_rebuild_options_does_not_reapply_a_follows_cursor_option(): + """A cursor-scoped session already re-resolves via follow_cursor as the + table's selection moves — rebuild_options should just swap the options + list in without an extra fetch.""" + ctl, app = _controller() + host = _StubHost() + app.all_entities = [{"entity_id": "light.a", "attributes": {}}] + old_options = [ctl.cursor_option("cursor", lambda: "light.a", with_device=False)] + ctl.open(host, options=old_options, option_id="cursor", hint="") + await app.run_spawned() + calls_before = len(app.client.logbook_calls) + + new_options = [ctl.cursor_option("cursor", lambda: "light.a", with_device=False)] + ctl.rebuild_options(host, new_options) + await app.run_spawned() + + assert len(app.client.logbook_calls) == calls_before + assert ctl.session_for(host).options is new_options + + +def test_rebuild_options_noop_without_an_open_session(): + ctl, app = _controller() + host = _StubHost() + options = [ctl.base_option("list", "list_b", ["light.b"], with_devices=False)] + ctl.rebuild_options(host, options) # must not raise + assert ctl.session_for(host) is None + + # ── follow_cursor ────────────────────────────────────────────────────────────