From 30d3d9f0cff94667d90c4ed2c8097c0c5985450b Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Tue, 30 Dec 2025 11:52:57 -0600 Subject: [PATCH 1/3] test: Add integration tests documenting DeckClient.update_card bugs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tests document current behavior of update_card method: - Updating without title fails (400) - title required but conditionally sent - Updating with title clears description - PUT is full replacement Related: #452 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- .../client/deck/test_deck_update_card_api.py | 200 ++++++++++++++++++ 1 file changed, 200 insertions(+) create mode 100644 tests/client/deck/test_deck_update_card_api.py diff --git a/tests/client/deck/test_deck_update_card_api.py b/tests/client/deck/test_deck_update_card_api.py new file mode 100644 index 0000000..cf772e5 --- /dev/null +++ b/tests/client/deck/test_deck_update_card_api.py @@ -0,0 +1,200 @@ +""" +Integration tests for DeckClient.update_card API behavior. + +This test suite documents the behavior of our DeckClient.update_card method +and identifies bugs in how it handles partial updates. + +FINDINGS: +The Deck API PUT endpoint is a FULL REPLACEMENT, not a partial update. +Fields not included in the request body are either: +- Required and cause 400 error (title, type, owner) +- Optional but get CLEARED if not sent (description) + +Related issues: +- nextcloud-mcp-server #452: DeckClient.update_card always sets owner/type +- deck #3127: REST API Docs: missing parameter in "update cards" +- deck #4106: Provide a working example of API usage to update a cards details +""" + +import httpx +import pytest + +pytestmark = [pytest.mark.integration] + + +@pytest.fixture +async def deck_test_card(nc_client): + """Create a board, stack, and card for testing, cleanup after.""" + board = await nc_client.deck.create_board("Test Update Card API", "FF0000") + stack = await nc_client.deck.create_stack(board.id, "Test Stack", 1) + card = await nc_client.deck.create_card( + board.id, + stack.id, + "Original Title", + type="plain", + description="Original description", + ) + + yield { + "board_id": board.id, + "stack_id": stack.id, + "card_id": card.id, + "card": card, + } + + # Cleanup + await nc_client.deck.delete_board(board.id) + + +class TestDeckClientUpdateCard: + """ + Test DeckClient.update_card() method behavior with various parameter combinations. + + These tests document the current buggy behavior where: + 1. Updating without title fails (400) - title is required but conditionally sent + 2. Updating with title clears description - description should be preserved + """ + + async def test_update_title_only_clears_description( + self, nc_client, deck_test_card + ): + """ + BUG: Updating only the title clears the description. + + The Deck PUT API is a full replacement. Our client doesn't send + description when not explicitly provided, so it gets cleared. + """ + await nc_client.deck.update_card( + board_id=deck_test_card["board_id"], + stack_id=deck_test_card["stack_id"], + card_id=deck_test_card["card_id"], + title="New Title", + ) + + updated = await nc_client.deck.get_card( + deck_test_card["board_id"], + deck_test_card["stack_id"], + deck_test_card["card_id"], + ) + assert updated.title == "New Title" + # BUG: Description was cleared instead of preserved + assert updated.description == "" # Should be "Original description" + + async def test_update_description_only_fails(self, nc_client, deck_test_card): + """ + BUG: Updating only the description fails with 400. + + The Deck PUT API requires title, type, and owner. + Our client doesn't send title when not explicitly provided. + """ + with pytest.raises(httpx.HTTPStatusError) as exc_info: + await nc_client.deck.update_card( + board_id=deck_test_card["board_id"], + stack_id=deck_test_card["stack_id"], + card_id=deck_test_card["card_id"], + description="New description only", + ) + + assert exc_info.value.response.status_code == 400 + + async def test_update_title_and_description(self, nc_client, deck_test_card): + """Updating title and description together works correctly.""" + await nc_client.deck.update_card( + board_id=deck_test_card["board_id"], + stack_id=deck_test_card["stack_id"], + card_id=deck_test_card["card_id"], + title="New Title", + description="New description", + ) + + updated = await nc_client.deck.get_card( + deck_test_card["board_id"], + deck_test_card["stack_id"], + deck_test_card["card_id"], + ) + assert updated.title == "New Title" + assert updated.description == "New description" + + async def test_update_duedate_only_fails(self, nc_client, deck_test_card): + """ + BUG: Updating only the duedate fails with 400. + + title is required but not sent when not explicitly provided. + """ + with pytest.raises(httpx.HTTPStatusError) as exc_info: + await nc_client.deck.update_card( + board_id=deck_test_card["board_id"], + stack_id=deck_test_card["stack_id"], + card_id=deck_test_card["card_id"], + duedate="2025-12-31T23:59:59+00:00", + ) + + assert exc_info.value.response.status_code == 400 + + async def test_update_archived_only_fails(self, nc_client, deck_test_card): + """ + BUG: Updating only the archived status fails with 400. + + title is required but not sent when not explicitly provided. + """ + with pytest.raises(httpx.HTTPStatusError) as exc_info: + await nc_client.deck.update_card( + board_id=deck_test_card["board_id"], + stack_id=deck_test_card["stack_id"], + card_id=deck_test_card["card_id"], + archived=True, + ) + + assert exc_info.value.response.status_code == 400 + + async def test_update_preserves_type(self, nc_client, deck_test_card): + """Type is correctly preserved (already always sent in current implementation).""" + original = deck_test_card["card"] + + await nc_client.deck.update_card( + board_id=deck_test_card["board_id"], + stack_id=deck_test_card["stack_id"], + card_id=deck_test_card["card_id"], + title="Changed Title", + ) + + updated = await nc_client.deck.get_card( + deck_test_card["board_id"], + deck_test_card["stack_id"], + deck_test_card["card_id"], + ) + assert updated.type == original.type + + async def test_update_preserves_owner(self, nc_client, deck_test_card): + """Owner is correctly preserved (already always sent in current implementation).""" + original = deck_test_card["card"] + + await nc_client.deck.update_card( + board_id=deck_test_card["board_id"], + stack_id=deck_test_card["stack_id"], + card_id=deck_test_card["card_id"], + title="Changed Title", + ) + + updated = await nc_client.deck.get_card( + deck_test_card["board_id"], + deck_test_card["stack_id"], + deck_test_card["card_id"], + ) + assert updated.owner == original.owner + + async def test_update_order_only_fails(self, nc_client, deck_test_card): + """ + BUG: Updating only the order fails with 400. + + title is required but not sent when not explicitly provided. + """ + with pytest.raises(httpx.HTTPStatusError) as exc_info: + await nc_client.deck.update_card( + board_id=deck_test_card["board_id"], + stack_id=deck_test_card["stack_id"], + card_id=deck_test_card["card_id"], + order=1, + ) + + assert exc_info.value.response.status_code == 400 From 71ace4719740ec975e041cc26e544133c11817bd Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Tue, 30 Dec 2025 22:28:23 -0600 Subject: [PATCH 2/3] test: Define expected partial update behavior for DeckClient.update_card MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactor tests to assert what SHOULD happen (partial updates preserve unchanged fields) rather than documenting current buggy behavior. Tests will fail until fix is implemented in client or upstream. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- .../client/deck/test_deck_update_card_api.py | 160 +++++++++--------- 1 file changed, 77 insertions(+), 83 deletions(-) diff --git a/tests/client/deck/test_deck_update_card_api.py b/tests/client/deck/test_deck_update_card_api.py index cf772e5..d110d9f 100644 --- a/tests/client/deck/test_deck_update_card_api.py +++ b/tests/client/deck/test_deck_update_card_api.py @@ -1,22 +1,16 @@ """ Integration tests for DeckClient.update_card API behavior. -This test suite documents the behavior of our DeckClient.update_card method -and identifies bugs in how it handles partial updates. - -FINDINGS: -The Deck API PUT endpoint is a FULL REPLACEMENT, not a partial update. -Fields not included in the request body are either: -- Required and cause 400 error (title, type, owner) -- Optional but get CLEARED if not sent (description) +These tests define the EXPECTED behavior for partial card updates: +- Only fields explicitly passed should be modified +- All other fields should be preserved unchanged Related issues: -- nextcloud-mcp-server #452: DeckClient.update_card always sets owner/type +- nextcloud-mcp-server #452: DeckClient.update_card partial update bugs - deck #3127: REST API Docs: missing parameter in "update cards" - deck #4106: Provide a working example of API usage to update a cards details """ -import httpx import pytest pytestmark = [pytest.mark.integration] @@ -48,22 +42,15 @@ async def deck_test_card(nc_client): class TestDeckClientUpdateCard: """ - Test DeckClient.update_card() method behavior with various parameter combinations. + Test DeckClient.update_card() partial update behavior. - These tests document the current buggy behavior where: - 1. Updating without title fails (400) - title is required but conditionally sent - 2. Updating with title clears description - description should be preserved + Expected: Only explicitly provided fields are updated, all others preserved. """ - async def test_update_title_only_clears_description( + async def test_update_title_only_preserves_description( self, nc_client, deck_test_card ): - """ - BUG: Updating only the title clears the description. - - The Deck PUT API is a full replacement. Our client doesn't send - description when not explicitly provided, so it gets cleared. - """ + """Updating only the title should preserve the description.""" await nc_client.deck.update_card( board_id=deck_test_card["board_id"], stack_id=deck_test_card["stack_id"], @@ -77,28 +64,27 @@ class TestDeckClientUpdateCard: deck_test_card["card_id"], ) assert updated.title == "New Title" - # BUG: Description was cleared instead of preserved - assert updated.description == "" # Should be "Original description" + assert updated.description == "Original description" - async def test_update_description_only_fails(self, nc_client, deck_test_card): - """ - BUG: Updating only the description fails with 400. + async def test_update_description_only(self, nc_client, deck_test_card): + """Updating only the description should work and preserve other fields.""" + await nc_client.deck.update_card( + board_id=deck_test_card["board_id"], + stack_id=deck_test_card["stack_id"], + card_id=deck_test_card["card_id"], + description="New description only", + ) - The Deck PUT API requires title, type, and owner. - Our client doesn't send title when not explicitly provided. - """ - with pytest.raises(httpx.HTTPStatusError) as exc_info: - await nc_client.deck.update_card( - board_id=deck_test_card["board_id"], - stack_id=deck_test_card["stack_id"], - card_id=deck_test_card["card_id"], - description="New description only", - ) - - assert exc_info.value.response.status_code == 400 + updated = await nc_client.deck.get_card( + deck_test_card["board_id"], + deck_test_card["stack_id"], + deck_test_card["card_id"], + ) + assert updated.title == "Original Title" + assert updated.description == "New description only" async def test_update_title_and_description(self, nc_client, deck_test_card): - """Updating title and description together works correctly.""" + """Updating title and description together should work.""" await nc_client.deck.update_card( board_id=deck_test_card["board_id"], stack_id=deck_test_card["stack_id"], @@ -115,40 +101,62 @@ class TestDeckClientUpdateCard: assert updated.title == "New Title" assert updated.description == "New description" - async def test_update_duedate_only_fails(self, nc_client, deck_test_card): - """ - BUG: Updating only the duedate fails with 400. + async def test_update_duedate_only(self, nc_client, deck_test_card): + """Updating only the duedate should work and preserve other fields.""" + await nc_client.deck.update_card( + board_id=deck_test_card["board_id"], + stack_id=deck_test_card["stack_id"], + card_id=deck_test_card["card_id"], + duedate="2025-12-31T23:59:59+00:00", + ) - title is required but not sent when not explicitly provided. - """ - with pytest.raises(httpx.HTTPStatusError) as exc_info: - await nc_client.deck.update_card( - board_id=deck_test_card["board_id"], - stack_id=deck_test_card["stack_id"], - card_id=deck_test_card["card_id"], - duedate="2025-12-31T23:59:59+00:00", - ) + updated = await nc_client.deck.get_card( + deck_test_card["board_id"], + deck_test_card["stack_id"], + deck_test_card["card_id"], + ) + assert updated.title == "Original Title" + assert updated.description == "Original description" + assert updated.duedate is not None - assert exc_info.value.response.status_code == 400 + async def test_update_archived_only(self, nc_client, deck_test_card): + """Updating only the archived status should work and preserve other fields.""" + await nc_client.deck.update_card( + board_id=deck_test_card["board_id"], + stack_id=deck_test_card["stack_id"], + card_id=deck_test_card["card_id"], + archived=True, + ) - async def test_update_archived_only_fails(self, nc_client, deck_test_card): - """ - BUG: Updating only the archived status fails with 400. + updated = await nc_client.deck.get_card( + deck_test_card["board_id"], + deck_test_card["stack_id"], + deck_test_card["card_id"], + ) + assert updated.title == "Original Title" + assert updated.description == "Original description" + assert updated.archived is True - title is required but not sent when not explicitly provided. - """ - with pytest.raises(httpx.HTTPStatusError) as exc_info: - await nc_client.deck.update_card( - board_id=deck_test_card["board_id"], - stack_id=deck_test_card["stack_id"], - card_id=deck_test_card["card_id"], - archived=True, - ) + async def test_update_order_only(self, nc_client, deck_test_card): + """Updating only the order should work and preserve other fields.""" + await nc_client.deck.update_card( + board_id=deck_test_card["board_id"], + stack_id=deck_test_card["stack_id"], + card_id=deck_test_card["card_id"], + order=99, + ) - assert exc_info.value.response.status_code == 400 + updated = await nc_client.deck.get_card( + deck_test_card["board_id"], + deck_test_card["stack_id"], + deck_test_card["card_id"], + ) + assert updated.title == "Original Title" + assert updated.description == "Original description" + assert updated.order == 99 async def test_update_preserves_type(self, nc_client, deck_test_card): - """Type is correctly preserved (already always sent in current implementation).""" + """Type should be preserved when not explicitly changed.""" original = deck_test_card["card"] await nc_client.deck.update_card( @@ -164,9 +172,10 @@ class TestDeckClientUpdateCard: deck_test_card["card_id"], ) assert updated.type == original.type + assert updated.description == "Original description" async def test_update_preserves_owner(self, nc_client, deck_test_card): - """Owner is correctly preserved (already always sent in current implementation).""" + """Owner should be preserved when not explicitly changed.""" original = deck_test_card["card"] await nc_client.deck.update_card( @@ -182,19 +191,4 @@ class TestDeckClientUpdateCard: deck_test_card["card_id"], ) assert updated.owner == original.owner - - async def test_update_order_only_fails(self, nc_client, deck_test_card): - """ - BUG: Updating only the order fails with 400. - - title is required but not sent when not explicitly provided. - """ - with pytest.raises(httpx.HTTPStatusError) as exc_info: - await nc_client.deck.update_card( - board_id=deck_test_card["board_id"], - stack_id=deck_test_card["stack_id"], - card_id=deck_test_card["card_id"], - order=1, - ) - - assert exc_info.value.response.status_code == 400 + assert updated.description == "Original description" From a26a470af61bb52330bb9eed4299c4a513d4194e Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Tue, 30 Dec 2025 23:30:01 -0600 Subject: [PATCH 3/3] fix(deck): Always preserve fields in update_card for partial updates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Deck PUT API is a full replacement, not a partial update. Previously, title and description were conditionally sent, causing: - 400 errors when title not provided (it's required) - Description being cleared when not explicitly set Now all required fields (title, type, owner) and description are always included in the payload using current card values when not explicitly provided. This matches the existing pattern for type/owner. Also simplified owner extraction since DeckCard.validate_owner already ensures it's always a string. Fixes #452 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- nextcloud_mcp_server/client/deck.py | 35 +++++++++++++---------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/nextcloud_mcp_server/client/deck.py b/nextcloud_mcp_server/client/deck.py index c8c3cc2..62b6f84 100644 --- a/nextcloud_mcp_server/client/deck.py +++ b/nextcloud_mcp_server/client/deck.py @@ -285,28 +285,23 @@ class DeckClient(BaseNextcloudClient): archived: Optional[bool] = None, done: Optional[str] = None, ) -> None: - # First, get the current card to use existing values for required fields + # Deck PUT API is a full replacement - all required fields must be sent. + # Fetch current card to preserve values for fields not being updated. current_card = await self.get_card(board_id, stack_id, card_id) - json_data = {} - if title is not None: - json_data["title"] = title - if description is not None: - json_data["description"] = description - # Type is required by the API, use provided or keep current - json_data["type"] = type if type is not None else current_card.type - # Owner is required by the API, use provided or keep current - json_data["owner"] = ( - owner - if owner is not None - else ( - current_card.owner - if isinstance(current_card.owner, str) - else current_card.owner.uid - if hasattr(current_card.owner, "uid") - else current_card.owner.primaryKey - ) - ) + # Build payload with required fields always included + json_data = { + # Title is required by the API + "title": title if title is not None else current_card.title, + # Type is required by the API + "type": type if type is not None else current_card.type, + # Owner is required by the API (model validator ensures it's a string) + "owner": owner if owner is not None else current_card.owner, + # Description must be sent to preserve it (PUT clears omitted fields) + "description": description + if description is not None + else (current_card.description or ""), + } if order is not None: json_data["order"] = order if duedate is not None: