feat(files-overlay): recursive directory delete + fix nested-file save misroute
Two related fixes to the overlay file manager, found in the same session. 1. Nested-file save was silently moving the file (fix). The Filename input is pre-filled with the full relative path (intentional: phone-friendly move-via-edit when drag-and-drop isn't an option). The save handler compared it against the basename only, so every save of a file in a subfolder built newPath = parent + "/" + editedFilename and re-routed the file into a doubly-nested path (e.g. cfg/tick60.cfg -> cfg/cfg/tick60.cfg). All three sites in editor.js now compare against relPath. Two e2e tests pin both directions: save-without- edit leaves the file untouched, edit-the-path performs the intended move. 2. Recursive directory delete + visible 409 errors (feature). GET /files/delete_preview enumerates what a recursive delete would remove (files + dirs + symlinks, capped at 500 entries, followlinks=False). POST /files/delete accepts an optional recursive=1 form param that uses shutil.rmtree (default still refuses non-empty dirs, preserving the historical safety guard). The delete confirm modal now opens an inline preview for non-empty folders, with a scrollable list and a count summary. The error handler falls back to r.rawText so the server's text bodies (like "directory is not empty") finally surface to the user instead of "HTTP 409". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
mwiegand
parent
122e0abddd
commit
058acb9c5c
7 changed files with 640 additions and 31 deletions
|
|
@ -21,8 +21,13 @@ Mutating endpoints (only `overlay.type == 'files'`, owner or admin):
|
|||
rename or move; refuses cycles via `safe_resolve_for_move`.
|
||||
- `POST /overlays/<id>/files/mkdir` — JSON `{path}`. Slashes allowed in
|
||||
`path`; intermediate directories are created.
|
||||
- `POST /overlays/<id>/files/delete` — form `path`. Refuses non-empty
|
||||
directories.
|
||||
- `POST /overlays/<id>/files/delete` — form `path` plus optional
|
||||
`recursive=1`. Without the flag, refuses non-empty directories
|
||||
(HTTP 409). With it, `shutil.rmtree`s the target without following
|
||||
symlinks.
|
||||
- `GET /overlays/<id>/files/delete_preview?path=<rel>` — JSON listing
|
||||
of what a recursive delete would remove. Powers the GUI confirm
|
||||
modal so the user sees the blast radius before clicking Delete.
|
||||
- `GET /overlays/<id>/files/download_zip?path=` — streams a zip of the
|
||||
folder's contents.
|
||||
"""
|
||||
|
|
@ -615,14 +620,100 @@ def overlay_file_mkdir(overlay_id: int):
|
|||
return jsonify({"path": path})
|
||||
|
||||
|
||||
@bp.post("/overlays/<int:overlay_id>/files/delete")
|
||||
# Cap on preview-listing entries so a directory with millions of files
|
||||
# doesn't bloat the response or the modal. Past this point the response
|
||||
# stops appending to `entries` but keeps counting so the totals stay
|
||||
# accurate — the JS surfaces this as "… and N more entries will also
|
||||
# be deleted."
|
||||
_DELETE_PREVIEW_MAX_ENTRIES = 500
|
||||
|
||||
|
||||
def _walk_delete_preview(target):
|
||||
"""Walk `target` once with followlinks=False, returning the
|
||||
payload for /files/delete_preview.
|
||||
|
||||
Symlinks are recorded with kind="symlink" but their targets are
|
||||
not descended into — matches what `shutil.rmtree` will actually
|
||||
remove. Sizes for symlinks are zero (we unlink the link, not the
|
||||
file it points to)."""
|
||||
entries: list[dict] = []
|
||||
file_count = 0
|
||||
dir_count = 0
|
||||
total_bytes = 0
|
||||
truncated = False
|
||||
|
||||
for dirpath, dirnames, filenames in os.walk(target, followlinks=False):
|
||||
# Make output deterministic — also gives the test suite a
|
||||
# stable order to assert on.
|
||||
dirnames.sort()
|
||||
filenames.sort()
|
||||
rel_base = os.path.relpath(dirpath, target)
|
||||
|
||||
# Symlinks-to-directories show up in `dirnames` for os.walk,
|
||||
# so we have to inspect each before descending. The followlinks=False
|
||||
# flag prevents the descent but doesn't filter the names list.
|
||||
for name in list(dirnames):
|
||||
child = os.path.join(dirpath, name)
|
||||
rel = name if rel_base == "." else os.path.join(rel_base, name)
|
||||
if os.path.islink(child):
|
||||
# Treat symlinked dirs like symlinked files: enumerated
|
||||
# as a leaf, not descended.
|
||||
dirnames.remove(name)
|
||||
if len(entries) < _DELETE_PREVIEW_MAX_ENTRIES:
|
||||
entries.append({"path": rel, "kind": "symlink"})
|
||||
else:
|
||||
truncated = True
|
||||
# symlinks count toward neither file_count nor dir_count;
|
||||
# they're a third category visible only in `entries`.
|
||||
continue
|
||||
dir_count += 1
|
||||
if len(entries) < _DELETE_PREVIEW_MAX_ENTRIES:
|
||||
entries.append({"path": rel + "/", "kind": "dir"})
|
||||
else:
|
||||
truncated = True
|
||||
|
||||
for name in filenames:
|
||||
child = os.path.join(dirpath, name)
|
||||
rel = name if rel_base == "." else os.path.join(rel_base, name)
|
||||
if os.path.islink(child):
|
||||
if len(entries) < _DELETE_PREVIEW_MAX_ENTRIES:
|
||||
entries.append({"path": rel, "kind": "symlink"})
|
||||
else:
|
||||
truncated = True
|
||||
continue
|
||||
file_count += 1
|
||||
try:
|
||||
total_bytes += os.stat(child, follow_symlinks=False).st_size
|
||||
except OSError:
|
||||
pass
|
||||
if len(entries) < _DELETE_PREVIEW_MAX_ENTRIES:
|
||||
entries.append({"path": rel, "kind": "file"})
|
||||
else:
|
||||
truncated = True
|
||||
|
||||
return {
|
||||
"entries": entries,
|
||||
"file_count": file_count,
|
||||
"dir_count": dir_count,
|
||||
"total_bytes": total_bytes,
|
||||
"truncated": truncated,
|
||||
}
|
||||
|
||||
|
||||
@bp.get("/overlays/<int:overlay_id>/files/delete_preview")
|
||||
@require_login
|
||||
def overlay_file_delete(overlay_id: int):
|
||||
"""Delete a file or empty folder. Refuses recursive directory removal."""
|
||||
def overlay_file_delete_preview(overlay_id: int):
|
||||
"""Enumerate what a recursive delete of `path` would remove.
|
||||
|
||||
Drives the confirm modal in dialogs.js — the user sees the full
|
||||
list of files and subdirectories before they click Delete.
|
||||
|
||||
Reuses safe_resolve_for_delete so the same guards (`..`, overlay
|
||||
root, symlink escapes) apply here as for the actual delete."""
|
||||
user = current_user()
|
||||
assert user is not None
|
||||
|
||||
path = (request.form.get("path") or "").strip()
|
||||
path = (request.args.get("path") or "").strip()
|
||||
if not path:
|
||||
return Response("missing path", status=400)
|
||||
|
||||
|
|
@ -631,6 +722,49 @@ def overlay_file_delete(overlay_id: int):
|
|||
return result
|
||||
overlay = result
|
||||
|
||||
try:
|
||||
target = safe_resolve_for_delete(overlay.path, path)
|
||||
except ValueError as exc:
|
||||
return Response(str(exc), status=422)
|
||||
|
||||
if not target.exists() and not target.is_symlink():
|
||||
return Response(status=404)
|
||||
|
||||
# Files and symlinks-to-anything: empty payload. The modal renders
|
||||
# the existing compact "Delete <name>?" view; the JS only opens
|
||||
# the preview details when entries is non-empty.
|
||||
if not target.is_dir() or target.is_symlink():
|
||||
return jsonify(
|
||||
entries=[], file_count=0, dir_count=0, total_bytes=0, truncated=False
|
||||
)
|
||||
|
||||
return jsonify(_walk_delete_preview(target))
|
||||
|
||||
|
||||
@bp.post("/overlays/<int:overlay_id>/files/delete")
|
||||
@require_login
|
||||
def overlay_file_delete(overlay_id: int):
|
||||
"""Delete a file, an empty folder, or — with `recursive=1` — an
|
||||
entire directory tree.
|
||||
|
||||
Without `recursive`, non-empty directories return HTTP 409 (the
|
||||
historical safety guard). The recursive branch uses
|
||||
`shutil.rmtree`, which does not follow symlinks by default, so
|
||||
a symlink inside the tree is unlinked rather than its target's
|
||||
contents being deleted."""
|
||||
user = current_user()
|
||||
assert user is not None
|
||||
|
||||
path = (request.form.get("path") or "").strip()
|
||||
if not path:
|
||||
return Response("missing path", status=400)
|
||||
recursive = request.form.get("recursive") == "1"
|
||||
|
||||
result = _load_files_overlay(overlay_id, user)
|
||||
if isinstance(result, Response):
|
||||
return result
|
||||
overlay = result
|
||||
|
||||
try:
|
||||
target = safe_resolve_for_delete(overlay.path, path)
|
||||
except ValueError as exc:
|
||||
|
|
@ -641,10 +775,15 @@ def overlay_file_delete(overlay_id: int):
|
|||
|
||||
try:
|
||||
if target.is_dir() and not target.is_symlink():
|
||||
try:
|
||||
target.rmdir()
|
||||
except OSError:
|
||||
return Response("directory is not empty", status=409)
|
||||
if recursive:
|
||||
# rmtree refuses to follow symlinks during the walk;
|
||||
# symlinks inside the tree get unlinked, not chased.
|
||||
shutil.rmtree(target)
|
||||
else:
|
||||
try:
|
||||
target.rmdir()
|
||||
except OSError:
|
||||
return Response("directory is not empty", status=409)
|
||||
else:
|
||||
os.unlink(target)
|
||||
except OSError as exc:
|
||||
|
|
|
|||
|
|
@ -665,6 +665,26 @@ button.danger-outline:hover {
|
|||
background: color-mix(in srgb, var(--color-success) 10%, transparent);
|
||||
}
|
||||
|
||||
/* Delete-confirm modal: scrollable preview list. The <details> wrapper
|
||||
collapses by default; when expanded, very large folders shouldn't
|
||||
push the modal off-screen. */
|
||||
.files-delete-preview-list {
|
||||
max-height: 40vh;
|
||||
overflow-y: auto;
|
||||
margin: 0.5rem 0 0;
|
||||
padding-left: 1.25rem;
|
||||
font-family: var(--font-mono, monospace);
|
||||
font-size: 0.875rem;
|
||||
}
|
||||
|
||||
.files-delete-preview-list li {
|
||||
list-style: square;
|
||||
}
|
||||
|
||||
.files-delete-preview-summary {
|
||||
cursor: pointer;
|
||||
}
|
||||
|
||||
/* Wider modal for the editor (textarea needs the breathing room). */
|
||||
dialog.modal.modal-wide,
|
||||
div.modal.modal-wide {
|
||||
|
|
|
|||
|
|
@ -37,7 +37,7 @@
|
|||
if (!fo) return;
|
||||
|
||||
const { baseUrl, csrfToken } = fo;
|
||||
const { joinPath, parentOf, basename, postJson, postForm, scheduleRefresh } = fo.helpers;
|
||||
const { joinPath, parentOf, basename, postJson, postForm, fetchJson, scheduleRefresh } = fo.helpers;
|
||||
|
||||
const newFolderDialog = document.getElementById("files-new-folder-modal");
|
||||
const deleteDialog = document.getElementById("files-delete-modal");
|
||||
|
|
@ -102,15 +102,101 @@
|
|||
|
||||
// ---------- delete-confirm modal ----------------------------------------
|
||||
|
||||
// For non-empty directories the modal shows a <details> preview of
|
||||
// every path that recursive delete would remove. The fetch is fired
|
||||
// before openModal() so the modal opens already populated — no
|
||||
// first-paint "Contents…" placeholder with empty content. Failure of
|
||||
// the preview fetch is non-fatal: we just open the modal without a
|
||||
// preview and the user can still confirm; the delete itself still
|
||||
// works.
|
||||
|
||||
let deleteState = null;
|
||||
|
||||
function openDelete(targetPath, kind, name) {
|
||||
function resetPreview() {
|
||||
if (!deleteDialog) return;
|
||||
const preview = deleteDialog.querySelector(".files-delete-preview");
|
||||
if (!preview) return;
|
||||
preview.hidden = true;
|
||||
preview.open = false;
|
||||
deleteDialog.querySelector(".files-delete-preview-list").innerHTML = "";
|
||||
const truncated = deleteDialog.querySelector(".files-delete-preview-truncated");
|
||||
truncated.hidden = true;
|
||||
deleteDialog.querySelector(".files-delete-preview-extra").textContent = "";
|
||||
const summary = deleteDialog.querySelector(".files-delete-preview-summary");
|
||||
if (summary) summary.textContent = "Contents to be deleted";
|
||||
}
|
||||
|
||||
function renderPreview(preview) {
|
||||
if (!deleteDialog) return;
|
||||
if (!preview || !Array.isArray(preview.entries) || preview.entries.length === 0) {
|
||||
return;
|
||||
}
|
||||
const container = deleteDialog.querySelector(".files-delete-preview");
|
||||
const list = deleteDialog.querySelector(".files-delete-preview-list");
|
||||
const summary = deleteDialog.querySelector(".files-delete-preview-summary");
|
||||
const truncated = deleteDialog.querySelector(".files-delete-preview-truncated");
|
||||
const extraEl = deleteDialog.querySelector(".files-delete-preview-extra");
|
||||
|
||||
const frag = document.createDocumentFragment();
|
||||
for (const entry of preview.entries) {
|
||||
const li = document.createElement("li");
|
||||
li.textContent = entry.path;
|
||||
if (entry.kind === "symlink") {
|
||||
const tag = document.createElement("span");
|
||||
tag.className = "muted";
|
||||
tag.textContent = " (link)";
|
||||
li.appendChild(tag);
|
||||
}
|
||||
frag.appendChild(li);
|
||||
}
|
||||
list.replaceChildren(frag);
|
||||
|
||||
const parts = [];
|
||||
if (preview.file_count > 0) {
|
||||
parts.push(`${preview.file_count} file${preview.file_count === 1 ? "" : "s"}`);
|
||||
}
|
||||
if (preview.dir_count > 0) {
|
||||
parts.push(`${preview.dir_count} folder${preview.dir_count === 1 ? "" : "s"}`);
|
||||
}
|
||||
if (summary) {
|
||||
summary.textContent = parts.length
|
||||
? `Contents to be deleted (${parts.join(" + ")})`
|
||||
: "Contents to be deleted";
|
||||
}
|
||||
|
||||
if (preview.truncated) {
|
||||
const shown = preview.entries.length;
|
||||
const total = (preview.file_count || 0) + (preview.dir_count || 0);
|
||||
const extra = Math.max(0, total - shown);
|
||||
extraEl.textContent = String(extra);
|
||||
truncated.hidden = false;
|
||||
}
|
||||
container.hidden = false;
|
||||
}
|
||||
|
||||
async function openDelete(targetPath, kind, name) {
|
||||
if (!deleteDialog) return;
|
||||
deleteDialog.querySelector(".files-delete-name").textContent = name;
|
||||
const errEl = deleteDialog.querySelector(".files-delete-error");
|
||||
errEl.hidden = true;
|
||||
errEl.textContent = "";
|
||||
deleteState = { path: targetPath, kind, errEl };
|
||||
resetPreview();
|
||||
// Default to non-recursive; flipped to true only when the preview
|
||||
// confirms there are entries inside.
|
||||
deleteState = { path: targetPath, kind, errEl, recursive: false };
|
||||
|
||||
if (kind === "dir") {
|
||||
const url = `${baseUrl}/files/delete_preview?path=${encodeURIComponent(targetPath)}`;
|
||||
const r = await fetchJson(url);
|
||||
if (r.ok && r.body && Array.isArray(r.body.entries) && r.body.entries.length > 0) {
|
||||
renderPreview(r.body);
|
||||
deleteState.recursive = true;
|
||||
}
|
||||
// On preview-fetch failure (or empty result), fall through to
|
||||
// the compact modal — the existing rmdir path still handles
|
||||
// empty dirs without recursive=1.
|
||||
}
|
||||
|
||||
openModal(deleteDialog);
|
||||
}
|
||||
|
||||
|
|
@ -119,10 +205,11 @@
|
|||
const btn = event.target?.closest?.(".files-delete-confirm");
|
||||
if (!btn) return;
|
||||
if (!deleteState) return;
|
||||
const { path, errEl } = deleteState;
|
||||
const { path, errEl, recursive } = deleteState;
|
||||
const fd = new FormData();
|
||||
fd.append("path", path);
|
||||
fd.append("csrf_token", csrfToken);
|
||||
if (recursive) fd.append("recursive", "1");
|
||||
const r = await postForm(`${baseUrl}/files/delete`, fd);
|
||||
if (r.ok) {
|
||||
deleteState = null;
|
||||
|
|
@ -130,12 +217,17 @@
|
|||
scheduleRefresh(parentOf(path));
|
||||
} else {
|
||||
errEl.hidden = false;
|
||||
// rawText fallback surfaces the server's text-body errors
|
||||
// (e.g. "directory is not empty") — mirrors editor.js.
|
||||
errEl.textContent =
|
||||
(r.body && r.body.error) || `Delete failed (HTTP ${r.status}).`;
|
||||
(r.body && r.body.error) ||
|
||||
r.rawText ||
|
||||
`Delete failed (HTTP ${r.status}).`;
|
||||
}
|
||||
});
|
||||
deleteDialog.addEventListener("close", () => {
|
||||
deleteState = null;
|
||||
resetPreview();
|
||||
});
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -69,17 +69,18 @@
|
|||
|
||||
// Enables Save (labeled "Replace" in binary mode) when either a file
|
||||
// is queued OR the filename input has been edited. Rename-only is a
|
||||
// valid Replace and routes to /files/move below.
|
||||
// valid Replace and routes to /files/move below. The filename input
|
||||
// holds the full relative path (phone-friendly move-via-edit), so the
|
||||
// "changed?" check compares against relPath, not its basename.
|
||||
function updateRoutedBinarySaveEnabled(modalContent) {
|
||||
const panel = modalContent.querySelector(".files-editor-binary[data-rel-path]");
|
||||
const saveBtn = modalContent.querySelector(".files-editor-save");
|
||||
if (!panel || !saveBtn) return;
|
||||
const relPath = panel.dataset.relPath || "";
|
||||
const originalLeaf = relPath.split("/").pop() || relPath;
|
||||
const filenameInput = modalContent.querySelector("[data-editor-filename]");
|
||||
const filenameChanged =
|
||||
filenameInput && filenameInput.value.trim() &&
|
||||
filenameInput.value.trim() !== originalLeaf;
|
||||
filenameInput.value.trim() !== relPath;
|
||||
saveBtn.disabled = !routedReplacement && !filenameChanged;
|
||||
}
|
||||
|
||||
|
|
@ -227,13 +228,12 @@
|
|||
return;
|
||||
}
|
||||
|
||||
// Edit mode: rename-on-save (sibling rename only — parent stays).
|
||||
const originalLeaf = relPath.split("/").pop() || relPath;
|
||||
const parent = relPath.includes("/") ? relPath.slice(0, relPath.lastIndexOf("/")) : "";
|
||||
let newPath = null;
|
||||
if (editedFilename && editedFilename !== originalLeaf) {
|
||||
newPath = parent ? `${parent}/${editedFilename}` : editedFilename;
|
||||
}
|
||||
// Edit mode: the filename input holds the FULL relative path, not
|
||||
// just the leaf — editing it moves or renames the file. This is
|
||||
// the phone-friendly alternative to drag-and-drop. /files/save +
|
||||
// safe_resolve_for_move validate the destination, so we forward
|
||||
// the input verbatim.
|
||||
const newPath = (editedFilename && editedFilename !== relPath) ? editedFilename : null;
|
||||
const payload = { path: relPath, content };
|
||||
if (newPath) payload.new_path = newPath;
|
||||
const r = await postJson(`${baseUrl}/files/save`, payload);
|
||||
|
|
@ -257,12 +257,10 @@
|
|||
|
||||
const filenameInput = modalContent.querySelector("[data-editor-filename]");
|
||||
const editedFilename = filenameInput ? filenameInput.value.trim() : "";
|
||||
const originalLeaf = relPath.split("/").pop() || relPath;
|
||||
const parent = relPath.includes("/") ? relPath.slice(0, relPath.lastIndexOf("/")) : "";
|
||||
const renaming = !!editedFilename && editedFilename !== originalLeaf;
|
||||
const newPath = renaming
|
||||
? (parent ? `${parent}/${editedFilename}` : editedFilename)
|
||||
: null;
|
||||
// Filename input is the full relative path; see routedSaveClicked
|
||||
// for the rationale.
|
||||
const renaming = !!editedFilename && editedFilename !== relPath;
|
||||
const newPath = renaming ? editedFilename : null;
|
||||
|
||||
if (routedReplacement) {
|
||||
const fd = new FormData();
|
||||
|
|
|
|||
|
|
@ -215,6 +215,16 @@
|
|||
</div>
|
||||
<div class="modal-body">
|
||||
<p>This cannot be undone.</p>
|
||||
{# Populated by dialogs.js when deleting a non-empty directory.
|
||||
Stays hidden for files and empty folders so the modal keeps its
|
||||
compact "Delete X?" look in those cases. #}
|
||||
<details class="files-delete-preview" hidden>
|
||||
<summary class="files-delete-preview-summary">Contents to be deleted</summary>
|
||||
<ul class="files-delete-preview-list"></ul>
|
||||
<p class="files-delete-preview-truncated muted" hidden>
|
||||
… and <span class="files-delete-preview-extra"></span> more entries will also be deleted.
|
||||
</p>
|
||||
</details>
|
||||
<p class="files-delete-error muted" hidden></p>
|
||||
</div>
|
||||
<div class="modal-footer">
|
||||
|
|
|
|||
|
|
@ -340,6 +340,63 @@ def test_new_folder_then_delete(page: Page, files_overlay_server) -> None:
|
|||
expect(folder_row).to_have_count(0, timeout=5000)
|
||||
|
||||
|
||||
def test_delete_populated_folder_shows_preview_and_recurses(
|
||||
page: Page, files_overlay_server
|
||||
) -> None:
|
||||
"""Click Delete on the seeded cfg/ folder (which contains files +
|
||||
a nested empty subdir). The modal must:
|
||||
* show every path that recursive delete will remove
|
||||
* include a summary count
|
||||
* succeed in one click, removing the whole tree
|
||||
Regression guard for the original UX bug: a 409 'directory is not
|
||||
empty' with no remediation, and no way to bulk-delete from the
|
||||
GUI.
|
||||
"""
|
||||
base = files_overlay_server["base_url"]
|
||||
overlay_id = files_overlay_server["overlay_id"]
|
||||
overlay_root = files_overlay_server["overlay_root"]
|
||||
|
||||
# Enrich the seeded cfg/ — fixture starts with just admins.txt.
|
||||
# The extra file + nested empty dir make the preview meaningful.
|
||||
(overlay_root / "cfg" / "motd.txt").write_text("welcome\n")
|
||||
(overlay_root / "cfg" / "nested").mkdir()
|
||||
|
||||
_open_overlay(page, base, overlay_id)
|
||||
|
||||
page.locator(
|
||||
'button[data-action="delete"]'
|
||||
'[data-target-path="cfg"]'
|
||||
'[data-row-kind="dir"]'
|
||||
).click(force=True)
|
||||
delete_modal = page.locator("#files-delete-modal")
|
||||
expect(delete_modal).to_be_visible(timeout=5000)
|
||||
|
||||
# Preview wakes up because cfg/ is non-empty. The <details> renders
|
||||
# closed by default — `inner_text` filters by layout visibility and
|
||||
# would return "" for the still-collapsed <li>s. Use
|
||||
# `all_text_contents()` (DOM-level, no visibility filter) so we can
|
||||
# assert on content without forcing an expand.
|
||||
preview = delete_modal.locator(".files-delete-preview")
|
||||
expect(preview).to_be_visible(timeout=5000)
|
||||
list_items = delete_modal.locator(".files-delete-preview-list li")
|
||||
rendered_paths = set(list_items.all_text_contents())
|
||||
# Strip the " (link)" suffix used for symlinks (none here, but
|
||||
# keeps the assertion stable if the fixture ever grows one).
|
||||
cleaned = {p.replace(" (link)", "").strip() for p in rendered_paths}
|
||||
assert {"admins.txt", "motd.txt", "nested/"} <= cleaned
|
||||
|
||||
summary = delete_modal.locator(".files-delete-preview-summary").inner_text()
|
||||
assert "2 files" in summary
|
||||
assert "1 folder" in summary
|
||||
|
||||
page.click(".files-delete-confirm")
|
||||
expect(delete_modal).to_be_hidden(timeout=5000)
|
||||
|
||||
# Whole tree gone; sibling overlay-root files untouched.
|
||||
assert not (overlay_root / "cfg").exists()
|
||||
assert (overlay_root / "server.cfg").exists()
|
||||
|
||||
|
||||
def test_filename_rename_on_save(page: Page, files_overlay_server) -> None:
|
||||
"""Open server.cfg, change the filename input to server-renamed.cfg,
|
||||
and click Save. The single POST /files/save call performs an atomic
|
||||
|
|
@ -387,6 +444,81 @@ def test_filename_rename_on_save(page: Page, files_overlay_server) -> None:
|
|||
).to_have_count(0)
|
||||
|
||||
|
||||
def test_edit_nested_file_save_no_rename(page: Page, files_overlay_server) -> None:
|
||||
"""Open a file inside a subfolder, click Save without touching the
|
||||
filename input, and assert the file stayed at its original path.
|
||||
|
||||
Regression for the rel_path/originalLeaf mismatch that turned every
|
||||
save of a nested file into a silent rename to a doubly-nested path
|
||||
(e.g. cfg/admins.txt → cfg/cfg/admins.txt). The filename input is
|
||||
pre-filled with the FULL relative path on purpose (phone-friendly
|
||||
move-via-edit), so the save handler must compare against relPath,
|
||||
not its basename — otherwise the equality check is permanently
|
||||
false and the rename branch fires on every save.
|
||||
"""
|
||||
base = files_overlay_server["base_url"]
|
||||
overlay_id = files_overlay_server["overlay_id"]
|
||||
overlay_root = files_overlay_server["overlay_root"]
|
||||
original_content = (overlay_root / "cfg" / "admins.txt").read_text()
|
||||
_open_overlay(page, base, overlay_id)
|
||||
|
||||
# Expand cfg/ so the file row is in the DOM (lazy-loaded children).
|
||||
page.click('li.file-tree-row-dir[data-target-path="cfg"] .file-tree-toggle')
|
||||
expect(
|
||||
page.locator('button.file-tree-name-button[data-target-path="cfg/admins.txt"]')
|
||||
).to_be_visible(timeout=5000)
|
||||
|
||||
page.click('button.file-tree-name-button[data-target-path="cfg/admins.txt"]')
|
||||
_wait_for_routed_editor(page)
|
||||
# Pin the design intent: the filename input shows the full path.
|
||||
assert (
|
||||
page.locator("input[data-editor-filename]").input_value() == "cfg/admins.txt"
|
||||
)
|
||||
|
||||
page.click(".files-editor-save")
|
||||
expect(page.locator("#modal-container")).to_be_hidden(timeout=5000)
|
||||
|
||||
# File stayed put. The doubling-bug signature would be
|
||||
# cfg/cfg/admins.txt; assert that ghost path doesn't exist.
|
||||
assert (overlay_root / "cfg" / "admins.txt").read_text() == original_content
|
||||
assert not (overlay_root / "cfg" / "cfg").exists()
|
||||
|
||||
|
||||
def test_edit_nested_file_rename_via_path_input(page: Page, files_overlay_server) -> None:
|
||||
"""Open cfg/admins.txt, change the filename input to a sibling path
|
||||
(cfg/admins-renamed.txt), and assert the file moved within cfg/.
|
||||
|
||||
Pins the intended phone-friendly feature: editing the filename
|
||||
input performs a rename/move because the input holds the full
|
||||
relative path. A future "fix" that re-introduces basename-only
|
||||
semantics (treating the input as just the leaf) would either
|
||||
leave the file in place or, worse, mis-route the new path —
|
||||
this test catches both.
|
||||
"""
|
||||
base = files_overlay_server["base_url"]
|
||||
overlay_id = files_overlay_server["overlay_id"]
|
||||
overlay_root = files_overlay_server["overlay_root"]
|
||||
original_content = (overlay_root / "cfg" / "admins.txt").read_text()
|
||||
_open_overlay(page, base, overlay_id)
|
||||
|
||||
page.click('li.file-tree-row-dir[data-target-path="cfg"] .file-tree-toggle')
|
||||
expect(
|
||||
page.locator('button.file-tree-name-button[data-target-path="cfg/admins.txt"]')
|
||||
).to_be_visible(timeout=5000)
|
||||
page.click('button.file-tree-name-button[data-target-path="cfg/admins.txt"]')
|
||||
_wait_for_routed_editor(page)
|
||||
|
||||
new_path = "cfg/admins-renamed.txt"
|
||||
page.fill("input[data-editor-filename]", new_path)
|
||||
page.click(".files-editor-save")
|
||||
|
||||
expect(page.locator("#modal-container")).to_be_hidden(timeout=5000)
|
||||
assert not (overlay_root / "cfg" / "admins.txt").exists()
|
||||
assert (overlay_root / "cfg" / "admins-renamed.txt").read_text() == original_content
|
||||
# Doubling-bug ghost path must not appear here either.
|
||||
assert not (overlay_root / "cfg" / "cfg").exists()
|
||||
|
||||
|
||||
def test_share_url_deep_link_reopens_editor(page: Page, files_overlay_server) -> None:
|
||||
"""Navigate directly to /overlays/<id>?modal=<urlencoded edit path>
|
||||
and assert the routed editor auto-opens for the right file. This
|
||||
|
|
|
|||
|
|
@ -636,6 +636,35 @@ def test_save_with_new_path_renames_atomically(app, left4me_root: Path) -> None:
|
|||
assert (overlay_dir / "motd_de.txt").read_text() == "willkommen"
|
||||
|
||||
|
||||
def test_save_with_new_path_moves_across_folders(app, left4me_root: Path) -> None:
|
||||
"""Cross-folder new_path is a supported rename target. Pins the
|
||||
contract the editor JS now relies on after the path-doubling fix:
|
||||
the filename input holds a full relative path, so the JS may send
|
||||
arbitrary inter-folder destinations as new_path. safe_resolve_for_move
|
||||
already handles this — this test makes the dependency explicit."""
|
||||
user_id = _make_user()
|
||||
overlay_id = _make_files_overlay(left4me_root, user_id=user_id, name="files")
|
||||
overlay_dir = left4me_root / "overlays" / str(overlay_id)
|
||||
(overlay_dir / "cfg").mkdir()
|
||||
(overlay_dir / "cfg" / "server.cfg").write_text("hostname original\n")
|
||||
(overlay_dir / "other").mkdir()
|
||||
|
||||
client = _client_for(app, user_id)
|
||||
r = client.post(
|
||||
f"/overlays/{overlay_id}/files/save",
|
||||
json={
|
||||
"path": "cfg/server.cfg",
|
||||
"new_path": "other/server.cfg",
|
||||
"content": "hostname moved\n",
|
||||
},
|
||||
headers=_csrf_headers(),
|
||||
)
|
||||
|
||||
assert r.status_code == 200
|
||||
assert not (overlay_dir / "cfg" / "server.cfg").exists()
|
||||
assert (overlay_dir / "other" / "server.cfg").read_text() == "hostname moved\n"
|
||||
|
||||
|
||||
def test_save_rejects_oversized_content(app, left4me_root: Path) -> None:
|
||||
user_id = _make_user()
|
||||
overlay_id = _make_files_overlay(left4me_root, user_id=user_id, name="files")
|
||||
|
|
@ -921,6 +950,195 @@ def test_delete_409_on_non_empty_dir(app, left4me_root: Path) -> None:
|
|||
assert (overlay_dir / "addons" / "file.txt").exists()
|
||||
|
||||
|
||||
def test_delete_recursive_removes_populated_tree(app, left4me_root: Path) -> None:
|
||||
"""recursive=1 turns POST /files/delete into an rmtree. The historical
|
||||
no-recursive default still 409s — covered by test_delete_409_on_non_empty_dir
|
||||
above. This is the opt-in branch that powers the GUI's preview-and-confirm
|
||||
flow."""
|
||||
user_id = _make_user()
|
||||
overlay_id = _make_files_overlay(left4me_root, user_id=user_id, name="files")
|
||||
overlay_dir = left4me_root / "overlays" / str(overlay_id)
|
||||
(overlay_dir / "addons").mkdir()
|
||||
(overlay_dir / "addons" / "a.txt").write_text("one")
|
||||
(overlay_dir / "addons" / "sub").mkdir()
|
||||
(overlay_dir / "addons" / "sub" / "b.txt").write_text("two")
|
||||
(overlay_dir / "addons" / "sub" / "c.txt").write_text("three")
|
||||
# Sibling outside the deletion target — must survive.
|
||||
(overlay_dir / "keep.txt").write_text("keep")
|
||||
|
||||
client = _client_for(app, user_id)
|
||||
r = client.post(
|
||||
f"/overlays/{overlay_id}/files/delete",
|
||||
data={"path": "addons", "recursive": "1", "csrf_token": "test-token"},
|
||||
)
|
||||
|
||||
assert r.status_code == 200
|
||||
assert not (overlay_dir / "addons").exists()
|
||||
assert (overlay_dir / "keep.txt").read_text() == "keep"
|
||||
|
||||
|
||||
def test_delete_recursive_unlinks_symlink_without_following(
|
||||
app, left4me_root: Path
|
||||
) -> None:
|
||||
"""A symlink inside the deletion target must be removed as a link —
|
||||
the symlink target's contents must NOT be wiped. shutil.rmtree's
|
||||
default (followlinks=False) gives us this; the test pins it so a
|
||||
future careless rewrite doesn't introduce a wipe-the-world bug."""
|
||||
user_id = _make_user()
|
||||
overlay_id = _make_files_overlay(left4me_root, user_id=user_id, name="files")
|
||||
overlay_dir = left4me_root / "overlays" / str(overlay_id)
|
||||
# Set up a "precious" file OUTSIDE the deletion target but still
|
||||
# inside left4me_root so safe_resolve doesn't reject the symlink at
|
||||
# resolve time. The recursive delete only operates on `target`'s
|
||||
# contents — the symlink target must be untouched after the call.
|
||||
precious = left4me_root / "precious.txt"
|
||||
precious.write_text("do-not-delete")
|
||||
(overlay_dir / "addons").mkdir()
|
||||
(overlay_dir / "addons" / "inner.txt").write_text("doomed")
|
||||
os.symlink(precious, overlay_dir / "addons" / "link-to-precious")
|
||||
|
||||
client = _client_for(app, user_id)
|
||||
r = client.post(
|
||||
f"/overlays/{overlay_id}/files/delete",
|
||||
data={"path": "addons", "recursive": "1", "csrf_token": "test-token"},
|
||||
)
|
||||
|
||||
assert r.status_code == 200
|
||||
assert not (overlay_dir / "addons").exists()
|
||||
assert precious.read_text() == "do-not-delete"
|
||||
|
||||
|
||||
# ---- /delete_preview ----------------------------------------------------
|
||||
|
||||
|
||||
def test_delete_preview_lists_paths_in_populated_tree(
|
||||
app, left4me_root: Path
|
||||
) -> None:
|
||||
user_id = _make_user()
|
||||
overlay_id = _make_files_overlay(left4me_root, user_id=user_id, name="files")
|
||||
overlay_dir = left4me_root / "overlays" / str(overlay_id)
|
||||
(overlay_dir / "tree").mkdir()
|
||||
(overlay_dir / "tree" / "a.txt").write_text("a")
|
||||
(overlay_dir / "tree" / "sub").mkdir()
|
||||
(overlay_dir / "tree" / "sub" / "b.txt").write_text("bb")
|
||||
|
||||
client = _client_for(app, user_id)
|
||||
r = client.get(f"/overlays/{overlay_id}/files/delete_preview?path=tree")
|
||||
|
||||
assert r.status_code == 200
|
||||
body = r.get_json()
|
||||
paths = {e["path"] for e in body["entries"]}
|
||||
assert paths == {"a.txt", "sub/", "sub/b.txt"}
|
||||
assert body["file_count"] == 2
|
||||
assert body["dir_count"] == 1
|
||||
assert body["total_bytes"] == 3 # "a" + "bb"
|
||||
assert body["truncated"] is False
|
||||
|
||||
|
||||
def test_delete_preview_empty_dir(app, left4me_root: Path) -> None:
|
||||
user_id = _make_user()
|
||||
overlay_id = _make_files_overlay(left4me_root, user_id=user_id, name="files")
|
||||
overlay_dir = left4me_root / "overlays" / str(overlay_id)
|
||||
(overlay_dir / "empty").mkdir()
|
||||
|
||||
client = _client_for(app, user_id)
|
||||
r = client.get(f"/overlays/{overlay_id}/files/delete_preview?path=empty")
|
||||
|
||||
assert r.status_code == 200
|
||||
body = r.get_json()
|
||||
assert body == {
|
||||
"entries": [],
|
||||
"file_count": 0,
|
||||
"dir_count": 0,
|
||||
"total_bytes": 0,
|
||||
"truncated": False,
|
||||
}
|
||||
|
||||
|
||||
def test_delete_preview_file_has_empty_payload(app, left4me_root: Path) -> None:
|
||||
"""Files don't get a preview — the existing compact modal shows the
|
||||
single-file confirm copy. Empty payload signals the JS to skip the
|
||||
preview UI."""
|
||||
user_id = _make_user()
|
||||
overlay_id = _make_files_overlay(left4me_root, user_id=user_id, name="files")
|
||||
overlay_dir = left4me_root / "overlays" / str(overlay_id)
|
||||
(overlay_dir / "motd.txt").write_text("hi")
|
||||
|
||||
client = _client_for(app, user_id)
|
||||
r = client.get(f"/overlays/{overlay_id}/files/delete_preview?path=motd.txt")
|
||||
|
||||
assert r.status_code == 200
|
||||
assert r.get_json()["entries"] == []
|
||||
|
||||
|
||||
def test_delete_preview_caps_at_500_entries(app, left4me_root: Path) -> None:
|
||||
"""Big trees still return — entries cap at 500 with truncated=True,
|
||||
but file_count keeps the full total so the summary line is accurate."""
|
||||
user_id = _make_user()
|
||||
overlay_id = _make_files_overlay(left4me_root, user_id=user_id, name="files")
|
||||
overlay_dir = left4me_root / "overlays" / str(overlay_id)
|
||||
(overlay_dir / "big").mkdir()
|
||||
for i in range(600):
|
||||
(overlay_dir / "big" / f"f{i:04d}.txt").write_text("x")
|
||||
|
||||
client = _client_for(app, user_id)
|
||||
r = client.get(f"/overlays/{overlay_id}/files/delete_preview?path=big")
|
||||
|
||||
assert r.status_code == 200
|
||||
body = r.get_json()
|
||||
assert len(body["entries"]) == 500
|
||||
assert body["truncated"] is True
|
||||
assert body["file_count"] == 600
|
||||
|
||||
|
||||
def test_delete_preview_does_not_follow_symlinks(
|
||||
app, left4me_root: Path
|
||||
) -> None:
|
||||
"""Symlinks inside the tree show up as kind=symlink. The link's
|
||||
target is NOT enumerated, so a huge external dir reachable via a
|
||||
link can't bloat the preview."""
|
||||
user_id = _make_user()
|
||||
overlay_id = _make_files_overlay(left4me_root, user_id=user_id, name="files")
|
||||
overlay_dir = left4me_root / "overlays" / str(overlay_id)
|
||||
external = left4me_root / "external"
|
||||
external.mkdir()
|
||||
(external / "unrelated.txt").write_text("unrelated")
|
||||
(overlay_dir / "tree").mkdir()
|
||||
os.symlink(external, overlay_dir / "tree" / "link")
|
||||
|
||||
client = _client_for(app, user_id)
|
||||
r = client.get(f"/overlays/{overlay_id}/files/delete_preview?path=tree")
|
||||
|
||||
assert r.status_code == 200
|
||||
body = r.get_json()
|
||||
paths = {(e["path"], e["kind"]) for e in body["entries"]}
|
||||
assert ("link", "symlink") in paths
|
||||
# The link's target's contents must not be enumerated.
|
||||
assert not any("unrelated" in e["path"] for e in body["entries"])
|
||||
|
||||
|
||||
def test_delete_preview_rejects_dotdot(app, left4me_root: Path) -> None:
|
||||
user_id = _make_user()
|
||||
overlay_id = _make_files_overlay(left4me_root, user_id=user_id, name="files")
|
||||
|
||||
client = _client_for(app, user_id)
|
||||
r = client.get(
|
||||
f"/overlays/{overlay_id}/files/delete_preview?path=../escape"
|
||||
)
|
||||
assert r.status_code == 422
|
||||
|
||||
|
||||
def test_delete_preview_404_on_missing(app, left4me_root: Path) -> None:
|
||||
user_id = _make_user()
|
||||
overlay_id = _make_files_overlay(left4me_root, user_id=user_id, name="files")
|
||||
|
||||
client = _client_for(app, user_id)
|
||||
r = client.get(
|
||||
f"/overlays/{overlay_id}/files/delete_preview?path=does-not-exist"
|
||||
)
|
||||
assert r.status_code == 404
|
||||
|
||||
|
||||
# ---- /replace -------------------------------------------------------------
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue