diff --git a/nextcloud_mcp_server/server/deck.py b/nextcloud_mcp_server/server/deck.py index b4dd7d3d..0c16b58e 100644 --- a/nextcloud_mcp_server/server/deck.py +++ b/nextcloud_mcp_server/server/deck.py @@ -34,14 +34,81 @@ logger = logging.getLogger(__name__) def _truncate_card_descriptions( cards: list[DeckCard], description_max_length: int | None ) -> None: - """Truncate each card's description in-place when it exceeds the limit.""" + """Truncate each card's description in-place when it strictly exceeds the + limit. + + Descriptions whose length is less than or equal to ``description_max_length`` + are left untouched (no ellipsis is appended). Descriptions longer than the + limit are truncated to ``description_max_length`` characters and an + ellipsis ("…") is appended, so the resulting string is + ``description_max_length + 1`` characters total. + + Args: + cards: Cards to mutate in place. + description_max_length: Positive truncation threshold, or ``None`` to + skip truncation entirely. + + Raises: + ValueError: If ``description_max_length`` is not positive. + """ if description_max_length is None: return + if description_max_length <= 0: + raise ValueError( + f"description_max_length must be positive, got {description_max_length}" + ) for card in cards: if card.description and len(card.description) > description_max_length: card.description = card.description[:description_max_length] + "…" +def _apply_board_filters( + board: DeckBoard, + *, + include_acl: bool, + include_users: bool, + include_labels: bool, +) -> DeckBoard: + """Drop board sub-fields the caller didn't request (in-place).""" + if not include_acl: + board.acl = [] + if not include_users: + board.users = [] + if not include_labels: + board.labels = [] + return board + + +def _apply_stack_filters( + stack: DeckStack, + *, + include_cards: bool, + include_archived_cards: bool, + description_max_length: int | None, +) -> DeckStack: + """Apply card-shaping filters to a single stack (in-place).""" + if not include_cards: + stack.cards = None + elif stack.cards: + if not include_archived_cards: + stack.cards = [c for c in stack.cards if not c.archived] + _truncate_card_descriptions(stack.cards, description_max_length) + return stack + + +def _apply_card_filters( + cards: list[DeckCard], + *, + include_archived: bool, + description_max_length: int | None, +) -> list[DeckCard]: + """Apply filters to a flat list of cards. Returns a (possibly new) list.""" + if not include_archived: + cards = [c for c in cards if not c.archived] + _truncate_card_descriptions(cards, description_max_length) + return cards + + def configure_deck_tools(mcp: FastMCP): """Configure Nextcloud Deck tools and resources for the MCP server.""" @@ -175,13 +242,12 @@ def configure_deck_tools(mcp: FastMCP): """ client = await get_client(ctx) board = await client.deck.get_board(board_id) - if not include_acl: - board.acl = [] - if not include_users: - board.users = [] - if not include_labels: - board.labels = [] - return board + return _apply_board_filters( + board, + include_acl=include_acl, + include_users=include_users, + include_labels=include_labels, + ) @mcp.tool( title="List Deck Stacks", @@ -212,13 +278,12 @@ def configure_deck_tools(mcp: FastMCP): client = await get_client(ctx) stacks = await client.deck.get_stacks(board_id) for stack in stacks: - if not include_cards: - stack.cards = None - continue - if stack.cards: - if not include_archived_cards: - stack.cards = [c for c in stack.cards if not c.archived] - _truncate_card_descriptions(stack.cards, description_max_length) + _apply_stack_filters( + stack, + include_cards=include_cards, + include_archived_cards=include_archived_cards, + description_max_length=description_max_length, + ) return ListStacksResponse(stacks=stacks, total=len(stacks)) @mcp.tool( @@ -248,13 +313,12 @@ def configure_deck_tools(mcp: FastMCP): """ client = await get_client(ctx) stack = await client.deck.get_stack(board_id, stack_id) - if not include_cards: - stack.cards = None - elif stack.cards: - if not include_archived_cards: - stack.cards = [c for c in stack.cards if not c.archived] - _truncate_card_descriptions(stack.cards, description_max_length) - return stack + return _apply_stack_filters( + stack, + include_cards=include_cards, + include_archived_cards=include_archived_cards, + description_max_length=description_max_length, + ) @mcp.tool( title="List Archived Deck Stacks", @@ -312,10 +376,11 @@ def configure_deck_tools(mcp: FastMCP): """ client = await get_client(ctx) stack = await client.deck.get_stack(board_id, stack_id) - cards = stack.cards or [] - if not include_archived: - cards = [c for c in cards if not c.archived] - _truncate_card_descriptions(cards, description_max_length) + cards = _apply_card_filters( + stack.cards or [], + include_archived=include_archived, + description_max_length=description_max_length, + ) return ListCardsResponse(cards=cards, total=len(cards)) @mcp.tool( diff --git a/tests/unit/test_deck_server.py b/tests/unit/test_deck_server.py index 3f48fbdf..4306f00f 100644 --- a/tests/unit/test_deck_server.py +++ b/tests/unit/test_deck_server.py @@ -1,24 +1,94 @@ import pytest -from nextcloud_mcp_server.models.deck import DeckCard -from nextcloud_mcp_server.server.deck import _truncate_card_descriptions +from nextcloud_mcp_server.models.deck import ( + DeckACL, + DeckBoard, + DeckCard, + DeckLabel, + DeckPermissions, + DeckStack, + DeckUser, +) +from nextcloud_mcp_server.server.deck import ( + _apply_board_filters, + _apply_card_filters, + _apply_stack_filters, + _truncate_card_descriptions, +) pytestmark = pytest.mark.unit -def _make_card(card_id: int, description: str | None) -> DeckCard: +# Fixtures ------------------------------------------------------------------ + + +def _make_card( + card_id: int, + description: str | None = "desc", + archived: bool = False, +) -> DeckCard: return DeckCard( id=card_id, title=f"Card {card_id}", stackId=1, type="plain", order=card_id, - archived=False, + archived=archived, owner="testuser", description=description, ) +def _make_user(uid: str = "testuser") -> DeckUser: + return DeckUser(primaryKey=uid, uid=uid, displayname=uid) + + +def _make_board( + board_id: int = 1, + *, + labels: list[DeckLabel] | None = None, + acl: list[DeckACL] | None = None, + users: list[DeckUser] | None = None, +) -> DeckBoard: + return DeckBoard( + id=board_id, + title=f"Board {board_id}", + owner=_make_user(), + color="FF0000", + archived=False, + labels=labels + if labels is not None + else [DeckLabel(id=1, title="L1", color="00FF00")], + acl=acl if acl is not None else [], + permissions=DeckPermissions( + PERMISSION_READ=True, + PERMISSION_EDIT=True, + PERMISSION_MANAGE=True, + PERMISSION_SHARE=True, + ), + users=users if users is not None else [_make_user("alice"), _make_user("bob")], + deletedAt=0, + ) + + +def _make_stack( + stack_id: int = 1, + *, + cards: list[DeckCard] | None = None, +) -> DeckStack: + return DeckStack( + id=stack_id, + title=f"Stack {stack_id}", + boardId=1, + order=stack_id, + deletedAt=0, + cards=cards, + ) + + +# _truncate_card_descriptions ---------------------------------------------- + + def test_truncate_card_descriptions_no_op_when_limit_is_none(): """When description_max_length is None, descriptions are left untouched.""" cards = [_make_card(1, "x" * 5000)] @@ -49,3 +119,206 @@ def test_truncate_card_descriptions_at_exact_boundary(): cards = [_make_card(1, "x" * 100)] _truncate_card_descriptions(cards, 100) assert cards[0].description == "x" * 100 + + +def test_truncate_card_descriptions_shorter_than_limit_no_ellipsis(): + """A description shorter than the limit must not have an ellipsis appended.""" + cards = [_make_card(1, "hello")] + _truncate_card_descriptions(cards, 1000) + assert cards[0].description == "hello" + + +def test_truncate_card_descriptions_rejects_zero(): + """A zero limit is invalid (would wipe descriptions to a single ellipsis).""" + cards = [_make_card(1, "anything")] + with pytest.raises(ValueError, match="must be positive"): + _truncate_card_descriptions(cards, 0) + + +def test_truncate_card_descriptions_rejects_negative(): + """Negative limits are invalid.""" + cards = [_make_card(1, "anything")] + with pytest.raises(ValueError, match="must be positive"): + _truncate_card_descriptions(cards, -10) + + +# _apply_board_filters ------------------------------------------------------ + + +def test_apply_board_filters_defaults_preserve_fields(): + """With all include_* flags True, no fields are cleared.""" + board = _make_board() + result = _apply_board_filters( + board, include_acl=True, include_users=True, include_labels=True + ) + assert len(result.labels) == 1 + assert len(result.users) == 2 + + +def test_apply_board_filters_excludes_acl(): + """include_acl=False clears the acl list.""" + board = _make_board( + acl=[ + DeckACL( + id=1, + participant=_make_user("alice"), + type=0, + boardId=1, + permissionEdit=True, + permissionShare=True, + permissionManage=False, + owner=False, + ) + ] + ) + result = _apply_board_filters( + board, include_acl=False, include_users=True, include_labels=True + ) + assert result.acl == [] + + +def test_apply_board_filters_excludes_users(): + """include_users=False clears the users list.""" + board = _make_board() + result = _apply_board_filters( + board, include_acl=True, include_users=False, include_labels=True + ) + assert result.users == [] + + +def test_apply_board_filters_excludes_labels(): + """include_labels=False clears the labels list.""" + board = _make_board() + result = _apply_board_filters( + board, include_acl=True, include_users=True, include_labels=False + ) + assert result.labels == [] + + +def test_apply_board_filters_excludes_all(): + """All include_* flags False clears every filterable list.""" + board = _make_board() + result = _apply_board_filters( + board, include_acl=False, include_users=False, include_labels=False + ) + assert result.acl == [] + assert result.users == [] + assert result.labels == [] + + +# _apply_stack_filters ------------------------------------------------------ + + +def test_apply_stack_filters_include_cards_false_strips_cards(): + """include_cards=False sets cards to None regardless of other flags.""" + stack = _make_stack(cards=[_make_card(1), _make_card(2, archived=True)]) + result = _apply_stack_filters( + stack, + include_cards=False, + include_archived_cards=True, + description_max_length=None, + ) + assert result.cards is None + + +def test_apply_stack_filters_excludes_archived_by_default(): + """include_archived_cards=False filters out archived cards.""" + stack = _make_stack( + cards=[_make_card(1, archived=False), _make_card(2, archived=True)] + ) + result = _apply_stack_filters( + stack, + include_cards=True, + include_archived_cards=False, + description_max_length=None, + ) + assert result.cards is not None + assert [c.id for c in result.cards] == [1] + + +def test_apply_stack_filters_keeps_archived_when_requested(): + """include_archived_cards=True retains archived cards.""" + stack = _make_stack( + cards=[_make_card(1, archived=False), _make_card(2, archived=True)] + ) + result = _apply_stack_filters( + stack, + include_cards=True, + include_archived_cards=True, + description_max_length=None, + ) + assert result.cards is not None + assert [c.id for c in result.cards] == [1, 2] + + +def test_apply_stack_filters_truncates_descriptions_after_archive_filter(): + """Truncation runs on the post-archive-filter card set.""" + stack = _make_stack( + cards=[ + _make_card(1, description="x" * 50, archived=False), + _make_card(2, description="y" * 50, archived=True), + ] + ) + result = _apply_stack_filters( + stack, + include_cards=True, + include_archived_cards=False, + description_max_length=10, + ) + assert result.cards is not None + assert len(result.cards) == 1 + assert result.cards[0].description is not None + assert result.cards[0].description.endswith("…") + + +def test_apply_stack_filters_handles_none_cards(): + """A stack with no cards (cards=None) is left untouched.""" + stack = _make_stack(cards=None) + result = _apply_stack_filters( + stack, + include_cards=True, + include_archived_cards=False, + description_max_length=10, + ) + assert result.cards is None + + +# _apply_card_filters ------------------------------------------------------- + + +def test_apply_card_filters_excludes_archived_by_default(): + """include_archived=False filters archived cards out of the flat list.""" + cards = [ + _make_card(1, archived=False), + _make_card(2, archived=True), + _make_card(3, archived=False), + ] + result = _apply_card_filters( + cards, include_archived=False, description_max_length=None + ) + assert [c.id for c in result] == [1, 3] + + +def test_apply_card_filters_keeps_archived_when_requested(): + """include_archived=True retains archived cards.""" + cards = [_make_card(1, archived=False), _make_card(2, archived=True)] + result = _apply_card_filters( + cards, include_archived=True, description_max_length=None + ) + assert [c.id for c in result] == [1, 2] + + +def test_apply_card_filters_truncates_descriptions(): + """description_max_length is honored on the returned cards.""" + cards = [_make_card(1, description="x" * 50)] + result = _apply_card_filters( + cards, include_archived=True, description_max_length=10 + ) + assert result[0].description is not None + assert result[0].description.endswith("…") + + +def test_apply_card_filters_empty_list_is_noop(): + """An empty input returns an empty output.""" + result = _apply_card_filters([], include_archived=False, description_max_length=10) + assert result == []