LR-LIB-FOLDER: delete populated album folders with undo (#501)

crates/engine/src/cmd/library.rs

@@ -874,14 +874,45 @@ pub fn specs() -> Vec<CommandSpec> {
             s.commit("Rename Album", Op::RenameAlbum { id, name })?;
             ok()
         }),
-        cmd!("album.delete", "Delete Album", [], None, "{id}", always, |s, p| {
-            let id = album_param(p, "id", "album.delete")?;
-            s.commit("Delete Album", Op::RemoveAlbum { id })?;
-            if s.source == LibrarySource::Album(id) {
-                s.source = LibrarySource::All;
+        cmd!(
+            "album.delete",
+            "Delete Album",
+            [],
+            None,
+            "{id} — deletes the album or folder and its descendants, keeping photos in the library; one undo step",
+            always,
+            |s, p| {
+                let id = album_param(p, "id", "album.delete")?;
+                let folder = s.catalog.album(id).ok_or_else(|| bad("album.delete", "no such album"))?.folder;
+                let mut children = std::collections::BTreeMap::<AlbumId, Vec<AlbumId>>::new();
+                for album in s.catalog.albums() {
+                    if let Some(parent) = album.parent {
+                        children.entry(parent).or_default().push(album.id);
+                    }
+                }
+                // Iterative traversal: even a malformed cycle is rejected before any change.
+                // Reverse parent-first order removes children first; the batch inverse restores
+                // parents first, including memberships, smart rules, covers and manual ordering.
+                let mut pending = vec![id];
+                let mut order = Vec::new();
+                let mut removed = std::collections::BTreeSet::new();
+                while let Some(next) = pending.pop() {
+                    if !removed.insert(next) {
+                        return Err(bad("album.delete", "cycle in album folders"));
+                    }
+                    order.push(next);
+                    if let Some(nested) = children.remove(&next) {
+                        pending.extend(nested);
+                    }
+                }
+                let ops = order.into_iter().rev().map(|id| Op::RemoveAlbum { id }).collect();
+                s.commit(if folder { "Delete Folder" } else { "Delete Album" }, Op::Batch { ops })?;
+                if matches!(s.source, LibrarySource::Album(current) if removed.contains(&current)) {
+                    s.source = LibrarySource::All;
+                }
+                ok()
             }
-            ok()
-        }),
+        ),
         cmd!("album.move", "Move Album", [], None, "{id, parent?: folderId|null}", always, |s, p| {
             let id = album_param(p, "id", "album.move")?;
             let parent = p.get("parent").and_then(Value::as_u64).map(AlbumId);

crates/engine/src/tests.rs

@@ -113,9 +113,10 @@ fn albums_crud() {
     s.execute("library.selectAll", &json!({})).unwrap();
     s.execute("album.addPhotos", &json!({"id": a})).unwrap();
     assert_eq!(s.catalog.album(lightcraft_catalog::AlbumId(a)).unwrap().photos.len(), 24);
-    assert!(s.execute("album.delete", &json!({"id": f})).is_err(), "non-empty folder");
-    s.execute("album.delete", &json!({"id": a})).unwrap();
     s.execute("album.delete", &json!({"id": f})).unwrap();
+    assert!(s.catalog.album(lightcraft_catalog::AlbumId(a)).is_none());
+    assert!(s.catalog.album(lightcraft_catalog::AlbumId(f)).is_none());
+    assert_eq!(s.catalog.photos().count(), 24, "album deletion keeps the photos");
 }
 
 /// Given an album and a folder, when something is created inside each, then only the folder

crates/engine/src/tests_organize.rs

@@ -256,3 +256,44 @@ fn random_sort_persists_with_the_view() {
     assert_eq!(s2.sort.seed, 123_456_789);
     let _ = std::fs::remove_dir_all(&dir);
 }
+
+/// Issue #501: deleting a populated folder is one reversible, durable catalog edit.
+#[test]
+fn deleting_an_album_folder_preserves_photos_and_restores_its_subtree() {
+    use lightcraft_catalog::AlbumId;
+    let dir = temp_dir("delete-folder");
+    let mut s = open(&dir);
+    let make = |s: &mut Session, p| s.execute("album.create", &p).unwrap()["id"].as_u64().unwrap();
+    let outer = make(&mut s, json!({"name": "Trips", "folder": true}));
+    let nested = make(&mut s, json!({"name": "Europe", "folder": true, "parent": outer}));
+    let album = make(&mut s, json!({"name": "Best", "parent": nested, "addSelected": true}));
+    let sibling = make(&mut s, json!({"name": "Kept", "addSelected": true}));
+    let smart = s.execute("album.createSmart", &json!({"name": "Rated", "parent": outer, "rules": {"rating": 4}})).unwrap()["id"].as_u64().unwrap();
+    s.execute("album.reorder", &json!({"id": album, "parent": nested})).unwrap();
+    s.execute("library.source", &json!({"kind": "album", "id": album})).unwrap();
+    let before = s.catalog.to_snapshot();
+    let photos = serde_json::to_value(s.catalog.photos().collect::<Vec<_>>()).unwrap();
+    let undo_before = s.undo.len();
+    s.execute("album.delete", &json!({"id": outer})).unwrap();
+    for id in [outer, nested, album, smart] {
+        assert!(s.catalog.album(AlbumId(id)).is_none());
+    }
+    assert!(s.catalog.album(AlbumId(sibling)).is_some());
+    assert_eq!(serde_json::to_value(s.catalog.photos().collect::<Vec<_>>()).unwrap(), photos);
+    assert_eq!(s.source, LibrarySource::All, "a deleted descendant cannot remain the source");
+    assert_eq!(s.undo.len(), undo_before + 1);
+    let deleted = s.catalog.to_snapshot();
+    s.execute("edit.undo", &json!({})).unwrap();
+    assert_eq!(s.catalog.to_snapshot(), before, "all album fields and memberships restored");
+    s.execute("edit.redo", &json!({})).unwrap();
+    assert_eq!(s.catalog.to_snapshot(), deleted);
+    let undo_after = s.undo.len();
+    assert!(s.execute("album.delete", &json!({"id": outer})).is_err());
+    assert_eq!(s.catalog.to_snapshot(), deleted, "missing id leaves the catalog unchanged");
+    assert_eq!(s.undo.len(), undo_after);
+    drop(s);
+    let reopened = open(&dir);
+    assert_eq!(reopened.catalog.to_snapshot(), deleted, "journal replay preserves the deletion");
+    drop(reopened);
+    let _ = std::fs::remove_dir_all(dir);
+}

crates/ui-egui/src/panels/left.rs

@@ -1087,8 +1087,15 @@ fn folder_menu(app: &mut LightcraftApp, resp: &egui::Response, a: &Album) {
         if ui.button(crate::i18n::tr("Rename…")).clicked() {
             app.ui.dialog = Some(crate::state::Dialog::RenameAlbum { id: a.id.0, name: a.name.clone() });
         }
-        if ui.button(crate::i18n::tr("Delete")).clicked() {
-            let _ = app.run("album.delete", json!({"id": a.id.0}));
+        let delete = ui.button(crate::i18n::tr("Delete"));
+        register(ui.ctx(), format!("albumDelete:{}", a.id.0), delete.rect);
+        let delete =
+            delete.on_hover_text(crate::i18n::tr("Photos stay in All Photos. Deleting a folder also deletes its albums. You can undo this."));
+        if delete.clicked() {
+            if let Err(e) = app.run("album.delete", json!({"id": a.id.0})) {
+                app.toast(ui.ctx(), e);
+            }
+            ui.close();
         }
     });
 }

crates/ui-egui/src/tests_panels.rs

@@ -1096,3 +1096,23 @@ fn date_and_keyword_rows_show_their_photos_from_any_source() {
     h.step();
     assert_eq!(h.app.session.filter, lightcraft_catalog::Filter::default(), "{key}");
 }
+
+/// Issue #501: use the actual context-menu button on the populated demo folder.
+#[test]
+fn deleting_a_populated_album_folder_from_its_menu_is_undoable() {
+    let mut h = demo([1400.0, 900.0], json!({"view": "photoGrid", "leftPanel": true}));
+    let folder = h.app.session.catalog.albums().find(|a| a.folder && a.name == "Travel 2026").unwrap().id;
+    assert!(!h.app.session.catalog.album_children(Some(folder)).is_empty());
+    let before = h.app.session.catalog.to_snapshot();
+    let photo_count = h.app.session.catalog.photos().count();
+    right_click(&mut h, &format!("source:folder:{}", folder.0));
+    click(&mut h, &format!("albumDelete:{}", folder.0));
+    assert!(h.app.session.catalog.album(folder).is_none());
+    assert_eq!(h.app.session.catalog.photos().count(), photo_count);
+    assert!(!popup_open(&h));
+    h.app.run("edit.undo", json!({})).unwrap();
+    h.step();
+    h.step();
+    assert_eq!(h.app.session.catalog.to_snapshot(), before);
+    assert!(has(&h, &format!("source:folder:{}", folder.0)));
+}

docs/parity.md

@@ -156,7 +156,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins.
 | LR-LIB-RECENT-ADDED | Recently added | P1 | ✅ | `cmd:library.source` (`recentlyAdded`), `crates/engine/src/view.rs` (`RECENT_DAYS`) | photos imported in the 30 days up to the latest import, newest import first, grouped by import day |
 | LR-LIB-BYDATE | Browse by date | P1 | ✅ | `cmd:library.filter` (`date`), `crates/ui-egui/src/panels/left.rs` (`date_row`), `crates/catalog/src/query.rs` (`date_groups`) | year → month → day tree with library-wide counts; a row filters the current source by that date. Active filters show as removable chips under the grid header (`crates/ui-egui/src/panels/chips.rs`, with “+N more” and Clear all), the header counts “N of M photos” (matching of the source's total), a badge on the top-bar filter icon counts them, and an empty result says no photos match the active filters |
 | LR-LIB-ALBUM | Albums | P0 | ✅ | `cmd:album.create`, `cmd:album.rename`, `cmd:album.delete`, `cmd:album.addPhotos`, `cmd:album.removePhotos`, `cmd:dialog.newAlbum` | photos dragged from the grid onto an album row are added to it (LR-BEHAV-DRAGDROP) |
-| LR-LIB-FOLDER | Folders of albums | P0 | ✅ | `cmd:album.create` (`folder`, `parent`), `cmd:album.move`, `cmd:album.reorder`, `cmd:album.sort`, `cmd:dialog.newAlbum` / `cmd:dialog.newFolder` / `cmd:dialog.smartAlbum` / `cmd:dialog.newSmartAlbum` (`parent`), `crates/ui-egui/src/panels/left.rs` (`albums_tree`, `folder_menu`, `disclosure_triangle`), `crates/ui-egui/src/tests_panels.rs` | folders nest and fold with the same disclosure triangle as Local / By Date / Folders / Keywords (`albumToggle:<id>`; the row folds them too; open by default); right-click a folder ▸ New ▸ Create Album… / Create Smart Album… / Create Smart Album from Filter… / Create Folder… creates inside it (the `dialog.new*` commands take `parent`), opening the folders down to the new row; creating is only allowed inside folders; moving: drag an album or folder row onto a folder (outlined when allowed; a closed folder opens after 0.6 s under the pointer; the "Albums" header takes an item that sits in a folder back to the top level while it is dragged), the Move to menu or `album.move` — one undo step each; not onto a plain album, itself or something inside it; drop on the top / bottom half of an album or the edge of a folder to place it there by hand (`cmd:album.reorder`, a line shows the place; ordered per folder, folders stay first, new albums go after the ordered ones; right-click ▸ Sort Contents A–Z or ＋ ▸ Sort Albums A–Z = `cmd:album.sort`) |
+| LR-LIB-FOLDER | Folders of albums | P0 | ✅ | `cmd:album.create` (`folder`, `parent`), `cmd:album.move`, `cmd:album.reorder`, `cmd:album.sort`, `cmd:dialog.newAlbum` / `cmd:dialog.newFolder` / `cmd:dialog.smartAlbum` / `cmd:dialog.newSmartAlbum` (`parent`), `crates/ui-egui/src/panels/left.rs` (`albums_tree`, `folder_menu`, `disclosure_triangle`), `crates/ui-egui/src/tests_panels.rs` | Delete removes a folder and its nested albums as one undoable edit, keeps photos in the library and returns a deleted album view to All Photos (issue #501); folders nest and fold with the same disclosure triangle as Local / By Date / Folders / Keywords (`albumToggle:<id>`; the row folds them too; open by default); right-click a folder ▸ New ▸ Create Album… / Create Smart Album… / Create Smart Album from Filter… / Create Folder… creates inside it (the `dialog.new*` commands take `parent`), opening the folders down to the new row; creating is only allowed inside folders; moving: drag an album or folder row onto a folder (outlined when allowed; a closed folder opens after 0.6 s under the pointer; the "Albums" header takes an item that sits in a folder back to the top level while it is dragged), the Move to menu or `album.move` — one undo step each; not onto a plain album, itself or something inside it; drop on the top / bottom half of an album or the edge of a folder to place it there by hand (`cmd:album.reorder`, a line shows the place; ordered per folder, folders stay first, new albums go after the ordered ones; right-click ▸ Sort Contents A–Z or ＋ ▸ Sort Albums A–Z = `cmd:album.sort`) |
 | LR-LIB-SMARTALBUM | Smart albums | P1 | ✅ | `cmd:album.createSmart`, `cmd:album.setRules`, `crates/catalog/src/query.rs`, `crates/ui-egui/src/panels/filterbar.rs`, `crates/ui-egui/src/lib.rs` (`album_counts`) | saved filters (rating/flag/label/kind/edited/keyword/camera/lens/date range/text/album), live; match-all only (no any/none rule groups, no rule editor dialog — rules come from the filter bar or `album.setRules`); sidebar counts are cached per catalog revision (and per minute while an “in the last…” rule is used), counted without building id lists |
 | LR-LIB-SHARED-ALBUM | Shared albums | P2 | ⬜ | | needs a sharing service |
 | LR-LIB-OFFLINE | Keep album offline | P2 | 🚫 | | not applicable: local-first library |