diff --git a/apps/lightcraft-web/src/backup.rs b/apps/lightcraft-web/src/backup.rs index 00ec12a41..281a32296 100644 --- a/apps/lightcraft-web/src/backup.rs +++ b/apps/lightcraft-web/src/backup.rs @@ -52,6 +52,18 @@ pub fn valid_hash(h: &str) -> bool { !h.is_empty() && h.len() <= 128 && h.chars().all(|c| c.is_ascii_alphanumeric()) } +/// Why a backup can't start yet, given [`lightcraft_engine::Session::unsaved`]: changes applied in +/// the app but not yet written to browser storage (a save failed). The zip copies the stored +/// library files, so it would report success while missing them; `None` when everything is saved. +pub fn unsaved_refusal(unsaved: Option<(usize, &str)>) -> Option { + let (ops, why) = unsaved?; + let changes = if ops == 1 { "1 change".to_string() } else { format!("{ops} changes") }; + Some(format!( + "Backup not started: {changes} could not be saved to browser storage yet ({why}), so the backup would miss them. \ + Free up storage space; LightCraft retries the save with every change. Back up once nothing is left unsaved." + )) +} + /// Zip entry name for an original. pub fn original_entry(hash: &str, file_name: &str) -> String { let name: String = file_name.chars().map(|c| if c == '/' || c == '\\' || c.is_control() { '_' } else { c }).collect(); diff --git a/apps/lightcraft-web/src/files.rs b/apps/lightcraft-web/src/files.rs index d0c9f348b..4b5f6289d 100644 --- a/apps/lightcraft-web/src/files.rs +++ b/apps/lightcraft-web/src/files.rs @@ -282,6 +282,32 @@ mod tests { assert_eq!(s2.catalog.photo(id).unwrap().rating, 5, "the rating survived"); } + /// A backup copies the stored library files, so while a save is failing it would miss the + /// queued changes: it is refused with the reason until they are written (#1508). + #[test] + fn backup_is_refused_while_changes_are_unsaved() { + let files = Files::default(); + let mut s = Session::new(); + let stores = LibraryStores { dir: "browser".into(), catalog: Box::new(files.store()), files: Box::new(files.store()), on_disk: false }; + s.open_library_in(stores, true).unwrap(); + assert_eq!(crate::backup::unsaved_refusal(s.unsaved()), None, "everything saved: the backup may start"); + files.set_error(Some("QuotaExceededError".into())); + let _ = s.execute("photo.rate", &json!({"rating": 5})); + let refusal = crate::backup::unsaved_refusal(s.unsaved()).expect("refused while the rating is unsaved"); + assert!(refusal.contains("could not be saved") && refusal.contains("QuotaExceeded"), "{refusal}"); + files.set_error(None); + s.persist_if_dirty(); + s.persist().unwrap(); + assert_eq!(crate::backup::unsaved_refusal(s.unsaved()), None, "allowed again once the change is written"); + } + + #[test] + fn unsaved_refusal_counts_changes() { + use crate::backup::unsaved_refusal; + assert!(unsaved_refusal(Some((1, "quota"))).unwrap().contains("1 change could not")); + assert!(unsaved_refusal(Some((3, "quota"))).unwrap().contains("3 changes could not")); + } + #[test] fn library_survives_reload_through_flushes() { let mut disk = BTreeMap::new(); diff --git a/apps/lightcraft-web/src/web.rs b/apps/lightcraft-web/src/web.rs index 6a4487d09..3ce8eb66c 100644 --- a/apps/lightcraft-web/src/web.rs +++ b/apps/lightcraft-web/src/web.rs @@ -173,6 +173,9 @@ fn services(originals: Originals, backend: Option, files: Files, frozen screen_profile: None, backup_library: Some(Box::new(move |session: &mut Session| { let Some(b) = backup_backend.clone() else { return Err("nothing is stored in this browser session (?store=memory)".into()) }; + if let Some(why) = crate::backup::unsaved_refusal(session.unsaved()) { + return Err(why); + } // the photos' own names for the originals in the zip let names: std::collections::HashMap = session .catalog diff --git a/crates/ui-egui/src/menubar.rs b/crates/ui-egui/src/menubar.rs index c5e4e45c7..96463df61 100644 --- a/crates/ui-egui/src/menubar.rs +++ b/crates/ui-egui/src/menubar.rs @@ -962,9 +962,10 @@ pub fn show_in_window(app: &mut LightcraftApp, ui: &mut egui::Ui, max_width: f32 } if let Some((id, params)) = clicked { let r = run_item(app, &id, params); - // an export that can't start (e.g. no folder) says why instead of doing nothing + // an export that can't start (e.g. no folder), or a backup / restore the host refuses + // (changes still unsaved, nothing stored in this browser), says why instead of doing nothing if let Err(e) = r - && matches!(id.as_str(), "app.export" | "app.exportPrevious") + && matches!(id.as_str(), "app.export" | "app.exportPrevious" | "file.backupLibrary" | "file.restoreLibrary") { app.toast(ui.ctx(), e); } @@ -1044,6 +1045,7 @@ fn nodes_ui(ui: &mut egui::Ui, nodes: &[MenuNode], mac: bool, clicked: &mut Opti b = b.shortcut_text(shortcut_text(sc, mac)); } let r = ui.add_enabled(*enabled, b); + crate::widgets::register(ui.ctx(), format!("menuitem:{id}"), r.rect); if *checked == Some(true) { let t = crate::theme::Tokens::get(ui.ctx()); let c = egui::Rect::from_center_size(egui::pos2(r.rect.left() + 11.0, r.rect.center().y), egui::vec2(13.0, 13.0)); @@ -1390,6 +1392,30 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + /// A backup the host refuses (changes still unsaved, issue #1508) says why when started from + /// the menu, as a refused export does; the refusal used to reach only the control channel. + #[test] + fn a_refused_backup_from_the_menu_shows_why() { + // the host refuses, as the web one does while changes are still unsaved (issue #1508) + let refusal = "Backup not started: 1 change could not be saved to browser storage yet"; + let services = crate::Services { + backup_library: Some(Box::new(move |_: &mut lightcraft_engine::Session| Err(refusal.to_string()))), + ..Default::default() + }; + let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), services); + let mut h = crate::headless::Headless::new(app, [1400.0, 900.0], 1.0); + let t = std::time::Duration::from_secs(30); + h.settle(t); + for id in ["menu:File", "menuitem:file.backupLibrary"] { + let r = h.request("ui.clickWidget", json!({"id": id}), t); + assert_eq!(r["ok"], true, "{id}: {r}"); + h.step(); + } + // (read at once: a toast keeps the UI repainting until it expires, so settling would outlive it) + let toast = h.app.ui.toast.as_ref().map(|t| t.0.clone()).unwrap_or_default(); + assert!(toast.contains(refusal), "the refusal is shown: {toast:?}"); + } + /// #1528: `file.findMissing` with `wait: true` must propagate folder validation and search /// failures as errors instead of returning `Ok(null)`. #[test] diff --git a/docs/web.md b/docs/web.md index b275e837d..a755bfd80 100644 --- a/docs/web.md +++ b/docs/web.md @@ -111,7 +111,9 @@ are the same in every version, so serve them with `Cache-Control: no-cache` (not grant it, a notice says the library may be evicted under storage pressure. - **File ▸ Back Up Library…** downloads a zip of the library files and every stored original (`originals//`, so an unzipped backup is browsable; entries are stored, not - compressed; at most 4 GB). **File ▸ Restore Library from Backup…** reads such a zip into a + compressed; at most 4 GB). While a save is failing it refuses to start and says how many + changes are unsaved and why: the zip copies the stored library files, so it would miss them + (issue #1508). **File ▸ Restore Library from Backup…** reads such a zip into a new library folder (`library-restored-