From 6cb2148244acfabb13f215b67532f19a764d08af Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Sat, 16 Sep 2023 19:58:51 +0000 Subject: [PATCH 01/21] Add local_todo component --- CODEOWNERS | 2 + .../components/local_todo/__init__.py | 49 +++ .../components/local_todo/config_flow.py | 40 ++ homeassistant/components/local_todo/const.py | 5 + .../components/local_todo/manifest.json | 9 + homeassistant/components/local_todo/store.py | 38 ++ .../components/local_todo/strings.json | 13 + homeassistant/components/local_todo/todo.py | 179 +++++++++ homeassistant/generated/config_flows.py | 1 + homeassistant/generated/integrations.json | 6 + requirements_all.txt | 1 + requirements_test_all.txt | 1 + script/hassfest/translations.py | 1 + tests/components/local_todo/__init__.py | 1 + tests/components/local_todo/conftest.py | 78 ++++ .../components/local_todo/test_config_flow.py | 33 ++ tests/components/local_todo/test_init.py | 18 + tests/components/local_todo/test_todo.py | 354 ++++++++++++++++++ 18 files changed, 829 insertions(+) create mode 100644 homeassistant/components/local_todo/__init__.py create mode 100644 homeassistant/components/local_todo/config_flow.py create mode 100644 homeassistant/components/local_todo/const.py create mode 100644 homeassistant/components/local_todo/manifest.json create mode 100644 homeassistant/components/local_todo/store.py create mode 100644 homeassistant/components/local_todo/strings.json create mode 100644 homeassistant/components/local_todo/todo.py create mode 100644 tests/components/local_todo/__init__.py create mode 100644 tests/components/local_todo/conftest.py create mode 100644 tests/components/local_todo/test_config_flow.py create mode 100644 tests/components/local_todo/test_init.py create mode 100644 tests/components/local_todo/test_todo.py diff --git a/CODEOWNERS b/CODEOWNERS index 87ac2cebfcb7d8..cf5866c3d5a701 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -708,6 +708,8 @@ build.json @home-assistant/supervisor /tests/components/local_calendar/ @allenporter /homeassistant/components/local_ip/ @issacg /tests/components/local_ip/ @issacg +/homeassistant/components/local_todo/ @allenporter +/tests/components/local_todo/ @allenporter /homeassistant/components/lock/ @home-assistant/core /tests/components/lock/ @home-assistant/core /homeassistant/components/logbook/ @home-assistant/core diff --git a/homeassistant/components/local_todo/__init__.py b/homeassistant/components/local_todo/__init__.py new file mode 100644 index 00000000000000..e805eea3818632 --- /dev/null +++ b/homeassistant/components/local_todo/__init__.py @@ -0,0 +1,49 @@ +"""The Local To-do integration.""" +from __future__ import annotations + +from pathlib import Path + +from homeassistant.config_entries import ConfigEntry +from homeassistant.const import Platform +from homeassistant.core import HomeAssistant +from homeassistant.util import slugify + +from .const import CONF_TODO_LIST_NAME, DOMAIN +from .store import LocalTodoListStore + +PLATFORMS: list[Platform] = [Platform.TODO] + +STORAGE_PATH = ".storage/local_todo.{key}.ics" + + +async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: + """Set up Local To-do from a config entry.""" + + hass.data.setdefault(DOMAIN, {}) + + key = slugify(entry.data[CONF_TODO_LIST_NAME]) + path = Path(hass.config.path(STORAGE_PATH.format(key=key))) + hass.data[DOMAIN][entry.entry_id] = LocalTodoListStore(hass, path) + + await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS) + + return True + + +async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: + """Unload a config entry.""" + if unload_ok := await hass.config_entries.async_unload_platforms(entry, PLATFORMS): + hass.data[DOMAIN].pop(entry.entry_id) + + return unload_ok + + +async def async_remove_entry(hass: HomeAssistant, entry: ConfigEntry) -> None: + """Handle removal of an entry.""" + key = slugify(entry.data[CONF_TODO_LIST_NAME]) + path = Path(hass.config.path(STORAGE_PATH.format(key=key))) + + def unlink(path: Path) -> None: + path.unlink(missing_ok=True) + + await hass.async_add_executor_job(unlink, path) diff --git a/homeassistant/components/local_todo/config_flow.py b/homeassistant/components/local_todo/config_flow.py new file mode 100644 index 00000000000000..02e95c77a3a7eb --- /dev/null +++ b/homeassistant/components/local_todo/config_flow.py @@ -0,0 +1,40 @@ +"""Config flow for Local To-do integration.""" +from __future__ import annotations + +import logging +from typing import Any + +import voluptuous as vol + +from homeassistant import config_entries +from homeassistant.data_entry_flow import FlowResult + +from .const import CONF_TODO_LIST_NAME, DOMAIN + +_LOGGER = logging.getLogger(__name__) + +STEP_USER_DATA_SCHEMA = vol.Schema( + { + vol.Required(CONF_TODO_LIST_NAME): str, + } +) + + +class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): + """Handle a config flow for Local To-do.""" + + VERSION = 1 + + async def async_step_user( + self, user_input: dict[str, Any] | None = None + ) -> FlowResult: + """Handle the initial step.""" + errors: dict[str, str] = {} + if user_input is not None: + return self.async_create_entry( + title=user_input[CONF_TODO_LIST_NAME], data=user_input + ) + + return self.async_show_form( + step_id="user", data_schema=STEP_USER_DATA_SCHEMA, errors=errors + ) diff --git a/homeassistant/components/local_todo/const.py b/homeassistant/components/local_todo/const.py new file mode 100644 index 00000000000000..707025d567244a --- /dev/null +++ b/homeassistant/components/local_todo/const.py @@ -0,0 +1,5 @@ +"""Constants for the Local To-do integration.""" + +DOMAIN = "local_todo" + +CONF_TODO_LIST_NAME = "todo_list_name" diff --git a/homeassistant/components/local_todo/manifest.json b/homeassistant/components/local_todo/manifest.json new file mode 100644 index 00000000000000..049a1824495959 --- /dev/null +++ b/homeassistant/components/local_todo/manifest.json @@ -0,0 +1,9 @@ +{ + "domain": "local_todo", + "name": "Local To-do", + "codeowners": ["@allenporter"], + "config_flow": true, + "documentation": "https://www.home-assistant.io/integrations/local_todo", + "iot_class": "local_polling", + "requirements": ["ical==5.1.0"] +} diff --git a/homeassistant/components/local_todo/store.py b/homeassistant/components/local_todo/store.py new file mode 100644 index 00000000000000..e88a7dd325d191 --- /dev/null +++ b/homeassistant/components/local_todo/store.py @@ -0,0 +1,38 @@ +"""Local storage for the Local To-do integration.""" + +import asyncio +from pathlib import Path + +from homeassistant.core import HomeAssistant + +STORAGE_PATH = ".storage/{key}.ics" + + +class LocalTodoListStore: + """Local storage for a single to-do list.""" + + def __init__(self, hass: HomeAssistant, path: Path) -> None: + """Initialize LocalTodoListStore.""" + self._hass = hass + self._path = path + self._lock = asyncio.Lock() + + async def async_load(self) -> str: + """Load the calendar from disk.""" + async with self._lock: + return await self._hass.async_add_executor_job(self._load) + + def _load(self) -> str: + """Load the calendar from disk.""" + if not self._path.exists(): + return "" + return self._path.read_text() + + async def async_store(self, ics_content: str) -> None: + """Persist the calendar to storage.""" + async with self._lock: + await self._hass.async_add_executor_job(self._store, ics_content) + + def _store(self, ics_content: str) -> None: + """Persist the calendar to storage.""" + self._path.write_text(ics_content) diff --git a/homeassistant/components/local_todo/strings.json b/homeassistant/components/local_todo/strings.json new file mode 100644 index 00000000000000..3531922c04e584 --- /dev/null +++ b/homeassistant/components/local_todo/strings.json @@ -0,0 +1,13 @@ +{ + "title": "Local To-do", + "config": { + "step": { + "user": { + "description": "Please choose a name for your new To-do list", + "data": { + "todo_list_name": "To-do List Name" + } + } + } + } +} diff --git a/homeassistant/components/local_todo/todo.py b/homeassistant/components/local_todo/todo.py new file mode 100644 index 00000000000000..354ccc1204a804 --- /dev/null +++ b/homeassistant/components/local_todo/todo.py @@ -0,0 +1,179 @@ +"""A local todo list todo platform.""" + +import dataclasses +import logging + +from ical.calendar import Calendar +from ical.calendar_stream import IcsCalendarStream +from ical.store import TodoStore +from ical.todo import Todo, TodoStatus +from pydantic import ValidationError +import voluptuous as vol + +from homeassistant.components.todo import ( + TodoItem, + TodoItemStatus, + TodoListEntity, + TodoListEntityFeature, +) +from homeassistant.config_entries import ConfigEntry +from homeassistant.core import HomeAssistant +from homeassistant.exceptions import HomeAssistantError +from homeassistant.helpers.entity_platform import AddEntitiesCallback + +from .const import CONF_TODO_LIST_NAME, DOMAIN +from .store import LocalTodoListStore + +_LOGGER = logging.getLogger(__name__) + + +PRODID = "-//homeassistant.io//local_todo 1.0//EN" + +ICS_TODO_STATUS_MAP = { + TodoStatus.IN_PROCESS: TodoItemStatus.NEEDS_ACTION, + TodoStatus.NEEDS_ACTION: TodoItemStatus.NEEDS_ACTION, + TodoStatus.COMPLETED: TodoItemStatus.COMPLETED, + TodoStatus.CANCELLED: TodoItemStatus.COMPLETED, +} + + +async def async_setup_entry( + hass: HomeAssistant, + config_entry: ConfigEntry, + async_add_entities: AddEntitiesCallback, +) -> None: + """Set up the local_todo todo platform.""" + + store = hass.data[DOMAIN][config_entry.entry_id] + ics = await store.async_load() + calendar = IcsCalendarStream.calendar_from_ics(ics) + calendar.prodid = PRODID + + name = config_entry.data[CONF_TODO_LIST_NAME] + entity = LocalTodoListEntity(store, calendar, name, unique_id=config_entry.entry_id) + async_add_entities([entity], True) + + +class LocalTodoListEntity(TodoListEntity): + """A To-do List representation of the Shopping List.""" + + _attr_has_entity_name = True + _attr_supported_features = ( + TodoListEntityFeature.CREATE_TODO_ITEM + | TodoListEntityFeature.DELETE_TODO_ITEM + | TodoListEntityFeature.UPDATE_TODO_ITEM + | TodoListEntityFeature.MOVE_TODO_ITEM + ) + + def __init__( + self, + store: LocalTodoListStore, + calendar: Calendar, + name: str, + unique_id: str, + ) -> None: + """Initialize LocalTodoListEntity.""" + self._store = store + self._calendar = calendar + self._attr_name = name.capitalize() + self._attr_unique_id = unique_id + self._compute_state() + + async def async_get_todo_items(self) -> list[TodoItem]: + """Get items in the To-do list.""" + return self._todo_items + + async def async_create_todo_item(self, item: TodoItem) -> None: + """Add an item to the To-do list.""" + try: + todo = Todo.parse_obj( + {k: v for k, v in dataclasses.asdict(item).items() if v is not None} + ) + except ValidationError as err: + _LOGGER.debug("Error parsing todo input fields: %s (%s)", item, str(err)) + raise vol.Invalid("Error parsing todo input fields") from err + TodoStore(self._calendar).add(todo) + await self._async_store() + self._compute_state() + await self.async_update_ha_state(force_refresh=True) + + async def async_update_todo_item(self, item: TodoItem) -> None: + """Update an item to the To-do list.""" + try: + todo = Todo.parse_obj( + {k: v for k, v in dataclasses.asdict(item).items() if v is not None} + ) + except ValidationError as err: + _LOGGER.debug("Error parsing todo input fields: %s (%s)", item, str(err)) + raise vol.Invalid("Error parsing todo input fields") from err + TodoStore(self._calendar).edit(todo.uid, todo) + await self._async_store() + self._compute_state() + await self.async_update_ha_state(force_refresh=True) + + async def async_delete_todo_items(self, uids: set[str]) -> None: + """Add an item to the To-do list.""" + store = TodoStore(self._calendar) + for uid in uids: + store.delete(uid) + await self._async_store() + self._compute_state() + await self.async_update_ha_state(force_refresh=True) + + async def async_move_todo_item(self, uid: str, previous: str | None = None) -> None: + """Re-order an item to the To-do list.""" + if uid == previous: + return + # Build a map of each item id to its position within the list + item_idx: dict[str, int] = {} + todos = self._calendar.todos + for idx, itm in enumerate(todos): + item_idx[itm.uid] = idx + if uid not in item_idx: + raise HomeAssistantError( + "Item '{uid}' not found in todo list {self.entity_id}" + ) + if previous and previous not in item_idx: + raise HomeAssistantError( + "Item '{previous}' not found in todo list {self.entity_id}" + ) + dst_idx = item_idx[previous] + 1 if previous else 0 + src_idx = item_idx[uid] + src_item = todos.pop(src_idx) + if dst_idx > src_idx: + dst_idx -= 1 + todos.insert(dst_idx, src_item) + self._calendar.todos = todos + await self._async_store() + self._compute_state() + await self.async_update_ha_state(force_refresh=True) + + async def _async_store(self) -> None: + """Persist the todo list to disk.""" + content = IcsCalendarStream.calendar_to_ics(self._calendar) + await self._store.async_store(content) + + @property + def _todo_items(self) -> list[TodoItem]: + """Get the current set of To-do items.""" + results = [] + for item in self._calendar.todos: + if item.status: + status = ICS_TODO_STATUS_MAP.get( + item.status, TodoItemStatus.NEEDS_ACTION + ) + else: + status = TodoItemStatus.NEEDS_ACTION + results.append( + TodoItem( + summary=item.summary or "", + uid=item.uid, + status=status, + ) + ) + return results + + def _compute_state(self) -> None: + self._attr_incomplete_count = sum( + item.status == TodoItemStatus.NEEDS_ACTION for item in self._todo_items + ) diff --git a/homeassistant/generated/config_flows.py b/homeassistant/generated/config_flows.py index fa83d93c87b23e..85c90f863f45aa 100644 --- a/homeassistant/generated/config_flows.py +++ b/homeassistant/generated/config_flows.py @@ -262,6 +262,7 @@ "livisi", "local_calendar", "local_ip", + "local_todo", "locative", "logi_circle", "lookin", diff --git a/homeassistant/generated/integrations.json b/homeassistant/generated/integrations.json index fb42c7f0e8eef3..aacc6cec723fe0 100644 --- a/homeassistant/generated/integrations.json +++ b/homeassistant/generated/integrations.json @@ -3105,6 +3105,11 @@ "config_flow": true, "iot_class": "local_polling" }, + "local_todo": { + "integration_type": "hub", + "config_flow": true, + "iot_class": "local_polling" + }, "locative": { "name": "Locative", "integration_type": "hub", @@ -6825,6 +6830,7 @@ "islamic_prayer_times", "local_calendar", "local_ip", + "local_todo", "min_max", "mobile_app", "moehlenhoff_alpha2", diff --git a/requirements_all.txt b/requirements_all.txt index fa334ab6ea61a4..4278ca53102084 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -1045,6 +1045,7 @@ ibeacon-ble==1.0.1 ibmiotf==0.3.4 # homeassistant.components.local_calendar +# homeassistant.components.local_todo ical==5.1.0 # homeassistant.components.ping diff --git a/requirements_test_all.txt b/requirements_test_all.txt index c28a50a81ee8c8..e22392e90869de 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -825,6 +825,7 @@ iaqualink==0.5.0 ibeacon-ble==1.0.1 # homeassistant.components.local_calendar +# homeassistant.components.local_todo ical==5.1.0 # homeassistant.components.ping diff --git a/script/hassfest/translations.py b/script/hassfest/translations.py index 5c6d7b19719ce4..4483aacd80475f 100644 --- a/script/hassfest/translations.py +++ b/script/hassfest/translations.py @@ -37,6 +37,7 @@ "islamic_prayer_times", "local_calendar", "local_ip", + "local_todo", "nmap_tracker", "rpi_power", "waze_travel_time", diff --git a/tests/components/local_todo/__init__.py b/tests/components/local_todo/__init__.py new file mode 100644 index 00000000000000..b4c21d557b0f28 --- /dev/null +++ b/tests/components/local_todo/__init__.py @@ -0,0 +1 @@ +"""Tests for the Local To-do integration.""" diff --git a/tests/components/local_todo/conftest.py b/tests/components/local_todo/conftest.py new file mode 100644 index 00000000000000..70f3e749a7f5fd --- /dev/null +++ b/tests/components/local_todo/conftest.py @@ -0,0 +1,78 @@ +"""Common fixtures for the NEW_NAME tests.""" +from collections.abc import Generator +from pathlib import Path +from unittest.mock import AsyncMock, patch + +import pytest + +from homeassistant.components.local_todo import LocalTodoListStore +from homeassistant.components.local_todo.const import CONF_TODO_LIST_NAME, DOMAIN +from homeassistant.core import HomeAssistant +from homeassistant.setup import async_setup_component + +from tests.common import MockConfigEntry + +TODO_NAME = "My Tasks" +FRIENDLY_NAME = "My tasks" +TEST_ENTITY = "todo.my_tasks" + + +@pytest.fixture +def mock_setup_entry() -> Generator[AsyncMock, None, None]: + """Override async_setup_entry.""" + with patch( + "homeassistant.components.local_todo.async_setup_entry", return_value=True + ) as mock_setup_entry: + yield mock_setup_entry + + +class FakeStore(LocalTodoListStore): + """Mock storage implementation.""" + + def __init__(self, hass: HomeAssistant, path: Path, ics_content: str) -> None: + """Initialize FakeStore.""" + super().__init__(hass, path) + self._content = ics_content + + def _load(self) -> str: + """Read from todo storage.""" + return self._content + + def _store(self, ics_content: str) -> None: + """Persist the todo storage.""" + self._content = ics_content + + +@pytest.fixture(name="ics_content", autouse=True) +def mock_ics_content() -> str: + """Fixture to allow tests to set initial ics content for the todo store.""" + return "" + + +@pytest.fixture(name="store", autouse=True) +def mock_store(ics_content: str) -> Generator[None, None, None]: + """Test cleanup, remove any media storage persisted during the test.""" + + stores: dict[Path, FakeStore] = {} + + def new_store(hass: HomeAssistant, path: Path) -> FakeStore: + if path not in stores: + stores[path] = FakeStore(hass, path, ics_content) + return stores[path] + + with patch("homeassistant.components.local_todo.LocalTodoListStore", new=new_store): + yield + + +@pytest.fixture(name="config_entry") +def mock_config_entry() -> MockConfigEntry: + """Fixture for mock configuration entry.""" + return MockConfigEntry(domain=DOMAIN, data={CONF_TODO_LIST_NAME: TODO_NAME}) + + +@pytest.fixture(name="setup_integration") +async def setup_integration(hass: HomeAssistant, config_entry: MockConfigEntry) -> None: + """Set up the integration.""" + config_entry.add_to_hass(hass) + assert await async_setup_component(hass, DOMAIN, {}) + await hass.async_block_till_done() diff --git a/tests/components/local_todo/test_config_flow.py b/tests/components/local_todo/test_config_flow.py new file mode 100644 index 00000000000000..b78031b1d5539a --- /dev/null +++ b/tests/components/local_todo/test_config_flow.py @@ -0,0 +1,33 @@ +"""Test the local_todo config flow.""" +from unittest.mock import AsyncMock + +import pytest + +from homeassistant import config_entries +from homeassistant.components.local_todo.const import DOMAIN +from homeassistant.core import HomeAssistant +from homeassistant.data_entry_flow import FlowResultType + +pytestmark = pytest.mark.usefixtures("mock_setup_entry") + + +async def test_form(hass: HomeAssistant, mock_setup_entry: AsyncMock) -> None: + """Test we get the form.""" + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": config_entries.SOURCE_USER} + ) + assert result["type"] == FlowResultType.FORM + assert not result.get("errors") + + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], + { + "todo_list_name": "My tasks", + }, + ) + await hass.async_block_till_done() + + assert result2["type"] == FlowResultType.CREATE_ENTRY + assert result2["title"] == "My tasks" + assert result2["data"] == {"todo_list_name": "My tasks"} + assert len(mock_setup_entry.mock_calls) == 1 diff --git a/tests/components/local_todo/test_init.py b/tests/components/local_todo/test_init.py new file mode 100644 index 00000000000000..0a69fd063b4670 --- /dev/null +++ b/tests/components/local_todo/test_init.py @@ -0,0 +1,18 @@ +"""Tests for init platform of local todo.""" + +from unittest.mock import patch + +from homeassistant.core import HomeAssistant + +from tests.common import MockConfigEntry + + +async def test_remove_config_entry( + hass: HomeAssistant, setup_integration: None, config_entry: MockConfigEntry +) -> None: + """Test removing a config entry.""" + + with patch("homeassistant.components.local_todo.Path.unlink") as unlink_mock: + assert await hass.config_entries.async_remove(config_entry.entry_id) + await hass.async_block_till_done() + unlink_mock.assert_called_once() diff --git a/tests/components/local_todo/test_todo.py b/tests/components/local_todo/test_todo.py new file mode 100644 index 00000000000000..2b9a2a2e78ea6f --- /dev/null +++ b/tests/components/local_todo/test_todo.py @@ -0,0 +1,354 @@ +"""Tests for todo platform of local_todo.""" + +from collections.abc import Awaitable, Callable + +import pytest + +from homeassistant.core import HomeAssistant + +from .conftest import TEST_ENTITY + +from tests.typing import WebSocketGenerator + + +@pytest.fixture +def ws_req_id() -> Callable[[], int]: + """Fixture for incremental websocket requests.""" + + id = 0 + + def next() -> int: + nonlocal id + id += 1 + return id + + return next + + +@pytest.fixture +async def ws_get_items( + hass_ws_client: WebSocketGenerator, ws_req_id: Callable[[], int] +) -> Callable[[], Awaitable[dict[str, str]]]: + """Fixture to fetch items from the todo websocket.""" + + async def get() -> list[dict[str, str]]: + # Fetch items using To-do platform + client = await hass_ws_client() + id = ws_req_id() + await client.send_json( + { + "id": id, + "type": "todo/item/list", + "entity_id": TEST_ENTITY, + } + ) + resp = await client.receive_json() + assert resp.get("id") == id + assert resp.get("success") + return resp.get("result", {}).get("items", []) + + return get + + +@pytest.fixture +async def ws_move_item( + hass_ws_client: WebSocketGenerator, + ws_req_id: Callable[[], int], +) -> Callable[[str, str | None], Awaitable[None]]: + """Fixture to move an item in the todo list.""" + + async def move(uid: str, previous: str | None) -> None: + # Fetch items using To-do platform + client = await hass_ws_client() + id = ws_req_id() + data = { + "id": id, + "type": "todo/item/move", + "entity_id": TEST_ENTITY, + "uid": uid, + } + if previous is not None: + data["previous"] = previous + await client.send_json(data) + resp = await client.receive_json() + assert resp.get("id") == id + assert resp.get("success") + + return move + + +async def test_create_item( + hass: HomeAssistant, + hass_ws_client: WebSocketGenerator, + setup_integration: None, + ws_req_id: Callable[[], int], + ws_get_items: Callable[[], Awaitable[dict[str, str]]], +) -> None: + """Test creating a todo item.""" + + state = hass.states.get(TEST_ENTITY) + assert state + assert state.state == "0" + + client = await hass_ws_client(hass) + await client.send_json( + { + "id": ws_req_id(), + "type": "todo/item/create", + "entity_id": TEST_ENTITY, + "item": { + "summary": "replace batteries", + }, + } + ) + resp = await client.receive_json() + assert resp.get("success") + + items = await ws_get_items() + assert len(items) == 1 + assert items[0]["summary"] == "replace batteries" + assert items[0]["status"] == "NEEDS-ACTION" + assert "uid" in items[0] + + state = hass.states.get(TEST_ENTITY) + assert state + assert state.state == "1" + + +async def test_delete_item( + hass: HomeAssistant, + hass_ws_client: WebSocketGenerator, + setup_integration: None, + ws_req_id: Callable[[], int], + ws_get_items: Callable[[], Awaitable[dict[str, str]]], +) -> None: + """Test deleting a todo item.""" + client = await hass_ws_client(hass) + + await client.send_json( + { + "id": ws_req_id(), + "type": "todo/item/create", + "entity_id": TEST_ENTITY, + "item": { + "summary": "replace batteries", + }, + } + ) + resp = await client.receive_json() + assert resp.get("success") + + items = await ws_get_items() + assert len(items) == 1 + assert items[0]["summary"] == "replace batteries" + assert items[0]["status"] == "NEEDS-ACTION" + assert "uid" in items[0] + + state = hass.states.get(TEST_ENTITY) + assert state + assert state.state == "1" + + await client.send_json( + { + "id": ws_req_id(), + "type": "todo/item/delete", + "entity_id": TEST_ENTITY, + "uids": [items[0]["uid"]], + } + ) + resp = await client.receive_json() + assert resp.get("success") + + items = await ws_get_items() + assert len(items) == 0 + + state = hass.states.get(TEST_ENTITY) + assert state + assert state.state == "0" + + +async def test_bulk_delete( + hass: HomeAssistant, + hass_ws_client: WebSocketGenerator, + setup_integration: None, + ws_req_id: Callable[[], int], + ws_get_items: Callable[[], Awaitable[dict[str, str]]], +) -> None: + """Test deleting a todo item.""" + client = await hass_ws_client(hass) + + for _i in range(0, 5): + await client.send_json( + { + "id": ws_req_id(), + "type": "todo/item/create", + "entity_id": TEST_ENTITY, + "item": { + "summary": "soda", + }, + } + ) + resp = await client.receive_json() + assert resp.get("success") + + items = await ws_get_items() + assert len(items) == 5 + uids = [item["uid"] for item in items] + + state = hass.states.get(TEST_ENTITY) + assert state + assert state.state == "5" + + await client.send_json( + { + "id": ws_req_id(), + "type": "todo/item/delete", + "entity_id": TEST_ENTITY, + "uids": uids, + } + ) + resp = await client.receive_json() + assert resp.get("success") + + items = await ws_get_items() + assert len(items) == 0 + + state = hass.states.get(TEST_ENTITY) + assert state + assert state.state == "0" + + +async def test_update_item( + hass: HomeAssistant, + hass_ws_client: WebSocketGenerator, + setup_integration: None, + ws_req_id: Callable[[], int], + ws_get_items: Callable[[], Awaitable[dict[str, str]]], +) -> None: + """Test updating a todo item.""" + client = await hass_ws_client(hass) + + # Create new item + await client.send_json( + { + "id": 5, + "type": "todo/item/create", + "entity_id": TEST_ENTITY, + "item": { + "summary": "soda", + }, + } + ) + resp = await client.receive_json() + assert resp.get("id") == 5 + assert resp.get("success") + + # Fetch item + items = await ws_get_items() + assert len(items) == 1 + item = items[0] + assert item["summary"] == "soda" + assert item["status"] == "NEEDS-ACTION" + + state = hass.states.get(TEST_ENTITY) + assert state + assert state.state == "1" + + # Mark item completed + item["status"] = "COMPLETED" + await client.send_json( + { + "id": 7, + "type": "todo/item/update", + "entity_id": TEST_ENTITY, + "item": item, + } + ) + resp = await client.receive_json() + assert resp.get("id") == 7 + assert resp.get("success") + + # Verify item is marked as completed + items = await ws_get_items() + assert len(items) == 1 + item = items[0] + assert item["summary"] == "soda" + assert item["status"] == "COMPLETED" + + state = hass.states.get(TEST_ENTITY) + assert state + assert state.state == "0" + + +@pytest.mark.parametrize( + ("src_idx", "dst_idx", "expected_items"), + [ + # Move any item to the front of the list + (0, None, ["item 1", "item 2", "item 3", "item 4"]), + (1, None, ["item 2", "item 1", "item 3", "item 4"]), + (2, None, ["item 3", "item 1", "item 2", "item 4"]), + (3, None, ["item 4", "item 1", "item 2", "item 3"]), + # Move items right + (0, 1, ["item 2", "item 1", "item 3", "item 4"]), + (0, 2, ["item 2", "item 3", "item 1", "item 4"]), + (0, 3, ["item 2", "item 3", "item 4", "item 1"]), + (1, 2, ["item 1", "item 3", "item 2", "item 4"]), + (1, 3, ["item 1", "item 3", "item 4", "item 2"]), + # Move items left + (2, 0, ["item 1", "item 3", "item 2", "item 4"]), + (3, 0, ["item 1", "item 4", "item 2", "item 3"]), + (3, 1, ["item 1", "item 2", "item 4", "item 3"]), + # No-ops + (0, 0, ["item 1", "item 2", "item 3", "item 4"]), + (2, 1, ["item 1", "item 2", "item 3", "item 4"]), + (2, 2, ["item 1", "item 2", "item 3", "item 4"]), + (3, 2, ["item 1", "item 2", "item 3", "item 4"]), + (3, 3, ["item 1", "item 2", "item 3", "item 4"]), + ], +) +async def test_move_item( + hass: HomeAssistant, + hass_ws_client: WebSocketGenerator, + setup_integration: None, + ws_req_id: Callable[[], int], + ws_get_items: Callable[[], Awaitable[dict[str, str]]], + ws_move_item: Callable[[str, str | None], Awaitable[None]], + src_idx: int, + dst_idx: int | None, + expected_items: list[str], +) -> None: + """Test moving a todo item within the list.""" + client = await hass_ws_client(hass) + + for i in range(1, 5): + await client.send_json( + { + "id": ws_req_id(), + "type": "todo/item/create", + "entity_id": TEST_ENTITY, + "item": { + "summary": f"item {i}", + }, + } + ) + resp = await client.receive_json() + assert resp.get("success") + + items = await ws_get_items() + assert len(items) == 4 + uids = [item["uid"] for item in items] + summaries = [item["summary"] for item in items] + assert summaries == ["item 1", "item 2", "item 3", "item 4"] + + # Prepare items for moving + uid = uids[src_idx] + previous = None + if dst_idx is not None: + previous = uids[dst_idx] + + await ws_move_item(uid, previous) + + items = await ws_get_items() + assert len(items) == 4 + summaries = [item["summary"] for item in items] + assert summaries == expected_items From 68b00f8bb85bd210a457eb1fea9f7056326cd06b Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Tue, 17 Oct 2023 01:10:45 +0000 Subject: [PATCH 02/21] Update Local To-do with latest To-do platform APIs --- homeassistant/components/local_todo/todo.py | 107 +++++------ tests/components/local_todo/test_todo.py | 198 +++++++------------- 2 files changed, 117 insertions(+), 188 deletions(-) diff --git a/homeassistant/components/local_todo/todo.py b/homeassistant/components/local_todo/todo.py index 354ccc1204a804..aaf5e0da8d972b 100644 --- a/homeassistant/components/local_todo/todo.py +++ b/homeassistant/components/local_todo/todo.py @@ -1,7 +1,9 @@ """A local todo list todo platform.""" +from collections.abc import Iterable import dataclasses import logging +from typing import Any from ical.calendar import Calendar from ical.calendar_stream import IcsCalendarStream @@ -35,6 +37,10 @@ TodoStatus.COMPLETED: TodoItemStatus.COMPLETED, TodoStatus.CANCELLED: TodoItemStatus.COMPLETED, } +ICS_TODO_STATUS_MAP_INV = { + TodoItemStatus.COMPLETED: TodoStatus.COMPLETED, + TodoItemStatus.NEEDS_ACTION: TodoStatus.NEEDS_ACTION, +} async def async_setup_entry( @@ -54,6 +60,17 @@ async def async_setup_entry( async_add_entities([entity], True) +def _todo_dict_factory(obj: Iterable[tuple[str, Any]]) -> dict[str, str]: + """Convert TodoItem dataclass items to dictionary of attributes for ical consumption.""" + result: dict[str, str] = {} + for name, value in obj: + if name == "status": + result[name] = ICS_TODO_STATUS_MAP_INV[value] + elif value is not None: + result[name] = value + return result + + class LocalTodoListEntity(TodoListEntity): """A To-do List representation of the Shopping List.""" @@ -64,6 +81,7 @@ class LocalTodoListEntity(TodoListEntity): | TodoListEntityFeature.UPDATE_TODO_ITEM | TodoListEntityFeature.MOVE_TODO_ITEM ) + _attr_should_poll = False def __init__( self, @@ -77,103 +95,72 @@ def __init__( self._calendar = calendar self._attr_name = name.capitalize() self._attr_unique_id = unique_id - self._compute_state() - async def async_get_todo_items(self) -> list[TodoItem]: - """Get items in the To-do list.""" - return self._todo_items + async def async_update(self) -> None: + """Update entity state based on the local To-do items.""" + self._attr_todo_items = [ + TodoItem( + uid=item.uid, + summary=item.summary or "", + status=ICS_TODO_STATUS_MAP.get( + item.status or TodoStatus.NEEDS_ACTION, TodoItemStatus.NEEDS_ACTION + ), + ) + for item in self._calendar.todos + ] async def async_create_todo_item(self, item: TodoItem) -> None: """Add an item to the To-do list.""" try: todo = Todo.parse_obj( - {k: v for k, v in dataclasses.asdict(item).items() if v is not None} + dataclasses.asdict(item, dict_factory=_todo_dict_factory) ) except ValidationError as err: _LOGGER.debug("Error parsing todo input fields: %s (%s)", item, str(err)) raise vol.Invalid("Error parsing todo input fields") from err TodoStore(self._calendar).add(todo) - await self._async_store() - self._compute_state() + await self._async_save() await self.async_update_ha_state(force_refresh=True) async def async_update_todo_item(self, item: TodoItem) -> None: """Update an item to the To-do list.""" try: todo = Todo.parse_obj( - {k: v for k, v in dataclasses.asdict(item).items() if v is not None} + dataclasses.asdict(item, dict_factory=_todo_dict_factory) ) except ValidationError as err: _LOGGER.debug("Error parsing todo input fields: %s (%s)", item, str(err)) raise vol.Invalid("Error parsing todo input fields") from err TodoStore(self._calendar).edit(todo.uid, todo) - await self._async_store() - self._compute_state() + await self._async_save() await self.async_update_ha_state(force_refresh=True) - async def async_delete_todo_items(self, uids: set[str]) -> None: + async def async_delete_todo_items(self, uids: list[str]) -> None: """Add an item to the To-do list.""" store = TodoStore(self._calendar) for uid in uids: store.delete(uid) - await self._async_store() - self._compute_state() + await self._async_save() await self.async_update_ha_state(force_refresh=True) - async def async_move_todo_item(self, uid: str, previous: str | None = None) -> None: + async def async_move_todo_item(self, uid: str, pos: int) -> None: """Re-order an item to the To-do list.""" - if uid == previous: - return - # Build a map of each item id to its position within the list - item_idx: dict[str, int] = {} todos = self._calendar.todos + found_item: Todo | None = None for idx, itm in enumerate(todos): - item_idx[itm.uid] = idx - if uid not in item_idx: + if itm.uid == uid: + found_item = itm + todos.pop(idx) + break + if found_item is None: raise HomeAssistantError( - "Item '{uid}' not found in todo list {self.entity_id}" + f"Item '{uid}' not found in todo list {self.entity_id}" ) - if previous and previous not in item_idx: - raise HomeAssistantError( - "Item '{previous}' not found in todo list {self.entity_id}" - ) - dst_idx = item_idx[previous] + 1 if previous else 0 - src_idx = item_idx[uid] - src_item = todos.pop(src_idx) - if dst_idx > src_idx: - dst_idx -= 1 - todos.insert(dst_idx, src_item) - self._calendar.todos = todos - await self._async_store() - self._compute_state() + todos.insert(pos, found_item) + await self._async_save() await self.async_update_ha_state(force_refresh=True) - async def _async_store(self) -> None: + async def _async_save(self) -> None: """Persist the todo list to disk.""" content = IcsCalendarStream.calendar_to_ics(self._calendar) await self._store.async_store(content) - - @property - def _todo_items(self) -> list[TodoItem]: - """Get the current set of To-do items.""" - results = [] - for item in self._calendar.todos: - if item.status: - status = ICS_TODO_STATUS_MAP.get( - item.status, TodoItemStatus.NEEDS_ACTION - ) - else: - status = TodoItemStatus.NEEDS_ACTION - results.append( - TodoItem( - summary=item.summary or "", - uid=item.uid, - status=status, - ) - ) - return results - - def _compute_state(self) -> None: - self._attr_incomplete_count = sum( - item.status == TodoItemStatus.NEEDS_ACTION for item in self._todo_items - ) diff --git a/tests/components/local_todo/test_todo.py b/tests/components/local_todo/test_todo.py index 2b9a2a2e78ea6f..b88847983a2a71 100644 --- a/tests/components/local_todo/test_todo.py +++ b/tests/components/local_todo/test_todo.py @@ -4,6 +4,7 @@ import pytest +from homeassistant.components.todo import DOMAIN as TODO_DOMAIN from homeassistant.core import HomeAssistant from .conftest import TEST_ENTITY @@ -57,7 +58,7 @@ async def ws_move_item( ) -> Callable[[str, str | None], Awaitable[None]]: """Fixture to move an item in the todo list.""" - async def move(uid: str, previous: str | None) -> None: + async def move(uid: str, pos: int) -> None: # Fetch items using To-do platform client = await hass_ws_client() id = ws_req_id() @@ -66,9 +67,8 @@ async def move(uid: str, previous: str | None) -> None: "type": "todo/item/move", "entity_id": TEST_ENTITY, "uid": uid, + "pos": pos, } - if previous is not None: - data["previous"] = previous await client.send_json(data) resp = await client.receive_json() assert resp.get("id") == id @@ -81,7 +81,6 @@ async def test_create_item( hass: HomeAssistant, hass_ws_client: WebSocketGenerator, setup_integration: None, - ws_req_id: Callable[[], int], ws_get_items: Callable[[], Awaitable[dict[str, str]]], ) -> None: """Test creating a todo item.""" @@ -90,24 +89,18 @@ async def test_create_item( assert state assert state.state == "0" - client = await hass_ws_client(hass) - await client.send_json( - { - "id": ws_req_id(), - "type": "todo/item/create", - "entity_id": TEST_ENTITY, - "item": { - "summary": "replace batteries", - }, - } + await hass.services.async_call( + TODO_DOMAIN, + "create_item", + {"summary": "replace batteries"}, + target={"entity_id": TEST_ENTITY}, + blocking=True, ) - resp = await client.receive_json() - assert resp.get("success") items = await ws_get_items() assert len(items) == 1 assert items[0]["summary"] == "replace batteries" - assert items[0]["status"] == "NEEDS-ACTION" + assert items[0]["status"] == "needs_action" assert "uid" in items[0] state = hass.states.get(TEST_ENTITY) @@ -117,47 +110,35 @@ async def test_create_item( async def test_delete_item( hass: HomeAssistant, - hass_ws_client: WebSocketGenerator, setup_integration: None, - ws_req_id: Callable[[], int], ws_get_items: Callable[[], Awaitable[dict[str, str]]], ) -> None: """Test deleting a todo item.""" - client = await hass_ws_client(hass) - - await client.send_json( - { - "id": ws_req_id(), - "type": "todo/item/create", - "entity_id": TEST_ENTITY, - "item": { - "summary": "replace batteries", - }, - } + await hass.services.async_call( + TODO_DOMAIN, + "create_item", + {"summary": "replace batteries"}, + target={"entity_id": TEST_ENTITY}, + blocking=True, ) - resp = await client.receive_json() - assert resp.get("success") items = await ws_get_items() assert len(items) == 1 assert items[0]["summary"] == "replace batteries" - assert items[0]["status"] == "NEEDS-ACTION" + assert items[0]["status"] == "needs_action" assert "uid" in items[0] state = hass.states.get(TEST_ENTITY) assert state assert state.state == "1" - await client.send_json( - { - "id": ws_req_id(), - "type": "todo/item/delete", - "entity_id": TEST_ENTITY, - "uids": [items[0]["uid"]], - } + await hass.services.async_call( + TODO_DOMAIN, + "delete_item", + {"uid": [items[0]["uid"]]}, + target={"entity_id": TEST_ENTITY}, + blocking=True, ) - resp = await client.receive_json() - assert resp.get("success") items = await ws_get_items() assert len(items) == 0 @@ -169,27 +150,18 @@ async def test_delete_item( async def test_bulk_delete( hass: HomeAssistant, - hass_ws_client: WebSocketGenerator, setup_integration: None, - ws_req_id: Callable[[], int], ws_get_items: Callable[[], Awaitable[dict[str, str]]], ) -> None: """Test deleting a todo item.""" - client = await hass_ws_client(hass) - - for _i in range(0, 5): - await client.send_json( - { - "id": ws_req_id(), - "type": "todo/item/create", - "entity_id": TEST_ENTITY, - "item": { - "summary": "soda", - }, - } + for i in range(0, 5): + await hass.services.async_call( + TODO_DOMAIN, + "create_item", + {"summary": f"soda #{i}"}, + target={"entity_id": TEST_ENTITY}, + blocking=True, ) - resp = await client.receive_json() - assert resp.get("success") items = await ws_get_items() assert len(items) == 5 @@ -199,16 +171,13 @@ async def test_bulk_delete( assert state assert state.state == "5" - await client.send_json( - { - "id": ws_req_id(), - "type": "todo/item/delete", - "entity_id": TEST_ENTITY, - "uids": uids, - } + await hass.services.async_call( + TODO_DOMAIN, + "delete_item", + {"uid": uids}, + target={"entity_id": TEST_ENTITY}, + blocking=True, ) - resp = await client.receive_json() - assert resp.get("success") items = await ws_get_items() assert len(items) == 0 @@ -220,60 +189,46 @@ async def test_bulk_delete( async def test_update_item( hass: HomeAssistant, - hass_ws_client: WebSocketGenerator, setup_integration: None, - ws_req_id: Callable[[], int], ws_get_items: Callable[[], Awaitable[dict[str, str]]], ) -> None: """Test updating a todo item.""" - client = await hass_ws_client(hass) # Create new item - await client.send_json( - { - "id": 5, - "type": "todo/item/create", - "entity_id": TEST_ENTITY, - "item": { - "summary": "soda", - }, - } + await hass.services.async_call( + TODO_DOMAIN, + "create_item", + {"summary": "soda"}, + target={"entity_id": TEST_ENTITY}, + blocking=True, ) - resp = await client.receive_json() - assert resp.get("id") == 5 - assert resp.get("success") # Fetch item items = await ws_get_items() assert len(items) == 1 item = items[0] assert item["summary"] == "soda" - assert item["status"] == "NEEDS-ACTION" + assert item["status"] == "needs_action" state = hass.states.get(TEST_ENTITY) assert state assert state.state == "1" # Mark item completed - item["status"] = "COMPLETED" - await client.send_json( - { - "id": 7, - "type": "todo/item/update", - "entity_id": TEST_ENTITY, - "item": item, - } + await hass.services.async_call( + TODO_DOMAIN, + "update_item", + {"uid": item["uid"], "status": "completed"}, + target={"entity_id": TEST_ENTITY}, + blocking=True, ) - resp = await client.receive_json() - assert resp.get("id") == 7 - assert resp.get("success") # Verify item is marked as completed items = await ws_get_items() assert len(items) == 1 item = items[0] assert item["summary"] == "soda" - assert item["status"] == "COMPLETED" + assert item["status"] == "completed" state = hass.states.get(TEST_ENTITY) assert state @@ -281,58 +236,50 @@ async def test_update_item( @pytest.mark.parametrize( - ("src_idx", "dst_idx", "expected_items"), + ("src_idx", "pos", "expected_items"), [ # Move any item to the front of the list - (0, None, ["item 1", "item 2", "item 3", "item 4"]), - (1, None, ["item 2", "item 1", "item 3", "item 4"]), - (2, None, ["item 3", "item 1", "item 2", "item 4"]), - (3, None, ["item 4", "item 1", "item 2", "item 3"]), + (0, 0, ["item 1", "item 2", "item 3", "item 4"]), + (1, 0, ["item 2", "item 1", "item 3", "item 4"]), + (2, 0, ["item 3", "item 1", "item 2", "item 4"]), + (3, 0, ["item 4", "item 1", "item 2", "item 3"]), # Move items right (0, 1, ["item 2", "item 1", "item 3", "item 4"]), (0, 2, ["item 2", "item 3", "item 1", "item 4"]), (0, 3, ["item 2", "item 3", "item 4", "item 1"]), (1, 2, ["item 1", "item 3", "item 2", "item 4"]), (1, 3, ["item 1", "item 3", "item 4", "item 2"]), + (1, 4, ["item 1", "item 3", "item 4", "item 2"]), + (1, 5, ["item 1", "item 3", "item 4", "item 2"]), # Move items left - (2, 0, ["item 1", "item 3", "item 2", "item 4"]), - (3, 0, ["item 1", "item 4", "item 2", "item 3"]), - (3, 1, ["item 1", "item 2", "item 4", "item 3"]), + (2, 1, ["item 1", "item 3", "item 2", "item 4"]), + (3, 1, ["item 1", "item 4", "item 2", "item 3"]), + (3, 2, ["item 1", "item 2", "item 4", "item 3"]), # No-ops - (0, 0, ["item 1", "item 2", "item 3", "item 4"]), - (2, 1, ["item 1", "item 2", "item 3", "item 4"]), + (1, 1, ["item 1", "item 2", "item 3", "item 4"]), (2, 2, ["item 1", "item 2", "item 3", "item 4"]), - (3, 2, ["item 1", "item 2", "item 3", "item 4"]), (3, 3, ["item 1", "item 2", "item 3", "item 4"]), + (3, 4, ["item 1", "item 2", "item 3", "item 4"]), ], ) async def test_move_item( hass: HomeAssistant, - hass_ws_client: WebSocketGenerator, setup_integration: None, - ws_req_id: Callable[[], int], ws_get_items: Callable[[], Awaitable[dict[str, str]]], ws_move_item: Callable[[str, str | None], Awaitable[None]], src_idx: int, - dst_idx: int | None, + pos: int, expected_items: list[str], ) -> None: """Test moving a todo item within the list.""" - client = await hass_ws_client(hass) - for i in range(1, 5): - await client.send_json( - { - "id": ws_req_id(), - "type": "todo/item/create", - "entity_id": TEST_ENTITY, - "item": { - "summary": f"item {i}", - }, - } + await hass.services.async_call( + TODO_DOMAIN, + "create_item", + {"summary": f"item {i}"}, + target={"entity_id": TEST_ENTITY}, + blocking=True, ) - resp = await client.receive_json() - assert resp.get("success") items = await ws_get_items() assert len(items) == 4 @@ -341,12 +288,7 @@ async def test_move_item( assert summaries == ["item 1", "item 2", "item 3", "item 4"] # Prepare items for moving - uid = uids[src_idx] - previous = None - if dst_idx is not None: - previous = uids[dst_idx] - - await ws_move_item(uid, previous) + await ws_move_item(uids[src_idx], pos) items = await ws_get_items() assert len(items) == 4 From 0163b3ab1ca761306702393d8f1b238bc8f73717 Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Tue, 24 Oct 2023 00:50:37 +0000 Subject: [PATCH 03/21] Add test coverage for moving an unknown item --- tests/components/local_todo/test_todo.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/components/local_todo/test_todo.py b/tests/components/local_todo/test_todo.py index b88847983a2a71..c54b6e09d55171 100644 --- a/tests/components/local_todo/test_todo.py +++ b/tests/components/local_todo/test_todo.py @@ -294,3 +294,27 @@ async def test_move_item( assert len(items) == 4 summaries = [item["summary"] for item in items] assert summaries == expected_items + + +async def test_move_item_unknown( + hass: HomeAssistant, + setup_integration: None, + hass_ws_client: WebSocketGenerator, +) -> None: + """Test moving a todo item that does not exist.""" + + # Prepare items for moving + client = await hass_ws_client() + data = { + "id": 1, + "type": "todo/item/move", + "entity_id": TEST_ENTITY, + "uid": "unknown", + "pos": 0, + } + await client.send_json(data) + resp = await client.receive_json() + assert resp.get("id") == 1 + assert not resp.get("success") + assert resp.get("error", {}).get("code") == "failed" + assert "not found in todo list" in resp["error"]["message"] From 610a783a5704049e72043475c818cbd3b1b9cbaa Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Tue, 24 Oct 2023 01:20:51 +0000 Subject: [PATCH 04/21] Update comments in conftest.py --- tests/components/local_todo/conftest.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/tests/components/local_todo/conftest.py b/tests/components/local_todo/conftest.py index 70f3e749a7f5fd..69c4bd42cb849a 100644 --- a/tests/components/local_todo/conftest.py +++ b/tests/components/local_todo/conftest.py @@ -1,4 +1,4 @@ -"""Common fixtures for the NEW_NAME tests.""" +"""Common fixtures for the local_todo tests.""" from collections.abc import Generator from pathlib import Path from unittest.mock import AsyncMock, patch @@ -43,21 +43,15 @@ def _store(self, ics_content: str) -> None: self._content = ics_content -@pytest.fixture(name="ics_content", autouse=True) -def mock_ics_content() -> str: - """Fixture to allow tests to set initial ics content for the todo store.""" - return "" - - @pytest.fixture(name="store", autouse=True) -def mock_store(ics_content: str) -> Generator[None, None, None]: +def mock_store() -> Generator[None, None, None]: """Test cleanup, remove any media storage persisted during the test.""" stores: dict[Path, FakeStore] = {} def new_store(hass: HomeAssistant, path: Path) -> FakeStore: if path not in stores: - stores[path] = FakeStore(hass, path, ics_content) + stores[path] = FakeStore(hass, path, "") return stores[path] with patch("homeassistant.components.local_todo.LocalTodoListStore", new=new_store): From a9a7b41121bbe549aa0a8da243ba86324dd427d7 Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Tue, 24 Oct 2023 01:30:21 +0000 Subject: [PATCH 05/21] Update comments for consistency --- homeassistant/components/local_todo/todo.py | 2 +- tests/components/local_todo/__init__.py | 2 +- tests/components/local_todo/test_init.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/homeassistant/components/local_todo/todo.py b/homeassistant/components/local_todo/todo.py index aaf5e0da8d972b..9a4f00ea9c54be 100644 --- a/homeassistant/components/local_todo/todo.py +++ b/homeassistant/components/local_todo/todo.py @@ -1,4 +1,4 @@ -"""A local todo list todo platform.""" +"""A Local To-do todo platform.""" from collections.abc import Iterable import dataclasses diff --git a/tests/components/local_todo/__init__.py b/tests/components/local_todo/__init__.py index b4c21d557b0f28..a96a2e85cbd52c 100644 --- a/tests/components/local_todo/__init__.py +++ b/tests/components/local_todo/__init__.py @@ -1 +1 @@ -"""Tests for the Local To-do integration.""" +"""Tests for the local_todo integration.""" diff --git a/tests/components/local_todo/test_init.py b/tests/components/local_todo/test_init.py index 0a69fd063b4670..2888c72c8168a0 100644 --- a/tests/components/local_todo/test_init.py +++ b/tests/components/local_todo/test_init.py @@ -1,4 +1,4 @@ -"""Tests for init platform of local todo.""" +"""Tests for init platform of local_todo.""" from unittest.mock import patch From 87943a9aed49d805e068984fc707e8d6780e82ef Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Tue, 24 Oct 2023 01:43:53 +0000 Subject: [PATCH 06/21] Update comments in To-do --- homeassistant/components/local_todo/store.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/local_todo/store.py b/homeassistant/components/local_todo/store.py index e88a7dd325d191..0811ce645a9b7a 100644 --- a/homeassistant/components/local_todo/store.py +++ b/homeassistant/components/local_todo/store.py @@ -9,7 +9,7 @@ class LocalTodoListStore: - """Local storage for a single to-do list.""" + """Local storage for a single To-do list.""" def __init__(self, hass: HomeAssistant, path: Path) -> None: """Initialize LocalTodoListStore.""" From b4ba165f6afa4849e5e38bd192eaad2f304dc533 Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Tue, 24 Oct 2023 04:27:54 +0000 Subject: [PATCH 07/21] Refactor to improve test coverage --- homeassistant/components/local_todo/todo.py | 26 +++++++++------------ tests/components/local_todo/conftest.py | 15 ++++++------ 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/homeassistant/components/local_todo/todo.py b/homeassistant/components/local_todo/todo.py index 9a4f00ea9c54be..833ff743f78da8 100644 --- a/homeassistant/components/local_todo/todo.py +++ b/homeassistant/components/local_todo/todo.py @@ -10,7 +10,6 @@ from ical.store import TodoStore from ical.todo import Todo, TodoStatus from pydantic import ValidationError -import voluptuous as vol from homeassistant.components.todo import ( TodoItem, @@ -71,6 +70,15 @@ def _todo_dict_factory(obj: Iterable[tuple[str, Any]]) -> dict[str, str]: return result +def _convert_item(item: TodoItem) -> Todo: + """Convert a HomeAssistant TodoItem to an ical Todo.""" + try: + return Todo.parse_obj(dataclasses.asdict(item, dict_factory=_todo_dict_factory)) + except ValidationError as err: + _LOGGER.debug("Error parsing todo input fields: %s (%s)", item, str(err)) + raise HomeAssistantError("Error parsing todo input fields") from err + + class LocalTodoListEntity(TodoListEntity): """A To-do List representation of the Shopping List.""" @@ -111,26 +119,14 @@ async def async_update(self) -> None: async def async_create_todo_item(self, item: TodoItem) -> None: """Add an item to the To-do list.""" - try: - todo = Todo.parse_obj( - dataclasses.asdict(item, dict_factory=_todo_dict_factory) - ) - except ValidationError as err: - _LOGGER.debug("Error parsing todo input fields: %s (%s)", item, str(err)) - raise vol.Invalid("Error parsing todo input fields") from err + todo = _convert_item(item) TodoStore(self._calendar).add(todo) await self._async_save() await self.async_update_ha_state(force_refresh=True) async def async_update_todo_item(self, item: TodoItem) -> None: """Update an item to the To-do list.""" - try: - todo = Todo.parse_obj( - dataclasses.asdict(item, dict_factory=_todo_dict_factory) - ) - except ValidationError as err: - _LOGGER.debug("Error parsing todo input fields: %s (%s)", item, str(err)) - raise vol.Invalid("Error parsing todo input fields") from err + todo = _convert_item(item) TodoStore(self._calendar).edit(todo.uid, todo) await self._async_save() await self.async_update_ha_state(force_refresh=True) diff --git a/tests/components/local_todo/conftest.py b/tests/components/local_todo/conftest.py index 69c4bd42cb849a..86f5e0fc5e650a 100644 --- a/tests/components/local_todo/conftest.py +++ b/tests/components/local_todo/conftest.py @@ -1,7 +1,7 @@ """Common fixtures for the local_todo tests.""" from collections.abc import Generator from pathlib import Path -from unittest.mock import AsyncMock, patch +from unittest.mock import AsyncMock, Mock, patch import pytest @@ -31,16 +31,17 @@ class FakeStore(LocalTodoListStore): def __init__(self, hass: HomeAssistant, path: Path, ics_content: str) -> None: """Initialize FakeStore.""" - super().__init__(hass, path) self._content = ics_content + mock_path = Mock() + mock_path.read_text = self._mock_read_text + mock_path.write_text = self._mock_write_text + super().__init__(hass, mock_path) - def _load(self) -> str: - """Read from todo storage.""" + def _mock_read_text(self) -> str: return self._content - def _store(self, ics_content: str) -> None: - """Persist the todo storage.""" - self._content = ics_content + def _mock_write_text(self, content: str) -> None: + self._content = content @pytest.fixture(name="store", autouse=True) From e8d839a013f10ebd64d875739e6460d1d10e84d6 Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Tue, 24 Oct 2023 05:23:23 +0000 Subject: [PATCH 08/21] Increase test coverage for local todo --- tests/components/local_todo/conftest.py | 20 ++++++-- tests/components/local_todo/test_todo.py | 62 ++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 4 deletions(-) diff --git a/tests/components/local_todo/conftest.py b/tests/components/local_todo/conftest.py index 86f5e0fc5e650a..985224b8d602bd 100644 --- a/tests/components/local_todo/conftest.py +++ b/tests/components/local_todo/conftest.py @@ -29,30 +29,42 @@ def mock_setup_entry() -> Generator[AsyncMock, None, None]: class FakeStore(LocalTodoListStore): """Mock storage implementation.""" - def __init__(self, hass: HomeAssistant, path: Path, ics_content: str) -> None: + def __init__( + self, hass: HomeAssistant, path: Path, ics_content: str | None + ) -> None: """Initialize FakeStore.""" self._content = ics_content mock_path = Mock() + mock_path.exists = self._mock_exists mock_path.read_text = self._mock_read_text mock_path.write_text = self._mock_write_text super().__init__(hass, mock_path) + def _mock_exists(self) -> bool: + return self._content is not None + def _mock_read_text(self) -> str: - return self._content + return self._content or "" def _mock_write_text(self, content: str) -> None: self._content = content +@pytest.fixture(name="ics_content") +def mock_ics_content() -> str | None: + """Fixture to set .ics file content.""" + return "" + + @pytest.fixture(name="store", autouse=True) -def mock_store() -> Generator[None, None, None]: +def mock_store(ics_content: str) -> Generator[None, None, None]: """Test cleanup, remove any media storage persisted during the test.""" stores: dict[Path, FakeStore] = {} def new_store(hass: HomeAssistant, path: Path) -> FakeStore: if path not in stores: - stores[path] = FakeStore(hass, path, "") + stores[path] = FakeStore(hass, path, ics_content) return stores[path] with patch("homeassistant.components.local_todo.LocalTodoListStore", new=new_store): diff --git a/tests/components/local_todo/test_todo.py b/tests/components/local_todo/test_todo.py index c54b6e09d55171..b1b7038fc83d41 100644 --- a/tests/components/local_todo/test_todo.py +++ b/tests/components/local_todo/test_todo.py @@ -1,6 +1,7 @@ """Tests for todo platform of local_todo.""" from collections.abc import Awaitable, Callable +import textwrap import pytest @@ -318,3 +319,64 @@ async def test_move_item_unknown( assert not resp.get("success") assert resp.get("error", {}).get("code") == "failed" assert "not found in todo list" in resp["error"]["message"] + + +@pytest.mark.parametrize( + ("ics_content", "expected_state"), + [ + ("", "0"), + (None, "0"), + ( + textwrap.dedent( + """\ + BEGIN:VCALENDAR + PRODID:-//homeassistant.io//local_todo 1.0//EN + VERSION:2.0 + BEGIN:VTODO + DTSTAMP:20231024T014011 + UID:077cb7f2-6c89-11ee-b2a9-0242ac110002 + CREATED:20231017T010348 + LAST-MODIFIED:20231024T014011 + SEQUENCE:1 + STATUS:COMPLETED + SUMMARY:Complete Task + END:VTODO + END:VCALENDAR + """ + ), + "0", + ), + ( + textwrap.dedent( + """\ + BEGIN:VCALENDAR + PRODID:-//homeassistant.io//local_todo 1.0//EN + VERSION:2.0 + BEGIN:VTODO + DTSTAMP:20231024T014011 + UID:077cb7f2-6c89-11ee-b2a9-0242ac110002 + CREATED:20231017T010348 + LAST-MODIFIED:20231024T014011 + SEQUENCE:1 + STATUS:NEEDS-ACTION + SUMMARY:Incomplete Task + END:VTODO + END:VCALENDAR + """ + ), + "1", + ), + ], + ids=("empty", "not_exists", "completed", "needs_action"), +) +async def test_parse_existing_ics( + hass: HomeAssistant, + hass_ws_client: WebSocketGenerator, + setup_integration: None, + expected_state: str, +) -> None: + """Test parsing ics content.""" + + state = hass.states.get(TEST_ENTITY) + assert state + assert state.state == expected_state From 328ecddc651b37b12b14adf7d6de6282760b330a Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Tue, 24 Oct 2023 07:51:05 -0700 Subject: [PATCH 09/21] Apply suggestions from code review Co-authored-by: Robert Resch Co-authored-by: Martin Hjelmare --- homeassistant/components/local_todo/store.py | 1 - homeassistant/components/local_todo/strings.json | 2 +- tests/components/local_todo/test_todo.py | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/homeassistant/components/local_todo/store.py b/homeassistant/components/local_todo/store.py index 0811ce645a9b7a..50ad5698f7b1ff 100644 --- a/homeassistant/components/local_todo/store.py +++ b/homeassistant/components/local_todo/store.py @@ -5,7 +5,6 @@ from homeassistant.core import HomeAssistant -STORAGE_PATH = ".storage/{key}.ics" class LocalTodoListStore: diff --git a/homeassistant/components/local_todo/strings.json b/homeassistant/components/local_todo/strings.json index 3531922c04e584..5829c292f9b11b 100644 --- a/homeassistant/components/local_todo/strings.json +++ b/homeassistant/components/local_todo/strings.json @@ -5,7 +5,7 @@ "user": { "description": "Please choose a name for your new To-do list", "data": { - "todo_list_name": "To-do List Name" + "todo_list_name": "To-do list name" } } } diff --git a/tests/components/local_todo/test_todo.py b/tests/components/local_todo/test_todo.py index b1b7038fc83d41..6d06649a6ba117 100644 --- a/tests/components/local_todo/test_todo.py +++ b/tests/components/local_todo/test_todo.py @@ -154,7 +154,7 @@ async def test_bulk_delete( setup_integration: None, ws_get_items: Callable[[], Awaitable[dict[str, str]]], ) -> None: - """Test deleting a todo item.""" + """Test deleting multiple todo items.""" for i in range(0, 5): await hass.services.async_call( TODO_DOMAIN, From 5ff632401b8b240c17422f436a18882b4dee4615 Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Tue, 24 Oct 2023 15:08:20 +0000 Subject: [PATCH 10/21] Add strict typing --- .strict-typing | 1 + homeassistant/components/local_todo/todo.py | 2 +- mypy.ini | 10 ++++++++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/.strict-typing b/.strict-typing index 97e3f5778494ec..1faf190a1de797 100644 --- a/.strict-typing +++ b/.strict-typing @@ -204,6 +204,7 @@ homeassistant.components.light.* homeassistant.components.litejet.* homeassistant.components.litterrobot.* homeassistant.components.local_ip.* +homeassistant.components.local_todo.* homeassistant.components.lock.* homeassistant.components.logbook.* homeassistant.components.logger.* diff --git a/homeassistant/components/local_todo/todo.py b/homeassistant/components/local_todo/todo.py index 833ff743f78da8..e705d5ab42657b 100644 --- a/homeassistant/components/local_todo/todo.py +++ b/homeassistant/components/local_todo/todo.py @@ -73,7 +73,7 @@ def _todo_dict_factory(obj: Iterable[tuple[str, Any]]) -> dict[str, str]: def _convert_item(item: TodoItem) -> Todo: """Convert a HomeAssistant TodoItem to an ical Todo.""" try: - return Todo.parse_obj(dataclasses.asdict(item, dict_factory=_todo_dict_factory)) + return Todo(**dataclasses.asdict(item, dict_factory=_todo_dict_factory)) except ValidationError as err: _LOGGER.debug("Error parsing todo input fields: %s (%s)", item, str(err)) raise HomeAssistantError("Error parsing todo input fields") from err diff --git a/mypy.ini b/mypy.ini index 43ec39ebc56b44..92b96e756598aa 100644 --- a/mypy.ini +++ b/mypy.ini @@ -1801,6 +1801,16 @@ disallow_untyped_defs = true warn_return_any = true warn_unreachable = true +[mypy-homeassistant.components.local_todo.*] +check_untyped_defs = true +disallow_incomplete_defs = true +disallow_subclassing_any = true +disallow_untyped_calls = true +disallow_untyped_decorators = true +disallow_untyped_defs = true +warn_return_any = true +warn_unreachable = true + [mypy-homeassistant.components.lock.*] check_untyped_defs = true disallow_incomplete_defs = true From ccc160c5337615241a0063fe093eb35dc6b4ea3d Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Tue, 24 Oct 2023 15:27:44 +0000 Subject: [PATCH 11/21] Update config flow to prevent multiple entities with the same name --- .../components/local_todo/config_flow.py | 4 +++ .../components/local_todo/strings.json | 3 ++ tests/components/local_todo/conftest.py | 7 +++- .../components/local_todo/test_config_flow.py | 34 +++++++++++++++++-- 4 files changed, 44 insertions(+), 4 deletions(-) diff --git a/homeassistant/components/local_todo/config_flow.py b/homeassistant/components/local_todo/config_flow.py index 02e95c77a3a7eb..f02e2dddf52c00 100644 --- a/homeassistant/components/local_todo/config_flow.py +++ b/homeassistant/components/local_todo/config_flow.py @@ -8,6 +8,7 @@ from homeassistant import config_entries from homeassistant.data_entry_flow import FlowResult +from homeassistant.util import slugify from .const import CONF_TODO_LIST_NAME, DOMAIN @@ -31,6 +32,9 @@ async def async_step_user( """Handle the initial step.""" errors: dict[str, str] = {} if user_input is not None: + _LOGGER.info(slugify(user_input[CONF_TODO_LIST_NAME])) + await self.async_set_unique_id(slugify(user_input[CONF_TODO_LIST_NAME])) + self._abort_if_unique_id_configured() return self.async_create_entry( title=user_input[CONF_TODO_LIST_NAME], data=user_input ) diff --git a/homeassistant/components/local_todo/strings.json b/homeassistant/components/local_todo/strings.json index 5829c292f9b11b..e6e72d2d4e534e 100644 --- a/homeassistant/components/local_todo/strings.json +++ b/homeassistant/components/local_todo/strings.json @@ -8,6 +8,9 @@ "todo_list_name": "To-do list name" } } + }, + "abort": { + "already_configured": "[%key:common::config_flow::abort::already_configured_account%]" } } } diff --git a/tests/components/local_todo/conftest.py b/tests/components/local_todo/conftest.py index 985224b8d602bd..12f3f62e36ef66 100644 --- a/tests/components/local_todo/conftest.py +++ b/tests/components/local_todo/conftest.py @@ -9,6 +9,7 @@ from homeassistant.components.local_todo.const import CONF_TODO_LIST_NAME, DOMAIN from homeassistant.core import HomeAssistant from homeassistant.setup import async_setup_component +from homeassistant.util import slugify from tests.common import MockConfigEntry @@ -74,7 +75,11 @@ def new_store(hass: HomeAssistant, path: Path) -> FakeStore: @pytest.fixture(name="config_entry") def mock_config_entry() -> MockConfigEntry: """Fixture for mock configuration entry.""" - return MockConfigEntry(domain=DOMAIN, data={CONF_TODO_LIST_NAME: TODO_NAME}) + return MockConfigEntry( + unique_id=slugify(TODO_NAME), + domain=DOMAIN, + data={CONF_TODO_LIST_NAME: TODO_NAME}, + ) @pytest.fixture(name="setup_integration") diff --git a/tests/components/local_todo/test_config_flow.py b/tests/components/local_todo/test_config_flow.py index b78031b1d5539a..52d88d6446e99f 100644 --- a/tests/components/local_todo/test_config_flow.py +++ b/tests/components/local_todo/test_config_flow.py @@ -8,6 +8,10 @@ from homeassistant.core import HomeAssistant from homeassistant.data_entry_flow import FlowResultType +from .conftest import TODO_NAME + +from tests.common import MockConfigEntry + pytestmark = pytest.mark.usefixtures("mock_setup_entry") @@ -22,12 +26,36 @@ async def test_form(hass: HomeAssistant, mock_setup_entry: AsyncMock) -> None: result2 = await hass.config_entries.flow.async_configure( result["flow_id"], { - "todo_list_name": "My tasks", + "todo_list_name": TODO_NAME, }, ) await hass.async_block_till_done() assert result2["type"] == FlowResultType.CREATE_ENTRY - assert result2["title"] == "My tasks" - assert result2["data"] == {"todo_list_name": "My tasks"} + assert result2["title"] == TODO_NAME + assert result2["data"] == {"todo_list_name": TODO_NAME} assert len(mock_setup_entry.mock_calls) == 1 + + +async def test_duplicate_todo_list_name( + hass: HomeAssistant, setup_integration: None, config_entry: MockConfigEntry +) -> None: + """Test two todo-lists cannot be added with the same name.""" + + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": config_entries.SOURCE_USER} + ) + assert result["type"] == FlowResultType.FORM + assert not result.get("errors") + + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], + { + # Pick a name that has the same slugify value as an existing config entry + "todo_list_name": "my tasks", + }, + ) + await hass.async_block_till_done() + + assert result2["type"] == FlowResultType.ABORT + assert result2["reason"] == "already_configured" From 6d2790aae26430267def827fb5d16f3900fb0c17 Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Tue, 24 Oct 2023 15:41:15 +0000 Subject: [PATCH 12/21] Add tests for raising OSError from the store --- homeassistant/components/local_todo/todo.py | 7 +++++-- tests/components/local_todo/conftest.py | 19 ++++++++++++++++--- tests/components/local_todo/test_todo.py | 18 ++++++++++++++++++ 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/homeassistant/components/local_todo/todo.py b/homeassistant/components/local_todo/todo.py index e705d5ab42657b..6837b81f51fed0 100644 --- a/homeassistant/components/local_todo/todo.py +++ b/homeassistant/components/local_todo/todo.py @@ -19,7 +19,7 @@ ) from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant -from homeassistant.exceptions import HomeAssistantError +from homeassistant.exceptions import HomeAssistantError, PlatformNotReady from homeassistant.helpers.entity_platform import AddEntitiesCallback from .const import CONF_TODO_LIST_NAME, DOMAIN @@ -50,7 +50,10 @@ async def async_setup_entry( """Set up the local_todo todo platform.""" store = hass.data[DOMAIN][config_entry.entry_id] - ics = await store.async_load() + try: + ics = await store.async_load() + except OSError as err: + raise PlatformNotReady() from err calendar = IcsCalendarStream.calendar_from_ics(ics) calendar.prodid = PRODID diff --git a/tests/components/local_todo/conftest.py b/tests/components/local_todo/conftest.py index 12f3f62e36ef66..705360fc6628b9 100644 --- a/tests/components/local_todo/conftest.py +++ b/tests/components/local_todo/conftest.py @@ -31,10 +31,15 @@ class FakeStore(LocalTodoListStore): """Mock storage implementation.""" def __init__( - self, hass: HomeAssistant, path: Path, ics_content: str | None + self, + hass: HomeAssistant, + path: Path, + ics_content: str | None, + raise_error: bool, ) -> None: """Initialize FakeStore.""" self._content = ics_content + self._raise_error = raise_error mock_path = Mock() mock_path.exists = self._mock_exists mock_path.read_text = self._mock_read_text @@ -45,6 +50,8 @@ def _mock_exists(self) -> bool: return self._content is not None def _mock_read_text(self) -> str: + if self._raise_error: + raise OSError("Fake Error") return self._content or "" def _mock_write_text(self, content: str) -> None: @@ -57,15 +64,21 @@ def mock_ics_content() -> str | None: return "" +@pytest.fixture(name="store_error") +def mock_store_error() -> bool: + """Fixture to raise errors from the FakeStore.""" + return False + + @pytest.fixture(name="store", autouse=True) -def mock_store(ics_content: str) -> Generator[None, None, None]: +def mock_store(ics_content: str, store_error: bool) -> Generator[None, None, None]: """Test cleanup, remove any media storage persisted during the test.""" stores: dict[Path, FakeStore] = {} def new_store(hass: HomeAssistant, path: Path) -> FakeStore: if path not in stores: - stores[path] = FakeStore(hass, path, ics_content) + stores[path] = FakeStore(hass, path, ics_content, store_error) return stores[path] with patch("homeassistant.components.local_todo.LocalTodoListStore", new=new_store): diff --git a/tests/components/local_todo/test_todo.py b/tests/components/local_todo/test_todo.py index 6d06649a6ba117..044af082fe604e 100644 --- a/tests/components/local_todo/test_todo.py +++ b/tests/components/local_todo/test_todo.py @@ -380,3 +380,21 @@ async def test_parse_existing_ics( state = hass.states.get(TEST_ENTITY) assert state assert state.state == expected_state + + +@pytest.mark.parametrize( + ("store_error"), + [ + (True), + ], +) +async def test_store_os_error( + hass: HomeAssistant, + hass_ws_client: WebSocketGenerator, + setup_integration: None, + # expected_state: str, +) -> None: + """Test failures loading the todo store.""" + + state = hass.states.get(TEST_ENTITY) + assert not state From 79299b9593452ffdc44fe4df81c2f40353074407 Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Tue, 24 Oct 2023 15:51:13 +0000 Subject: [PATCH 13/21] Update test fixtures from PR feedback --- tests/components/local_todo/conftest.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/components/local_todo/conftest.py b/tests/components/local_todo/conftest.py index 705360fc6628b9..d24701a7440bc9 100644 --- a/tests/components/local_todo/conftest.py +++ b/tests/components/local_todo/conftest.py @@ -8,7 +8,6 @@ from homeassistant.components.local_todo import LocalTodoListStore from homeassistant.components.local_todo.const import CONF_TODO_LIST_NAME, DOMAIN from homeassistant.core import HomeAssistant -from homeassistant.setup import async_setup_component from homeassistant.util import slugify from tests.common import MockConfigEntry @@ -72,7 +71,7 @@ def mock_store_error() -> bool: @pytest.fixture(name="store", autouse=True) def mock_store(ics_content: str, store_error: bool) -> Generator[None, None, None]: - """Test cleanup, remove any media storage persisted during the test.""" + """Fixture that sets up a fake local storage object.""" stores: dict[Path, FakeStore] = {} @@ -99,5 +98,5 @@ def mock_config_entry() -> MockConfigEntry: async def setup_integration(hass: HomeAssistant, config_entry: MockConfigEntry) -> None: """Set up the integration.""" config_entry.add_to_hass(hass) - assert await async_setup_component(hass, DOMAIN, {}) + await hass.config_entries.async_setup(config_entry.entry_id) await hass.async_block_till_done() From b979230a40fcc7640c483d5962e651b25e8a6ca7 Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Tue, 24 Oct 2023 15:55:36 +0000 Subject: [PATCH 14/21] Add test that loads/unloads config entry --- tests/components/local_todo/test_init.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/components/local_todo/test_init.py b/tests/components/local_todo/test_init.py index 2888c72c8168a0..627e8f276dfa75 100644 --- a/tests/components/local_todo/test_init.py +++ b/tests/components/local_todo/test_init.py @@ -2,11 +2,34 @@ from unittest.mock import patch +from homeassistant.config_entries import ConfigEntryState from homeassistant.core import HomeAssistant +from .conftest import TEST_ENTITY + from tests.common import MockConfigEntry +async def test_load_unload( + hass: HomeAssistant, setup_integration: None, config_entry: MockConfigEntry +) -> None: + """Test loading and unloading a config entry.""" + + assert config_entry.state == ConfigEntryState.LOADED + + state = hass.states.get(TEST_ENTITY) + assert state + assert state.state == "0" + + await hass.config_entries.async_unload(config_entry.entry_id) + await hass.async_block_till_done() + + assert config_entry.state == ConfigEntryState.NOT_LOADED + state = hass.states.get(TEST_ENTITY) + assert state + assert state.state == "unavailable" + + async def test_remove_config_entry( hass: HomeAssistant, setup_integration: None, config_entry: MockConfigEntry ) -> None: From 166f0551a3bdae7ddd1785d8b22b1f85471c6066 Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Tue, 24 Oct 2023 15:59:12 +0000 Subject: [PATCH 15/21] Fix formatting error --- homeassistant/components/local_todo/store.py | 1 - 1 file changed, 1 deletion(-) diff --git a/homeassistant/components/local_todo/store.py b/homeassistant/components/local_todo/store.py index 50ad5698f7b1ff..79d5adb217f9b0 100644 --- a/homeassistant/components/local_todo/store.py +++ b/homeassistant/components/local_todo/store.py @@ -6,7 +6,6 @@ from homeassistant.core import HomeAssistant - class LocalTodoListStore: """Local storage for a single To-do list.""" From bee1f600c09b4f304185aab929ff5fa5aea29d59 Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Wed, 25 Oct 2023 03:57:34 +0000 Subject: [PATCH 16/21] Update mock store side effects --- tests/components/local_todo/conftest.py | 25 ++++++++++++++---------- tests/components/local_todo/test_todo.py | 4 ++-- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/tests/components/local_todo/conftest.py b/tests/components/local_todo/conftest.py index d24701a7440bc9..14bd0dd6f12269 100644 --- a/tests/components/local_todo/conftest.py +++ b/tests/components/local_todo/conftest.py @@ -1,6 +1,7 @@ """Common fixtures for the local_todo tests.""" from collections.abc import Generator from pathlib import Path +from typing import Any from unittest.mock import AsyncMock, Mock, patch import pytest @@ -34,23 +35,25 @@ def __init__( hass: HomeAssistant, path: Path, ics_content: str | None, - raise_error: bool, + read_side_effect: Any | None = None, ) -> None: """Initialize FakeStore.""" self._content = ics_content - self._raise_error = raise_error + mock_path = Mock() mock_path.exists = self._mock_exists - mock_path.read_text = self._mock_read_text + mock_path.read_text = Mock() + mock_path.read_text.side_effect = ( + read_side_effect if read_side_effect else self._mock_read_text + ) mock_path.write_text = self._mock_write_text + super().__init__(hass, mock_path) def _mock_exists(self) -> bool: return self._content is not None def _mock_read_text(self) -> str: - if self._raise_error: - raise OSError("Fake Error") return self._content or "" def _mock_write_text(self, content: str) -> None: @@ -63,21 +66,23 @@ def mock_ics_content() -> str | None: return "" -@pytest.fixture(name="store_error") -def mock_store_error() -> bool: +@pytest.fixture(name="store_read_side_effect") +def mock_store_read_side_effect() -> Any | None: """Fixture to raise errors from the FakeStore.""" - return False + return None @pytest.fixture(name="store", autouse=True) -def mock_store(ics_content: str, store_error: bool) -> Generator[None, None, None]: +def mock_store( + ics_content: str, store_read_side_effect: Any | None +) -> Generator[None, None, None]: """Fixture that sets up a fake local storage object.""" stores: dict[Path, FakeStore] = {} def new_store(hass: HomeAssistant, path: Path) -> FakeStore: if path not in stores: - stores[path] = FakeStore(hass, path, ics_content, store_error) + stores[path] = FakeStore(hass, path, ics_content, store_read_side_effect) return stores[path] with patch("homeassistant.components.local_todo.LocalTodoListStore", new=new_store): diff --git a/tests/components/local_todo/test_todo.py b/tests/components/local_todo/test_todo.py index 044af082fe604e..54ced8d7782a77 100644 --- a/tests/components/local_todo/test_todo.py +++ b/tests/components/local_todo/test_todo.py @@ -383,9 +383,9 @@ async def test_parse_existing_ics( @pytest.mark.parametrize( - ("store_error"), + ("store_read_side_effect"), [ - (True), + (OSError("read error")), ], ) async def test_store_os_error( From 59adec4481400b1eaacd3c9e9f3aca78b737efde Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Wed, 25 Oct 2023 04:15:28 +0000 Subject: [PATCH 17/21] Updated failure checking for config entry loading --- .../components/local_todo/__init__.py | 9 ++++++++- homeassistant/components/local_todo/todo.py | 7 ++----- tests/components/local_todo/test_init.py | 19 +++++++++++++++++++ tests/components/local_todo/test_todo.py | 18 ------------------ 4 files changed, 29 insertions(+), 24 deletions(-) diff --git a/homeassistant/components/local_todo/__init__.py b/homeassistant/components/local_todo/__init__.py index e805eea3818632..78fba50e3a026a 100644 --- a/homeassistant/components/local_todo/__init__.py +++ b/homeassistant/components/local_todo/__init__.py @@ -6,6 +6,7 @@ from homeassistant.config_entries import ConfigEntry from homeassistant.const import Platform from homeassistant.core import HomeAssistant +from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.util import slugify from .const import CONF_TODO_LIST_NAME, DOMAIN @@ -23,7 +24,13 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: key = slugify(entry.data[CONF_TODO_LIST_NAME]) path = Path(hass.config.path(STORAGE_PATH.format(key=key))) - hass.data[DOMAIN][entry.entry_id] = LocalTodoListStore(hass, path) + store = LocalTodoListStore(hass, path) + try: + await store.async_load() + except OSError as err: + raise ConfigEntryNotReady("Failed to load file {path}: {err}") from err + + hass.data[DOMAIN][entry.entry_id] = store await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS) diff --git a/homeassistant/components/local_todo/todo.py b/homeassistant/components/local_todo/todo.py index 6837b81f51fed0..e705d5ab42657b 100644 --- a/homeassistant/components/local_todo/todo.py +++ b/homeassistant/components/local_todo/todo.py @@ -19,7 +19,7 @@ ) from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant -from homeassistant.exceptions import HomeAssistantError, PlatformNotReady +from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.entity_platform import AddEntitiesCallback from .const import CONF_TODO_LIST_NAME, DOMAIN @@ -50,10 +50,7 @@ async def async_setup_entry( """Set up the local_todo todo platform.""" store = hass.data[DOMAIN][config_entry.entry_id] - try: - ics = await store.async_load() - except OSError as err: - raise PlatformNotReady() from err + ics = await store.async_load() calendar = IcsCalendarStream.calendar_from_ics(ics) calendar.prodid = PRODID diff --git a/tests/components/local_todo/test_init.py b/tests/components/local_todo/test_init.py index 627e8f276dfa75..98da2ef3c122d5 100644 --- a/tests/components/local_todo/test_init.py +++ b/tests/components/local_todo/test_init.py @@ -2,6 +2,8 @@ from unittest.mock import patch +import pytest + from homeassistant.config_entries import ConfigEntryState from homeassistant.core import HomeAssistant @@ -39,3 +41,20 @@ async def test_remove_config_entry( assert await hass.config_entries.async_remove(config_entry.entry_id) await hass.async_block_till_done() unlink_mock.assert_called_once() + + +@pytest.mark.parametrize( + ("store_read_side_effect"), + [ + (OSError("read error")), + ], +) +async def test_load_failure( + hass: HomeAssistant, setup_integration: None, config_entry: MockConfigEntry +) -> None: + """Test failures loading the todo store.""" + + assert config_entry.state == ConfigEntryState.SETUP_RETRY + + state = hass.states.get(TEST_ENTITY) + assert not state diff --git a/tests/components/local_todo/test_todo.py b/tests/components/local_todo/test_todo.py index 54ced8d7782a77..6d06649a6ba117 100644 --- a/tests/components/local_todo/test_todo.py +++ b/tests/components/local_todo/test_todo.py @@ -380,21 +380,3 @@ async def test_parse_existing_ics( state = hass.states.get(TEST_ENTITY) assert state assert state.state == expected_state - - -@pytest.mark.parametrize( - ("store_read_side_effect"), - [ - (OSError("read error")), - ], -) -async def test_store_os_error( - hass: HomeAssistant, - hass_ws_client: WebSocketGenerator, - setup_integration: None, - # expected_state: str, -) -> None: - """Test failures loading the todo store.""" - - state = hass.states.get(TEST_ENTITY) - assert not state From a1f242507f55c89151cb1d285be2fb9a8ffcc2e1 Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Wed, 25 Oct 2023 04:27:36 +0000 Subject: [PATCH 18/21] Update unique identifier check based on storage key --- homeassistant/components/local_todo/__init__.py | 5 ++--- homeassistant/components/local_todo/config_flow.py | 8 ++++---- homeassistant/components/local_todo/const.py | 1 + homeassistant/components/local_todo/strings.json | 2 +- tests/components/local_todo/conftest.py | 11 +++++++---- tests/components/local_todo/test_config_flow.py | 7 +++++-- 6 files changed, 20 insertions(+), 14 deletions(-) diff --git a/homeassistant/components/local_todo/__init__.py b/homeassistant/components/local_todo/__init__.py index 78fba50e3a026a..f8403251ba0d56 100644 --- a/homeassistant/components/local_todo/__init__.py +++ b/homeassistant/components/local_todo/__init__.py @@ -9,7 +9,7 @@ from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.util import slugify -from .const import CONF_TODO_LIST_NAME, DOMAIN +from .const import CONF_STORAGE_KEY, CONF_TODO_LIST_NAME, DOMAIN from .store import LocalTodoListStore PLATFORMS: list[Platform] = [Platform.TODO] @@ -22,8 +22,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: hass.data.setdefault(DOMAIN, {}) - key = slugify(entry.data[CONF_TODO_LIST_NAME]) - path = Path(hass.config.path(STORAGE_PATH.format(key=key))) + path = Path(hass.config.path(STORAGE_PATH.format(key=entry.data[CONF_STORAGE_KEY]))) store = LocalTodoListStore(hass, path) try: await store.async_load() diff --git a/homeassistant/components/local_todo/config_flow.py b/homeassistant/components/local_todo/config_flow.py index f02e2dddf52c00..73328358a3cbca 100644 --- a/homeassistant/components/local_todo/config_flow.py +++ b/homeassistant/components/local_todo/config_flow.py @@ -10,7 +10,7 @@ from homeassistant.data_entry_flow import FlowResult from homeassistant.util import slugify -from .const import CONF_TODO_LIST_NAME, DOMAIN +from .const import CONF_STORAGE_KEY, CONF_TODO_LIST_NAME, DOMAIN _LOGGER = logging.getLogger(__name__) @@ -32,9 +32,9 @@ async def async_step_user( """Handle the initial step.""" errors: dict[str, str] = {} if user_input is not None: - _LOGGER.info(slugify(user_input[CONF_TODO_LIST_NAME])) - await self.async_set_unique_id(slugify(user_input[CONF_TODO_LIST_NAME])) - self._abort_if_unique_id_configured() + key = slugify(user_input[CONF_TODO_LIST_NAME]) + self._async_abort_entries_match({CONF_STORAGE_KEY: key}) + user_input[CONF_STORAGE_KEY] = key return self.async_create_entry( title=user_input[CONF_TODO_LIST_NAME], data=user_input ) diff --git a/homeassistant/components/local_todo/const.py b/homeassistant/components/local_todo/const.py index 707025d567244a..4677ed42178d8b 100644 --- a/homeassistant/components/local_todo/const.py +++ b/homeassistant/components/local_todo/const.py @@ -3,3 +3,4 @@ DOMAIN = "local_todo" CONF_TODO_LIST_NAME = "todo_list_name" +CONF_STORAGE_KEY = "storage_key" diff --git a/homeassistant/components/local_todo/strings.json b/homeassistant/components/local_todo/strings.json index e6e72d2d4e534e..2403fae60a5e3d 100644 --- a/homeassistant/components/local_todo/strings.json +++ b/homeassistant/components/local_todo/strings.json @@ -10,7 +10,7 @@ } }, "abort": { - "already_configured": "[%key:common::config_flow::abort::already_configured_account%]" + "already_configured": "[%key:common::config_flow::abort::already_configured_service%]" } } } diff --git a/tests/components/local_todo/conftest.py b/tests/components/local_todo/conftest.py index 14bd0dd6f12269..d893a6f694b25b 100644 --- a/tests/components/local_todo/conftest.py +++ b/tests/components/local_todo/conftest.py @@ -7,14 +7,18 @@ import pytest from homeassistant.components.local_todo import LocalTodoListStore -from homeassistant.components.local_todo.const import CONF_TODO_LIST_NAME, DOMAIN +from homeassistant.components.local_todo.const import ( + CONF_STORAGE_KEY, + CONF_TODO_LIST_NAME, + DOMAIN, +) from homeassistant.core import HomeAssistant -from homeassistant.util import slugify from tests.common import MockConfigEntry TODO_NAME = "My Tasks" FRIENDLY_NAME = "My tasks" +STORAGE_KEY = "my_tasks" TEST_ENTITY = "todo.my_tasks" @@ -93,9 +97,8 @@ def new_store(hass: HomeAssistant, path: Path) -> FakeStore: def mock_config_entry() -> MockConfigEntry: """Fixture for mock configuration entry.""" return MockConfigEntry( - unique_id=slugify(TODO_NAME), domain=DOMAIN, - data={CONF_TODO_LIST_NAME: TODO_NAME}, + data={CONF_STORAGE_KEY: STORAGE_KEY, CONF_TODO_LIST_NAME: TODO_NAME}, ) diff --git a/tests/components/local_todo/test_config_flow.py b/tests/components/local_todo/test_config_flow.py index 52d88d6446e99f..6677a39e54a41c 100644 --- a/tests/components/local_todo/test_config_flow.py +++ b/tests/components/local_todo/test_config_flow.py @@ -8,7 +8,7 @@ from homeassistant.core import HomeAssistant from homeassistant.data_entry_flow import FlowResultType -from .conftest import TODO_NAME +from .conftest import STORAGE_KEY, TODO_NAME from tests.common import MockConfigEntry @@ -33,7 +33,10 @@ async def test_form(hass: HomeAssistant, mock_setup_entry: AsyncMock) -> None: assert result2["type"] == FlowResultType.CREATE_ENTRY assert result2["title"] == TODO_NAME - assert result2["data"] == {"todo_list_name": TODO_NAME} + assert result2["data"] == { + "todo_list_name": TODO_NAME, + "storage_key": STORAGE_KEY, + } assert len(mock_setup_entry.mock_calls) == 1 From 202901728fa056511d69f2d9fd761a4ec07ea69c Mon Sep 17 00:00:00 2001 From: Martin Hjelmare Date: Wed, 25 Oct 2023 11:46:37 +0200 Subject: [PATCH 19/21] Clean up test fixture --- tests/components/local_todo/conftest.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/components/local_todo/conftest.py b/tests/components/local_todo/conftest.py index d893a6f694b25b..838c0b87c3f6b1 100644 --- a/tests/components/local_todo/conftest.py +++ b/tests/components/local_todo/conftest.py @@ -47,9 +47,8 @@ def __init__( mock_path = Mock() mock_path.exists = self._mock_exists mock_path.read_text = Mock() - mock_path.read_text.side_effect = ( - read_side_effect if read_side_effect else self._mock_read_text - ) + mock_path.read_text.return_value = self._content + mock_path.read_text.side_effect = read_side_effect mock_path.write_text = self._mock_write_text super().__init__(hass, mock_path) @@ -57,9 +56,6 @@ def __init__( def _mock_exists(self) -> bool: return self._content is not None - def _mock_read_text(self) -> str: - return self._content or "" - def _mock_write_text(self, content: str) -> None: self._content = content From cab3577b1500e4f2ec76191242dcf2c804607af8 Mon Sep 17 00:00:00 2001 From: Martin Hjelmare Date: Wed, 25 Oct 2023 11:47:24 +0200 Subject: [PATCH 20/21] Clean not needed str copy --- homeassistant/components/local_todo/todo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/local_todo/todo.py b/homeassistant/components/local_todo/todo.py index e705d5ab42657b..14d14316faf39f 100644 --- a/homeassistant/components/local_todo/todo.py +++ b/homeassistant/components/local_todo/todo.py @@ -75,7 +75,7 @@ def _convert_item(item: TodoItem) -> Todo: try: return Todo(**dataclasses.asdict(item, dict_factory=_todo_dict_factory)) except ValidationError as err: - _LOGGER.debug("Error parsing todo input fields: %s (%s)", item, str(err)) + _LOGGER.debug("Error parsing todo input fields: %s (%s)", item, err) raise HomeAssistantError("Error parsing todo input fields") from err From 431f6337f41233ebcb01bc937006578cca065caa Mon Sep 17 00:00:00 2001 From: Martin Hjelmare Date: Wed, 25 Oct 2023 12:03:27 +0200 Subject: [PATCH 21/21] Clean up fixture more --- tests/components/local_todo/conftest.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/components/local_todo/conftest.py b/tests/components/local_todo/conftest.py index 838c0b87c3f6b1..5afa005dd64df8 100644 --- a/tests/components/local_todo/conftest.py +++ b/tests/components/local_todo/conftest.py @@ -42,22 +42,20 @@ def __init__( read_side_effect: Any | None = None, ) -> None: """Initialize FakeStore.""" - self._content = ics_content - - mock_path = Mock() + mock_path = self._mock_path = Mock() mock_path.exists = self._mock_exists mock_path.read_text = Mock() - mock_path.read_text.return_value = self._content + mock_path.read_text.return_value = ics_content mock_path.read_text.side_effect = read_side_effect mock_path.write_text = self._mock_write_text super().__init__(hass, mock_path) def _mock_exists(self) -> bool: - return self._content is not None + return self._mock_path.read_text.return_value is not None def _mock_write_text(self, content: str) -> None: - self._content = content + self._mock_path.read_text.return_value = content @pytest.fixture(name="ics_content")