Skip to content

[stable35] fix(viewer): handle public shares on delete - #3460

Open
bulungasdeveloper wants to merge 4 commits into
nextcloud:stable35from
bulungasdeveloper:fix-viewer-public-delete-stable35
Open

bulungasdeveloper wants to merge 4 commits into
nextcloud:stable35from
bulungasdeveloper:fix-viewer-public-delete-stable35

Conversation

@bulungasdeveloper

Copy link
Copy Markdown

Summary

Deleting a file from the viewer in a public link share with delete permission
failed with Missing mandatory root in the browser console. The file was not
removed: the exception is thrown while building the temporary NcFile node,
before the DELETE request is sent.

onDelete only treated remote.php/dav URLs as DAV resources. Files opened
from a public share are served over public.php/dav (web UI) or
public.php/webdav (legacy clients), so the node was created with
root: undefined.

Recognise (remote|public).php/(web)?dav as DAV endpoints. Public shares now
get root = getRootPath() (/files/<token>) and the same decoded source the
store uses. Behaviour for authenticated users is unchanged.

Testing

  • New Playwright spec playwright/e2e/sharing/delete-share.spec.ts: public
    link share with delete permission → open image in viewer → Delete → viewer
    closes and the file disappears from the list.
    • 7/7 new spec.
    • 21/21 together with the existing delete and download-share specs.
  • npm ci && npm run build (Node 24, Linux): OK. The five generated assets are
    in a separate chore(assets) commit because the build workflow checks for a
    clean tree.
  • npm run stylelint: 0 errors (11 pre-existing warnings).
  • npx eslint src: 0 errors (24 pre-existing warnings).

No unit tests exist in this app; the Playwright suite is the relevant coverage.

Notes

  • The test creates a folder link share with
    READ | UPDATE | CREATE | DELETE permissions; the viewer only offers Delete
    when the file carries the D permission.
  • The source passed to the temporary node is intentionally decoded, matching the
    files store; the DELETE request still uses the encoded URL.
  • Targets stable35. main no longer ships the app.

🤖 AI (if applicable)

  • The content of this PR was partly or fully generated using AI: code and test drafted with opencode/DeepSeek V4.1 Flash Free, reviewed and tested manually.

@bulungasdeveloper

Copy link
Copy Markdown
Author

Playwright is intentionally disabled on fork PRs, so Playwright E2E tests and
playwright-summary will stay red. I ran the suite locally on Node 24: 21/21
(new public-share delete spec 7/7, plus the existing delete and download-share
specs). Could a maintainer run the workflow from an upstream branch or merge as
in #3457? All other checks are green.

Assisted-by: opencode:opencode
Signed-off-by: Braux Martinez <martinezjb@hitss.com>
@bulungasdeveloper
bulungasdeveloper force-pushed the fix-viewer-public-delete-stable35 branch from df69773 to 56282c6 Compare October 25, 2026 06:37
Add engine-strict=false to allow npm to run on newer Node versions
used by GitHub Actions runners. This does not change code.

Assisted-by: opencode:opencode
Signed-off-by: Braux Martinez <martinezjb@hitss.com>
Build assets in Docker node:24 so CI git status check passes.

Assisted-by: opencode:opencode
Signed-off-by: Braux Martinez <martinezjb@hitss.com>
Assisted-by: opencode:opencode
Signed-off-by: Braux Martinez <martinezjb@hitss.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant