From 39f5f114de066cdc41ea61fc6fe584d3d9bbe80f Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Thu, 8 Oct 2026 20:30:52 -0400 Subject: [PATCH] Catalog: albums can be ordered by hand (format version 3) Album.order and Op::SetAlbumOrder; album_children lists folders first, then by place, then by name. Moving an album drops its place and undo restores it. Co-Authored-By: Claude Sonnet 5.5 --- crates/catalog/src/journal.rs | 3 +- crates/catalog/src/lib.rs | 34 ++++++- crates/catalog/src/local.rs | 1 + crates/catalog/src/model.rs | 6 +- crates/catalog/src/tests.rs | 14 ++- crates/catalog/src/tests_album_order.rs | 102 +++++++++++++++++++++ crates/catalog/src/tests_format_version.rs | 16 +++- crates/catalog/src/tests_journal.rs | 2 + crates/engine/src/cmd/library.rs | 2 +- crates/engine/src/demo.rs | 16 +++- 10 files changed, 187 insertions(+), 9 deletions(-) create mode 100644 crates/catalog/src/tests_album_order.rs diff --git a/crates/catalog/src/journal.rs b/crates/catalog/src/journal.rs index 96187cfad..5973975d6 100644 --- a/crates/catalog/src/journal.rs +++ b/crates/catalog/src/journal.rs @@ -45,6 +45,7 @@ //! | (none) | early builds: a log without a snapshot | | //! | 1 | up to v0.2.0 | | //! | 2 | after v0.2.0 | `Op::SetBrowsed`, `Catalog.browsed`, `Photo.local_baseline` | +//! | 3 | albums by hand | `Op::SetAlbumOrder`, `Album.order` | //! //! Rules: //! - **Bump [`VERSION`]** (and add a row above) in the change that adds an [`Op`] variant or a @@ -69,7 +70,7 @@ pub const LOG: &str = "catalog.log"; const FORMAT: &str = "lightcraft-catalog"; /// The catalog format this build writes (and the newest it reads). See the module docs → /// *Format versions*; bump it whenever an [`Op`] variant or a serialized field is added. -pub const VERSION: u32 = 2; +pub const VERSION: u32 = 3; /// When [`Journal::wants_snapshot`] says it's time to compact the log. #[derive(Clone, Copy, Debug)] diff --git a/crates/catalog/src/lib.rs b/crates/catalog/src/lib.rs index 2934c7a60..8a19b6e22 100644 --- a/crates/catalog/src/lib.rs +++ b/crates/catalog/src/lib.rs @@ -136,6 +136,11 @@ pub enum Op { id: AlbumId, parent: Option, }, + /// Where the album stands among its siblings (`None`: by name). Format version 3. + SetAlbumOrder { + id: AlbumId, + order: Option, + }, SetAlbumPhotos { id: AlbumId, photos: Vec, @@ -282,6 +287,18 @@ impl Catalog { pub fn albums(&self) -> impl Iterator { self.albums.values() } + /// The albums and folders directly inside `parent` (`None`: the top level), in the order the + /// sidebar lists them: folders first, then by their place if the user ordered them (those + /// without one after, by name), else by name. + pub fn album_children(&self, parent: Option) -> Vec<&Album> { + let mut kids: Vec<&Album> = self.albums.values().filter(|a| a.parent == parent).collect(); + kids.sort_by_cached_key(|a| (!a.folder, a.order.is_none(), a.order.unwrap_or(0), a.name.to_lowercase(), a.id)); + kids + } + /// Whether anything inside `parent` has a place of its own (the folder is ordered by hand). + pub fn album_children_are_ordered(&self, parent: Option) -> bool { + self.albums.values().any(|a| a.parent == parent && a.order.is_some()) + } /// The Quick Collection, once something was added to it. pub fn quick_collection(&self) -> Option { self.albums.values().find(|a| a.quick).map(|a| a.id) @@ -479,7 +496,20 @@ impl Catalog { } } let a = self.album_mut(id)?; - Op::MoveAlbum { id, parent: std::mem::replace(&mut a.parent, parent) } + if a.parent == parent { + return Ok(Op::MoveAlbum { id, parent }); + } + // its place belonged to the old neighbours: the undo gives it back with the folder + let old_order = a.order.take(); + let old_parent = std::mem::replace(&mut a.parent, parent); + match old_order { + Some(_) => Op::Batch { ops: vec![Op::MoveAlbum { id, parent: old_parent }, Op::SetAlbumOrder { id, order: old_order }] }, + None => Op::MoveAlbum { id, parent: old_parent }, + } + } + Op::SetAlbumOrder { id, order } => { + let a = self.album_mut(id)?; + Op::SetAlbumOrder { id, order: std::mem::replace(&mut a.order, order) } } Op::SetAlbumPhotos { id, photos } => { let a = self.album_mut(id)?; @@ -671,6 +701,8 @@ impl Catalog { #[cfg(test)] mod tests; #[cfg(test)] +mod tests_album_order; +#[cfg(test)] mod tests_background; #[cfg(test)] mod tests_folders; diff --git a/crates/catalog/src/local.rs b/crates/catalog/src/local.rs index 79a4a42d7..4291f45fa 100644 --- a/crates/catalog/src/local.rs +++ b/crates/catalog/src/local.rs @@ -295,6 +295,7 @@ impl Op { Op::RemoveAlbum { .. } | Op::RenameAlbum { .. } | Op::MoveAlbum { .. } + | Op::SetAlbumOrder { .. } | Op::SetAlbumRules { .. } | Op::RemoveStack { .. } | Op::SetLabelName { .. } diff --git a/crates/catalog/src/model.rs b/crates/catalog/src/model.rs index ae53b486a..957f5b68c 100644 --- a/crates/catalog/src/model.rs +++ b/crates/catalog/src/model.rs @@ -405,12 +405,16 @@ pub struct Album { /// The Quick Collection: a regular album that B adds to (one per library). #[serde(default, skip_serializing_if = "std::ops::Not::not")] pub quick: bool, + /// Its place among the albums of the same kind in its folder, once the user ordered them by + /// hand (see [`crate::Catalog::album_children`]); `None`: listed by name. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub order: Option, } impl Album { /// A regular (manual) album. pub fn new(id: AlbumId, name: impl Into) -> Album { - Album { id, name: name.into(), parent: None, folder: false, photos: Vec::new(), cover: None, smart: None, quick: false } + Album { id, name: name.into(), parent: None, folder: false, photos: Vec::new(), cover: None, smart: None, quick: false, order: None } } pub fn is_smart(&self) -> bool { self.smart.is_some() diff --git a/crates/catalog/src/tests.rs b/crates/catalog/src/tests.rs index 49f0ac4de..8d34d5efa 100644 --- a/crates/catalog/src/tests.rs +++ b/crates/catalog/src/tests.rs @@ -58,12 +58,22 @@ fn albums_and_folders() { let a = photo(&mut c, "a.jpg", "2026-04-01"); let f = c.alloc_album_id(); c.apply(Op::AddAlbum { - album: Album { id: f, name: "Trips".into(), parent: None, folder: true, photos: vec![], cover: None, smart: None, quick: false }, + album: Album { id: f, name: "Trips".into(), parent: None, folder: true, photos: vec![], cover: None, smart: None, quick: false, order: None }, }) .unwrap(); let al = c.alloc_album_id(); c.apply(Op::AddAlbum { - album: Album { id: al, name: "Alps".into(), parent: Some(f), folder: false, photos: vec![a], cover: None, smart: None, quick: false }, + album: Album { + id: al, + name: "Alps".into(), + parent: Some(f), + folder: false, + photos: vec![a], + cover: None, + smart: None, + quick: false, + order: None, + }, }) .unwrap(); assert_eq!(c.albums_of(a), vec![al]); diff --git a/crates/catalog/src/tests_album_order.rs b/crates/catalog/src/tests_album_order.rs new file mode 100644 index 000000000..dfd463f9c --- /dev/null +++ b/crates/catalog/src/tests_album_order.rs @@ -0,0 +1,102 @@ +//! Album order: the children of a folder (or of the top level) are listed folders first, then +//! alphabetically, until some of them are given a place of their own (`Album.order`); moving an +//! album to another folder drops its place, and undoing the move brings it back. + +use crate::*; + +fn add(c: &mut Catalog, name: &str, parent: Option, folder: bool) -> AlbumId { + let id = c.alloc_album_id(); + c.apply(Op::AddAlbum { album: Album { parent, folder, ..Album::new(id, name) } }).unwrap(); + id +} + +fn names(c: &Catalog, parent: Option) -> Vec { + c.album_children(parent).iter().map(|a| a.name.clone()).collect() +} + +fn place(c: &mut Catalog, id: AlbumId, order: Option) { + c.apply(Op::SetAlbumOrder { id, order }).unwrap(); +} + +/// Given albums that were never ordered, when they are listed, then folders come first and each +/// group is alphabetical regardless of case. +#[test] +fn children_are_folders_first_then_alphabetical() { + let mut c = Catalog::new(); + add(&mut c, "beta", None, false); + add(&mut c, "Alpha", None, false); + add(&mut c, "zoo", None, true); + add(&mut c, "Archive", None, true); + assert_eq!(names(&c, None), ["Archive", "zoo", "Alpha", "beta"]); + assert!(!c.album_children_are_ordered(None)); +} + +/// Given some children with a place of their own, then they follow it; the others come after, by +/// name; folders stay ahead of albums. +#[test] +fn ordered_children_follow_their_order() { + let mut c = Catalog::new(); + let a = add(&mut c, "A", None, false); + let b = add(&mut c, "B", None, false); + let cc = add(&mut c, "C", None, false); + add(&mut c, "D", None, false); + let f = add(&mut c, "F", None, true); + place(&mut c, cc, Some(0)); + place(&mut c, a, Some(1)); + assert_eq!(names(&c, None), ["F", "C", "A", "B", "D"]); + assert!(c.album_children_are_ordered(None)); + // equal places fall back to the name, so a damaged catalog still lists deterministically + place(&mut c, b, Some(1)); + assert_eq!(names(&c, None), ["F", "C", "A", "B", "D"]); + // another container is unaffected + let inner = add(&mut c, "Z", Some(f), false); + add(&mut c, "Y", Some(f), false); + assert_eq!(names(&c, Some(f)), ["Y", "Z"]); + assert!(!c.album_children_are_ordered(Some(f))); + place(&mut c, inner, Some(0)); + assert_eq!(names(&c, Some(f)), ["Z", "Y"]); +} + +/// A place is one undo step, and `None` puts the album back among the alphabetical ones. +#[test] +fn a_place_can_be_set_cleared_and_undone() { + let mut c = Catalog::new(); + let a = add(&mut c, "A", None, false); + add(&mut c, "B", None, false); + let inv = c.apply(Op::SetAlbumOrder { id: a, order: Some(5) }).unwrap(); + assert_eq!(c.album(a).unwrap().order, Some(5)); + c.apply(inv).unwrap(); + assert_eq!(c.album(a).unwrap().order, None); + assert!(c.apply(Op::SetAlbumOrder { id: AlbumId(999), order: Some(1) }).is_err(), "no such album"); +} + +/// Moving to another folder drops the album's place there (it belonged to the old neighbours), +/// and undoing the move gives it back. +#[test] +fn moving_drops_the_place_and_undo_restores_it() { + let mut c = Catalog::new(); + let from = add(&mut c, "From", None, true); + let to = add(&mut c, "To", None, true); + let a = add(&mut c, "A", Some(from), false); + place(&mut c, a, Some(3)); + let inv = c.apply(Op::MoveAlbum { id: a, parent: Some(to) }).unwrap(); + assert_eq!((c.album(a).unwrap().parent, c.album(a).unwrap().order), (Some(to), None)); + c.apply(inv).unwrap(); + assert_eq!((c.album(a).unwrap().parent, c.album(a).unwrap().order), (Some(from), Some(3))); + // "moving" to where it already is changes nothing + c.apply(Op::MoveAlbum { id: a, parent: Some(from) }).unwrap(); + assert_eq!(c.album(a).unwrap().order, Some(3)); +} + +/// Catalogs written before albums had an order load as they were; an unset order is not written. +#[test] +fn albums_without_an_order_load_and_save_unchanged() { + let old = r#"{"id":4,"name":"Old","parent":null,"folder":false,"photos":[],"cover":null}"#; + let a: Album = serde_json::from_str(old).unwrap(); + assert_eq!(a.order, None); + assert!(!serde_json::to_string(&a).unwrap().contains("order")); + let mut a = a; + a.order = Some(2); + let back: Album = serde_json::from_str(&serde_json::to_string(&a).unwrap()).unwrap(); + assert_eq!(back.order, Some(2)); +} diff --git a/crates/catalog/src/tests_format_version.rs b/crates/catalog/src/tests_format_version.rs index 3f2e8816c..cb80ce67e 100644 --- a/crates/catalog/src/tests_format_version.rs +++ b/crates/catalog/src/tests_format_version.rs @@ -47,6 +47,19 @@ fn v1_library_loads_and_is_upgraded() { assert_eq!((r.replayed, r.upgraded_from), (1, None)); } +/// A library saved before albums could be ordered by hand opens as it was and is rewritten as v3. +#[test] +fn v2_library_loads_and_is_upgraded() { + let (base, log, full) = legacy_parts(); + let m = MemStore::new(); + m.set(SNAPSHOT, format!("{{\"format\":\"lightcraft-catalog\",\"version\":2,\"seq\":2,\"catalog\":{}}}\n", base.to_snapshot()).into_bytes()); + m.set(LOG, log.into_bytes()); + let (_, c, r) = Journal::open(Box::new(m.clone())).unwrap(); + assert_eq!(c.to_snapshot(), full.to_snapshot()); + assert_eq!((r.replayed, r.upgraded_from), (1, Some(2))); + assert_eq!(snapshot_version(&m), 3); +} + #[test] fn versionless_log_only_library_is_upgraded() { let mut c = Catalog::new(); @@ -179,8 +192,9 @@ fn op_variants_are_versioned() { | Op::SetLabelName { .. } | Op::Batch { .. } => 1, Op::SetBrowsed { .. } => 2, + Op::SetAlbumOrder { .. } => 3, } } - let newest = since(&Op::SetBrowsed { folder: String::new(), at: None }); + let newest = since(&Op::SetAlbumOrder { id: AlbumId(0), order: None }); assert_eq!(newest, VERSION, "the newest op's version must be the current format version"); } diff --git a/crates/catalog/src/tests_journal.rs b/crates/catalog/src/tests_journal.rs index 0c9513e34..0594b6d83 100644 --- a/crates/catalog/src/tests_journal.rs +++ b/crates/catalog/src/tests_journal.rs @@ -59,6 +59,7 @@ fn op_for(c: &mut Catalog, kind: u8, a: u8, b: u8) -> Op { cover: None, smart: None, quick: false, + order: None, }, } } @@ -77,6 +78,7 @@ fn op_for(c: &mut Catalog, kind: u8, a: u8, b: u8) -> Op { 13 => Op::Batch { ops: c.remove_from_stacks_ops(&[pid]) }, 14 => c.set_top_ops(pid).unwrap_or(Op::Batch { ops: vec![] }), 9 if b.is_multiple_of(5) => Op::SetLabelName { label: ColorLabel::ALL[a as usize % 5], name: (a % 3 != 1).then(|| format!("Label {b}")) }, + 9 if b % 7 == 3 => Op::SetAlbumOrder { id: aid, order: (!b.is_multiple_of(3)).then_some(u32::from(b % 5)) }, 9 if b.is_multiple_of(2) => { Op::SetFile { id: pid, file_name: format!("Renamed_{b}.jpg"), source: Source::File { path: format!("/p/Renamed_{b}.jpg") } } } diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index 436ad123b..4ff64eb3a 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -784,7 +784,7 @@ pub fn specs() -> Vec { let cover = photos.first().copied(); s.commit( if folder { "New Folder" } else { "New Album" }, - Op::AddAlbum { album: Album { id, name, parent, folder, photos, cover, smart: None, quick: false } }, + Op::AddAlbum { album: Album { id, name, parent, folder, photos, cover, smart: None, quick: false, order: None } }, )?; Ok(json!({"id": id.0})) }), diff --git a/crates/engine/src/demo.rs b/crates/engine/src/demo.rs index 2f392cb79..f9d8b7c96 100644 --- a/crates/engine/src/demo.rs +++ b/crates/engine/src/demo.rs @@ -62,7 +62,17 @@ pub fn load(s: &mut Session) { } let folder = s.catalog.alloc_album_id(); ops.push(Op::AddAlbum { - album: Album { id: folder, name: "Travel 2026".into(), parent: None, folder: true, photos: vec![], cover: None, smart: None, quick: false }, + album: Album { + id: folder, + name: "Travel 2026".into(), + parent: None, + folder: true, + photos: vec![], + cover: None, + smart: None, + quick: false, + order: None, + }, }); let by_kw = |kw: &str| -> Vec<_> { scenes.iter().zip(&ids).filter(|(sc, _)| sc.meta.keywords.contains(&kw)).map(|(_, id)| *id).collect() }; for (name, parent, photos) in [ @@ -75,7 +85,9 @@ pub fn load(s: &mut Session) { ] { let id = s.catalog.alloc_album_id(); let cover = photos.first().copied(); - ops.push(Op::AddAlbum { album: Album { id, name: name.into(), parent, folder: false, photos, cover, smart: None, quick: false } }); + ops.push(Op::AddAlbum { + album: Album { id, name: name.into(), parent, folder: false, photos, cover, smart: None, quick: false, order: None }, + }); } for op in ops { let _ = s.catalog.apply(op);