diff --git a/docs/deck.md b/docs/deck.md index 56c0978b..46dd9ae7 100644 --- a/docs/deck.md +++ b/docs/deck.md @@ -23,7 +23,8 @@ | `deck_delete_card` | Delete a card | | `deck_archive_card` | Archive a card | | `deck_unarchive_card` | Unarchive a card | -| `deck_reorder_card` | Move/reorder cards within or between stacks | +| `deck_reorder_card` | Reorder/move a card within a single board (between stacks on that board) | +| `deck_move_card_to_board` | Move a card to a stack on a different board, remapping board-scoped labels | | `deck_create_label` | Create a new label in a board | | `deck_update_label` | Update label title and color | | `deck_delete_label` | Delete a label | diff --git a/nextcloud_mcp_server/client/deck.py b/nextcloud_mcp_server/client/deck.py index 2b9d05ff..6d096af6 100644 --- a/nextcloud_mcp_server/client/deck.py +++ b/nextcloud_mcp_server/client/deck.py @@ -386,6 +386,20 @@ class DeckClient(BaseNextcloudClient): order: int, target_stack_id: int, ) -> None: + # Reorder is intentionally restricted to moves *within* a single board. + # Deck's reorder route (CardService::reorder) only reassigns stackId and + # does NOT remap board-scoped labels, so handing it a stack on another + # board silently orphans the card's labels (they keep the old boardId). + # Cross-board moves must go through move_card_to_board(), which uses the + # card-update route (CardService::update) and remaps labels by title. + board_stack_ids = {stack.id for stack in await self.get_stacks(board_id)} + if target_stack_id not in board_stack_ids: + raise ValueError( + f"target_stack_id {target_stack_id} is not a stack on board " + f"{board_id}; reorder_card only moves cards within a board. " + "Use move_card_to_board() to move a card to another board." + ) + # Use the non-API route /cards/{cardId}/reorder which correctly reads # stackId from the body. The API route /api/.../stacks/{stackId}/cards/... # has a parameter conflict where URL stackId overrides body stackId. @@ -399,6 +413,58 @@ class DeckClient(BaseNextcloudClient): headers=headers, ) + async def move_card_to_board( + self, + source_board_id: int, + source_stack_id: int, + card_id: int, + target_stack_id: int, + order: int = 0, + ) -> DeckCard: + """Move a card to a stack on a different (or the same) board. + + Unlike :meth:`reorder_card`, this goes through the card-update route + (``CardService::update``, which honours ``stackId`` in the body). When + the destination stack is on a different board, Deck remaps the card's + board-scoped labels by title — assigning the same-titled label on the + destination board, or cloning it there when the user has board-manage + permission — instead of leaving orphaned labels behind. This mirrors + Deck's native "Move/copy card" action. Card identity (id, comments, + attachments) is preserved. + + The card-update route is a full replacement, so the current card is + fetched first to preserve fields that are not being changed. The + internal ``/apps/deck/cards/{cardId}`` route is used (rather than the + board/stack-scoped API route) because the latter reads ``stackId`` from + the URL, which would override the target stack in the body — the same + parameter conflict that affects reorder (issue #469). The internal + route derives ``owner`` from the session user server-side. + """ + current = await self.get_card(source_board_id, source_stack_id, card_id) + + json_data: Dict[str, Any] = { + # The route placeholder is {cardId} but the controller reads the + # card id from the body, so it must be sent explicitly. + "id": card_id, + "title": current.title, + "type": current.type, + "stackId": target_stack_id, + "order": order, + "description": current.description or "", + # ISO string preserves the due date; None leaves it cleared + "duedate": current.duedate.isoformat() if current.duedate else None, + # 0 keeps the card live (a positive value would soft-delete it) + "deletedAt": current.deletedAt or 0, + } + headers = self._get_deck_headers() + response = await self._make_request( + "PUT", + f"/apps/deck/cards/{card_id}", + json=json_data, + headers=headers, + ) + return DeckCard(**response.json()) + # Labels async def get_label(self, board_id: int, label_id: int) -> DeckLabel: headers = self._get_deck_headers() diff --git a/nextcloud_mcp_server/server/deck.py b/nextcloud_mcp_server/server/deck.py index 2638f099..daaad483 100644 --- a/nextcloud_mcp_server/server/deck.py +++ b/nextcloud_mcp_server/server/deck.py @@ -1212,7 +1212,7 @@ def configure_deck_tools(mcp: FastMCP): ) @mcp.tool( - title="Reorder/Move Deck Card", + title="Reorder Deck Card", annotations=ToolAnnotations(idempotentHint=False, openWorldHint=True), ) @require_scopes("deck.write") @@ -1225,14 +1225,19 @@ def configure_deck_tools(mcp: FastMCP): order: int, target_stack_id: int, ) -> CardOperationResponse: - """Reorder/move a Nextcloud Deck card + """Reorder a Nextcloud Deck card within a board. + + Moves a card to a new position, optionally into a different stack on + the SAME board. To move a card to a stack on a DIFFERENT board, use + deck_move_card_to_board instead — reordering across boards is rejected + because it would orphan the card's board-scoped labels. Args: board_id: The ID of the board stack_id: The ID of the current stack card_id: The ID of the card order: New position in the target stack - target_stack_id: The ID of the target stack + target_stack_id: The ID of the target stack (must be on board_id) """ client = await get_client(ctx) await client.deck.reorder_card( @@ -1246,6 +1251,52 @@ def configure_deck_tools(mcp: FastMCP): board_id=board_id, ) + @mcp.tool( + title="Move Deck Card to Another Board", + annotations=ToolAnnotations(idempotentHint=False, openWorldHint=True), + ) + @require_scopes("deck.write") + @instrument_tool + async def deck_move_card_to_board( + ctx: Context, + source_board_id: int, + source_stack_id: int, + card_id: int, + target_board_id: int, + target_stack_id: int, + order: int = 0, + ) -> CardOperationResponse: + """Move a Nextcloud Deck card to a stack on a different board. + + The card keeps its identity (same id, comments, attachments). Deck + remaps the card's board-scoped labels to the destination board by + title — reusing a same-titled label there, or cloning it when you have + board-manage permission. Use deck_reorder_card for moves within a + single board. + + The destination is determined by target_stack_id; target_board_id must + be the board that owns it (it is used to report the resulting location). + + Args: + source_board_id: The ID of the board the card currently lives on + source_stack_id: The ID of the stack the card currently lives in + card_id: The ID of the card to move + target_board_id: The ID of the destination board (must own target_stack_id) + target_stack_id: The ID of the destination stack + order: Position within the destination stack (default 0 = top) + """ + client = await get_client(ctx) + await client.deck.move_card_to_board( + source_board_id, source_stack_id, card_id, target_stack_id, order + ) + return CardOperationResponse( + success=True, + message="Card moved to board successfully", + card_id=card_id, + stack_id=target_stack_id, + board_id=target_board_id, + ) + # Label Tools @mcp.tool( title="Create Deck Label", diff --git a/tests/client/deck/test_deck_move_card_api.py b/tests/client/deck/test_deck_move_card_api.py new file mode 100644 index 00000000..79f609eb --- /dev/null +++ b/tests/client/deck/test_deck_move_card_api.py @@ -0,0 +1,153 @@ +"""Unit tests for DeckClient.move_card_to_board and the reorder same-board guard. + +These mock the HTTP layer to assert request construction without a live server: +- move_card_to_board must send the card ``id`` and target ``stackId`` in the + body (the route placeholder is ``{cardId}`` but the controller reads ``id`` + from the body), and preserve due/deleted state. +- reorder_card must reject a target stack that is not on the given board before + issuing the reorder request, steering callers to move_card_to_board. +""" + +import httpx +import pytest + +from nextcloud_mcp_server.client.deck import DeckClient +from nextcloud_mcp_server.models.deck import DeckCard +from tests.client.conftest import ( + create_mock_deck_card_response, + create_mock_response, +) + +pytestmark = pytest.mark.unit + + +def _stacks_list_response(stacks: list[dict]) -> httpx.Response: + """Mock response for get_stacks (a JSON array of stack objects).""" + return create_mock_response(status_code=200, json_data=stacks) + + +async def test_move_card_to_board_sends_id_and_target_stack(mocker): + """The PUT body carries the card id and the destination stack id.""" + get_card_response = create_mock_deck_card_response( + card_id=42, title="Movable", stack_id=10, description="keep me" + ) + put_response = create_mock_deck_card_response( + card_id=42, title="Movable", stack_id=99, description="keep me" + ) + + mock_make_request = mocker.patch.object( + DeckClient, + "_make_request", + side_effect=[get_card_response, put_response], + ) + + client = DeckClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser") + moved = await client.move_card_to_board( + source_board_id=1, + source_stack_id=10, + card_id=42, + target_stack_id=99, + ) + + assert isinstance(moved, DeckCard) + assert moved.stackId == 99 + + # Second call is the update PUT to the internal card route + put_call = mock_make_request.call_args_list[1] + method, url = put_call.args[0], put_call.args[1] + body = put_call.kwargs["json"] + assert method == "PUT" + assert url == "/apps/deck/cards/42" + assert body["id"] == 42 + assert body["stackId"] == 99 + assert body["title"] == "Movable" + assert body["description"] == "keep me" + # No live due/deleted state on the source card + assert body["duedate"] is None + assert body["deletedAt"] == 0 + + +async def test_move_card_to_board_preserves_duedate(mocker): + """A due date on the source card is forwarded as an ISO-8601 string.""" + get_card_response = create_mock_deck_card_response( + card_id=7, stack_id=10, duedate="2030-01-02T03:04:05+00:00" + ) + put_response = create_mock_deck_card_response(card_id=7, stack_id=99) + + mock_make_request = mocker.patch.object( + DeckClient, + "_make_request", + side_effect=[get_card_response, put_response], + ) + + client = DeckClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser") + await client.move_card_to_board( + source_board_id=1, + source_stack_id=10, + card_id=7, + target_stack_id=99, + ) + + body = mock_make_request.call_args_list[1].kwargs["json"] + assert body["duedate"] == "2030-01-02T03:04:05+00:00" + + +async def test_reorder_card_rejects_cross_board_target(mocker): + """A target stack absent from the board is rejected before any reorder PUT.""" + # get_stacks returns stacks 10 and 11 — target 99 is on another board + mock_make_request = mocker.patch.object( + DeckClient, + "_make_request", + return_value=_stacks_list_response( + [ + {"id": 10, "title": "A", "boardId": 1, "order": 1, "deletedAt": 0}, + {"id": 11, "title": "B", "boardId": 1, "order": 2, "deletedAt": 0}, + ] + ), + ) + + client = DeckClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser") + with pytest.raises(ValueError, match="move_card_to_board"): + await client.reorder_card( + board_id=1, + stack_id=10, + card_id=42, + order=0, + target_stack_id=99, + ) + + # Only get_stacks was issued; the reorder PUT was never sent + mock_make_request.assert_called_once() + assert "/stacks" in mock_make_request.call_args.args[1] + + +async def test_reorder_card_allows_same_board_target(mocker): + """A target stack on the same board passes the guard and issues the PUT.""" + mock_make_request = mocker.patch.object( + DeckClient, + "_make_request", + side_effect=[ + _stacks_list_response( + [ + {"id": 10, "title": "A", "boardId": 1, "order": 1, "deletedAt": 0}, + {"id": 11, "title": "B", "boardId": 1, "order": 2, "deletedAt": 0}, + ] + ), + create_mock_response(status_code=200, json_data={}), + ], + ) + + client = DeckClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser") + await client.reorder_card( + board_id=1, + stack_id=10, + card_id=42, + order=0, + target_stack_id=11, + ) + + assert mock_make_request.call_count == 2 + reorder_call = mock_make_request.call_args_list[1] + assert reorder_call.args[0] == "PUT" + assert reorder_call.args[1] == "/apps/deck/cards/42/reorder" + assert reorder_call.kwargs["json"] == {"order": 0, "stackId": 11} diff --git a/tests/integration/test_deck_move_card_to_board.py b/tests/integration/test_deck_move_card_to_board.py new file mode 100644 index 00000000..8469a0ea --- /dev/null +++ b/tests/integration/test_deck_move_card_to_board.py @@ -0,0 +1,177 @@ +"""Integration tests for moving Deck cards between boards. + +Covers ``move_card_to_board`` and the same-board restriction on +``reorder_card``. The key behaviour under test is that a cross-board move +remaps the card's board-scoped labels to the destination board (by title) +rather than leaving orphaned labels that still reference the source board. +""" + +import logging +import uuid + +import pytest + +from nextcloud_mcp_server.client import NextcloudClient + +logger = logging.getLogger(__name__) +pytestmark = pytest.mark.integration + + +@pytest.fixture +async def two_boards_with_stacks(nc_client: NextcloudClient): + """Create two temporary boards, each with a single stack. + + Yields: + tuple: (source_board_id, source_stack_id, target_board_id, target_stack_id) + """ + unique_suffix = uuid.uuid4().hex[:8] + source_board = None + target_board = None + try: + source_board = await nc_client.deck.create_board( + f"Move Test Source {unique_suffix}", "FF0000" + ) + target_board = await nc_client.deck.create_board( + f"Move Test Target {unique_suffix}", "0000FF" + ) + source_stack = await nc_client.deck.create_stack( + source_board.id, f"Source Stack {unique_suffix}", order=1 + ) + target_stack = await nc_client.deck.create_stack( + target_board.id, f"Target Stack {unique_suffix}", order=1 + ) + logger.info( + "Created source board %s/stack %s and target board %s/stack %s", + source_board.id, + source_stack.id, + target_board.id, + target_stack.id, + ) + yield (source_board.id, source_stack.id, target_board.id, target_stack.id) + finally: + for board in (source_board, target_board): + if board: + try: + await nc_client.deck.delete_board(board.id) + except Exception as e: + logger.warning("Error cleaning up board %s: %s", board.id, e) + + +async def test_move_card_to_board_preserves_identity( + nc_client: NextcloudClient, two_boards_with_stacks: tuple +): + """A cross-board move relocates the card (same id) to the target board.""" + source_board_id, source_stack_id, target_board_id, target_stack_id = ( + two_boards_with_stacks + ) + + suffix = uuid.uuid4().hex[:8] + card = await nc_client.deck.create_card( + source_board_id, source_stack_id, f"Move me {suffix}", description="payload" + ) + logger.info("Created card %s on source board %s", card.id, source_board_id) + + moved = await nc_client.deck.move_card_to_board( + source_board_id=source_board_id, + source_stack_id=source_stack_id, + card_id=card.id, + target_stack_id=target_stack_id, + ) + + # Same card id, now on the target stack, with its description preserved + assert moved.id == card.id + assert moved.stackId == target_stack_id + + # The card is readable on the target board and gone from the source stack + on_target = await nc_client.deck.get_card(target_board_id, target_stack_id, card.id) + assert on_target.stackId == target_stack_id + assert on_target.description == "payload" + + source_cards = await nc_client.deck.get_stack(source_board_id, source_stack_id) + source_card_ids = {c.id for c in (source_cards.cards or [])} + assert card.id not in source_card_ids + + +async def test_move_card_to_board_remaps_labels( + nc_client: NextcloudClient, two_boards_with_stacks: tuple +): + """A board-scoped label is remapped to the destination board, not orphaned. + + Deck auto-creates the same default labels (e.g. "Finished") on every board, + so the moved card's "Finished" label should end up pointing at the target + board's "Finished" label rather than keeping the source board's id. + """ + source_board_id, source_stack_id, target_board_id, target_stack_id = ( + two_boards_with_stacks + ) + + # Pick a default label that exists on both boards by title + source_board = await nc_client.deck.get_board(source_board_id) + target_board = await nc_client.deck.get_board(target_board_id) + source_label = next( + label for label in source_board.labels if label.title == "Finished" + ) + target_label = next( + label for label in target_board.labels if label.title == "Finished" + ) + + suffix = uuid.uuid4().hex[:8] + card = await nc_client.deck.create_card( + source_board_id, source_stack_id, f"Labeled {suffix}" + ) + await nc_client.deck.assign_label_to_card( + source_board_id, source_stack_id, card.id, source_label.id + ) + + # Sanity check: the card carries the source board's label before the move + before = await nc_client.deck.get_card(source_board_id, source_stack_id, card.id) + assert any(label.id == source_label.id for label in (before.labels or [])) + + await nc_client.deck.move_card_to_board( + source_board_id=source_board_id, + source_stack_id=source_stack_id, + card_id=card.id, + target_stack_id=target_stack_id, + ) + + after = await nc_client.deck.get_card(target_board_id, target_stack_id, card.id) + labels = after.labels or [] + assert labels, "Card lost its label during the move" + + finished = [label for label in labels if label.title == "Finished"] + assert finished, "The 'Finished' label was not preserved across the move" + # Remapped to the target board's label; the source board's label is gone + assert all(label.boardId == target_board_id for label in finished), ( + f"Label still references source board: {[label.boardId for label in finished]}" + ) + assert any(label.id == target_label.id for label in finished) + assert all(label.id != source_label.id for label in finished) + + +async def test_reorder_card_rejects_cross_board_target( + nc_client: NextcloudClient, two_boards_with_stacks: tuple +): + """reorder_card refuses a target stack on a different board.""" + source_board_id, source_stack_id, _target_board_id, target_stack_id = ( + two_boards_with_stacks + ) + + suffix = uuid.uuid4().hex[:8] + card = await nc_client.deck.create_card( + source_board_id, source_stack_id, f"No cross-board reorder {suffix}" + ) + + with pytest.raises(ValueError, match="move_card_to_board"): + await nc_client.deck.reorder_card( + board_id=source_board_id, + stack_id=source_stack_id, + card_id=card.id, + order=0, + target_stack_id=target_stack_id, # belongs to the other board + ) + + # The card stayed put + still_there = await nc_client.deck.get_card( + source_board_id, source_stack_id, card.id + ) + assert still_there.stackId == source_stack_id