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) <noreply@anthropic.com>
This commit is contained in:
Chris Coutinho
co-authored by
Claude Opus 4.8
Claude Opus 4.8
parent
ec075b4ed9
commit
437eaa0872
@@ -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}
|
||||
@@ -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
|
||||
Reference in New Issue
Block a user