From 3ff3bc6705cea48cda9191a166d34ed90afc6137 Mon Sep 17 00:00:00 2001 From: Michael Hansen Date: Mon, 6 Jul 2026 16:11:51 -0500 Subject: [PATCH] Make archive the only finish action --- .../components/local_timer_list/timer_list.py | 50 +++++----- .../components/timer_list/__init__.py | 20 +--- homeassistant/components/timer_list/const.py | 9 -- .../components/timer_list/services.yaml | 9 -- .../components/timer_list/strings.json | 11 --- tests/components/timer_list/__init__.py | 46 +++++---- tests/components/timer_list/test_init.py | 95 ++++++------------- tests/components/timer_list/test_trigger.py | 4 +- 8 files changed, 79 insertions(+), 165 deletions(-) diff --git a/homeassistant/components/local_timer_list/timer_list.py b/homeassistant/components/local_timer_list/timer_list.py index f03993191d5e..e84815d06a04 100644 --- a/homeassistant/components/local_timer_list/timer_list.py +++ b/homeassistant/components/local_timer_list/timer_list.py @@ -6,7 +6,6 @@ from typing import override from homeassistant.components.timer_list import ( DOMAIN as TIMER_LIST_DOMAIN, - TimerFinishAction, TimerItem, TimerListEntity, TimerListEntityFeature, @@ -23,6 +22,7 @@ from homeassistant.util import dt as dt_util, ulid as ulid_util from .const import CONF_TIMER_LIST_NAME _FINISHED_STATUSES = (TimerStatus.FINISHED, TimerStatus.CANCELLED) +MAX_ARCHIVED_TIMERS = 10 async def async_setup_entry( @@ -59,13 +59,7 @@ class LocalTimerListEntity(TimerListEntity): return list(self._timers.values()) @override - async def async_start_timer( - self, - *, - name: str | None, - duration: timedelta, - finish_action: TimerFinishAction, - ) -> str: + async def async_start_timer(self, *, name: str | None, duration: timedelta) -> str: """Create and start a new timer, returning its id.""" now = dt_util.utcnow() timer_id = ulid_util.ulid_now() @@ -73,7 +67,6 @@ class LocalTimerListEntity(TimerListEntity): timer_id=timer_id, name=name, status=TimerStatus.ACTIVE, - finish_action=finish_action, duration=duration, created_at=now, finishes_at=now + duration, @@ -109,11 +102,7 @@ class LocalTimerListEntity(TimerListEntity): @override async def async_cancel_timer(self, timer_id: str) -> None: - """Cancel a timer. - - The timer is retained in the ``cancelled`` state only when its finish - action is ``archive``; otherwise it is removed. - """ + """Cancel a timer, archiving it in the ``cancelled`` state.""" timer = self._get_timer(timer_id) self._unschedule(timer_id) timer.status = TimerStatus.CANCELLED @@ -121,9 +110,7 @@ class LocalTimerListEntity(TimerListEntity): timer.remaining = None timer.finished_at = dt_util.utcnow() self._notify(TimerListEventType.CANCELLED, timer) - if timer.finish_action != TimerFinishAction.ARCHIVE: - del self._timers[timer_id] - self._notify(TimerListEventType.REMOVED, timer) + self._enforce_archive_limit() @override async def async_cancel_all_timers(self) -> None: @@ -202,7 +189,7 @@ class LocalTimerListEntity(TimerListEntity): @callback def _async_timer_finished(self, timer_id: str, now: datetime) -> None: - """Handle a timer reaching its finish time.""" + """Handle a timer reaching its finish time, archiving it.""" self._cancel_callbacks.pop(timer_id, None) if (timer := self._timers.get(timer_id)) is None: return @@ -212,18 +199,25 @@ class LocalTimerListEntity(TimerListEntity): timer.remaining = None timer.finished_at = dt_util.utcnow() self._notify(TimerListEventType.FINISHED, timer) + self._enforce_archive_limit() - if timer.finish_action == TimerFinishAction.REMOVE: - self._timers.pop(timer_id, None) + @callback + def _enforce_archive_limit(self) -> None: + """Evict the oldest archived timers beyond ``MAX_ARCHIVED_TIMERS``.""" + archived = sorted( + ( + timer + for timer in self._timers.values() + if timer.status in _FINISHED_STATUSES + ), + key=lambda timer: timer.finished_at or dt_util.utcnow(), + ) + excess = len(archived) - MAX_ARCHIVED_TIMERS + if excess <= 0: + return + for timer in archived[:excess]: + del self._timers[timer.timer_id] self._notify(TimerListEventType.REMOVED, timer) - elif timer.finish_action == TimerFinishAction.RESTART: - restarted_at = dt_util.utcnow() - timer.status = TimerStatus.ACTIVE - timer.created_at = restarted_at - timer.finishes_at = restarted_at + timer.duration - timer.finished_at = None - self._schedule(timer) - self._notify(TimerListEventType.STARTED, timer) @override async def async_will_remove_from_hass(self) -> None: diff --git a/homeassistant/components/timer_list/__init__.py b/homeassistant/components/timer_list/__init__.py index 7a3eafbb9e0a..cd91915252c4 100644 --- a/homeassistant/components/timer_list/__init__.py +++ b/homeassistant/components/timer_list/__init__.py @@ -35,12 +35,10 @@ from homeassistant.util import dt as dt_util from .const import ( ATTR_DURATION, - ATTR_FINISH_ACTION, ATTR_STATUS, ATTR_TIMER_ID, DATA_COMPONENT, DOMAIN, - TimerFinishAction, TimerListEntityFeature, TimerListEventType, TimerListServices, @@ -67,11 +65,8 @@ class TimerItem: status: TimerStatus """Current status of the timer.""" - finish_action: TimerFinishAction - """What happens to the timer once it finishes.""" - duration: timedelta - """Original duration the timer was created with (used by ``restart``).""" + """Original duration the timer was created with.""" created_at: datetime """When the timer was (re)started, in UTC.""" @@ -109,7 +104,6 @@ def timer_to_dict(item: TimerItem, now: datetime) -> dict[str, Any]: ATTR_TIMER_ID: item.timer_id, ATTR_NAME: item.name, ATTR_STATUS: item.status.value, - ATTR_FINISH_ACTION: item.finish_action.value, "duration": item.duration.total_seconds(), "created_at": item.created_at.isoformat(), "finishes_at": item.finishes_at.isoformat() if item.finishes_at else None, @@ -132,9 +126,6 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: { vol.Optional(ATTR_NAME): cv.string, vol.Required(ATTR_DURATION): cv.positive_time_period, - vol.Optional( - ATTR_FINISH_ACTION, default=TimerFinishAction.REMOVE - ): vol.Coerce(TimerFinishAction), }, _async_start_timer, required_features=[TimerListEntityFeature.START_TIMER], @@ -238,13 +229,7 @@ class TimerListEntity(Entity): """Return the timers in the list.""" raise NotImplementedError - async def async_start_timer( - self, - *, - name: str | None, - duration: timedelta, - finish_action: TimerFinishAction, - ) -> str: + async def async_start_timer(self, *, name: str | None, duration: timedelta) -> str: """Create and start a new timer, returning its id.""" raise NotImplementedError @@ -311,7 +296,6 @@ async def _async_start_timer( timer_id = await entity.async_start_timer( name=call.data.get(ATTR_NAME), duration=call.data[ATTR_DURATION], - finish_action=call.data[ATTR_FINISH_ACTION], ) return {ATTR_TIMER_ID: timer_id} diff --git a/homeassistant/components/timer_list/const.py b/homeassistant/components/timer_list/const.py index 7e832add07ae..537a8c919211 100644 --- a/homeassistant/components/timer_list/const.py +++ b/homeassistant/components/timer_list/const.py @@ -15,7 +15,6 @@ DATA_COMPONENT: HassKey[EntityComponent[TimerListEntity]] = HassKey(DOMAIN) ATTR_TIMER_ID = "timer_id" ATTR_DURATION = "duration" -ATTR_FINISH_ACTION = "finish_action" ATTR_FINISHES_AT = "finishes_at" ATTR_CREATED_AT = "created_at" ATTR_FINISHED_AT = "finished_at" @@ -49,14 +48,6 @@ class TimerStatus(StrEnum): CANCELLED = "cancelled" -class TimerFinishAction(StrEnum): - """What happens to a timer once it finishes.""" - - REMOVE = "remove" - ARCHIVE = "archive" - RESTART = "restart" - - class TimerListEventType(StrEnum): """Type of change pushed to timer list subscribers.""" diff --git a/homeassistant/components/timer_list/services.yaml b/homeassistant/components/timer_list/services.yaml index 9245036cc4b9..1e922fb69ffa 100644 --- a/homeassistant/components/timer_list/services.yaml +++ b/homeassistant/components/timer_list/services.yaml @@ -14,15 +14,6 @@ start_timer: example: "00:05:00" selector: duration: - finish_action: - default: remove - selector: - select: - translation_key: finish_action - options: - - remove - - archive - - restart pause_timer: target: entity: diff --git a/homeassistant/components/timer_list/strings.json b/homeassistant/components/timer_list/strings.json index 175629c130fa..39a930e3e295 100644 --- a/homeassistant/components/timer_list/strings.json +++ b/homeassistant/components/timer_list/strings.json @@ -10,13 +10,6 @@ } }, "selector": { - "finish_action": { - "options": { - "archive": "Archive", - "remove": "Remove", - "restart": "Restart" - } - }, "status": { "options": { "active": "Active", @@ -110,10 +103,6 @@ "description": "How long the timer should run for.", "name": "Duration" }, - "finish_action": { - "description": "What happens to the timer once it finishes.", - "name": "Finish action" - }, "name": { "description": "Optional name for the timer.", "name": "Name" diff --git a/tests/components/timer_list/__init__.py b/tests/components/timer_list/__init__.py index e04a0488acba..9dba7af610c2 100644 --- a/tests/components/timer_list/__init__.py +++ b/tests/components/timer_list/__init__.py @@ -6,7 +6,6 @@ from typing import override from homeassistant.components.timer_list import ( DOMAIN, - TimerFinishAction, TimerItem, TimerListEntity, TimerListEntityFeature, @@ -32,6 +31,7 @@ ALL_FEATURES = ( ) _FINISHED_STATUSES = (TimerStatus.FINISHED, TimerStatus.CANCELLED) +MAX_ARCHIVED_TIMERS = 10 class MockFlow(ConfigFlow): @@ -62,13 +62,7 @@ class MockTimerListEntity(TimerListEntity): return list(self._timers.values()) @override - async def async_start_timer( - self, - *, - name: str | None, - duration: timedelta, - finish_action: TimerFinishAction, - ) -> str: + async def async_start_timer(self, *, name: str | None, duration: timedelta) -> str: """Create and start a new timer, returning its id.""" now = dt_util.utcnow() timer_id = ulid_util.ulid_now() @@ -76,7 +70,6 @@ class MockTimerListEntity(TimerListEntity): timer_id=timer_id, name=name, status=TimerStatus.ACTIVE, - finish_action=finish_action, duration=duration, created_at=now, finishes_at=now + duration, @@ -112,7 +105,7 @@ class MockTimerListEntity(TimerListEntity): @override async def async_cancel_timer(self, timer_id: str) -> None: - """Cancel a timer.""" + """Cancel a timer, archiving it in the ``cancelled`` state.""" timer = self._get_timer(timer_id) self._unschedule(timer_id) timer.status = TimerStatus.CANCELLED @@ -120,9 +113,7 @@ class MockTimerListEntity(TimerListEntity): timer.remaining = None timer.finished_at = dt_util.utcnow() self._notify(TimerListEventType.CANCELLED, timer) - if timer.finish_action != TimerFinishAction.ARCHIVE: - del self._timers[timer_id] - self._notify(TimerListEventType.REMOVED, timer) + self._enforce_archive_limit() @override async def async_cancel_all_timers(self) -> None: @@ -201,7 +192,7 @@ class MockTimerListEntity(TimerListEntity): @callback def _async_timer_finished(self, timer_id: str, now: datetime) -> None: - """Handle a timer reaching its finish time.""" + """Handle a timer reaching its finish time, archiving it.""" self._cancel_callbacks.pop(timer_id, None) if (timer := self._timers.get(timer_id)) is None: return @@ -211,18 +202,25 @@ class MockTimerListEntity(TimerListEntity): timer.remaining = None timer.finished_at = dt_util.utcnow() self._notify(TimerListEventType.FINISHED, timer) + self._enforce_archive_limit() - if timer.finish_action == TimerFinishAction.REMOVE: - self._timers.pop(timer_id, None) + @callback + def _enforce_archive_limit(self) -> None: + """Evict the oldest archived timers beyond ``MAX_ARCHIVED_TIMERS``.""" + archived = sorted( + ( + timer + for timer in self._timers.values() + if timer.status in _FINISHED_STATUSES + ), + key=lambda timer: timer.finished_at or dt_util.utcnow(), + ) + excess = len(archived) - MAX_ARCHIVED_TIMERS + if excess <= 0: + return + for timer in archived[:excess]: + del self._timers[timer.timer_id] self._notify(TimerListEventType.REMOVED, timer) - elif timer.finish_action == TimerFinishAction.RESTART: - restarted_at = dt_util.utcnow() - timer.status = TimerStatus.ACTIVE - timer.created_at = restarted_at - timer.finishes_at = restarted_at + timer.duration - timer.finished_at = None - self._schedule(timer) - self._notify(TimerListEventType.STARTED, timer) @override async def async_will_remove_from_hass(self) -> None: diff --git a/tests/components/timer_list/test_init.py b/tests/components/timer_list/test_init.py index 404e22275c6a..6f6e64c00236 100644 --- a/tests/components/timer_list/test_init.py +++ b/tests/components/timer_list/test_init.py @@ -23,13 +23,9 @@ async def _start_timer( *, duration: int = 60, name: str | None = None, - finish_action: str = "remove", ) -> str: """Start a timer and return its id.""" - data: dict[str, Any] = { - "duration": {"seconds": duration}, - "finish_action": finish_action, - } + data: dict[str, Any] = {"duration": {"seconds": duration}} if name is not None: data[ATTR_NAME] = name result = await hass.services.async_call( @@ -104,27 +100,11 @@ async def test_get_timers_status_filter(hass: HomeAssistant) -> None: @pytest.mark.usefixtures("test_entity") -async def test_finish_action_remove( +async def test_timer_finishes_and_is_archived( hass: HomeAssistant, freezer: FrozenDateTimeFactory ) -> None: - """Test a timer is removed after finishing with the remove action.""" - await _start_timer(hass, duration=60, finish_action="remove") - assert hass.states.get(TEST_ENTITY_ID).state == "1" - - freezer.tick(timedelta(seconds=61)) - async_fire_time_changed(hass) - await hass.async_block_till_done() - - assert hass.states.get(TEST_ENTITY_ID).state == "0" - assert await _get_timers(hass) == [] - - -@pytest.mark.usefixtures("test_entity") -async def test_finish_action_archive( - hass: HomeAssistant, freezer: FrozenDateTimeFactory -) -> None: - """Test a timer is retained as finished with the archive action.""" - await _start_timer(hass, duration=60, finish_action="archive") + """Test a finished timer is archived as ``finished``.""" + await _start_timer(hass, duration=60) freezer.tick(timedelta(seconds=61)) async_fire_time_changed(hass) @@ -137,24 +117,6 @@ async def test_finish_action_archive( assert timers[0]["finished_at"] is not None -@pytest.mark.usefixtures("test_entity") -async def test_finish_action_restart( - hass: HomeAssistant, freezer: FrozenDateTimeFactory -) -> None: - """Test a timer restarts itself with the restart action.""" - timer_id = await _start_timer(hass, duration=60, finish_action="restart") - - freezer.tick(timedelta(seconds=61)) - async_fire_time_changed(hass) - await hass.async_block_till_done() - - assert hass.states.get(TEST_ENTITY_ID).state == "1" - timers = await _get_timers(hass) - assert len(timers) == 1 - assert timers[0]["timer_id"] == timer_id - assert timers[0]["status"] == "active" - - @pytest.mark.usefixtures("test_entity") async def test_pause_and_unpause(hass: HomeAssistant) -> None: """Test pausing and resuming a timer.""" @@ -190,7 +152,7 @@ async def test_add_and_remove_time( @pytest.mark.usefixtures("test_entity") async def test_remove_time_finishes_timer(hass: HomeAssistant) -> None: """Test removing more time than remaining finishes the timer immediately.""" - timer_id = await _start_timer(hass, duration=60, finish_action="archive") + timer_id = await _start_timer(hass, duration=60) await _call(hass, "remove_time", timer_id=timer_id, duration={"seconds": 120}) @@ -199,19 +161,9 @@ async def test_remove_time_finishes_timer(hass: HomeAssistant) -> None: @pytest.mark.usefixtures("test_entity") -async def test_cancel_timer_remove(hass: HomeAssistant) -> None: - """Test cancelling a remove-action timer deletes it.""" - timer_id = await _start_timer(hass, finish_action="remove") - await _call(hass, "cancel_timer", timer_id=timer_id) - - assert hass.states.get(TEST_ENTITY_ID).state == "0" - assert await _get_timers(hass) == [] - - -@pytest.mark.usefixtures("test_entity") -async def test_cancel_timer_archive(hass: HomeAssistant) -> None: - """Test cancelling an archive-action timer retains it as cancelled.""" - timer_id = await _start_timer(hass, finish_action="archive") +async def test_cancel_timer_archives_timer(hass: HomeAssistant) -> None: + """Test cancelling a timer retains it as cancelled.""" + timer_id = await _start_timer(hass) await _call(hass, "cancel_timer", timer_id=timer_id) assert hass.states.get(TEST_ENTITY_ID).state == "0" @@ -222,21 +174,20 @@ async def test_cancel_timer_archive(hass: HomeAssistant) -> None: @pytest.mark.usefixtures("test_entity") async def test_cancel_all_timers(hass: HomeAssistant) -> None: - """Test cancelling all timers.""" + """Test cancelling all timers archives them.""" + await _start_timer(hass) await _start_timer(hass) - await _start_timer(hass, finish_action="archive") await _call(hass, "cancel_all_timers") assert hass.states.get(TEST_ENTITY_ID).state == "0" - # The archived timer is retained as cancelled, the remove timer is deleted. - assert len(await _get_timers(hass)) == 1 + assert len(await _get_timers(hass)) == 2 @pytest.mark.usefixtures("test_entity") async def test_clear_finished_timers(hass: HomeAssistant) -> None: """Test clearing finished and cancelled timers.""" - timer_id = await _start_timer(hass, finish_action="archive") + timer_id = await _start_timer(hass) await _call(hass, "cancel_timer", timer_id=timer_id) await _start_timer(hass) assert len(await _get_timers(hass)) == 2 @@ -248,6 +199,25 @@ async def test_clear_finished_timers(hass: HomeAssistant) -> None: assert timers[0]["status"] == "active" +@pytest.mark.usefixtures("test_entity") +async def test_archive_limit_evicts_oldest( + hass: HomeAssistant, freezer: FrozenDateTimeFactory +) -> None: + """Test only the 10 most recently archived timers are retained.""" + timer_ids = [] + for _ in range(11): + timer_id = await _start_timer(hass) + await _call(hass, "cancel_timer", timer_id=timer_id) + timer_ids.append(timer_id) + freezer.tick(timedelta(seconds=1)) + + timers = await _get_timers(hass) + assert len(timers) == 10 + archived_ids = {timer["timer_id"] for timer in timers} + assert timer_ids[0] not in archived_ids + assert set(timer_ids[1:]) == archived_ids + + @pytest.mark.usefixtures("test_entity") async def test_remove_timer(hass: HomeAssistant) -> None: """Test removing a single timer regardless of status.""" @@ -291,9 +261,6 @@ async def test_websocket_subscribe( await _call(hass, "cancel_timer", timer_id=timer_id) msg = await client.receive_json() assert msg["event"]["event_type"] == "cancelled" - # remove-action timers also emit a removed event after cancellation. - msg = await client.receive_json() - assert msg["event"]["event_type"] == "removed" @pytest.mark.usefixtures("test_entity") diff --git a/tests/components/timer_list/test_trigger.py b/tests/components/timer_list/test_trigger.py index 5e54f9480b72..c5b1b67df6a0 100644 --- a/tests/components/timer_list/test_trigger.py +++ b/tests/components/timer_list/test_trigger.py @@ -66,12 +66,12 @@ async def _setup_automation(hass: HomeAssistant, trigger_type: str) -> None: await hass.async_block_till_done() -async def _start_timer(hass: HomeAssistant, finish_action: str = "remove") -> str: +async def _start_timer(hass: HomeAssistant) -> str: """Start a timer and return its id.""" result = await hass.services.async_call( DOMAIN, "start_timer", - {"duration": {"seconds": 60}, "finish_action": finish_action}, + {"duration": {"seconds": 60}}, target={ATTR_ENTITY_ID: TEST_ENTITY_ID}, blocking=True, return_response=True,