feat: enhance file deletion functionality with multi-select by Jessica765 · Pull Request #682 · crosspoint-reader/crosspoint-reader

Jessica765 · 2026-02-03T13:40:37Z

Summary

What is the goal of this PR? Enhances the file manager with multi-select deletion functionality and improved UI formatting.
What changes are included?
- Added multi-select capability for file deletion in the web interface
- Fixed formatting issues in file table for folder rows
- Updated [.gitignore] to exclude additional build artifacts and cache files
- Refactored CrossPointWebServer.cpp to support batch file deletion
- Enhanced FilesPage.html with improved UI for file selection and deletion

Additional Context

The file deletion endpoint now handles multiple files in a single request, improving efficiency when removing multiple files
Changes are focused on the web file manager component only

AI Usage

While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it
helps set the right context for reviewers.

Did you use AI tools to help write this code? PARTIALLY

daveallie · 2026-02-10T09:04:07Z

.gitignore

+node_modules/
+package.json
+package-lock.json
+mise.toml


Are these necessary, this repo doesn't rely on Mise or Node?

Fixed, apologies for the unnecessary additions

coderabbitai · 2026-02-22T06:48:32Z

📝 Walkthrough

Walkthrough

Backend and frontend now support batch file deletion: the server accepts a JSON array of paths and processes each item with per-item validation and aggregated reporting; the UI adds per-row and select-all checkboxes plus a batch delete modal to submit multiple paths.

Changes

Cohort / File(s)	Summary
Backend batch deletion `src/network/CrossPointWebServer.cpp`	Reworked `handleDelete` to accept a JSON `paths` array, normalize/validate each path, reject hidden/protected items, check existence and directory emptiness, delete files or empty folders, accumulate per-item failures, and return 200 on full success or 500 with a failure list.
Frontend multi-selection UI `src/network/html/FilesPage.html`	Added select-all and per-row checkboxes, a "Delete Selected" action, a multi-item delete modal and list rendering, selection helpers (`toggleSelectAll`, `getSelectedItems`) and batch deletion flow (`openDeleteSelectedModal`, `openDeleteModalForItems`, `confirmDelete`, `closeDeleteModal`), while retaining single-item entry compatibility.
Ignore list update `.gitignore`	Added `.history/` to the ignored paths.

Sequence Diagram(s)

sequenceDiagram
    participant Browser
    participant WebServer
    participant FileSystem

    Browser->>Browser: User selects items (checkboxes)
    Browser->>Browser: Click "Delete Selected" → collect paths array
    Browser->>WebServer: POST /delete with JSON { "paths": [...] }

    WebServer->>WebServer: Parse JSON array
    loop For each path
        WebServer->>WebServer: Normalize & validate path (non-empty, not root, starts with "/")
        WebServer->>WebServer: Reject hidden/system/protected names
        WebServer->>FileSystem: Check existence & type
        alt Path is folder
            WebServer->>FileSystem: Check if folder is empty
            alt Empty
                WebServer->>FileSystem: Delete folder
            else Not empty
                WebServer->>WebServer: Record failure for item
            end
        else Path is file
            WebServer->>FileSystem: Delete file
            alt Deletion failed
                WebServer->>WebServer: Record failure for item
            end
        end
    end

    alt All deletions succeeded
        WebServer->>Browser: 200 "All items deleted successfully"
    else Any deletion failed
        WebServer->>Browser: 500 with concatenated failure list
    end

    Browser->>Browser: Close modal and refresh file list

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main feature addition: multi-select capability for file deletion, which is the primary change across multiple files in the changeset.
Description check	✅ Passed	The description is directly related to the changeset, providing clear context about multi-select deletion, batch processing, and UI enhancements with specific file modifications mentioned.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 2

🧹 Nitpick comments (2)

src/network/CrossPointWebServer.cpp (1)

957-979: Consider using existing isProtectedItemName() helper.

