Merge PR #726

crates/engine/src/cmd/browse.rs

@@ -39,7 +39,9 @@ fn browse(s: &mut Session, p: &Value) -> Result<Value> {
         v.sort();
         v
     };
-    let report = import_with(s, &files, &ImportOptions { mode: ImportMode::Add, local: true, ..Default::default() })?;
+    // looking at a folder never brings its deleted photos back (importing the files does)
+    let opts = ImportOptions { mode: ImportMode::Add, local: true, on_deleted: crate::import::OnDeleted::Skip, ..Default::default() };
+    let report = import_with(s, &files, &opts)?;
     stamp_browsed(s, &dir_s, &files);
     s.browse = Some(Browse { path: dir_s.clone(), subfolders });
     s.source = LibrarySource::Folder;

crates/engine/src/cmd/library.rs

@@ -82,7 +82,7 @@ pub fn import_params(s: &Session, p: &Value) -> Result<ImportRequest> {
     }
     let on_deleted = match str_param(p, "onDeleted") {
         Some(v) => {
-            crate::import::OnDeleted::parse(v).ok_or_else(|| bad("library.import", format!("unknown onDeleted `{v}` (skip|restore|fresh)")))?
+            crate::import::OnDeleted::parse(v).ok_or_else(|| bad("library.import", format!("unknown onDeleted `{v}` (restore|skip|fresh)")))?
         }
         None => Default::default(),
     };
@@ -1246,7 +1246,9 @@ pub fn specs() -> Vec<CommandSpec> {
                     return Ok(json!({"imported": [], "folder": folder}));
                 }
                 let mode = if s.import_defaults.auto_copy { "copy" } else { "add" };
