From 437eaa087285e3dbfbfacd3f15d5024ca97552de Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Wed, 10 Jun 2026 23:25:15 +0200 Subject: [PATCH 1/7] feat(deck): add deck_move_card_to_board tool for cross-board moves deck_reorder_card only relocated a card between stacks on the same board. Moving a card to another board now has a dedicated tool that goes through Deck's card-update route (CardService::update), which remaps the card's board-scoped labels to the destination board by title instead of leaving orphaned labels behind. Card identity (id, comments, attachments) is preserved. reorder_card is now restricted to same-board moves: it rejects a target_stack_id on another board (which Deck's reorder route would accept but with orphaned labels), steering clients to deck_move_card_to_board. Verified empirically against Deck 1.15.9: the reorder route leaves a moved card carrying its source board's label (boardId mismatch); the update route remaps it to the destination board's same-titled label. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/deck.md | 3 +- nextcloud_mcp_server/client/deck.py | 66 +++++++ nextcloud_mcp_server/server/deck.py | 57 +++++- tests/client/deck/test_deck_move_card_api.py | 153 +++++++++++++++ .../test_deck_move_card_to_board.py | 177 ++++++++++++++++++ 5 files changed, 452 insertions(+), 4 deletions(-) create mode 100644 tests/client/deck/test_deck_move_card_api.py create mode 100644 tests/integration/test_deck_move_card_to_board.py 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 From 798a00d89dccd2ef1db298dcdec81061b60363f2 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Wed, 10 Jun 2026 23:41:07 +0200 Subject: [PATCH 2/7] fix(deck): preserve done/archived and validate target board on move MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the round-1 review on PR #885: - Preserve `done` across a cross-board move. The internal card-update route (the only one that works cross-board — the board/stack-scoped route 404s for a card not already on that board) does not accept a done value, so a "done" card is re-marked done after the move. Deck stamps the current time there, so the original timestamp isn't preserved — documented as a route limitation. (`archived` is already preserved: CardService only mutates it when sent.) - Validate that target_stack_id is on target_board_id before moving, so the parameter is load-bearing and a mismatch fails loudly instead of misreporting. - Skip the same-board guard's get_stacks round-trip on a same-stack reorder. - Add unit coverage (done-restore call, destination validation, same-stack skip) and integration coverage (done preservation, target-board mismatch). Co-Authored-By: Claude Opus 4.8 (1M context) --- nextcloud_mcp_server/client/deck.py | 72 +++++--- nextcloud_mcp_server/server/deck.py | 15 +- tests/client/deck/test_deck_move_card_api.py | 155 +++++++++++++----- .../test_deck_move_card_to_board.py | 68 +++++++- 4 files changed, 243 insertions(+), 67 deletions(-) diff --git a/nextcloud_mcp_server/client/deck.py b/nextcloud_mcp_server/client/deck.py index 6d096af6..c17bbe79 100644 --- a/nextcloud_mcp_server/client/deck.py +++ b/nextcloud_mcp_server/client/deck.py @@ -392,13 +392,15 @@ class DeckClient(BaseNextcloudClient): # 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." - ) + # A same-stack reorder can't cross a board boundary, so skip the lookup. + if target_stack_id != stack_id: + 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/... @@ -418,31 +420,45 @@ class DeckClient(BaseNextcloudClient): source_board_id: int, source_stack_id: int, card_id: int, + target_board_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. + (``CardService::update``). 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), ``archived`` state and the due + date are 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. + The internal ``/apps/deck/cards/{cardId}`` route is used: it reads the + target ``stackId`` from the body (the board/stack-scoped API route + instead binds ``stackId`` from the URL — issue #469 — and 404s for a + card that isn't already on that board). The controller derives ``owner`` + from the session user and does not accept a ``done`` value, so a "done" + card is re-marked done after the move; Deck stamps the current time + there, so the original done timestamp is not preserved (a limitation of + what this route exposes). """ + # Validate the destination so target_board_id is load-bearing: the move + # itself is driven by stackId, so without this a stack on another board + # would relocate the card while the reported board is wrong. + target_stack_ids = { + stack.id for stack in await self.get_stacks(target_board_id) + } + if target_stack_id not in target_stack_ids: + raise ValueError( + f"target_stack_id {target_stack_id} is not a stack on target " + f"board {target_board_id}." + ) + current = await self.get_card(source_board_id, source_stack_id, card_id) - json_data: Dict[str, Any] = { + 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, @@ -463,7 +479,17 @@ class DeckClient(BaseNextcloudClient): json=json_data, headers=headers, ) - return DeckCard(**response.json()) + moved = DeckCard(**response.json()) + + # This route clears `done`; restore the done *state* if the card had it + # (the timestamp is refreshed to now — see the docstring note). + if current.done is not None: + await self._make_request( + "PUT", f"/apps/deck/cards/{card_id}/done", headers=headers + ) + moved = await self.get_card(target_board_id, target_stack_id, card_id) + + return moved # Labels async def get_label(self, board_id: int, label_id: int) -> DeckLabel: diff --git a/nextcloud_mcp_server/server/deck.py b/nextcloud_mcp_server/server/deck.py index daaad483..9455cdcf 100644 --- a/nextcloud_mcp_server/server/deck.py +++ b/nextcloud_mcp_server/server/deck.py @@ -1274,20 +1274,25 @@ def configure_deck_tools(mcp: FastMCP): 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). + target_stack_id must be a stack on target_board_id; the move is + rejected otherwise. 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 + target_board_id: The ID of the destination board + target_stack_id: The ID of the destination stack (must be on target_board_id) 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 + source_board_id, + source_stack_id, + card_id, + target_board_id, + target_stack_id, + order, ) return CardOperationResponse( success=True, diff --git a/tests/client/deck/test_deck_move_card_api.py b/tests/client/deck/test_deck_move_card_api.py index 79f609eb..172047ec 100644 --- a/tests/client/deck/test_deck_move_card_api.py +++ b/tests/client/deck/test_deck_move_card_api.py @@ -1,11 +1,12 @@ """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. +- move_card_to_board must validate the destination board, then PUT to the + internal card route with the card id and target stackId in the body (a + cross-board move the board/stack-scoped route can't do), restoring done state + afterwards since that route clears it. - 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. + issuing the reorder request, while skipping the lookup for same-stack reorders. """ import httpx @@ -21,24 +22,31 @@ from tests.client.conftest import ( pytestmark = pytest.mark.unit -def _stacks_list_response(stacks: list[dict]) -> httpx.Response: +def _stacks_list_response(stack_ids: list[int]) -> httpx.Response: """Mock response for get_stacks (a JSON array of stack objects).""" - return create_mock_response(status_code=200, json_data=stacks) + return create_mock_response( + status_code=200, + json_data=[ + {"id": sid, "title": f"S{sid}", "boardId": 1, "order": i, "deletedAt": 0} + for i, sid in enumerate(stack_ids) + ], + ) 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" - ) - + """The PUT lands on the internal card route with id + target stack in body.""" mock_make_request = mocker.patch.object( DeckClient, "_make_request", - side_effect=[get_card_response, put_response], + side_effect=[ + _stacks_list_response([99]), # destination validation + create_mock_deck_card_response( # get_card (source) + card_id=42, title="Movable", stack_id=10, description="keep me" + ), + create_mock_deck_card_response( # move PUT + card_id=42, title="Movable", stack_id=99, description="keep me" + ), + ], ) client = DeckClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser") @@ -46,14 +54,14 @@ async def test_move_card_to_board_sends_id_and_target_stack(mocker): source_board_id=1, source_stack_id=10, card_id=42, + target_board_id=2, 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] + put_call = mock_make_request.call_args_list[2] method, url = put_call.args[0], put_call.args[1] body = put_call.kwargs["json"] assert method == "PUT" @@ -62,22 +70,24 @@ async def test_move_card_to_board_sends_id_and_target_stack(mocker): 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 + # Not-done card: no follow-up done call + assert mock_make_request.call_count == 3 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], + side_effect=[ + _stacks_list_response([99]), + create_mock_deck_card_response( + card_id=7, stack_id=10, duedate="2030-01-02T03:04:05+00:00" + ), + create_mock_deck_card_response(card_id=7, stack_id=99), + ], ) client = DeckClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser") @@ -85,25 +95,75 @@ async def test_move_card_to_board_preserves_duedate(mocker): source_board_id=1, source_stack_id=10, card_id=7, + target_board_id=2, target_stack_id=99, ) - body = mock_make_request.call_args_list[1].kwargs["json"] + body = mock_make_request.call_args_list[2].kwargs["json"] assert body["duedate"] == "2030-01-02T03:04:05+00:00" +async def test_move_card_to_board_restores_done_state(mocker): + """A done card triggers a follow-up PUT to the done endpoint after the move.""" + mock_make_request = mocker.patch.object( + DeckClient, + "_make_request", + side_effect=[ + _stacks_list_response([99]), + create_mock_deck_card_response( # source card is done + card_id=8, stack_id=10, done="2029-12-31T23:59:00+00:00" + ), + create_mock_deck_card_response(card_id=8, stack_id=99, done=None), + create_mock_deck_card_response(card_id=8, stack_id=99), # done PUT + create_mock_deck_card_response( # re-fetch after restore + card_id=8, stack_id=99, done="2031-01-01T00:00:00+00:00" + ), + ], + ) + + client = DeckClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser") + await client.move_card_to_board( + source_board_id=1, + source_stack_id=10, + card_id=8, + target_board_id=2, + target_stack_id=99, + ) + + # 4th call restores done on the moved card + done_call = mock_make_request.call_args_list[3] + assert done_call.args[0] == "PUT" + assert done_call.args[1] == "/apps/deck/cards/8/done" + + +async def test_move_card_to_board_rejects_stack_not_on_target_board(mocker): + """A target stack absent from the target board is rejected before any move.""" + mock_make_request = mocker.patch.object( + DeckClient, + "_make_request", + return_value=_stacks_list_response([50, 51]), # 99 not present + ) + + client = DeckClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser") + with pytest.raises(ValueError, match="not a stack on target board"): + await client.move_card_to_board( + source_board_id=1, + source_stack_id=10, + card_id=42, + target_board_id=2, + target_stack_id=99, + ) + + # Only the destination validation ran; no get_card / move PUT + mock_make_request.assert_called_once() + + 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}, - ] - ), + return_value=_stacks_list_response([10, 11]), # target 99 on another board ) client = DeckClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser") @@ -121,18 +181,37 @@ async def test_reorder_card_rejects_cross_board_target(mocker): assert "/stacks" in mock_make_request.call_args.args[1] +async def test_reorder_card_same_stack_skips_lookup(mocker): + """A same-stack reorder issues only the PUT — no get_stacks round-trip.""" + mock_make_request = mocker.patch.object( + DeckClient, + "_make_request", + return_value=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=3, + target_stack_id=10, # same stack — pure reorder + ) + + mock_make_request.assert_called_once() + call = mock_make_request.call_args + assert call.args[0] == "PUT" + assert call.args[1] == "/apps/deck/cards/42/reorder" + assert call.kwargs["json"] == {"order": 3, "stackId": 10} + + 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}, - ] - ), + _stacks_list_response([10, 11]), create_mock_response(status_code=200, json_data={}), ], ) diff --git a/tests/integration/test_deck_move_card_to_board.py b/tests/integration/test_deck_move_card_to_board.py index 8469a0ea..a2e85c55 100644 --- a/tests/integration/test_deck_move_card_to_board.py +++ b/tests/integration/test_deck_move_card_to_board.py @@ -75,6 +75,7 @@ async def test_move_card_to_board_preserves_identity( source_board_id=source_board_id, source_stack_id=source_stack_id, card_id=card.id, + target_board_id=target_board_id, target_stack_id=target_stack_id, ) @@ -105,7 +106,8 @@ async def test_move_card_to_board_remaps_labels( two_boards_with_stacks ) - # Pick a default label that exists on both boards by title + # "Finished" is one of the four labels Deck auto-creates on every new + # board, so it is reliably present on both the source and target boards. 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( @@ -131,6 +133,7 @@ async def test_move_card_to_board_remaps_labels( source_board_id=source_board_id, source_stack_id=source_stack_id, card_id=card.id, + target_board_id=target_board_id, target_stack_id=target_stack_id, ) @@ -148,6 +151,69 @@ async def test_move_card_to_board_remaps_labels( assert all(label.id != source_label.id for label in finished) +async def test_move_card_to_board_preserves_done_status( + nc_client: NextcloudClient, two_boards_with_stacks: tuple +): + """A card marked done keeps its done timestamp across a cross-board move.""" + 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"Done card {suffix}" + ) + done_ts = "2030-01-02T03:04:05+00:00" + await nc_client.deck.update_card( + board_id=source_board_id, + stack_id=source_stack_id, + card_id=card.id, + done=done_ts, + ) + before = await nc_client.deck.get_card(source_board_id, source_stack_id, card.id) + assert before.done is not None, "Card should be done before the move" + + 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_board_id=target_board_id, + target_stack_id=target_stack_id, + ) + + after = await nc_client.deck.get_card(target_board_id, target_stack_id, card.id) + assert after.done is not None, "done status was cleared during the move" + + +async def test_move_card_to_board_rejects_stack_not_on_target_board( + nc_client: NextcloudClient, two_boards_with_stacks: tuple +): + """The move is rejected when target_stack_id is not on target_board_id.""" + 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"Mismatch {suffix}" + ) + + with pytest.raises(ValueError, match="not a stack on target board"): + 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_board_id=target_board_id, + target_stack_id=source_stack_id, # on the source board, not target + ) + + # 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 + + async def test_reorder_card_rejects_cross_board_target( nc_client: NextcloudClient, two_boards_with_stacks: tuple ): From 7a39767482f9fead0a921f8805ba66698ffe28ba Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Wed, 10 Jun 2026 23:47:15 +0200 Subject: [PATCH 3/7] docs(deck): note owner reassignment on move; add archived-preservation test Round-2 review polish on PR #885: - Document in the deck_move_card_to_board tool that the move reassigns the card owner to the calling user and resets the done timestamp (both are limitations of Deck's move route), so an LLM reading only the tool description isn't misled about preserved fields. - Fix the done integration-test docstring to say "done state (not timestamp)". - Add test_move_card_to_board_preserves_archived_status to lock in the documented archived-preservation behaviour. Co-Authored-By: Claude Opus 4.8 (1M context) --- nextcloud_mcp_server/server/deck.py | 15 ++++++---- .../test_deck_move_card_to_board.py | 29 ++++++++++++++++++- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/nextcloud_mcp_server/server/deck.py b/nextcloud_mcp_server/server/deck.py index 9455cdcf..9fa8069b 100644 --- a/nextcloud_mcp_server/server/deck.py +++ b/nextcloud_mcp_server/server/deck.py @@ -1268,11 +1268,16 @@ def configure_deck_tools(mcp: FastMCP): ) -> 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 card keeps its identity (same id, comments, attachments), along + with its archived state and due date. 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. + + Two caveats from Deck's move route: the card's owner is reassigned to + the user performing the move (the original owner is not preserved), and + a card marked done keeps its done state but its done timestamp is reset + to the time of the move. target_stack_id must be a stack on target_board_id; the move is rejected otherwise. diff --git a/tests/integration/test_deck_move_card_to_board.py b/tests/integration/test_deck_move_card_to_board.py index a2e85c55..6ea524eb 100644 --- a/tests/integration/test_deck_move_card_to_board.py +++ b/tests/integration/test_deck_move_card_to_board.py @@ -154,7 +154,7 @@ async def test_move_card_to_board_remaps_labels( async def test_move_card_to_board_preserves_done_status( nc_client: NextcloudClient, two_boards_with_stacks: tuple ): - """A card marked done keeps its done timestamp across a cross-board move.""" + """A card marked done keeps its done state (not timestamp) across a move.""" source_board_id, source_stack_id, target_board_id, target_stack_id = ( two_boards_with_stacks ) @@ -185,6 +185,33 @@ async def test_move_card_to_board_preserves_done_status( assert after.done is not None, "done status was cleared during the move" +async def test_move_card_to_board_preserves_archived_status( + nc_client: NextcloudClient, two_boards_with_stacks: tuple +): + """An archived card stays archived across a cross-board move.""" + 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"Archived card {suffix}" + ) + await nc_client.deck.archive_card(source_board_id, source_stack_id, card.id) + + 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_board_id=target_board_id, + target_stack_id=target_stack_id, + ) + + after = await nc_client.deck.get_card(target_board_id, target_stack_id, card.id) + assert after.stackId == target_stack_id + assert after.archived is True, "archived state was lost during the move" + + async def test_move_card_to_board_rejects_stack_not_on_target_board( nc_client: NextcloudClient, two_boards_with_stacks: tuple ): From 69b32f345c936cdc696ea80ae863e35c39d89e18 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Wed, 10 Jun 2026 23:53:01 +0200 Subject: [PATCH 4/7] feat(deck): surface remapped labels in move-card response Round-3 review polish on PR #885: - deck_move_card_to_board now captures the moved DeckCard and returns its post-move label titles in CardOperationResponse.labels, so LLM clients can confirm the cross-board label remap (the tool's headline behaviour) without a follow-up deck_get_card. The field is optional and defaults to None for the other card operations that share this response model. - Tighten test_move_card_to_board_restores_done_state to assert the returned card reflects the restored done state. Co-Authored-By: Claude Opus 4.8 (1M context) --- nextcloud_mcp_server/models/deck.py | 8 ++++++++ nextcloud_mcp_server/server/deck.py | 5 ++++- tests/client/deck/test_deck_move_card_api.py | 4 +++- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/nextcloud_mcp_server/models/deck.py b/nextcloud_mcp_server/models/deck.py index 1d9890a1..98eb1a6b 100644 --- a/nextcloud_mcp_server/models/deck.py +++ b/nextcloud_mcp_server/models/deck.py @@ -337,6 +337,14 @@ class CardOperationResponse(StatusResponse): card_id: int = Field(description="ID of the affected card") stack_id: int = Field(description="ID of the stack containing the card") board_id: int = Field(description="ID of the board containing the card") + labels: list[str] | None = Field( + default=None, + description=( + "Label titles on the card after the operation, when relevant — " + "e.g. after a cross-board move that remaps board-scoped labels to " + "the destination board" + ), + ) # Label Response Models diff --git a/nextcloud_mcp_server/server/deck.py b/nextcloud_mcp_server/server/deck.py index 9fa8069b..4a8a7632 100644 --- a/nextcloud_mcp_server/server/deck.py +++ b/nextcloud_mcp_server/server/deck.py @@ -1291,7 +1291,7 @@ def configure_deck_tools(mcp: FastMCP): order: Position within the destination stack (default 0 = top) """ client = await get_client(ctx) - await client.deck.move_card_to_board( + moved = await client.deck.move_card_to_board( source_board_id, source_stack_id, card_id, @@ -1299,12 +1299,15 @@ def configure_deck_tools(mcp: FastMCP): target_stack_id, order, ) + # Surface the post-move labels so callers can confirm the remap without + # a follow-up get_card (label remapping is this tool's whole point). return CardOperationResponse( success=True, message="Card moved to board successfully", card_id=card_id, stack_id=target_stack_id, board_id=target_board_id, + labels=[label.title for label in (moved.labels or [])], ) # Label Tools diff --git a/tests/client/deck/test_deck_move_card_api.py b/tests/client/deck/test_deck_move_card_api.py index 172047ec..32c2cb73 100644 --- a/tests/client/deck/test_deck_move_card_api.py +++ b/tests/client/deck/test_deck_move_card_api.py @@ -122,7 +122,7 @@ async def test_move_card_to_board_restores_done_state(mocker): ) client = DeckClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser") - await client.move_card_to_board( + result = await client.move_card_to_board( source_board_id=1, source_stack_id=10, card_id=8, @@ -134,6 +134,8 @@ async def test_move_card_to_board_restores_done_state(mocker): done_call = mock_make_request.call_args_list[3] assert done_call.args[0] == "PUT" assert done_call.args[1] == "/apps/deck/cards/8/done" + # The returned card reflects the re-fetched (restored) done state + assert result.done is not None async def test_move_card_to_board_rejects_stack_not_on_target_board(mocker): From 5ae9cc2a9827853f70653fc452c1d75e803973ba Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Thu, 11 Jun 2026 00:00:21 +0200 Subject: [PATCH 5/7] fix(deck): make done-restore best-effort on move; cover combined states MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-4 review polish on PR #885: - The post-move done re-mark is now best-effort: the move PUT has already committed by then, so if the /done call (or its re-fetch) fails, log a warning with the card's new location and return the moved card instead of raising as if the whole move failed. Documented in the docstring. - Note that duedate is sent explicitly as None (vs update_card omitting it) — equivalent for this route. - Add unit coverage for the swallowed done-restore failure, and an integration test for a card that is both done and archived (exercises the done-restore re-fetch on an archived card). Co-Authored-By: Claude Opus 4.8 (1M context) --- nextcloud_mcp_server/client/deck.py | 36 ++++++++++++++--- tests/client/deck/test_deck_move_card_api.py | 37 +++++++++++++++++ .../test_deck_move_card_to_board.py | 40 +++++++++++++++++++ 3 files changed, 107 insertions(+), 6 deletions(-) diff --git a/nextcloud_mcp_server/client/deck.py b/nextcloud_mcp_server/client/deck.py index c17bbe79..b2a23611 100644 --- a/nextcloud_mcp_server/client/deck.py +++ b/nextcloud_mcp_server/client/deck.py @@ -1,5 +1,8 @@ +import logging from typing import Any, Dict, List, Optional +from httpx import HTTPStatusError, RequestError + from nextcloud_mcp_server.client.base import BaseNextcloudClient from nextcloud_mcp_server.models.deck import ( DeckACL, @@ -13,6 +16,8 @@ from nextcloud_mcp_server.models.deck import ( DeckStack, ) +logger = logging.getLogger(__name__) + class DeckClient(BaseNextcloudClient): """Client for Nextcloud Deck app operations.""" @@ -443,6 +448,11 @@ class DeckClient(BaseNextcloudClient): card is re-marked done after the move; Deck stamps the current time there, so the original done timestamp is not preserved (a limitation of what this route exposes). + + The done re-mark is best-effort: the move itself has already committed + by then, so if that follow-up fails the card is left on the target + board without its done state (logged as a warning) rather than raising + and implying the move failed. """ # Validate the destination so target_board_id is load-bearing: the move # itself is driven by stackId, so without this a stack on another board @@ -467,7 +477,9 @@ class DeckClient(BaseNextcloudClient): "stackId": target_stack_id, "order": order, "description": current.description or "", - # ISO string preserves the due date; None leaves it cleared + # Sent explicitly as None when absent (update_card omits the key); + # both are equivalent here — the route reads duedate and there's + # nothing to clear on a card that never had one. "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, @@ -482,12 +494,24 @@ class DeckClient(BaseNextcloudClient): moved = DeckCard(**response.json()) # This route clears `done`; restore the done *state* if the card had it - # (the timestamp is refreshed to now — see the docstring note). + # (the timestamp is refreshed to now — see the docstring note). The move + # has already committed, so this is best-effort: on failure, warn with + # the card's new location rather than raising as if the move failed. if current.done is not None: - await self._make_request( - "PUT", f"/apps/deck/cards/{card_id}/done", headers=headers - ) - moved = await self.get_card(target_board_id, target_stack_id, card_id) + try: + await self._make_request( + "PUT", f"/apps/deck/cards/{card_id}/done", headers=headers + ) + moved = await self.get_card(target_board_id, target_stack_id, card_id) + except (HTTPStatusError, RequestError) as e: + logger.warning( + "Card %s moved to board %s stack %s but restoring done " + "state failed: %s", + card_id, + target_board_id, + target_stack_id, + e, + ) return moved diff --git a/tests/client/deck/test_deck_move_card_api.py b/tests/client/deck/test_deck_move_card_api.py index 32c2cb73..56da76ba 100644 --- a/tests/client/deck/test_deck_move_card_api.py +++ b/tests/client/deck/test_deck_move_card_api.py @@ -138,6 +138,43 @@ async def test_move_card_to_board_restores_done_state(mocker): assert result.done is not None +async def test_move_card_to_board_done_restore_failure_is_swallowed(mocker): + """If the post-move done PUT fails, the move still succeeds (best-effort).""" + mock_make_request = mocker.patch.object( + DeckClient, + "_make_request", + side_effect=[ + _stacks_list_response([99]), + create_mock_deck_card_response( # source card is done + card_id=8, stack_id=10, done="2029-12-31T23:59:00+00:00" + ), + create_mock_deck_card_response(card_id=8, stack_id=99, done=None), # move + httpx.HTTPStatusError( # done PUT fails + "500 Server Error", + request=httpx.Request("PUT", "http://test.local"), + response=create_mock_response(status_code=500, json_data={}), + ), + ], + ) + + client = DeckClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser") + # Does not raise — the move already committed + result = await client.move_card_to_board( + source_board_id=1, + source_stack_id=10, + card_id=8, + target_board_id=2, + target_stack_id=99, + ) + + # Returns the moved card from the (successful) move PUT, done unrestored + assert result.stackId == 99 + assert result.done is None + # The done restore was attempted (and failed), with no re-fetch after it + assert mock_make_request.call_count == 4 + assert mock_make_request.call_args_list[3].args[1] == "/apps/deck/cards/8/done" + + async def test_move_card_to_board_rejects_stack_not_on_target_board(mocker): """A target stack absent from the target board is rejected before any move.""" mock_make_request = mocker.patch.object( diff --git a/tests/integration/test_deck_move_card_to_board.py b/tests/integration/test_deck_move_card_to_board.py index 6ea524eb..334b3e57 100644 --- a/tests/integration/test_deck_move_card_to_board.py +++ b/tests/integration/test_deck_move_card_to_board.py @@ -212,6 +212,46 @@ async def test_move_card_to_board_preserves_archived_status( assert after.archived is True, "archived state was lost during the move" +async def test_move_card_to_board_preserves_done_and_archived( + nc_client: NextcloudClient, two_boards_with_stacks: tuple +): + """A done *and* archived card keeps both states across a cross-board move. + + This exercises the done-restore path (post-move done PUT + re-fetch) on an + archived card, confirming get_card on the destination handles it. + """ + 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"Done+archived {suffix}" + ) + await nc_client.deck.update_card( + board_id=source_board_id, + stack_id=source_stack_id, + card_id=card.id, + done="2030-01-02T03:04:05+00:00", + ) + await nc_client.deck.archive_card(source_board_id, source_stack_id, card.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_board_id=target_board_id, + target_stack_id=target_stack_id, + ) + assert moved.done is not None + assert moved.archived is True + + after = await nc_client.deck.get_card(target_board_id, target_stack_id, card.id) + assert after.stackId == target_stack_id + assert after.done is not None, "done state was lost during the move" + assert after.archived is True, "archived state was lost during the move" + + async def test_move_card_to_board_rejects_stack_not_on_target_board( nc_client: NextcloudClient, two_boards_with_stacks: tuple ): From 98c9d58e54adfc7276edc732d4bd28ac6456a406 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Thu, 11 Jun 2026 00:05:30 +0200 Subject: [PATCH 6/7] test(deck): use https in mock request URL to clear Sonar hotspot The move-card unit tests added a mock httpx.Request with an http:// URL, which SonarCloud flags as a new security hotspot (insecure protocol), failing the new-code quality gate. The URL is never dialed (it only labels a synthetic HTTPStatusError), but switch it to https to keep the gate green. Also simplify the done-PUT mock to a bare 200 response, since that response is discarded by the implementation. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/client/deck/test_deck_move_card_api.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/client/deck/test_deck_move_card_api.py b/tests/client/deck/test_deck_move_card_api.py index 56da76ba..d4a29d38 100644 --- a/tests/client/deck/test_deck_move_card_api.py +++ b/tests/client/deck/test_deck_move_card_api.py @@ -114,7 +114,7 @@ async def test_move_card_to_board_restores_done_state(mocker): card_id=8, stack_id=10, done="2029-12-31T23:59:00+00:00" ), create_mock_deck_card_response(card_id=8, stack_id=99, done=None), - create_mock_deck_card_response(card_id=8, stack_id=99), # done PUT + create_mock_response(status_code=200, json_data={}), # done PUT (ignored) create_mock_deck_card_response( # re-fetch after restore card_id=8, stack_id=99, done="2031-01-01T00:00:00+00:00" ), @@ -151,7 +151,7 @@ async def test_move_card_to_board_done_restore_failure_is_swallowed(mocker): create_mock_deck_card_response(card_id=8, stack_id=99, done=None), # move httpx.HTTPStatusError( # done PUT fails "500 Server Error", - request=httpx.Request("PUT", "http://test.local"), + request=httpx.Request("PUT", "https://test.local"), response=create_mock_response(status_code=500, json_data={}), ), ], From d887181307b8476d2a5c3341099e3b72f6a5bb69 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Thu, 11 Jun 2026 00:12:35 +0200 Subject: [PATCH 7/7] docs(deck): document assignedUsers preservation on cross-board move Round-6 review: the docstrings listed preserved fields but omitted assignedUsers. Verified empirically (Deck 1.15.9) that the update route's board-change handling only remaps labels and leaves user assignments untouched, so assignees carry over. Documented in both the client and MCP tool docstrings, with the caveat that an assignee lacking access to the target board stays assigned but cannot act on the card. Added test_move_card_to_board_preserves_assigned_users to lock it in. Co-Authored-By: Claude Opus 4.8 (1M context) --- nextcloud_mcp_server/client/deck.py | 7 ++-- nextcloud_mcp_server/server/deck.py | 10 +++--- .../test_deck_move_card_to_board.py | 36 +++++++++++++++++++ 3 files changed, 47 insertions(+), 6 deletions(-) diff --git a/nextcloud_mcp_server/client/deck.py b/nextcloud_mcp_server/client/deck.py index b2a23611..2a0c3081 100644 --- a/nextcloud_mcp_server/client/deck.py +++ b/nextcloud_mcp_server/client/deck.py @@ -437,8 +437,11 @@ class DeckClient(BaseNextcloudClient): 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), ``archived`` state and the due - date are preserved. + identity (id, comments, attachments), ``archived`` state, the due date + and ``assignedUsers`` are preserved — the move does not re-validate + assignees against the destination board, so an assigned user without + access to the target board stays assigned but may not be able to act on + the card. The internal ``/apps/deck/cards/{cardId}`` route is used: it reads the target ``stackId`` from the body (the board/stack-scoped API route diff --git a/nextcloud_mcp_server/server/deck.py b/nextcloud_mcp_server/server/deck.py index 4a8a7632..68e0b4ed 100644 --- a/nextcloud_mcp_server/server/deck.py +++ b/nextcloud_mcp_server/server/deck.py @@ -1269,10 +1269,12 @@ def configure_deck_tools(mcp: FastMCP): """Move a Nextcloud Deck card to a stack on a different board. The card keeps its identity (same id, comments, attachments), along - with its archived state and due date. 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. + with its archived state, due date and user assignments (an assignee + without access to the target board stays assigned but cannot act on the + card). 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. Two caveats from Deck's move route: the card's owner is reassigned to the user performing the move (the original owner is not preserved), and diff --git a/tests/integration/test_deck_move_card_to_board.py b/tests/integration/test_deck_move_card_to_board.py index 334b3e57..4d7e9844 100644 --- a/tests/integration/test_deck_move_card_to_board.py +++ b/tests/integration/test_deck_move_card_to_board.py @@ -252,6 +252,42 @@ async def test_move_card_to_board_preserves_done_and_archived( assert after.archived is True, "archived state was lost during the move" +async def test_move_card_to_board_preserves_assigned_users( + nc_client: NextcloudClient, two_boards_with_stacks: tuple +): + """Assigned users carry over a cross-board move (the route doesn't touch them). + + Deck's update route remaps labels on a board change but leaves the card's + user assignments untouched, so an assignee that exists on both boards stays + assigned. + """ + 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"Assigned {suffix}" + ) + # The test user owns both temporary boards, so it is a valid assignee on each + await nc_client.deck.assign_user_to_card( + source_board_id, source_stack_id, card.id, nc_client.username + ) + before = await nc_client.deck.get_card(source_board_id, source_stack_id, card.id) + assert before.assignedUsers, "Card should have an assignee before the move" + + 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_board_id=target_board_id, + target_stack_id=target_stack_id, + ) + + after = await nc_client.deck.get_card(target_board_id, target_stack_id, card.id) + assert after.assignedUsers, "Assigned users were dropped during the move" + + async def test_move_card_to_board_rejects_stack_not_on_target_board( nc_client: NextcloudClient, two_boards_with_stacks: tuple ):