This segment duplicates the protection logic already implemented in isProtectedItemName() (lines 70-80). Using the helper would reduce duplication and ensure consistency.

♻️ Suggested refactor

     // Security check: prevent deletion of protected items
     const String itemName = itemPath.substring(itemPath.lastIndexOf('/') + 1);
 
-    // Hidden/system files are protected
-    if (itemName.startsWith(".")) {
-      failedItems += itemPath + " (hidden/system file); ";
-      allSuccess = false;
-      continue;
-    }
-
-    // Check against explicitly protected items
-    bool isProtected = false;
-    for (size_t i = 0; i < HIDDEN_ITEMS_COUNT; i++) {
-      if (itemName.equals(HIDDEN_ITEMS[i])) {
-        isProtected = true;
-        break;
-      }
-    }
-    if (isProtected) {
-      failedItems += itemPath + " (protected file); ";
+    if (isProtectedItemName(itemName)) {
+      failedItems += itemPath + " (protected item); ";
       allSuccess = false;
       continue;
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/network/CrossPointWebServer.cpp` around lines 957 - 979, The block
duplicates protection checks already implemented by isProtectedItemName();
replace the manual hidden-dot and HIDDEN_ITEMS loop with a single call to
isProtectedItemName(itemName) (using the existing itemName variable), and on
true append the same failure handling (failedItems += itemPath + " (protected
file); "; allSuccess = false; continue;) so protection logic is centralized and
consistent with the helper.

src/network/html/FilesPage.html (1)

919-925: Optional: Sync select-all checkbox with individual selections.

The select-all checkbox doesn't update when individual items are toggled. This could confuse users who manually select all items or deselect some after "select all".

💡 Optional enhancement

   function toggleSelectAll(master) {
     const checked = master.checked;
     document.querySelectorAll('.select-item').forEach(cb => {
       cb.checked = checked;
     });
   }
+
+  function updateSelectAllState() {
+    const all = document.querySelectorAll('.select-item');
+    const checked = document.querySelectorAll('.select-item:checked');
+    const master = document.getElementById('selectAllCheckbox');
+    if (!master) return;
+    master.checked = all.length > 0 && checked.length === all.length;
+    master.indeterminate = checked.length > 0 && checked.length < all.length;
+  }

Then add onchange="updateSelectAllState()" to each .select-item checkbox in the template.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/network/html/FilesPage.html` around lines 919 - 925, The select-all
checkbox isn't synced when individual item checkboxes change; add a new function
updateSelectAllState that queries all '.select-item' checkboxes, sets the master
select-all checkbox checked/indeterminate based on whether all/none/some are
selected, and wire each '.select-item' checkbox to call updateSelectAllState on
change (as suggested, add onchange="updateSelectAllState()" in the template).
Use the existing toggleSelectAll(master) behavior for the master checkbox and
reference the same master selector (the select-all element) inside
updateSelectAllState to keep both directions of syncing consistent.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f62529a and 67257a3.

📒 Files selected for processing (3)

.gitignore
src/network/CrossPointWebServer.cpp
src/network/html/FilesPage.html

🔇 Additional comments (6)

.gitignore (1)

13-13: LGTM!

Adding .history/ is appropriate — it's commonly generated by VS Code's Local History extension and follows the existing pattern of ignoring IDE-specific directories.

src/network/CrossPointWebServer.cpp (1)

1015-1019: LGTM!

The aggregated response approach correctly distinguishes full success from partial failure, providing useful feedback for debugging failed deletions.

src/network/html/FilesPage.html (4)

839-842: LGTM!

The select-all checkbox is correctly placed in the table header with proper event binding.

859-874: LGTM!

Per-row checkboxes correctly store path (URL-encoded) and name (HTML-escaped) data attributes for both folders and files.

919-937: LGTM!

Selection helper functions correctly handle checkbox state propagation and item collection with proper URL decoding.

973-1003: LGTM!

The confirmDelete() function correctly:

Validates items exist before proceeding

Normalizes paths to ensure leading slash

Sends JSON-encoded paths array matching the backend API

Handles success/error responses appropriately

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/network/CrossPointWebServer.cpp`:
- Around line 1003-1007: In the deletion branch after calling
Storage.remove(itemPath.c_str()) (the block that closes f and sets success),
call clearEpubCacheIfNeeded(...) with the deleted path (itemPath) when removal
succeeds so EPUB metadata cache entries are cleared for deleted files; ensure
this call happens after Storage.remove returns true and use the same itemPath
variable to identify the cache to clear.
- Around line 923-936: Replace the fixed-size v6-style DynamicJsonDocument usage
in the paths handler: stop constructing DynamicJsonDocument with a hardcoded
capacity and instead use v7 elastic allocation (i.e., default-construct or use
the v7 JsonDocument API), then after calling deserializeJson(doc,
server->arg("paths")) check both the returned DeserializationError and
doc.overflowed(); if overflowed() is true respond with server->send(400,
"text/plain", "Paths too large") (or similar) and return, otherwise continue to
obtain JsonArray paths = doc.as<JsonArray>() and validate paths.isNull() or
paths.size() == 0 as before.

---

Nitpick comments:
In `@src/network/CrossPointWebServer.cpp`:
- Around line 957-979: The block duplicates protection checks already
implemented by isProtectedItemName(); replace the manual hidden-dot and
HIDDEN_ITEMS loop with a single call to isProtectedItemName(itemName) (using the
existing itemName variable), and on true append the same failure handling
(failedItems += itemPath + " (protected file); "; allSuccess = false; continue;)
so protection logic is centralized and consistent with the helper.

In `@src/network/html/FilesPage.html`:
- Around line 919-925: The select-all checkbox isn't synced when individual item
checkboxes change; add a new function updateSelectAllState that queries all
'.select-item' checkboxes, sets the master select-all checkbox
checked/indeterminate based on whether all/none/some are selected, and wire each
'.select-item' checkbox to call updateSelectAllState on change (as suggested,
add onchange="updateSelectAllState()" in the template). Use the existing
toggleSelectAll(master) behavior for the master checkbox and reference the same
master selector (the select-all element) inside updateSelectAllState to keep
both directions of syncing consistent.

src/network/CrossPointWebServer.cpp

coderabbitai

🧹 Nitpick comments (2)

src/network/CrossPointWebServer.cpp (2)

952-979: Consider reusing isProtectedItemName() and normalizing trailing slashes.

Lines 961-979 duplicate the logic in isProtectedItemName() (lines 70-80). Using the helper improves maintainability.
Trailing slash normalization is missing—paths like /foo/ won't match /foo in subsequent operations.

Suggested refactor

     // Ensure path starts with /
     if (!itemPath.startsWith("/")) {
       itemPath = "/" + itemPath;
     }
+    // Remove trailing slash (except for root)
+    if (itemPath.length() > 1 && itemPath.endsWith("/")) {
+      itemPath = itemPath.substring(0, itemPath.length() - 1);
+    }

     // Security check: prevent deletion of protected items
     const String itemName = itemPath.substring(itemPath.lastIndexOf('/') + 1);

-    // Hidden/system files are protected
-    if (itemName.startsWith(".")) {
-      failedItems += itemPath + " (hidden/system file); ";
-      allSuccess = false;
-      continue;
-    }
-
-    // Check against explicitly protected items
-    bool isProtected = false;
-    for (size_t i = 0; i < HIDDEN_ITEMS_COUNT; i++) {
-      if (itemName.equals(HIDDEN_ITEMS[i])) {
-        isProtected = true;
-        break;
-      }
-    }
-    if (isProtected) {
-      failedItems += itemPath + " (protected file); ";
+    if (isProtectedItemName(itemName)) {
+      failedItems += itemPath + " (protected item); ";
       allSuccess = false;
       continue;
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/network/CrossPointWebServer.cpp` around lines 952 - 979, Replace the
duplicated protection-check and add trailing-slash normalization: before
extracting itemName from itemPath, normalize itemPath to remove any trailing
slash (e.g., turn "/foo/" into "/foo" but keep "/" intact), then compute
itemName = itemPath.substring(itemPath.lastIndexOf('/') + 1) and call the
existing helper isProtectedItemName(itemName) instead of re-iterating
HIDDEN_ITEMS/HIDDEN_ITEMS_COUNT; if isProtectedItemName(...) returns true or
itemName startsWith("."), mark the item as failed (append to failedItems and set
allSuccess=false) and continue.

1003-1008: Cache clearing order inconsistency (minor).

The clearEpubCacheIfNeeded() is called after Storage.remove() regardless of success, while other handlers (rename at line 810, move at line 903) clear cache before the operation. If deletion fails, the cache is unnecessarily invalidated.

Consider moving the cache clear inside a success check or before the removal for consistency:

Suggested adjustment

     } else {
       // It's a file (or couldn't open as dir) — remove file
       if (f) f.close();
+      clearEpubCacheIfNeeded(itemPath);  // Clear before removal (consistent with rename/move)
       success = Storage.remove(itemPath.c_str());
-      clearEpubCacheIfNeeded(itemPath);
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/network/CrossPointWebServer.cpp` around lines 1003 - 1008, The cache is
cleared unconditionally after attempting file deletion; update the block
handling file removal so clearEpubCacheIfNeeded(itemPath) is only invoked when
the deletion actually succeeds (i.e., after Storage.remove(itemPath.c_str())
returns true), or alternatively move the cache-clear call to run before
Storage.remove for consistency with rename/move handlers; adjust the code around
the existing f.close(), Storage.remove, and clearEpubCacheIfNeeded calls to
ensure cache invalidation only happens on successful removal (reference
clearEpubCacheIfNeeded, Storage.remove, and itemPath).

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 67257a3 and d1c3895.

📒 Files selected for processing (1)

src/network/CrossPointWebServer.cpp

🔇 Additional comments (3)

src/network/CrossPointWebServer.cpp (3)

917-921: LGTM!

The argument validation correctly checks for the required paths parameter before proceeding.

938-950: LGTM!

Good defensive checks: tracking variables are properly initialized, and root path deletion is correctly prevented.

1016-1020: LGTM!

The response handling clearly communicates success or failure with descriptive error messages listing which items failed and why.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/network/CrossPointWebServer.cpp`:
- Around line 923-936: After deserializeJson(doc, pathsArg) add a check for
doc.overflowed() and return a 400 with a clear message if true; also enforce a
maximum allowed array size (e.g., define a MAX_PATHS constant) before using auto
paths = doc.as<JsonArray>() and reject requests exceeding that limit with a 400
(or trim/limit processing to MAX_PATHS if preferred). Reference deserializeJson,
doc.overflowed(), pathsArg, JsonArray/paths, and server->send to locate where to
add these checks and the error responses.

---

Nitpick comments:
In `@src/network/CrossPointWebServer.cpp`:
- Around line 952-979: Replace the duplicated protection-check and add
trailing-slash normalization: before extracting itemName from itemPath,
normalize itemPath to remove any trailing slash (e.g., turn "/foo/" into "/foo"
but keep "/" intact), then compute itemName =
itemPath.substring(itemPath.lastIndexOf('/') + 1) and call the existing helper
isProtectedItemName(itemName) instead of re-iterating
HIDDEN_ITEMS/HIDDEN_ITEMS_COUNT; if isProtectedItemName(...) returns true or
itemName startsWith("."), mark the item as failed (append to failedItems and set
allSuccess=false) and continue.
- Around line 1003-1008: The cache is cleared unconditionally after attempting
file deletion; update the block handling file removal so
clearEpubCacheIfNeeded(itemPath) is only invoked when the deletion actually
succeeds (i.e., after Storage.remove(itemPath.c_str()) returns true), or
alternatively move the cache-clear call to run before Storage.remove for
consistency with rename/move handlers; adjust the code around the existing
f.close(), Storage.remove, and clearEpubCacheIfNeeded calls to ensure cache
invalidation only happens on successful removal (reference
clearEpubCacheIfNeeded, Storage.remove, and itemPath).

…nt-reader#682) * **What is the goal of this PR?** Enhances the file manager with multi-select deletion functionality and improved UI formatting. * **What changes are included?** * Added multi-select capability for file deletion in the web interface * Fixed formatting issues in file table for folder rows * Updated [.gitignore] to exclude additional build artifacts and cache files * Refactored CrossPointWebServer.cpp to support batch file deletion * Enhanced FilesPage.html with improved UI for file selection and deletion * The file deletion endpoint now handles multiple files in a single request, improving efficiency when removing multiple files * Changes are focused on the web file manager component only --- While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? _**PARTIALLY**_ --------- Co-authored-by: Jessica Harrison <[email protected]> Co-authored-by: Dave Allie <[email protected]> (cherry picked from commit 786b438)

…nt-reader#682) ## Summary * **What is the goal of this PR?** Enhances the file manager with multi-select deletion functionality and improved UI formatting. * **What changes are included?** * Added multi-select capability for file deletion in the web interface * Fixed formatting issues in file table for folder rows * Updated [.gitignore] to exclude additional build artifacts and cache files * Refactored CrossPointWebServer.cpp to support batch file deletion * Enhanced FilesPage.html with improved UI for file selection and deletion ## Additional Context * The file deletion endpoint now handles multiple files in a single request, improving efficiency when removing multiple files * Changes are focused on the web file manager component only --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? _**PARTIALLY**_ --------- Co-authored-by: Jessica Harrison <[email protected]> Co-authored-by: Dave Allie <[email protected]>

Jessica Harrison added 4 commits February 3, 2026 15:14

feat: enhance file deletion functionality with multi-select

5f1c614

fix: update .gitignore

254f2b4

fix: correct formatting in file table for folder rows

4f18ec2

fix: remove test script from .gitignore

3a9b8c9

daveallie reviewed Feb 10, 2026

View reviewed changes

Jessica Harrison and others added 2 commits February 15, 2026 17:52

fix: remove unnecessary .gitignore files

71675c6

Merge branch 'master' into fork/Jessica765/feat/extend-web-file-manager

67257a3

daveallie previously approved these changes Feb 22, 2026

View reviewed changes

coderabbitai bot reviewed Feb 22, 2026

View reviewed changes

src/network/CrossPointWebServer.cpp Show resolved Hide resolved

src/network/CrossPointWebServer.cpp Show resolved Hide resolved

Replace DynamicJsonDocument with JsonDocument

e5ac9f2

daveallie dismissed their stale review via e5ac9f2 February 22, 2026 06:55

Call clearEpubCacheIfNeeded when deleting files

d1c3895

daveallie approved these changes Feb 22, 2026

View reviewed changes

coderabbitai bot reviewed Feb 22, 2026

View reviewed changes

daveallie merged commit 786b438 into crosspoint-reader:master Feb 22, 2026
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: enhance file deletion functionality with multi-select#682

feat: enhance file deletion functionality with multi-select#682
daveallie merged 8 commits intocrosspoint-reader:masterfrom
Jessica765:feat/extend-web-file-manager

Jessica765 commented Feb 3, 2026 •

edited

Loading

Uh oh!

daveallie Feb 10, 2026

Uh oh!

Jessica765 Feb 15, 2026

Uh oh!

coderabbitai bot commented Feb 22, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Jessica765 commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Additional Context

AI Usage

Uh oh!

daveallie Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Jessica765 Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot commented Feb 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Jessica765 commented Feb 3, 2026 •

edited

Loading

coderabbitai bot commented Feb 22, 2026 •

edited

Loading