Import: a deferred review never replaces a running import

A finished scan's review that waits for a dialog to close counts as an
import in progress: dropped photos and another scan are refused
meanwhile. Confirming a review while an import (or a Local folder
browse scan, which ends in one) runs is refused too, since replacing
it would stop its worker.
This commit is contained in:
aiongg
2026-10-24 03:34:40 +00:00
parent 9c9f87e589
commit dc406df9b4
2 changed files with 40 additions and 4 deletions
+13 -4
View File
@@ -480,6 +480,9 @@ pub fn open(app: &mut LightcraftApp, paths: Vec<String>) -> Result<Value, String
if app.scan.is_some() {
return Err("a scan is already running".into());
}
if app.review_pending.is_some() {
return Err("an import review is waiting for the open dialog to close".into());
}
let sources = paths.clone();
let (input, paths) = ScanInput::new(&mut app.session, &paths);
let progress = std::sync::Arc::new(ScanProgress::default());
@@ -677,10 +680,11 @@ pub(crate) fn stall_scan_of(app: &mut LightcraftApp, browse: bool) -> Box<dyn st
Box::new(tx)
}
/// Whether an import or any scan (the review's, or a Local folder browse) is running, which a drop of
/// photos must not replace: `open` refuses while a scan runs, and a browse that finishes replaces `app.import`.
/// Whether an import or any scan (the review's, or a Local folder browse) is running, or a finished scan's
/// review waits for a dialog to close, which a drop of photos must not replace: `open` refuses while a scan
/// runs, and a browse that finishes replaces `app.import`.
pub fn running(app: &LightcraftApp) -> bool {
app.import.is_some() || app.scan.is_some()
app.import.is_some() || app.scan.is_some() || app.review_pending.is_some()
}
impl ScanTask {
@@ -690,12 +694,17 @@ impl ScanTask {
}
}
/// Start importing the dialog's checked files (the dialog's OK / `ui.dialog.confirm`).
/// Start importing the dialog's checked files (the dialog's OK / `ui.dialog.confirm`). Refused while another
/// import runs: replacing `app.import` would stop its worker.
pub fn start(app: &mut LightcraftApp, d: &ImportDialog) -> Result<Value, String> {
busy_synchronizing(app)?;
if !d.in_library(app) {
return Err("another library was opened since the photos were scanned: nothing was imported".into());
}
// a browse scan that finishes starts its own import, replacing this one
if app.import.is_some() || app.scan.as_ref().is_some_and(|t| t.browse) {
return Err(crate::i18n::tr("An import is running").to_string());
}
let queue = d.selected_paths();
if queue.is_empty() {
return Err("no photos selected".into());
+27
View File
@@ -193,6 +193,33 @@ fn a_waiting_review_is_dropped_when_another_library_opens() {
assert_eq!(photos(&h), 0);
}
/// A review waiting behind another dialog counts as an import in progress: a direct drop meanwhile is
/// refused, and the review's OK never replaces an import that runs when it is confirmed.
#[test]
fn a_waiting_review_never_replaces_a_running_import() {
let dir = Scratch::new("pending-running");
let mut h = app(&dir);
h.app.ui.settings.import_dropped_directly = true;
write_png(&dir.path("more/c.png"), 3);
crate::import::open(&mut h.app, vec![dir.path("trip")]).unwrap();
h.app.ui.dialog = Some(Dialog::WhatsNew);
assert!(h.step_until(SETTLE, |h| h.app.scan.is_none()), "the scan ends");
h.step();
assert!(h.app.review_pending.is_some(), "the review waits behind the dialog");
assert!(crate::import::start_paths(&mut h.app, vec![dir.path("more")]).is_err(), "no second import starts");
assert!(h.app.import.is_none());
h.app.ui.dialog = None;
h.step();
let Some(review) = h.app.ui.dialog.clone() else { panic!("the review opens once the dialog is closed") };
assert!(matches!(review, Dialog::Import { .. }));
// an import that started anyway (a Local folder browse, say) is the one that runs
h.app.import = Some(crate::import::ImportTask::new(vec![dir.path("more")], json!({"mode": "add"}), 0, false));
assert!(crate::panels::dialogs::confirm_dialog(&mut h.app, &review).is_err(), "the review's OK is refused");
assert_eq!(h.app.import.as_ref().map(|t| t.total), Some(1), "the running import stays");
assert!(h.step_until(SETTLE, |h| h.app.import.is_none()), "it runs to the end");
assert_eq!(photos(&h), 1, "only the running import's photo arrives");
}
/// A drop while an import runs would open a review whose OK replaces the running import: it is
/// refused instead, and the card says so.
#[test]