Chris CoutinhoandClaude Opus 4.7
271904c227
PR #781 review round 1: - 🔴 Fix notesPath key: the Notes API returns the folder under camelCase ``notesPath`` (see models/notes.py:43), but `deck_attach_note` was looking up snake_case ``notes_path`` and silently falling back to ``"Notes"``. Users with a non-default notes folder would have produced shares pointing at non-existent files (404 on click in Deck UI). - 🔴 Add wire-through unit test that would have caught the above: extract `_resolve_note_attach_path(client, note_id)` as a testable helper that encapsulates the camelCase-key lookup. Three new tests: custom notesPath honored, missing key falls back to default, null category handled. - 🟡 Modernize new fields on `DeckAttachmentExtendedData` to PEP 604 (`X | None`) per CLAUDE.md. - 🟡 Drop unnecessary string forward reference on `ListAttachmentsResponse.results` — DeckAttachment is defined earlier in the same module. - 🟢 Move `pytestmark = pytest.mark.unit` to module level in test_sharing_client.py to match the convention in test_deck_server.py. Per user request: `deck_attach_file` is now scoped `deck.write` + ``files.read`` (was just `deck.write`) so the generic file-share permission story is consistent — only `deck_attach_note` keeps `notes.read` since it specifically reads from the Notes app. Docstring updated to emphasise the tool is generic over the user's Files (PDFs/images/etc., not just markdown). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
90 lines
3.0 KiB
Python
90 lines
3.0 KiB
Python
"""Unit tests for SharingClient — wire-format checks for the OCS Sharing API.
|
|
|
|
These verify the payload shape sent to Nextcloud, particularly for
|
|
``shareType=12`` (``IShare::TYPE_DECK``), which is what powers Deck card
|
|
file attachments. The Deck UI fires this exact request — see
|
|
``~/Software/deck/src/components/card/AttachmentList.vue:223-238``.
|
|
"""
|
|
|
|
import pytest
|
|
from httpx import AsyncClient
|
|
|
|
from nextcloud_mcp_server.client.sharing import SharingClient
|
|
|
|
pytestmark = pytest.mark.unit
|
|
|
|
|
|
@pytest.fixture
|
|
def sharing_client(mocker):
|
|
"""SharingClient with a mocked underlying httpx client."""
|
|
mock_http = mocker.AsyncMock(spec=AsyncClient)
|
|
return SharingClient(mock_http, "testuser")
|
|
|
|
|
|
def _ok_share_response(mocker, share_id: int = 4242, **extra):
|
|
"""Build a fake OCS create-share success response."""
|
|
response = mocker.Mock()
|
|
response.raise_for_status = mocker.Mock()
|
|
response.json.return_value = {
|
|
"ocs": {
|
|
"meta": {"statuscode": 200, "message": "OK"},
|
|
"data": {"id": share_id, **extra},
|
|
}
|
|
}
|
|
return response
|
|
|
|
|
|
async def test_create_share_deck_type_payload(sharing_client, mocker):
|
|
"""create_share(share_type=12) must POST exactly what the Deck UI does:
|
|
{path, shareType: 12, shareWith: "<cardId>"} to /ocs/v2.php/apps/files_sharing/api/v1/shares.
|
|
|
|
Drift here would silently break Deck attachments — Nextcloud's
|
|
ShareAPIController routes shareType=12 to DeckShareProvider, which
|
|
creates the deck-card share row binding the file to the card.
|
|
"""
|
|
sharing_client._client.post.return_value = _ok_share_response(mocker, share_id=99)
|
|
|
|
share = await sharing_client.create_share(
|
|
path="/Notes/My Note.md",
|
|
share_with="123",
|
|
share_type=12,
|
|
permissions=1,
|
|
)
|
|
|
|
assert share["id"] == 99
|
|
sharing_client._client.post.assert_called_once()
|
|
call = sharing_client._client.post.call_args
|
|
assert call.args[0] == "/ocs/v2.php/apps/files_sharing/api/v1/shares"
|
|
assert call.kwargs["data"] == {
|
|
"path": "/Notes/My Note.md",
|
|
"shareType": 12,
|
|
"shareWith": "123",
|
|
"permissions": 1,
|
|
}
|
|
# Nextcloud demands this header on OCS endpoints; without it the request
|
|
# is rejected as a CSRF risk.
|
|
assert call.kwargs["headers"]["OCS-APIRequest"] == "true"
|
|
|
|
|
|
async def test_create_share_raises_on_ocs_failure(sharing_client, mocker):
|
|
"""OCS error responses (statuscode != 100/200) raise RuntimeError."""
|
|
response = mocker.Mock()
|
|
response.raise_for_status = mocker.Mock()
|
|
response.json.return_value = {
|
|
"ocs": {
|
|
"meta": {
|
|
"statuscode": 404,
|
|
"message": "Wrong path, file/folder doesn't exist",
|
|
},
|
|
"data": [],
|
|
}
|
|
}
|
|
sharing_client._client.post.return_value = response
|
|
|
|
with pytest.raises(RuntimeError, match="Wrong path"):
|
|
await sharing_client.create_share(
|
|
path="/nope.md",
|
|
share_with="1",
|
|
share_type=12,
|
|
)
|