-                let mut params = json!({"paths": fresh, "mode": mode});
+                // a photo deleted from the library doesn't come back on its own because its file
+                // is still in the watched folder (a copy's source is not a known path)
+                let mut params = json!({"paths": fresh, "mode": mode, "onDeleted": "skip"});
                 if let Some(a) = s.import_defaults.auto_album.clone() {
                     match s.catalog.albums().find(|al| al.name.eq_ignore_ascii_case(&a) && !al.folder && !al.is_smart()) {
                         Some(al) => params["album"] = json!(al.id.0),
@@ -1326,7 +1328,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Import Photos",
             ["File"],
             Some("Cmd+Shift+I"),
-            "{paths: [file or folder (recursive)], mode?: add|copy|move (add = reference the files in place; copy = into the library's Originals/YYYY/YYYY-MM-DD/; move = as copy, then each original and its XMP sidecars are removed from the source — only after the copy is verified (a hard link on the same volume, else copied, synced and compared byte for byte) and its catalog record is saved; failed, duplicate and unchecked files keep their sources; a taken name gets -1, -2…; undo removes the photos from the library but leaves the files at the destination), destination?: folder for copies / moves, organize?: date (YYYY/YYYY-MM-DD) | month (YYYY/YYYY-MM) | flat | a folder template, e.g. `{date:%Y}/{date:%Y%m%d}` → 2026/20260114 (the template's `/` make the folders, each level expanded with the rename tokens and made a safe folder name: never outside the destination; must be relative, no `..`; a level with missing metadata is `unknown`) — dated by capture time, else the import time, rename?: file-name template for copies, original extension added (tokens: {name} {num} {seq} {seq:N} {date} {date:%Y%m%d} {folder} {camera} {lens} {iso} {rating} {title} {creator} {ext}; photo.renameTokens explains each; blank = keep names), renameStart?: 1, metadataPreset?: name, dng?: bool (copy raws as DNG; copy only), local?: bool (browsing: the photos stay out of the library, like library.browse; not with move), album?: albumId, albumName?: new album, preset?: presetId, keywords?: [..], onDeleted?: skip|restore|fresh (a file that is in Recently Deleted: skip = leave it there and say so, restore = bring the photo back with its edits, fresh = delete the trashed record and import the file as new)} → {imported, duplicates: [{path, existing, reason, existingDeleted?}], restored?: [photoId], failed, moved?: [{from, to, sidecars?}], kept?: [{path, reason}] (move: sources left in place and why), album?}",
+            "{paths: [file or folder (recursive)], mode?: add|copy|move (add = reference the files in place; copy = into the library's Originals/YYYY/YYYY-MM-DD/; move = as copy, then each original and its XMP sidecars are removed from the source — only after the copy is verified (a hard link on the same volume, else copied, synced and compared byte for byte) and its catalog record is saved; failed, duplicate and unchecked files keep their sources; a taken name gets -1, -2…; undo removes the photos from the library but leaves the files at the destination), destination?: folder for copies / moves, organize?: date (YYYY/YYYY-MM-DD) | month (YYYY/YYYY-MM) | flat | a folder template, e.g. `{date:%Y}/{date:%Y%m%d}` → 2026/20260114 (the template's `/` make the folders, each level expanded with the rename tokens and made a safe folder name: never outside the destination; must be relative, no `..`; a level with missing metadata is `unknown`) — dated by capture time, else the import time, rename?: file-name template for copies, original extension added (tokens: {name} {num} {seq} {seq:N} {date} {date:%Y%m%d} {folder} {camera} {lens} {iso} {rating} {title} {creator} {ext}; photo.renameTokens explains each; blank = keep names), renameStart?: 1, metadataPreset?: name, dng?: bool (copy raws as DNG; copy only), local?: bool (browsing: the photos stay out of the library, like library.browse; not with move), album?: albumId, albumName?: new album, preset?: presetId, keywords?: [..], onDeleted?: restore|skip|fresh (a file that is in Recently Deleted: restore (default) = bring the photo back with its edits — relinked to the file when its own is gone, skip = leave it there and say so, fresh = delete the trashed record and import the file as new)} → {imported, duplicates: [{path, existing, reason, existingDeleted?}], restored?: [photoId], failed, moved?: [{from, to, sidecars?}], kept?: [{path, reason}] (move: sources left in place and why), album?}",
             always,
             |s, p| {
                 let req = import_params(s, p)?;

crates/engine/src/import.rs

@@ -6,6 +6,8 @@
 //! 3. Probe the rest — dimensions, metadata, and the **content hash** of the bytes — in parallel
 //!    on native targets.
 //! 4. Skip **duplicates by content** (same bytes already in the library, or twice in this batch).
+//!    A duplicate (by path or content) of a photo in Recently Deleted is restored instead, with
+//!    its edits, unless the import says otherwise ([`OnDeleted`]).
 //! 5. *Add* in place (the photo points at the original file), *copy* into the library's
 //!    `Originals/YYYY/YYYY-MM-DD/` folder (names made unique) and point at the copy, or *move*
 //!    there: placed like a copy (with its XMP sidecars), and the original removed only once its
@@ -53,12 +55,19 @@ pub enum ImportMode {
 }
 
 /// What an import does with a file the library already has in Recently Deleted.
+///
+/// Someone who imports such a file wants the photo in the library again, so the default brings
+/// it back (issues #298, #706: before, the import skipped it as "already in the library" while
+/// the grid showed it deleted, and the user was stuck). Restoring loses nothing — the edits,
+/// rating and albums come back with the photo and Reset Edits gives a clean start — where
+/// importing afresh would silently drop them.
 #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
 pub enum OnDeleted {
     /// Leave it there; the report says so ([`Duplicate::existing_deleted`]).
-    #[default]
     Skip,
-    /// Bring the trashed photo back, with its edits ([`ImportReport::restored`]).
+    /// Bring the trashed photo back, with its edits ([`ImportReport::restored`]); a photo whose
+    /// file is no longer where the library had it is relinked to the imported file.
+    #[default]
     Restore,
     /// Delete the trashed record permanently and import the file as a new photo.
     Fresh,
@@ -78,7 +87,7 @@ impl OnDeleted {
 /// What an import does besides adding the photos.
 #[derive(Clone, Debug, Default)]
 pub struct ImportOptions {
-    /// A file that is in Recently Deleted: skip (default), restore, or import afresh.
+    /// A file that is in Recently Deleted: restore (default), skip, or import afresh.
     pub on_deleted: OnDeleted,
     pub mode: ImportMode,
     /// Applied to every imported photo (one History entry).
@@ -1170,6 +1179,16 @@ fn run_transfer(t: Transfer, file_bytes: Option<&crate::merge::ByteReader>) -> P
     PreparedItem::Ready(Box::new(ReadyFile { path, stored, info, sidecar, placed }))
 }
 
+/// A trashed photo's file is not where the library had it (never on the web, where paths are
+/// storage keys, not files).
+fn trashed_file_is_gone(cat: &lightcraft_catalog::Catalog, id: PhotoId) -> bool {
+    !cfg!(target_arch = "wasm32")
+        && match cat.photo(id).map(|p| &p.source) {
+            Some(Source::File { path }) => !Path::new(path).exists(),
+            _ => false,
+        }
+}
+
 /// The catalog half of an import: add the photos [`ImportJob::prepare`] readied (one undoable op;
 /// a Move's sources are removed once it is saved). On a failed commit a Move's placed files are
 /// taken back.
@@ -1210,9 +1229,15 @@ pub fn commit_prepared(s: &mut Session, opts: &ImportOptions, now: &str, prepare
                 });
                 let trashed = existing.filter(|id| s.catalog.photo(*id).is_some_and(|p| p.deleted));
                 match trashed {
-                    Some(id) if opts.on_deleted == OnDeleted::Restore => {
+                    // (browsing a folder in Local only looks: it never restores)
+                    Some(id) if opts.on_deleted == OnDeleted::Restore && !opts.local => {
                         if !report.restored.contains(&id.0) {
                             ops.push(Op::SetDeleted { id, deleted: false });
+                            // matched by content from another path, and the file the record
+                            // points at is gone (moved, renamed): the photo follows the file
+                            if reason == "content" && trashed_file_is_gone(&s.catalog, id) {
+                                ops.push(crate::cmd::missing::relink_op(id, &path));
+                            }
                             report.restored.push(id.0);
                         }
                     }

crates/engine/src/lightroom_job.rs

@@ -141,7 +141,8 @@ pub struct LightroomJob {
 impl LightroomJob {
     /// Snapshot session state only.  This constructor performs no filesystem work.
     pub fn new(s: &mut crate::Session, path: PathBuf, update_existing: bool) -> crate::Result<Self> {
-        let opts = ImportOptions { mode: ImportMode::Add, ..ImportOptions::default() };
+        // a photo deleted here stays deleted: the catalog's record is matched, not restored
+        let opts = ImportOptions { mode: ImportMode::Add, on_deleted: crate::import::OnDeleted::Skip, ..ImportOptions::default() };
         let import = ImportJob::new(s, opts)?;
         let archive_dir = s.library.as_ref().filter(|library| library.on_disk).map(|library| library.dir.join("Interop"));
         Ok(Self { path, update_existing, import, token: LightroomLibraryToken::capture(s), archive_dir })
@@ -242,7 +243,7 @@ pub fn commit_prepared(s: &mut crate::Session, mut prepared: PreparedLightroom)
 
     let undo0 = s.undo.len();
     prepared.normal.revalidate_add(s);
-    let options = ImportOptions { mode: ImportMode::Add, ..ImportOptions::default() };
+    let options = ImportOptions { mode: ImportMode::Add, on_deleted: crate::import::OnDeleted::Skip, ..ImportOptions::default() };
     let now = prepared.now.clone();
     let files = crate::import::commit_prepared(s, &options, &now, prepared.normal)?;
     let applied = lightroom_catalog::apply_prepared(

crates/engine/src/tests_import.rs

@@ -870,8 +870,8 @@ fn missing_photos_skip_local_browse_records() {
     assert_eq!(s.visible_cloned().len(), 4, "the view lists what the count counts");
 }
 
-/// Import a file that is in Recently Deleted (issue #298): by default it is skipped but the
-/// report says it is in the trash; `onDeleted: restore` brings it back with its edits;
+/// Import a file that is in Recently Deleted (issues #298, #706): by default the photo comes
+/// back with its edits; `onDeleted: skip` leaves it there but the report says it is in the trash;
 /// `onDeleted: fresh` replaces the trashed record with a new one.
 fn trashed_photo(tag: &str) -> (Session, std::path::PathBuf, u64) {
     let src = temp_dir(tag);
@@ -884,14 +884,115 @@ fn trashed_photo(tag: &str) -> (Session, std::path::PathBuf, u64) {
     (s, src, id)
 }
 
+/// Issue #706: import a photo, delete it, import the same file again. Before, the import skipped
+/// it as "already in the library" (a duplicate by path) while the grid showed it deleted, and the
+/// user was stuck. Now the photo is restored: it is back in All Photos, with its edits.
+#[test]
+fn importing_a_deleted_photo_again_brings_it_back() {
+    let (mut s, src, id) = trashed_photo("trash-reimport");
+    assert_eq!(s.visible_cloned().len(), 0, "deleted: not in All Photos");
+    let r = s.execute("library.import", &json!({"paths": [src.join("a.png").to_string_lossy()]})).unwrap();
+    assert_eq!(r["restored"], json!([id]), "{r}");
+    assert_eq!(ids(&r, "duplicates"), 0, "not reported as a duplicate: {r}");
+    let p = s.catalog.photo(lightcraft_catalog::PhotoId(id)).unwrap();
+    assert!(!p.deleted && p.rating == 4, "back with its edits");
+    assert_eq!(s.catalog.len(), 1, "no second record");
+    assert_eq!(s.visible_cloned(), vec![lightcraft_catalog::PhotoId(id)], "shown again in All Photos");
+    // undo puts it back in Recently Deleted
+    s.execute("edit.undo", &json!({})).unwrap();
+    assert!(s.catalog.photo(lightcraft_catalog::PhotoId(id)).unwrap().deleted);
+    let _ = std::fs::remove_dir_all(&src);
+}
+
+/// The same sequence with the library on disk and a restart between the delete and the import:
+/// the trashed record is read back from the catalog and restored, not refused.
+#[test]
+fn importing_a_deleted_photo_again_after_reopening_the_library_brings_it_back() {
+    let src = temp_dir("trash-reimport-reopen");
+    write_png(&src.join("a.png"), 1);
+    let lib = src.join("library");
+    let id = {
+        let mut s = Session::new().with_fs();
+        s.open_library(&lib, false).unwrap();
+        let r = s.execute("library.import", &json!({"paths": [src.join("a.png").to_string_lossy()], "albumName": "Trip"})).unwrap();
+        let id = r["imported"][0].as_u64().unwrap();
+        s.execute("photo.rate", &json!({"ids": [id], "rating": 2})).unwrap();
+        s.execute("photo.delete", &json!({"ids": [id]})).unwrap();
+        s.close_library().unwrap();
+        id
+    };
+    let mut s = Session::new().with_fs();
+    s.open_library(&lib, false).unwrap();
+    assert!(s.catalog.photo(lightcraft_catalog::PhotoId(id)).unwrap().deleted, "deleted state persisted");
+    let r = s.execute("library.import", &json!({"paths": [src.join("a.png").to_string_lossy()]})).unwrap();
+    assert_eq!(r["restored"], json!([id]), "{r}");
+    assert_eq!(ids(&r, "duplicates"), 0, "{r}");
+    let p = s.catalog.photo(lightcraft_catalog::PhotoId(id)).unwrap();
+    assert!(!p.deleted && p.rating == 2, "restored with its edits");
+    let album = s.catalog.albums().find(|a| a.name == "Trip").unwrap();
+    assert_eq!(album.photos, vec![lightcraft_catalog::PhotoId(id)], "still in its album");
+    assert_eq!(s.visible_cloned().len(), 1);
+    let _ = std::fs::remove_dir_all(&src);
+}
+
+/// A deleted photo whose file was moved meanwhile: importing the file from its new place restores
+/// the photo and points it at the file (matched by content), instead of bringing back a record
+/// whose file is missing.
+#[test]
+fn importing_a_deleted_photo_from_its_new_place_restores_and_relinks_it() {
+    let (mut s, src, id) = trashed_photo("trash-reimport-moved");
+    std::fs::create_dir_all(src.join("moved")).unwrap();
+    std::fs::rename(src.join("a.png"), src.join("moved/b.png")).unwrap();
+    let r = s.execute("library.import", &json!({"paths": [src.join("moved/b.png").to_string_lossy()]})).unwrap();
+    assert_eq!(r["restored"], json!([id]), "{r}");
+    let p = s.catalog.photo(lightcraft_catalog::PhotoId(id)).unwrap();
+    assert!(!p.deleted && p.rating == 4);
+    assert_eq!(p.source, lightcraft_catalog::Source::File { path: src.join("moved/b.png").to_string_lossy().to_string() });
+    assert_eq!(p.file_name, "b.png");
+    assert_eq!(s.catalog.len(), 1);
+    let _ = std::fs::remove_dir_all(&src);
+}
+
+/// Only an import brings a deleted photo back. Looking at its folder in Local, and Auto Import
+/// seeing its file again in the watched folder (a copy: the source is not a path the library
+/// knows), leave it in Recently Deleted.
+#[test]
+fn browsing_and_auto_import_never_restore_a_deleted_photo() {
+    let (mut s, src, id) = trashed_photo("trash-no-silent-restore");
+    let r = s.execute("library.browse", &json!({"path": src.to_string_lossy()})).unwrap();
+    assert_eq!(r["new"], 0, "{r}");
+    assert!(s.catalog.photo(lightcraft_catalog::PhotoId(id)).unwrap().deleted, "browsing only looks");
+    assert_eq!(s.catalog.len(), 1);
+    // auto import in copy mode: the trashed photo's copy is in the library's folder, so the watched
+    // folder's file is new by path and a duplicate by content
+    let lib = src.join("library");
+    let watched = src.join("watched");
+    write_png(&watched.join("w.png"), 7);
+    let mut s = Session::new().with_fs();
+    s.open_library(&lib, false).unwrap();
+    let r = s.execute("library.import", &json!({"paths": [watched.join("w.png").to_string_lossy()], "mode": "copy"})).unwrap();
+    let copied = r["imported"][0].as_u64().unwrap();
+    s.execute("photo.delete", &json!({"ids": [copied]})).unwrap();
+    s.execute("library.autoImport", &json!({"folder": watched.to_string_lossy(), "copy": true})).unwrap();
+    for _ in 0..3 {
+        let r = s.execute("library.autoImportScan", &json!({})).unwrap();
+        assert_eq!(r["imported"], json!([]), "{r}");
+    }
+    assert!(s.catalog.photo(lightcraft_catalog::PhotoId(copied)).unwrap().deleted, "still deleted");
+    assert_eq!(s.catalog.len(), 1, "no second copy");
+    let _ = std::fs::remove_dir_all(&src);
+}
+
+/// `onDeleted: skip` keeps the old behaviour for callers that want it: nothing imported, the
+/// duplicate reported with `existingDeleted`.
 #[test]
 fn duplicate_in_recently_deleted_is_reported_as_such() {
     let (mut s, src, id) = trashed_photo("trash-report");
-    let r = s.execute("library.import", &json!({"paths": [src.to_string_lossy()]})).unwrap();
+    let r = s.execute("library.import", &json!({"paths": [src.to_string_lossy()], "onDeleted": "skip"})).unwrap();
     assert_eq!(ids(&r, "imported"), 0, "{r}");
     assert_eq!(r["duplicates"][0]["existing"], id, "{r}");
     assert_eq!(r["duplicates"][0]["existingDeleted"], true, "{r}");
-    assert!(s.catalog.photo(lightcraft_catalog::PhotoId(id)).unwrap().deleted, "default: left in the trash");
+    assert!(s.catalog.photo(lightcraft_catalog::PhotoId(id)).unwrap().deleted, "skip: left in the trash");
     let _ = std::fs::remove_dir_all(&src);
 }