diff --git a/homeassistant/components/local_todo/todo.py b/homeassistant/components/local_todo/todo.py index 14d14316faf..7e23d01ee46 100644 --- a/homeassistant/components/local_todo/todo.py +++ b/homeassistant/components/local_todo/todo.py @@ -139,20 +139,28 @@ class LocalTodoListEntity(TodoListEntity): await self._async_save() await self.async_update_ha_state(force_refresh=True) - async def async_move_todo_item(self, uid: str, pos: int) -> None: + async def async_move_todo_item( + self, uid: str, previous_uid: str | None = None + ) -> None: """Re-order an item to the To-do list.""" + if uid == previous_uid: + return todos = self._calendar.todos - found_item: Todo | None = None - for idx, itm in enumerate(todos): - if itm.uid == uid: - found_item = itm - todos.pop(idx) - break - if found_item is None: + item_idx: dict[str, int] = {itm.uid: idx for idx, itm in enumerate(todos)} + if uid not in item_idx: raise HomeAssistantError( - f"Item '{uid}' not found in todo list {self.entity_id}" + "Item '{uid}' not found in todo list {self.entity_id}" ) - todos.insert(pos, found_item) + if previous_uid and previous_uid not in item_idx: + raise HomeAssistantError( + "Item '{previous_uid}' not found in todo list {self.entity_id}" + ) + dst_idx = item_idx[previous_uid] + 1 if previous_uid 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) await self._async_save() await self.async_update_ha_state(force_refresh=True) diff --git a/homeassistant/components/shopping_list/__init__.py b/homeassistant/components/shopping_list/__init__.py index f2de59b10af..e2f04b5d880 100644 --- a/homeassistant/components/shopping_list/__init__.py +++ b/homeassistant/components/shopping_list/__init__.py @@ -322,17 +322,23 @@ class ShoppingData: context=context, ) - async def async_move_item(self, uid: str, pos: int) -> None: + async def async_move_item(self, uid: str, previous: str | None = None) -> None: """Re-order a shopping list item.""" - found_item: dict[str, Any] | None = None - for idx, itm in enumerate(self.items): - if cast(str, itm["id"]) == uid: - found_item = itm - self.items.pop(idx) - break - if not found_item: + if uid == previous: + return + item_idx = {cast(str, itm["id"]): idx for idx, itm in enumerate(self.items)} + if uid not in item_idx: raise NoMatchingShoppingListItem(f"Item '{uid}' not found in shopping list") - self.items.insert(pos, found_item) + if previous and previous not in item_idx: + raise NoMatchingShoppingListItem( + f"Item '{previous}' not found in shopping list" + ) + dst_idx = item_idx[previous] + 1 if previous else 0 + src_idx = item_idx[uid] + src_item = self.items.pop(src_idx) + if dst_idx > src_idx: + dst_idx -= 1 + self.items.insert(dst_idx, src_item) await self.hass.async_add_executor_job(self.save) self._async_notify() self.hass.bus.async_fire( diff --git a/homeassistant/components/shopping_list/todo.py b/homeassistant/components/shopping_list/todo.py index 53c9e6b6d74..d89f376d662 100644 --- a/homeassistant/components/shopping_list/todo.py +++ b/homeassistant/components/shopping_list/todo.py @@ -71,11 +71,13 @@ class ShoppingTodoListEntity(TodoListEntity): """Add an item to the To-do list.""" await self._data.async_remove_items(set(uids)) - async def async_move_todo_item(self, uid: str, pos: int) -> None: + async def async_move_todo_item( + self, uid: str, previous_uid: str | None = None + ) -> None: """Re-order an item to the To-do list.""" try: - await self._data.async_move_item(uid, pos) + await self._data.async_move_item(uid, previous_uid) except NoMatchingShoppingListItem as err: raise HomeAssistantError( f"Shopping list item '{uid}' could not be re-ordered" diff --git a/homeassistant/components/todo/__init__.py b/homeassistant/components/todo/__init__.py index a6660b0231a..12eac858f75 100644 --- a/homeassistant/components/todo/__init__.py +++ b/homeassistant/components/todo/__init__.py @@ -152,8 +152,15 @@ class TodoListEntity(Entity): """Delete an item in the To-do list.""" raise NotImplementedError() - async def async_move_todo_item(self, uid: str, pos: int) -> None: - """Move an item in the To-do list.""" + async def async_move_todo_item( + self, uid: str, previous_uid: str | None = None + ) -> None: + """Move an item in the To-do list. + + The To-do item with the specified `uid` should be moved to the position + in the list after the specified by `previous_uid` or `None` for the first + position in the To-do list. + """ raise NotImplementedError() @@ -190,7 +197,7 @@ async def websocket_handle_todo_item_list( vol.Required("type"): "todo/item/move", vol.Required("entity_id"): cv.entity_id, vol.Required("uid"): cv.string, - vol.Optional("pos", default=0): cv.positive_int, + vol.Optional("previous_uid"): cv.string, } ) @websocket_api.async_response @@ -215,9 +222,10 @@ async def websocket_handle_todo_item_move( ) ) return - try: - await entity.async_move_todo_item(uid=msg["uid"], pos=msg["pos"]) + await entity.async_move_todo_item( + uid=msg["uid"], previous_uid=msg.get("previous_uid") + ) except HomeAssistantError as ex: connection.send_error(msg["id"], "failed", str(ex)) else: diff --git a/pylint/plugins/hass_enforce_type_hints.py b/pylint/plugins/hass_enforce_type_hints.py index 845b70b72ba..f43dd9b6672 100644 --- a/pylint/plugins/hass_enforce_type_hints.py +++ b/pylint/plugins/hass_enforce_type_hints.py @@ -2469,7 +2469,7 @@ _INHERITANCE_MATCH: dict[str, list[ClassTypeHintMatch]] = { function_name="async_move_todo_item", arg_types={ 1: "str", - 2: "int", + 2: "str | None", }, return_type="None", ), diff --git a/tests/components/local_todo/test_todo.py b/tests/components/local_todo/test_todo.py index 6d06649a6ba..8a7e38c9773 100644 --- a/tests/components/local_todo/test_todo.py +++ b/tests/components/local_todo/test_todo.py @@ -59,7 +59,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, pos: int) -> None: + async def move(uid: str, previous_uid: str | None) -> None: # Fetch items using To-do platform client = await hass_ws_client() id = ws_req_id() @@ -68,8 +68,9 @@ async def ws_move_item( "type": "todo/item/move", "entity_id": TEST_ENTITY, "uid": uid, - "pos": pos, } + if previous_uid is not None: + data["previous_uid"] = previous_uid await client.send_json(data) resp = await client.receive_json() assert resp.get("id") == id @@ -237,30 +238,29 @@ async def test_update_item( @pytest.mark.parametrize( - ("src_idx", "pos", "expected_items"), + ("src_idx", "dst_idx", "expected_items"), [ # Move any item to the front of the list - (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"]), + (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"]), - (1, 4, ["item 1", "item 3", "item 4", "item 2"]), - (1, 5, ["item 1", "item 3", "item 4", "item 2"]), # Move items left - (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"]), + (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 - (1, 1, ["item 1", "item 2", "item 3", "item 4"]), + (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"]), - (3, 4, ["item 1", "item 2", "item 3", "item 4"]), ], ) async def test_move_item( @@ -269,7 +269,7 @@ async def test_move_item( ws_get_items: Callable[[], Awaitable[dict[str, str]]], ws_move_item: Callable[[str, str | None], Awaitable[None]], src_idx: int, - pos: int, + dst_idx: int | None, expected_items: list[str], ) -> None: """Test moving a todo item within the list.""" @@ -289,7 +289,10 @@ async def test_move_item( assert summaries == ["item 1", "item 2", "item 3", "item 4"] # Prepare items for moving - await ws_move_item(uids[src_idx], pos) + previous_uid = None + if dst_idx is not None: + previous_uid = uids[dst_idx] + await ws_move_item(uids[src_idx], previous_uid) items = await ws_get_items() assert len(items) == 4 @@ -311,7 +314,42 @@ async def test_move_item_unknown( "type": "todo/item/move", "entity_id": TEST_ENTITY, "uid": "unknown", - "pos": 0, + "previous_uid": "item-2", + } + 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"] + + +async def test_move_item_previous_unknown( + hass: HomeAssistant, + setup_integration: None, + hass_ws_client: WebSocketGenerator, + ws_get_items: Callable[[], Awaitable[dict[str, str]]], +) -> None: + """Test moving a todo item that does not exist.""" + + await hass.services.async_call( + TODO_DOMAIN, + "create_item", + {"summary": "item 1"}, + target={"entity_id": TEST_ENTITY}, + blocking=True, + ) + items = await ws_get_items() + assert len(items) == 1 + + # Prepare items for moving + client = await hass_ws_client() + data = { + "id": 1, + "type": "todo/item/move", + "entity_id": TEST_ENTITY, + "uid": items[0]["uid"], + "previous_uid": "unknown", } await client.send_json(data) resp = await client.receive_json() diff --git a/tests/components/shopping_list/test_todo.py b/tests/components/shopping_list/test_todo.py index 15f1e50bdb9..ab28c6cbe6d 100644 --- a/tests/components/shopping_list/test_todo.py +++ b/tests/components/shopping_list/test_todo.py @@ -57,10 +57,10 @@ async def ws_get_items( async def ws_move_item( hass_ws_client: WebSocketGenerator, ws_req_id: Callable[[], int], -) -> Callable[[str, int | None], Awaitable[None]]: +) -> Callable[[str, str | None], Awaitable[None]]: """Fixture to move an item in the todo list.""" - async def move(uid: str, pos: int | None) -> dict[str, Any]: + async def move(uid: str, previous_uid: str | None) -> dict[str, Any]: # Fetch items using To-do platform client = await hass_ws_client() id = ws_req_id() @@ -70,8 +70,8 @@ async def ws_move_item( "entity_id": TEST_ENTITY, "uid": uid, } - if pos is not None: - data["pos"] = pos + if previous_uid is not None: + data["previous_uid"] = previous_uid await client.send_json(data) resp = await client.receive_json() assert resp.get("id") == id @@ -406,10 +406,10 @@ async def test_update_invalid_item( ("src_idx", "dst_idx", "expected_items"), [ # Move any item to the front of the list - (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"]), + (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"]), @@ -417,15 +417,15 @@ async def test_update_invalid_item( (1, 2, ["item 1", "item 3", "item 2", "item 4"]), (1, 3, ["item 1", "item 3", "item 4", "item 2"]), # Move items left - (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"]), + (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"]), - (1, 1, ["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"]), - (3, 4, ["item 1", "item 2", "item 3", "item 4"]), ], ) async def test_move_item( @@ -433,7 +433,7 @@ async def test_move_item( sl_setup: None, ws_req_id: Callable[[], int], ws_get_items: Callable[[], Awaitable[dict[str, str]]], - ws_move_item: Callable[[str, int | None], Awaitable[dict[str, Any]]], + ws_move_item: Callable[[str, str | None], Awaitable[dict[str, Any]]], src_idx: int, dst_idx: int | None, expected_items: list[str], @@ -457,7 +457,12 @@ async def test_move_item( summaries = [item["summary"] for item in items] assert summaries == ["item 1", "item 2", "item 3", "item 4"] - resp = await ws_move_item(uids[src_idx], dst_idx) + # Prepare items for moving + previous_uid: str | None = None + if dst_idx is not None: + previous_uid = uids[dst_idx] + + resp = await ws_move_item(uids[src_idx], previous_uid) assert resp.get("success") items = await ws_get_items() diff --git a/tests/components/todo/test_init.py b/tests/components/todo/test_init.py index 833a4ea266b..f4d671ad352 100644 --- a/tests/components/todo/test_init.py +++ b/tests/components/todo/test_init.py @@ -571,7 +571,7 @@ async def test_move_todo_item_service_by_id( "type": "todo/item/move", "entity_id": "todo.entity1", "uid": "item-1", - "pos": "1", + "previous_uid": "item-2", } ) resp = await client.receive_json() @@ -581,7 +581,7 @@ async def test_move_todo_item_service_by_id( args = test_entity.async_move_todo_item.call_args assert args assert args.kwargs.get("uid") == "item-1" - assert args.kwargs.get("pos") == 1 + assert args.kwargs.get("previous_uid") == "item-2" async def test_move_todo_item_service_raises( @@ -601,7 +601,7 @@ async def test_move_todo_item_service_raises( "type": "todo/item/move", "entity_id": "todo.entity1", "uid": "item-1", - "pos": "1", + "previous_uid": "item-2", } ) resp = await client.receive_json() @@ -620,15 +620,10 @@ async def test_move_todo_item_service_raises( ), ({"entity_id": "todo.entity1"}, "invalid_format", "required key not provided"), ( - {"entity_id": "todo.entity1", "pos": "2"}, + {"entity_id": "todo.entity1", "previous_uid": "item-2"}, "invalid_format", "required key not provided", ), - ( - {"entity_id": "todo.entity1", "uid": "item-1", "pos": "-2"}, - "invalid_format", - "value must be at least 0", - ), ], ) async def test_move_todo_item_service_invalid_input( @@ -722,7 +717,7 @@ async def test_move_item_unsupported( "type": "todo/item/move", "entity_id": "todo.entity1", "uid": "item-1", - "pos": "1", + "previous_uid": "item-2", } ) resp = await client.receive_json()