Change todo move API to reference previous uid (#102795)

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
+        item_idx: dict[str, int] = {itm.uid: idx for idx, itm in enumerate(todos)}
-        for idx, itm in enumerate(todos):
+        if uid not in item_idx:
-            if itm.uid == uid:
-                found_item = itm
-                todos.pop(idx)
-                break
-        if found_item is None:
             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)
 

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
+        if uid == previous:
-        for idx, itm in enumerate(self.items):
+            return
-            if cast(str, itm["id"]) == uid:
+        item_idx = {cast(str, itm["id"]): idx for idx, itm in enumerate(self.items)}
-                found_item = itm
+        if uid not in item_idx:
-                self.items.pop(idx)
-                break
-        if not found_item:
             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(

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"

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:
+    async def async_move_todo_item(
-        """Move an item in the To-do list."""
+        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:

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",
                 ),

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"]),
+        (0, None, ["item 1", "item 2", "item 3", "item 4"]),
-        (1, 0, ["item 2", "item 1", "item 3", "item 4"]),
+        (1, None, ["item 2", "item 1", "item 3", "item 4"]),
-        (2, 0, ["item 3", "item 1", "item 2", "item 4"]),
+        (2, None, ["item 3", "item 1", "item 2", "item 4"]),
-        (3, 0, ["item 4", "item 1", "item 2", "item 3"]),
+        (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"]),
+        (2, 0, ["item 1", "item 3", "item 2", "item 4"]),
-        (3, 1, ["item 1", "item 4", "item 2", "item 3"]),
+        (3, 0, ["item 1", "item 4", "item 2", "item 3"]),
-        (3, 2, ["item 1", "item 2", "item 4", "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()

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:
+        if previous_uid is not None:
-            data["pos"] = pos
+            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"]),
+        (0, None, ["item 1", "item 2", "item 3", "item 4"]),
-        (1, 0, ["item 2", "item 1", "item 3", "item 4"]),
+        (1, None, ["item 2", "item 1", "item 3", "item 4"]),
-        (2, 0, ["item 3", "item 1", "item 2", "item 4"]),
+        (2, None, ["item 3", "item 1", "item 2", "item 4"]),
-        (3, 0, ["item 4", "item 1", "item 2", "item 3"]),
+        (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"]),
+        (2, 0, ["item 1", "item 3", "item 2", "item 4"]),
-        (3, 1, ["item 1", "item 4", "item 2", "item 3"]),
+        (3, 0, ["item 1", "item 4", "item 2", "item 3"]),
-        (3, 2, ["item 1", "item 2", "item 4", "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()

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()