Reuse Slides: insert from the bytes that were listed; a failed insert keeps the dialog open (#1103)

Review on #1109: the file is read once when the list is made and the same bytes
are inserted, so a file changed meanwhile can't swap the slides; `reuse_slides`
returns whether it inserted and the dialog closes only then. A kittest drives the
dialog.

Co-Authored-By: Claude <noreply@anthropic.com>

crates/ui-egui/src/dialogs.rs

@@ -1623,9 +1623,11 @@ fn body(app: &mut SlideApp, ui: &mut Ui, d: &mut Dialog) -> bool {
                     app.set_status("Tick at least one slide to insert".to_string());
                     return false;
                 }
-                app.reuse_slides(&path, keep, &picked);
+                // A failed insert keeps the dialog (and the ticked slides) open; the error is in the status bar.
+                let source = d.params.get("source").cloned().unwrap_or_default();
+                return app.reuse_slides(&source, keep, &picked);
             }
-            ok || cancel
+            cancel
         }
         "renameSection" | "renameShape" | "renameLayout" => {
             let cur = d.params.get("name").and_then(Value::as_str).unwrap_or("").to_string();

crates/ui-egui/src/lib.rs

@@ -1270,22 +1270,47 @@ impl SlideApp {
             return;
         };
         let Some(path) = pick("open") else { return };
+        // Read the file once: the list shown and the slides inserted come from the same bytes, even
+        // if the file changes while the dialog is open. Without a reader the path is used as is.
+        let mut source = json!({});
+        match self.services.read.as_ref().map(|r| r(&path)) {
+            Some(Ok(bytes)) => {
+                let name = std::path::Path::new(&path).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_default();
+                source["name"] = json!(name);
+                source["data"] = json!(deckcraft_engine::cmd::base64_encode(&bytes));
+            }
+            Some(Err(e)) => {
+                self.set_status(e.to_string());
+                return;
+            }
+            None => source["path"] = json!(path),
+        }
         // Offer the file's slides to choose from; all of them are ticked to begin with.
-        match self.session.execute("slide.reuseList", &json!({"path": path})) {
+        match self.session.execute("slide.reuseList", &source) {
             Ok(list) => {
                 let mut d = dialogs::Dialog::new("reuseSlides");
-                d.params = json!({"path": path, "keep": keep, "slides": list["slides"]});
+                d.params = json!({"path": path, "source": source, "keep": keep, "slides": list["slides"]});
                 self.dialog = Some(d);
             }
             Err(e) => self.set_status(e.to_string()),
         }
     }
 
-    /// Insert the slides `picked` (indices) of the presentation at `path` after the current slide.
-    pub fn reuse_slides(&mut self, path: &str, keep: bool, picked: &[usize]) {
-        match self.session.execute("slide.reuse", &json!({"path": path, "keepFormatting": keep, "slides": picked})) {
-            Ok(r) => self.set_status(format!("Inserted {} slides from {path}", r["slides"])),
-            Err(e) => self.set_status(e.to_string()),
+    /// Insert the slides `picked` (indices) of `source` (the `name` + `data`, or `path`, that
+    /// `slide.reuseList` listed) after the current slide. Returns whether they were inserted.
+    pub fn reuse_slides(&mut self, source: &Value, keep: bool, picked: &[usize]) -> bool {
+        let mut args = if source.is_object() { source.clone() } else { json!({}) };
+        args["keepFormatting"] = json!(keep);
+        args["slides"] = json!(picked);
+        match self.session.execute("slide.reuse", &args) {
+            Ok(r) => {
+                self.set_status(format!("Inserted {} slides", r["slides"]));
+                true
+            }
+            Err(e) => {
+                self.set_status(e.to_string());
+                false
+            }
         }
     }
 
@@ -2130,6 +2155,8 @@ mod last_color_tests;
 #[cfg(test)]
 mod replacefonts_tests;
 #[cfg(test)]
+mod reuse_picker_tests;
+#[cfg(test)]
 mod slide_size_tests;
 #[cfg(test)]
 mod texture_limit_tests;

crates/ui-egui/src/reuse_picker_tests.rs

@@ -0,0 +1,107 @@
+//! Home › New Slide › Reuse Slides…: the dialog lists the slides of the picked file and inserts the
+//! ticked ones; what is inserted is what was listed, and a failed insert keeps the dialog open.
+
+use std::cell::RefCell;
+use std::rc::Rc;
+
+use deckcraft_engine::Session;
+use egui_kittest::Harness;
+use egui_kittest::kittest::Queryable;
+use serde_json::json;
+
+use crate::{Services, SlideApp};
+
+fn deck_bytes(titles: &[&str]) -> Vec<u8> {
+    let mut s = Session::with_new();
+    for t in titles {
+        s.execute("slide.new", &json!({"layout": "titleAndContent", "title": t})).unwrap();
+    }
+    // Drop the new deck's empty first slide, so slide `i` is `titles[i]`.
+    s.execute("slide.delete", &json!({"index": 0})).unwrap();
+    deckcraft_engine::cmd::file::save_bytes(&s.doc().unwrap().doc, "deckcraft").unwrap()
+}
+
+/// An app whose open dialog picks `src.deckcraft`, read from the shared `file` bytes.
+fn harness(file: &Rc<RefCell<Vec<u8>>>) -> Harness<'static, SlideApp> {
+    let mut s = Session::with_new();
+    s.execute("slide.new", &json!({"layout": "blank"})).unwrap();
+    let f = Rc::clone(file);
+    let services = Services {
+        pick_open: Some(Box::new(|_| Some("/tmp/src.deckcraft".to_string()))),
+        read: Some(Box::new(move |_| Ok(f.borrow().clone()))),
+        ..Services::default()
+    };
+    let app = SlideApp::new(s, services);
+    let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).with_step_dt(1.0 / 60.0).build_ui_state(
+        |ui, app: &mut SlideApp| {
+            app.logic(ui.ctx());
+            app.ui(ui);
+        },
+        app,
+    );
+    h.run_steps(3);
+    h
+}
+
+fn titles(h: &Harness<'_, SlideApp>) -> Vec<String> {
+    h.state().session.active().unwrap().doc.slides.iter().map(|s| s.title()).collect()
+}
+
+fn open_dialog(h: &mut Harness<'_, SlideApp>) {
+    h.state_mut().run("app.reuseSlidesDialog", json!({})).unwrap();
+    h.run_steps(3);
+    assert_eq!(h.state().dialog.as_ref().map(|d| d.id.as_str()), Some("reuseSlides"));
+}
+
+fn press_insert(h: &mut Harness<'_, SlideApp>) {
+    // The ribbon has an Insert tab too; the dialog's button is drawn last.
+    h.query_all_by_label("Insert").last().expect("the dialog's Insert button").click();
+    h.run_steps(3);
+}
+
+#[test]
+fn inserts_the_ticked_slides_and_closes() {
+    let file = Rc::new(RefCell::new(deck_bytes(&["Alpha", "Beta", "Gamma"])));
+    let mut h = harness(&file);
+    open_dialog(&mut h);
+    // Untick the middle slide.
+    h.state_mut().dialog.as_mut().unwrap().fields.insert("off1".into(), "1".into());
+    h.run_steps(2);
+    press_insert(&mut h);
+    assert!(h.state().dialog.is_none());
+    let t = titles(&h);
+    assert!(t.iter().any(|x| x == "Alpha") && t.iter().any(|x| x == "Gamma") && !t.iter().any(|x| x == "Beta"), "{t:?}");
+}
+
+#[test]
+fn inserts_what_was_listed_even_if_the_file_changed_meanwhile() {
+    let file = Rc::new(RefCell::new(deck_bytes(&["Alpha", "Beta"])));
+    let mut h = harness(&file);
+    open_dialog(&mut h);
+    // Someone replaces the file while the dialog is open: slide 0 is now a different one.
+    *file.borrow_mut() = deck_bytes(&["Other", "Beta"]);
+    h.state_mut().dialog.as_mut().unwrap().fields.insert("off1".into(), "1".into());
+    h.run_steps(2);
+    press_insert(&mut h);
+    let t = titles(&h);
+    assert!(t.iter().any(|x| x == "Alpha"), "{t:?}");
+    assert!(!t.iter().any(|x| x == "Other"), "{t:?}");
+}
+
+#[test]
+fn a_failed_insert_keeps_the_dialog_open() {
+    let file = Rc::new(RefCell::new(deck_bytes(&["Alpha"])));
+    let mut h = harness(&file);
+    open_dialog(&mut h);
+    let before = titles(&h);
+    // Offer a slide the file doesn't have, so the insert is refused.
+    if let Some(d) = h.state_mut().dialog.as_mut()
+        && let Some(a) = d.params["slides"].as_array_mut()
+    {
+        a.push(json!({"index": 9, "title": "Ghost", "hidden": false}));
+    }
+    h.run_steps(2);
+    press_insert(&mut h);
+    assert_eq!(h.state().dialog.as_ref().map(|d| d.id.as_str()), Some("reuseSlides"), "the dialog closed on a failed insert");
+    assert_eq!(titles(&h), before);
+}