Przeglądaj źródła

Add external folder subfolder preservation and fix file manager stale UI after delete

  External folder scan now mirrors disk subfolder structure into the folder
  tree instead of flattening all files into root. Hidden directories are
  filtered, orphaned subfolders are cleaned up on rescan. Fixes #890.

  File manager delete endpoints (folder, file, bulk) now commit before
  returning the response — previously relied on post-response auto-commit,
  causing a race where the frontend refetch arrived before the commit.
maziggy 1 miesiąc temu
rodzic
commit
1645b51dad

+ 2 - 0
CHANGELOG.md

@@ -7,6 +7,7 @@ All notable changes to Bambuddy will be documented in this file.
 ### New Features
 - **Optional PostgreSQL Database Support** — Bambuddy can now use an external PostgreSQL database instead of the built-in SQLite. Set the `DATABASE_URL` environment variable (e.g., `postgresql+asyncpg://user:pass@host:5432/bambuddy`) to connect to Postgres. SQLite remains the default when no `DATABASE_URL` is set. All features work with both backends including full-text archive search (FTS5 on SQLite, tsvector+GIN on PostgreSQL), backup/restore (file copy vs pg_dump/pg_restore), health diagnostics, and cross-database restore (import a SQLite backup into PostgreSQL with automatic type conversion and FK handling).
 - **Shortest Job First Queue Scheduling** ([#879](https://github.com/maziggy/bambuddy/issues/879)) — New SJF toggle badge on the queue page header. When enabled, the scheduler starts shorter print jobs before longer ones instead of FIFO order. A starvation guard ensures long jobs that get skipped once are protected from being skipped again — they move to the front of the queue on the next cycle. The queue display automatically reorders to show the scheduler's actual execution order. Print duration is cached on queue items at creation time from the 3MF metadata.
+- **External Folder Subfolder Preservation** ([#890](https://github.com/maziggy/bambuddy/issues/890)) — Scanning an external folder now mirrors the real directory structure into the file manager folder tree instead of flattening all files into the root. Subdirectories are created as child LibraryFolders with correct parent/child hierarchy, and files are assigned to their matching subfolder. Hidden directories are skipped when "Show hidden files" is disabled. Subfolders that are deleted from disk are automatically cleaned up on the next scan. Created subfolders inherit the parent's read-only and show-hidden settings.
 
 ### Improved
 - **Database Engine Info on System Page** — The System Information page now shows the active database engine (SQLite or PostgreSQL) and its version in the Database section, making it easy to verify which backend is in use.
@@ -23,6 +24,7 @@ All notable changes to Bambuddy will be documented in this file.
 - **Camera Snapshot Temp Files World-Readable** — Camera snapshot and plate detection endpoints created temporary JPEG files in `/tmp` with default 0644 permissions, making them readable by any local user. Switched from `NamedTemporaryFile(delete=False)` to `mkstemp` with explicit 0600 permissions so only the application user can read them. Cleanup was already handled via `finally` blocks. Reported responsibly by Sacha Vaudey via security@bambuddy.cool.
 
 ### Fixed
+- **File Manager Stale UI After Deleting Folders/Files** — Deleting a folder, file, or bulk-deleting items in the file manager appeared to succeed (toast shown) but the UI didn't update until a page reload. The delete endpoints (`delete_folder`, `delete_file`, `bulk_delete`) relied on FastAPI's dependency cleanup auto-commit which runs after the response is sent — the frontend received the success response, refetched the folder/file list, but the delete hadn't been committed yet. Added explicit `db.commit()` before returning in all three endpoints.
 - **Spool Manager Deducts Double the Filament Used** ([#880](https://github.com/maziggy/bambuddy/issues/880)) — After a print completed, the built-in spool manager subtracted twice the actual filament consumption. The printer's MQTT status message contains both updated AMS remain percentages and the `FINISH` state, which triggered two independent deduction paths in the same event loop cycle: the AMS weight sync (absolute SET from remain%) and the usage tracker (additive delta from 3MF data). The AMS weight sync now skips updates while a print session is active, letting the usage tracker handle deductions precisely via 3MF slicer data.
 - **Thumbnails Broken After Backend Restart** — Archive and library thumbnails returned 401 Unauthorized after a backend restart because stream tokens are stored in memory and lost on restart. The frontend now detects failed token-protected image loads and automatically refreshes the stream token, so thumbnails recover without a page reload.
 - **SpoolBuddy Kiosk Screen Blanks on Boot** — The touchscreen display would blank immediately after the RPi booted, requiring a touch to wake. Added `consoleblank=0` to the kernel cmdline to disable Linux console blanking during the Plymouth-to-labwc transition, and changed the `wlr-randr` anti-blank loop to fire immediately instead of sleeping 60 seconds first.

+ 116 - 5
backend/app/api/routes/library.py

@@ -641,6 +641,7 @@ async def delete_folder(
 
     # Delete folder (cascade will handle files and subfolders)
     await db.delete(folder)
+    await db.commit()
 
     return {"status": "success", "message": "Folder deleted"}
 
@@ -780,18 +781,105 @@ async def scan_external_folder(
     if not ext_path.exists() or not ext_path.is_dir():
         raise HTTPException(status_code=400, detail=f"External path is not accessible: {folder.external_path}")
 
-    # Get existing DB files for this folder
+    # Collect all existing child external subfolder IDs (single query)
+    all_folder_ids = [folder_id]
+    child_result = await db.execute(
+        select(LibraryFolder).where(
+            LibraryFolder.is_external.is_(True),
+            LibraryFolder.parent_id.isnot(None),
+        )
+    )
+    all_child_folders = child_result.scalars().all()
+
+    # Walk the parent chain to find all descendants of folder_id
+    parent_to_children: dict[int, list] = {}
+    for cf in all_child_folders:
+        parent_to_children.setdefault(cf.parent_id, []).append(cf)
+
+    queue = [folder_id]
+    while queue:
+        pid = queue.pop()
+        for child in parent_to_children.get(pid, []):
+            all_folder_ids.append(child.id)
+            queue.append(child.id)
+
+    # Get existing DB files across root and all subfolders
     existing_result = await db.execute(
-        select(LibraryFile).where(LibraryFile.folder_id == folder_id, LibraryFile.is_external.is_(True))
+        select(LibraryFile).where(
+            LibraryFile.folder_id.in_(all_folder_ids),
+            LibraryFile.is_external.is_(True),
+        )
     )
     existing_files = {f.file_path: f for f in existing_result.scalars().all()}
 
+    # Build folder cache: relative path -> folder_id (for resolving subfolders)
+    # Pre-populate with existing child folders keyed by their external_path
+    folder_cache: dict[str, int] = {"": folder_id}
+    for fid in all_folder_ids:
+        if fid == folder_id:
+            continue
+        # Find the child folder object
+        for cf in all_child_folders:
+            if cf.id == fid and cf.external_path:
+                try:
+                    rel = str(Path(cf.external_path).relative_to(ext_path))
+                    if rel != ".":
+                        folder_cache[rel] = cf.id
+                except ValueError:
+                    pass
+
     # Scan the directory
     added = 0
     removed = 0
-    found_paths = set()
+    found_paths: set[str] = set()
+    seen_rel_dirs: set[str] = set()
+
+    for dirpath, dirnames, filenames in os.walk(ext_path):
+        # Filter hidden directories unless configured
+        if not folder.external_show_hidden:
+            dirnames[:] = [d for d in dirnames if not d.startswith(".")]
+
+        rel_dir = str(Path(dirpath).relative_to(ext_path))
+        if rel_dir == ".":
+            rel_dir = ""
+        seen_rel_dirs.add(rel_dir)
+
+        # Resolve or create subfolder chain for this directory
+        if rel_dir and rel_dir not in folder_cache:
+            parts = Path(rel_dir).parts
+            current_path = ""
+            current_parent = folder_id
+            for part in parts:
+                current_path = f"{current_path}/{part}".lstrip("/")
+                if current_path in folder_cache:
+                    current_parent = folder_cache[current_path]
+                else:
+                    existing_sub = await db.execute(
+                        select(LibraryFolder).where(
+                            LibraryFolder.name == part,
+                            LibraryFolder.parent_id == current_parent,
+                            LibraryFolder.is_external.is_(True),
+                        )
+                    )
+                    existing_folder = existing_sub.scalar_one_or_none()
+                    if existing_folder:
+                        current_parent = existing_folder.id
+                    else:
+                        new_folder = LibraryFolder(
+                            name=part,
+                            parent_id=current_parent,
+                            is_external=True,
+                            external_path=str(ext_path / current_path),
+                            external_readonly=folder.external_readonly,
+                            external_show_hidden=folder.external_show_hidden,
+                        )
+                        db.add(new_folder)
+                        await db.flush()
+                        current_parent = new_folder.id
+                    folder_cache[current_path] = current_parent
+
+        target_folder_id = folder_cache.get(rel_dir, folder_id)
 
-    for dirpath, _dirnames, filenames in os.walk(ext_path):
         for filename in filenames:
             # Skip hidden files unless configured
             if not folder.external_show_hidden and filename.startswith("."):
@@ -896,7 +984,7 @@ async def scan_external_folder(
                     thumbnail_path = to_relative_path(Path(thumbnail_path_str))
 
             db_file = LibraryFile(
-                folder_id=folder_id,
+                folder_id=target_folder_id,
                 is_external=True,
                 filename=filename,
                 file_path=file_path_str,
@@ -923,6 +1011,26 @@ async def scan_external_folder(
             await db.delete(db_file)
             removed += 1
 
+    # Remove empty subfolders whose directories no longer exist on disk
+    # Process deepest-first by sorting on path depth (descending)
+    subfolder_entries = [(rel, fid) for rel, fid in folder_cache.items() if rel and fid != folder_id]
+    subfolder_entries.sort(key=lambda x: x[0].count("/"), reverse=True)
+    for rel_path, sub_fid in subfolder_entries:
+        if rel_path in seen_rel_dirs:
+            continue  # Directory still exists on disk
+        # Check if subfolder has any remaining files
+        file_count_result = await db.execute(select(func.count(LibraryFile.id)).where(LibraryFile.folder_id == sub_fid))
+        if (file_count_result.scalar() or 0) == 0:
+            # Check if it has any remaining child folders
+            child_count_result = await db.execute(
+                select(func.count(LibraryFolder.id)).where(LibraryFolder.parent_id == sub_fid)
+            )
+            if (child_count_result.scalar() or 0) == 0:
+                sub_folder_result = await db.execute(select(LibraryFolder).where(LibraryFolder.id == sub_fid))
+                sub_folder_obj = sub_folder_result.scalar_one_or_none()
+                if sub_folder_obj:
+                    await db.delete(sub_folder_obj)
+
     await db.commit()
 
     return {"status": "success", "added": added, "removed": removed}
@@ -2328,6 +2436,7 @@ async def delete_file(
         logger.warning("Failed to delete file from disk: %s", e)
 
     await db.delete(file)
+    await db.commit()
 
     return {"status": "success", "message": "File deleted"}
 
@@ -2594,6 +2703,8 @@ async def bulk_delete(
             await db.delete(folder)
             deleted_folders += 1
 
+    await db.commit()
+
     return BulkDeleteResponse(deleted_files=deleted_files, deleted_folders=deleted_folders)
 
 

+ 167 - 4
backend/tests/integration/test_external_folders_api.py

@@ -125,6 +125,26 @@ class TestExternalFolderCreation:
         assert ext_folder["external_readonly"] is True
 
 
+def find_folder_in_tree(folders: list, name: str) -> dict | None:
+    """Recursively search a folder tree for a folder by name."""
+    for f in folders:
+        if f["name"] == name:
+            return f
+        result = find_folder_in_tree(f.get("children", []), name)
+        if result:
+            return result
+    return None
+
+
+def collect_folder_names(folders: list) -> list[str]:
+    """Recursively collect all folder names from a tree."""
+    names = []
+    for f in folders:
+        names.append(f["name"])
+        names.extend(collect_folder_names(f.get("children", [])))
+    return names
+
+
 class TestExternalFolderScan:
     """Tests for POST /library/folders/{id}/scan."""
 
@@ -158,22 +178,42 @@ class TestExternalFolderScan:
     @pytest.mark.asyncio
     @pytest.mark.integration
     async def test_scan_discovers_files(self, async_client: AsyncClient, db_session, external_folder):
-        """Verify scan discovers supported files."""
+        """Verify scan discovers supported files and creates subfolders."""
         response = await async_client.post(f"/api/v1/library/folders/{external_folder['id']}/scan")
         assert response.status_code == 200
         result = response.json()
-        # Should find: benchy.3mf, bracket.stl, print.gcode, subfolder/nested.stl
+        # Should find: benchy.3mf, bracket.stl, print.gcode (root) + subfolder/nested.stl
         # Should skip: readme.txt (unsupported), .hidden.3mf (hidden)
         assert result["added"] == 4
         assert result["removed"] == 0
 
+        # Root folder should have 3 files (nested.stl is in subfolder)
+        response = await async_client.get(f"/api/v1/library/files?folder_id={external_folder['id']}")
+        root_files = response.json()
+        assert len(root_files) == 3
+        root_filenames = {f["filename"] for f in root_files}
+        assert root_filenames == {"benchy.3mf", "bracket.stl", "print.gcode"}
+
+        # Subfolder should exist in the tree and contain nested.stl
+        response = await async_client.get("/api/v1/library/folders")
+        folders = response.json()
+        subfolder = find_folder_in_tree(folders, "subfolder")
+        assert subfolder is not None
+        assert subfolder["is_external"] is True
+        assert subfolder["parent_id"] == external_folder["id"]
+
+        response = await async_client.get(f"/api/v1/library/files?folder_id={subfolder['id']}")
+        sub_files = response.json()
+        assert len(sub_files) == 1
+        assert sub_files[0]["filename"] == "nested.stl"
+
     @pytest.mark.asyncio
     @pytest.mark.integration
     async def test_scan_skips_hidden_files(self, async_client: AsyncClient, db_session, external_folder):
         """Verify hidden files are skipped by default."""
         await async_client.post(f"/api/v1/library/folders/{external_folder['id']}/scan")
 
-        # List files in folder
+        # List files in root folder
         response = await async_client.get(f"/api/v1/library/files?folder_id={external_folder['id']}")
         assert response.status_code == 200
         files = response.json()
@@ -240,15 +280,138 @@ class TestExternalFolderScan:
     @pytest.mark.asyncio
     @pytest.mark.integration
     async def test_scan_files_marked_external(self, async_client: AsyncClient, db_session, external_folder):
-        """Verify scanned files have is_external=True."""
+        """Verify scanned files have is_external=True in root and subfolders."""
         await async_client.post(f"/api/v1/library/folders/{external_folder['id']}/scan")
 
+        # Check root folder files
         response = await async_client.get(f"/api/v1/library/files?folder_id={external_folder['id']}")
         files = response.json()
         assert len(files) > 0
         for f in files:
             assert f["is_external"] is True
 
+        # Check subfolder files
+        response = await async_client.get("/api/v1/library/folders")
+        folders = response.json()
+        subfolder = find_folder_in_tree(folders, "subfolder")
+        assert subfolder is not None
+        response = await async_client.get(f"/api/v1/library/files?folder_id={subfolder['id']}")
+        sub_files = response.json()
+        for f in sub_files:
+            assert f["is_external"] is True
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_scan_creates_nested_subfolders(self, async_client: AsyncClient, db_session, external_dir):
+        """Verify deeply nested directories create correct folder hierarchy."""
+        # Create nested structure: deep/nested/dir/model.stl
+        deep = external_dir / "deep" / "nested" / "dir"
+        deep.mkdir(parents=True)
+        (deep / "model.stl").write_bytes(b"deepstl")
+
+        data = {
+            "name": "Nested Test",
+            "external_path": str(external_dir),
+            "readonly": True,
+            "show_hidden": False,
+        }
+        response = await async_client.post("/api/v1/library/folders/external", json=data)
+        root = response.json()
+
+        response = await async_client.post(f"/api/v1/library/folders/{root['id']}/scan")
+        assert response.status_code == 200
+
+        # Verify folder chain: root -> deep -> nested -> dir
+        response = await async_client.get("/api/v1/library/folders")
+        all_folders = response.json()
+
+        deep = find_folder_in_tree(all_folders, "deep")
+        assert deep is not None
+        assert deep["parent_id"] == root["id"]
+        assert deep["is_external"] is True
+
+        nested = find_folder_in_tree(all_folders, "nested")
+        assert nested is not None
+        assert nested["parent_id"] == deep["id"]
+
+        dir_folder = find_folder_in_tree(all_folders, "dir")
+        assert dir_folder is not None
+        assert dir_folder["parent_id"] == nested["id"]
+
+        # model.stl should be in the "dir" folder
+        response = await async_client.get(f"/api/v1/library/files?folder_id={dir_folder['id']}")
+        files = response.json()
+        assert len(files) == 1
+        assert files[0]["filename"] == "model.stl"
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_scan_skips_hidden_directories(self, async_client: AsyncClient, db_session, external_dir):
+        """Verify hidden directories are skipped when show_hidden=False."""
+        hidden_dir = external_dir / ".hidden_dir"
+        hidden_dir.mkdir()
+        (hidden_dir / "secret.stl").write_bytes(b"secret")
+
+        data = {
+            "name": "Hidden Dir Test",
+            "external_path": str(external_dir),
+            "readonly": True,
+            "show_hidden": False,
+        }
+        response = await async_client.post("/api/v1/library/folders/external", json=data)
+        root = response.json()
+
+        response = await async_client.post(f"/api/v1/library/folders/{root['id']}/scan")
+        result = response.json()
+        # Should find 4 files (root 3 + subfolder/nested.stl) but NOT .hidden_dir/secret.stl
+        assert result["added"] == 4
+
+        # No ".hidden_dir" folder should be created
+        response = await async_client.get("/api/v1/library/folders")
+        folder_names = collect_folder_names(response.json())
+        assert ".hidden_dir" not in folder_names
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_scan_removes_deleted_subfolder(
+        self, async_client: AsyncClient, db_session, external_folder, external_dir
+    ):
+        """Verify scan removes empty subfolder entries when directory deleted from disk."""
+        await async_client.post(f"/api/v1/library/folders/{external_folder['id']}/scan")
+
+        # Verify subfolder exists
+        response = await async_client.get("/api/v1/library/folders")
+        subfolder = find_folder_in_tree(response.json(), "subfolder")
+        assert subfolder is not None
+
+        # Delete the subfolder from disk
+        import shutil
+
+        shutil.rmtree(external_dir / "subfolder")
+
+        # Re-scan
+        response = await async_client.post(f"/api/v1/library/folders/{external_folder['id']}/scan")
+        result = response.json()
+        assert result["removed"] == 1  # nested.stl removed
+
+        # Subfolder should be cleaned up (empty + directory gone)
+        response = await async_client.get("/api/v1/library/folders")
+        subfolder = find_folder_in_tree(response.json(), "subfolder")
+        assert subfolder is None
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_scan_subfolder_inherits_readonly(
+        self, async_client: AsyncClient, db_session, external_folder, external_dir
+    ):
+        """Verify created subfolders inherit external_readonly from parent."""
+        await async_client.post(f"/api/v1/library/folders/{external_folder['id']}/scan")
+
+        response = await async_client.get("/api/v1/library/folders")
+        subfolder = find_folder_in_tree(response.json(), "subfolder")
+        assert subfolder is not None
+        assert subfolder["external_readonly"] is True
+
 
 class TestExternalFolderProtections:
     """Tests for read-only protections on external folders."""