Import review: choose what happens to files in Recently Deleted (#298)

Files whose photo is in the trash stay unchecked until Restore them or Import as
new is chosen; the toast and undo label say what was restored.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
Nathaniel Ledford 2026-10-08 11:39:28 -04:00
parent 6c19090fde
commit 0b5ef3fe11
No known key found for this signature in database
3 changed files with 147 additions and 8 deletions

View File

@ -1358,6 +1358,71 @@ mod tests {
let _ = std::fs::remove_dir_all(&dir);
}
/// The import review offers what to do with files that are in Recently Deleted (issue #298):
/// they are unchecked until Restore or Import as new is chosen; then confirming restores the
/// photo (with its edits) or imports the file afresh.
#[test]
fn import_review_offers_recently_deleted_files() {
for (choice, tag) in [("restore", "r"), ("fresh", "f")] {
let dir = std::env::temp_dir().join(format!("lc-ui-import-trash-{tag}-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(&dir).unwrap();
for i in 0..2u8 {
let data: Vec<[u8; 4]> = (0..40 * 30).map(|k| [(k % 40 * 6) as u8, i * 90, 7, 255]).collect();
let img = lightcraft_raster::Rgba8 { width: 40, height: 30, data };
let png =
lightcraft_codecs::encode_png(&lightcraft_codecs::EncodeImage::rgba8(&img), &lightcraft_codecs::EncodeMeta::default()).unwrap();
std::fs::write(dir.join(format!("img{i}.png")), png).unwrap();
}
let services = crate::Services { png: None, ..Default::default() };
let mut app = LightcraftApp::new(lightcraft_engine::Session::new().with_fs(), services);
app.ui.view = crate::state::ViewMode::PhotoGrid;
let mut h = Headless::new(app, [1300.0, 900.0], 1.0);
let t = Duration::from_secs(10);
let r =
h.request("engine.execute", json!({"command": "library.import", "params": {"paths": [dir.join("img0.png").to_string_lossy()]}}), t);
let id = r["result"]["imported"][0].as_u64().unwrap();
h.request("engine.execute", json!({"command": "photo.rate", "params": {"ids": [id], "rating": 3}}), t);
h.request("engine.execute", json!({"command": "photo.delete", "params": {"ids": [id]}}), t);
h.request("engine.execute", json!({"command": "file.addPhotos", "params": {"paths": [dir.to_string_lossy()]}}), t);
h.step_until(SETTLE, |h| matches!(h.app.ui.dialog, Some(crate::state::Dialog::Import { .. })));
let Some(crate::state::Dialog::Import { opts }) = &mut h.app.ui.dialog else { panic!("no import review") };
assert_eq!(opts.selected_paths().len(), 1, "only the new file: the trashed one waits for a choice");
opts.set_on_deleted(choice);
assert_eq!(opts.selected_paths().len(), 2, "the trashed file is checked once a choice is made");
// a per-cell choice survives switching between the two options, and "Leave them" unchecks
let trashed = (0..opts.candidates.len()).find(|i| opts.is_trashed(*i)).unwrap();
opts.checked[trashed] = false;
opts.set_on_deleted(if choice == "restore" { "fresh" } else { "restore" });
assert_eq!(opts.selected_paths().len(), 1, "still unchecked");
opts.set_on_deleted("");
assert_eq!(opts.selected_paths().len(), 1);
opts.set_on_deleted(choice);
assert_eq!(opts.selected_paths().len(), 2);
let r = h.request("ui.dialog.confirm", json!({}), t);
assert_eq!(r["ok"], true, "{r}");
// frames until the import has finished, so its (timed) toast is read before it expires
let t0 = std::time::Instant::now();
while h.app.import.is_some() && t0.elapsed() < Duration::from_secs(30) {
h.step();
}
assert!(h.app.import.is_none(), "finished");
let toast = h.app.ui.toast.clone().map(|t| t.0).unwrap_or_default();
let photos = &h.app.session.catalog;
assert_eq!(photos.photos().filter(|p| !p.deleted).count(), 2, "{choice}");
let old = photos.photo(lightcraft_catalog::PhotoId(id));
if choice == "restore" {
assert!(toast.contains("1 restored"), "{toast}");
assert!(old.is_some_and(|p| !p.deleted && p.rating == 3), "restored with its edits");
assert_eq!(photos.len(), 2);
} else {
assert!(old.is_none(), "the trashed record is gone");
assert_eq!(photos.len(), 2);
}
let _ = std::fs::remove_dir_all(&dir);
}
}
/// Adding a file again that is in Recently Deleted shows it there (side panel opened, photo
/// selected) instead of only saying "duplicate skipped"; its menus offer Restore, and once
/// restored a re-add selects it in All Photos.

View File

@ -53,6 +53,10 @@ pub struct ImportDialog {
pub metadata_preset: String,
/// Copy: raws are copied as DNG.
pub dng: bool,
/// Per candidate: the library already has it in Recently Deleted.
pub trashed: Vec<bool>,
/// What to do with those: "" (leave them), `restore` or `fresh` (the `onDeleted` param).
pub on_deleted: String,
}
impl ImportDialog {
@ -61,14 +65,33 @@ impl ImportDialog {
ImportDialog { candidates, checked, ..Default::default() }
}
pub fn importable(&self, i: usize) -> bool {
self.candidates.get(i).is_some_and(|c| c.duplicate.is_none() && c.error.is_none())
self.candidates.get(i).is_some_and(|c| c.error.is_none() && (c.duplicate.is_none() || self.is_trashed(i) && !self.on_deleted.is_empty()))
}
/// Candidate `i` is a file whose photo is in Recently Deleted.
pub fn is_trashed(&self, i: usize) -> bool {
self.trashed.get(i).copied().unwrap_or(false)
}
/// Choose what happens to the files in Recently Deleted (`""`, `restore` or `fresh`); they are
/// checked once there is a choice and unchecked again without one.
pub fn set_on_deleted(&mut self, choice: &str) {
// switching between restore and fresh keeps the user's per-cell choices
let changed = self.on_deleted.is_empty() != choice.is_empty();
self.on_deleted = choice.to_string();
for i in (0..self.candidates.len()).filter(|_| changed) {
if self.is_trashed(i)
&& let Some(c) = self.checked.get_mut(i)
{
*c = !choice.is_empty();
}
}
}
pub fn selected_paths(&self) -> Vec<String> {
self.candidates
.iter()
.zip(&self.checked)
.filter(|(c, on)| **on && c.duplicate.is_none() && c.error.is_none())
.map(|(c, _)| c.path.clone())
.enumerate()
.filter(|(i, (_, on))| **on && self.importable(*i))
.map(|(_, (c, _))| c.path.clone())
.collect()
}
/// The `organize` param of `library.import` (`None` = the default, by day); an unusable
@ -104,6 +127,8 @@ pub struct ImportTask {
pub done: usize,
params: Value,
pub imported: usize,
/// Brought back from Recently Deleted (`onDeleted: restore`).
pub restored: usize,
pub duplicates: usize,
/// The library photos the skipped duplicates match (shown when nothing new was added).
existing: Vec<u64>,
@ -400,6 +425,14 @@ pub fn poll_scan(app: &mut LightcraftApp, ctx: &egui::Context) {
}
app.renderer.forget_imports();
let mut d = ImportDialog::new(out.candidates);
d.trashed = d
.candidates
.iter()
.map(|c| {
c.duplicate.is_some()
&& c.existing.is_some_and(|id| app.session.catalog.photo(lightcraft_catalog::PhotoId(id)).is_some_and(|p| p.deleted))
})
.collect();
d.copy = task.copy;
d.sources = task.sources;
app.ui.dialog = Some(crate::state::Dialog::Import { opts: Box::new(d) });
@ -461,6 +494,9 @@ pub fn start(app: &mut LightcraftApp, d: &ImportDialog) -> Result<Value, String>
(true, true) => "move",
};
let mut params = json!({"mode": mode, "keywords": d.keywords()});
if !d.on_deleted.is_empty() {
params["onDeleted"] = json!(d.on_deleted);
}
if let Some(a) = album {
params["album"] = json!(a);
}
@ -564,6 +600,7 @@ fn commit_batch(app: &mut LightcraftApp, task: &mut ImportTask, prepared: lightc
}
let len = |k: &str| v[k].as_array().map_or(0, Vec::len);
task.imported += len("imported");
task.restored += len("restored");
task.duplicates += len("duplicates");
task.existing.extend(v["duplicates"].as_array().into_iter().flatten().filter_map(|d| d["existing"].as_u64()));
task.failed += len("failed");
@ -573,7 +610,7 @@ fn commit_batch(app: &mut LightcraftApp, task: &mut ImportTask, prepared: lightc
log::warn!("import: kept {}: {}", k["path"].as_str().unwrap_or(""), k["reason"].as_str().unwrap_or(""));
}
if task.first.is_none() {
task.first = v["imported"].as_array().and_then(|a| a.first()).and_then(Value::as_u64);
task.first = ["imported", "restored"].iter().find_map(|k| v[*k].as_array().and_then(|a| a.first()).and_then(Value::as_u64));
}
}
Err(e) => {
@ -586,7 +623,14 @@ fn commit_batch(app: &mut LightcraftApp, task: &mut ImportTask, prepared: lightc
/// The import is done (or cancelled): one undo step, select the first photo, say what happened.
fn finish(app: &mut LightcraftApp, ctx: &egui::Context, task: ImportTask) {
let steps = app.session.undo.len().saturating_sub(task.undo0);
let label = crate::i18n::tr_format!("Add {} Photo{}", task.imported, if task.imported == 1 { "" } else { "s" });
let plural = |n: usize| if n == 1 { "" } else { "s" };
let label = if task.imported == 0 && task.restored > 0 {
format!("Restore {} Photo{}", task.restored, plural(task.restored))
} else if task.restored > 0 {
format!("Add {} Photo{}, restore {}", task.imported, plural(task.imported), task.restored)
} else {
crate::i18n::tr_format!("Add {} Photo{}", task.imported, if task.imported == 1 { "" } else { "s" })
};
app.session.merge_undo(steps, &label);
if task.auto {
if task.imported > 0 {
@ -606,7 +650,7 @@ fn finish(app: &mut LightcraftApp, ctx: &egui::Context, task: ImportTask) {
let plural = |n: usize| if n == 1 { "" } else { "s" };
// nothing new, only files the library already has: show where they are (Recently Deleted is
// easy to miss, and the side panel that lists it starts collapsed)
if task.imported == 0 && task.failed == 0 && !task.cancelled && show_existing(app, ctx, &task.existing) {
if task.imported == 0 && task.restored == 0 && task.failed == 0 && !task.cancelled && show_existing(app, ctx, &task.existing) {
return;
}
let mut msg = if task.cancelled {
@ -614,6 +658,9 @@ fn finish(app: &mut LightcraftApp, ctx: &egui::Context, task: ImportTask) {
} else {
crate::i18n::tr_format!("Imported {} photo{}", task.imported, plural(task.imported))
};
if task.restored > 0 {
msg.push_str(&format!(" · {} restored from Recently Deleted", task.restored));
}
if task.params["mode"] == "move" {
msg.push_str(&crate::i18n::tr_format!(" · {} moved", task.moved));
if task.kept > 0 {
@ -895,6 +942,32 @@ pub fn body(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &mut ImportDialog) {
);
}
}
if d.trashed.iter().any(|t| *t) {
field(ui, "In Recently Deleted", |ui| {
let cur = match d.on_deleted.as_str() {
"restore" => "Restore them",
"fresh" => "Import as new",
_ => "Leave them",
};
let mut pick = None;
egui::ComboBox::from_id_salt("import-on-deleted").selected_text(crate::i18n::tr(cur)).show_ui(ui, |ui| {
for (v, label) in [("", "Leave them"), ("restore", "Restore them"), ("fresh", "Import as new")] {
if ui.selectable_label(d.on_deleted == v, crate::i18n::tr(label)).clicked() {
pick = Some(v);
}
}
});
if let Some(v) = pick {
d.set_on_deleted(v);
}
let t = Tokens::get(ui.ctx());
ui.label(
egui::RichText::new(crate::i18n::tr("Restore keeps their edits; Import as new deletes the old record first."))
.color(t.text_dim)
.small(),
);
});
}
let albums: Vec<(u64, String)> = {
let mut v: Vec<(u64, String)> =
app.session.catalog.albums().filter(|a| !a.folder && !a.is_smart()).map(|a| (a.id.0, a.name.clone())).collect();
@ -1000,6 +1073,7 @@ fn candidate_cell(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &mut ImportDial
}
let in_trash = c.existing.is_some_and(|id| app.session.catalog.photo(lightcraft_catalog::PhotoId(id)).is_some_and(|p| p.deleted));
let badge = match (&c.duplicate, &c.error) {
(Some(_), _) if in_trash && ok => Some(if d.on_deleted == "restore" { "Will be restored" } else { "Will be replaced" }),
(Some(_), _) if in_trash => Some("In Recently Deleted"),
(Some(r), _) if r == "path" => Some("In library"),
(Some(_), _) => Some("Duplicate"),
@ -1117,7 +1191,7 @@ pub const DEFAULT_FOLDER_TEMPLATE: &str = "{date:%Y}/{date:%Y%m%d}";
/// Where the first selected photo would be copied to (destination, folders, name), for the
/// dialog's example line; `None` without a photo or with an unusable folder template.
pub fn example_destination(app: &LightcraftApp, d: &ImportDialog) -> Option<String> {
let c = d.candidates.iter().zip(&d.checked).find(|(c, on)| **on && c.duplicate.is_none() && c.error.is_none()).map(|(c, _)| c)?;
let c = d.candidates.iter().zip(&d.checked).enumerate().find(|(i, (_, on))| **on && d.importable(*i)).map(|(_, (c, _))| c)?;
let organize = match d.organize_param().ok()? {
Some(o) => lightcraft_engine::import::Organize::parse(&o)?,
None => Default::default(),

View File

@ -150,7 +150,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins.
| LR-LIB-PEOPLE | People / faces | P2 | 🟡 | `cmd:view.people`, `cmd:view.faceBoxes`, `cmd:photo.removeRegion`, `cmd:photo.setRegion`, `cmd:library.filter`, `crates/ui-egui/src/panels/people.rs`, `crates/catalog/src/query.rs` (`people`), `crates/engine/src/media.rs` (`face_job`), `crates/meta/src/xmp.rs` | MVP: named face regions read from XMP (MWG-RS, read-only, see `docs/xmp-interop.md`; clipped to the photo; Lightroom's `Rotation` of ±π/2 and π handled, mirrored orientations taken as written; boxes follow Rotate Left/Right and flips; a sidecar stating `mwg-rs:Regions`, even empty, replaces the photo's on re-read, one without leaves them) become cards in the People view (View ▸ People, or the People button in the toolbar): a close-up of the person's largest face, name and photo count; a click filters the grid (`person` in `library.filter`, `person:` search token, filter chip). Face boxes in the loupe can be switched off (View ▸ Face Boxes, toolbar) resized with eight drag handles and removed with the × shown on hover (catalog-only and undoable, one undo step per drag; the sidecar is never rewritten for either); no moving a box by dragging it yet. No face detection, no unnamed people / suggestions, no manual tagging, no writing regions back |
| LR-LIB-STACK | Stacks | P1 | ✅ | `cmd:stack.group`, `cmd:stack.ungroup`, `cmd:stack.toggle`, `cmd:stack.setTop`, `cmd:stack.remove`, `cmd:stack.auto`, `crates/catalog/src/stacks.rs` | grid/filmstrip count badges, expand/collapse, auto-stack by capture time; no visual-similarity auto-stack |
| LR-LIB-VERSIONS | Versions | P1 | ✅ | `cmd:version.create` | see section M |
| LR-LIB-DELETE | Delete / Recently Deleted | P0 | ✅ | `cmd:photo.delete`, `cmd:photo.restore`, `cmd:photo.deletePermanently` | Restore and Delete Permanently in the Photo menu and the photo context menus for photos in Recently Deleted (before, the UI had no way to restore); adding a file that is in Recently Deleted again selects it there (side panel opened) and says how to restore it or import it afresh; Photo ▸ Empty Recently Deleted (`cmd:library.emptyRecentlyDeleted`, one undo step, shown while the trash is the source); import takes `onDeleted` skip / restore / fresh for files in the trash (`cmd:library.import`; the report flags `existingDeleted` and lists `restored`; the desktop import dialog still uses skip and points at Restore); no confirmation dialog, no auto-purge after N days (needs a deleted-at date in the catalog) |
| LR-LIB-DELETE | Delete / Recently Deleted | P0 | ✅ | `cmd:photo.delete`, `cmd:photo.restore`, `cmd:photo.deletePermanently` | Restore and Delete Permanently in the Photo menu and the photo context menus for photos in Recently Deleted (before, the UI had no way to restore); adding a file that is in Recently Deleted again selects it there (side panel opened) and says how to restore it or import it afresh; Photo ▸ Empty Recently Deleted (`cmd:library.emptyRecentlyDeleted`, one undo step, shown while the trash is the source); import takes `onDeleted` skip / restore / fresh for files in the trash (`cmd:library.import`; the report flags `existingDeleted` and lists `restored`; the import review dialog offers Leave them / Restore them / Import as new for files that are in the trash; dropped files and Add Photos without a review still skip and point at Restore); no confirmation dialog, no auto-purge after N days (needs a deleted-at date in the catalog) |
| LR-LIB-REMOVE-ALBUM | Remove from album | P0 | ✅ | `cmd:album.removePhotos` | |
| LR-LIB-DUPLICATE | Duplicate a photo | P2 | ✅ | `cmd:photo.duplicate` | Photo ▸ Duplicate: a real `-copy` file with the same settings, metadata and albums (virtual copies share the file) |
| LR-LIB-RENAME | Batch rename | P1 | ✅ | `cmd:photo.rename`, `cmd:photo.renamePreview`, `cmd:photo.renameTokens`, `cmd:dialog.rename`, `crates/engine/src/rename.rs` (`TOKENS`) | template tokens {name} {num} {seq:N} {date:%Y%m%d} {folder} {camera} {lens} {iso} {rating} {title} {creator} {ext} (shared with export naming and import rename); a Tags picker beside every template field (Rename Photos, import Rename, export File name) lists each tag with meaning + live example, the date directives and the rules, and inserts the clicked tag at the text cursor; unknown tags are flagged; preview; renames files on disk with their XMP sidecars (a stem sidecar another file still shares, e.g. raw + JPEG, is copied rather than taken), never overwriting (-1, -2… suffixes; a change of letter case only is checked by file identity, so on case-sensitive volumes `img_1.jpg` next to `IMG_1.JPG` is a collision), rolls back on failure (a file that can't be moved back is reported old → new and the library follows it as an undoable partial rename; the same for undo/redo); undo/redo move the files; virtual copies follow |