Fix update_share: support clearing password, expiration, note, label, and toggling hide_download/public_upload

Author: bigcat88

PROGRESS.md

@@ -62,6 +62,6 @@
 | Errors | — | 10 |
 | Config | — | 12 |
 | State | — | 2 |
-| Shares | 5 | 33 |
+| Shares | 5 | 40 |
 | System Tags | 6 | 22 |
-| **Total** | **47** | **349** |
+| **Total** | **47** | **356** |

src/nextcloud_mcp/tools/shares.py

@@ -151,48 +151,47 @@ def _register_update_share(mcp: FastMCP) -> None:
     @require_permission(PermissionLevel.WRITE)
     async def update_share(
         share_id: int,
-        permissions: int = 0,
-        password: str = "",
-        expire_date: str = "",
-        note: str = "",
-        label: str = "",
-        public_upload: bool = False,
-        hide_download: bool = False,
+        permissions: int | None = None,
+        password: str | None = None,
+        expire_date: str | None = None,
+        note: str | None = None,
+        label: str | None = None,
+        public_upload: bool | None = None,
+        hide_download: bool | None = None,
     ) -> str:
         """Update properties of an existing share.
 
-        Only provided (non-default) parameters are changed. To remove a password or
-        expiration, use delete_share and create a new one.
+        Only provided parameters are changed. Omitted parameters keep their current value.
 
         Args:
             share_id: The numeric share ID to update.
             permissions: New permission flags (1=read, 2=update, 4=create, 8=delete, 16=share).
-            password: Set or change password (link/email shares only).
-            expire_date: Set expiration date in "YYYY-MM-DD" format.
-            note: Set or update the share note.
-            label: Set or update the share label.
-            public_upload: Enable/disable public upload on shared folders (link shares only).
-            hide_download: Hide the download button on public link shares.
+            password: Set or change password (link/email shares only). Pass "" to remove password.
+            expire_date: Set expiration in "YYYY-MM-DD" format. Pass "" to remove expiration.
+            note: Set or update the share note. Pass "" to clear.
+            label: Set or update the share label. Pass "" to clear.
+            public_upload: Enable (true) or disable (false) public upload on shared folders (link shares only).
+            hide_download: Show (false) or hide (true) the download button on public link shares.
 
         Returns:
             JSON object with the updated share details.
         """
         client = get_client()
         data: dict[str, str | int] = {}
-        if permissions > 0:
+        if permissions is not None:
             data["permissions"] = permissions
-        if password:
+        if password is not None:
             data["password"] = password
-        if expire_date:
+        if expire_date is not None:
             data["expireDate"] = expire_date
-        if note:
+        if note is not None:
             data["note"] = note
-        if label:
+        if label is not None:
             data["label"] = label
-        if public_upload:
-            data["publicUpload"] = "true"
-        if hide_download:
-            data["hideDownload"] = "true"
+        if public_upload is not None:
+            data["publicUpload"] = "true" if public_upload else "false"
+        if hide_download is not None:
+            data["hideDownload"] = "true" if hide_download else "false"
         result = await client.ocs_put(f"{SHARES_API}/{share_id}", data=data)
         return json.dumps(_format_share(result), indent=2, default=str)
 

tests/integration/test_shares.py

@@ -238,6 +238,83 @@ async def test_update_password(self, nc_mcp: McpTestHelper) -> None:
         updated = json.loads(result)
         assert updated.get("has_password") is True
 
+    @pytest.mark.asyncio
+    async def test_update_remove_password(self, nc_mcp: McpTestHelper) -> None:
+        await _setup_share_file(nc_mcp)
+        created = json.loads(
+            await nc_mcp.call("create_share", path=f"/{SHARE_FILE}", share_type=3, password="s3Cr3t!Pw9#xK")
+        )
+        share_id = int(created["id"])
+        assert created.get("has_password") is True
+        result = await nc_mcp.call("update_share", share_id=share_id, password="")
+        updated = json.loads(result)
+        assert updated.get("has_password") is not True
+
+    @pytest.mark.asyncio
+    async def test_update_remove_expiration(self, nc_mcp: McpTestHelper) -> None:
+        await _setup_share_file(nc_mcp)
+        created = json.loads(
+            await nc_mcp.call("create_share", path=f"/{SHARE_FILE}", share_type=3, expire_date="2099-12-31")
+        )
+        share_id = int(created["id"])
+        assert created["expiration"] is not None
+        result = await nc_mcp.call("update_share", share_id=share_id, expire_date="")
+        updated = json.loads(result)
+        assert updated["expiration"] is None
+
+    @pytest.mark.asyncio
+    async def test_update_clear_note(self, nc_mcp: McpTestHelper) -> None:
+        await _setup_share_file(nc_mcp)
+        created = json.loads(
+            await nc_mcp.call("create_share", path=f"/{SHARE_FILE}", share_type=3, note="initial note")
+        )
+        share_id = int(created["id"])
+        assert created["note"] == "initial note"
+        result = await nc_mcp.call("update_share", share_id=share_id, note="")
+        updated = json.loads(result)
+        assert updated["note"] == ""
+
+    @pytest.mark.asyncio
+    async def test_update_clear_label(self, nc_mcp: McpTestHelper) -> None:
+        await _setup_share_file(nc_mcp)
+        created = json.loads(
+            await nc_mcp.call("create_share", path=f"/{SHARE_FILE}", share_type=3, label="Initial Label")
+        )
+        share_id = int(created["id"])
+        assert created["label"] == "Initial Label"
+        result = await nc_mcp.call("update_share", share_id=share_id, label="")
+        updated = json.loads(result)
+        assert updated["label"] == ""
+
+    @pytest.mark.asyncio
+    async def test_update_enable_hide_download(self, nc_mcp: McpTestHelper) -> None:
+        await _setup_share_file(nc_mcp)
+        created = json.loads(await nc_mcp.call("create_share", path=f"/{SHARE_FILE}", share_type=3))
+        share_id = int(created["id"])
+        result = await nc_mcp.call("update_share", share_id=share_id, hide_download=True)
+        updated = json.loads(result)
+        assert updated.get("hide_download") == 1
+
+    @pytest.mark.asyncio
+    async def test_update_disable_hide_download(self, nc_mcp: McpTestHelper) -> None:
+        await _setup_share_file(nc_mcp)
+        created = json.loads(await nc_mcp.call("create_share", path=f"/{SHARE_FILE}", share_type=3))
+        share_id = int(created["id"])
+        await nc_mcp.call("update_share", share_id=share_id, hide_download=True)
+        result = await nc_mcp.call("update_share", share_id=share_id, hide_download=False)
+        updated = json.loads(result)
+        assert not updated.get("hide_download")
+
+    @pytest.mark.asyncio
+    async def test_update_disable_public_upload(self, nc_mcp: McpTestHelper) -> None:
+        await _setup_share_file(nc_mcp)
+        created = json.loads(await nc_mcp.call("create_share", path=f"/{SHARE_DIR}", share_type=3, public_upload=True))
+        share_id = int(created["id"])
+        assert created["permissions"] & 4, "Should have create permission"
+        result = await nc_mcp.call("update_share", share_id=share_id, public_upload=False)
+        updated = json.loads(result)
+        assert not (updated["permissions"] & 4), "Create permission should be removed"
+
     @pytest.mark.asyncio
     async def test_update_nonexistent_share_fails(self, nc_mcp: McpTestHelper) -> None:
         with pytest.raises(ToolError):