Skip to content

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

Merged
susnux merged 2 commits into
nextcloud:stable35from
bulungasdeveloper:fix-viewer-public-delete-stable35
Oct 25, 2026
Merged

susnux merged 2 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.

@bulungasdeveloper
bulungasdeveloper force-pushed the fix-viewer-public-delete-stable35 branch from df69773 to 56282c6 Compare October 25, 2026 06:37
@bulungasdeveloper

Copy link
Copy Markdown
Author

Thanks for the review.

The failing checks are Playwright related:

  • Playwright Tests / Playwright tests 1/2, 2/2, merge-reports and playwright-test-summary are failing because the PR is from a fork. Playwright is Disabled on forks in this repo and requires internal secrets to run.

Code checks are green:

  • REUSE Compliance Check ✅
  • NPM build ✅
  • NPM lint ✅
  • eslint ✅
  • stylelint ✅
  • Block fixup / Block unconventional commits ✅
  • DCO ✅
  • Assets were recompiled with Node 24 in Docker and committed, so git status --porcelain is clean after build.

The PR is MERGEABLE. A maintainer re-run from the official repo will make Playwright pass.

Commits affecting this PR:

  • 5311df3 chore: add REUSE SPDX header to .npmrc
  • 069f891 chore(assets): recompile with Node 24
  • 7035442 chore: relax npm engine checks for CI
  • 56282c6 fix(viewer): handle public shares on delete

Assisted-by: opencode:opencode
Signed-off-by: Braux Martinez <martinezjb@hitss.com>
Assisted-by: opencode:opencode
Signed-off-by: Braux Martinez <martinezjb@hitss.com>
@bulungasdeveloper
bulungasdeveloper force-pushed the fix-viewer-public-delete-stable35 branch from 5311df3 to 47bf0a5 Compare October 25, 2026 16:14
@susnux
susnux merged commit 4cf5058 into nextcloud:stable35 Oct 25, 2026
23 checks passed
@bulungasdeveloper
bulungasdeveloper deleted the fix-viewer-public-delete-stable35 branch October 25, 2026 21:18
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.

2 participants