Merge pull request #646 from marcoarnulfo/fix/web-open-dirty-session

Ask before a session opened in the browser replaces a modified one

apps/soundcraft-web/src/web.rs

@@ -1,6 +1,5 @@
 //! The browser shell: the eframe web runner around [`SoundApp`].
 
-use serde_json::json;
 use soundcraft_engine::{Engine, io};
 use soundcraft_ui_egui::dialogs::Dialog;
 use soundcraft_ui_egui::sampler_window::SampleRead;
@@ -166,30 +165,23 @@ impl WebShell {
         }
     }
 
-    fn handle(&mut self, purpose: Purpose, name: &str, bytes: &[u8]) {
-        if matches!(purpose, Purpose::Session) && soundcraft_ui_egui::bounce::refuses_session_switch(&mut self.app) {
-            return;
-        }
+    fn handle(&mut self, purpose: Purpose, name: String, bytes: Vec<u8>) {
         let e = &mut self.app.engine;
         let at = e.session().edit.selection.start.max(0);
         let result = match purpose {
-            Purpose::Session => {
-                let _ = self.app.run("transport.stop", json!({}));
-                io::open_session_bytes(&mut self.app.engine, name, bytes).map(|missing| {
-                    if missing.is_empty() { format!("Opened {name}") } else { format!("Opened {name} ({} missing media)", missing.len()) }
-                })
-            }
+            // Asks first when the open session has changes: the bytes wait for the answer.
+            Purpose::Session => return soundcraft_ui_egui::dialogs::open_session_bytes(&mut self.app, name, bytes),
             // Imports are edits: one undo step each, like the import commands.
             Purpose::Audio => {
                 let before = e.session_arc();
-                io::import_audio_bytes(e, name, bytes, None, None, at).map(|_| {
+                io::import_audio_bytes(e, &name, &bytes, None, None, at).map(|_| {
                     e.push_undo("Import Audio", before);
                     format!("Imported {name}")
                 })
             }
             Purpose::Midi => {
                 let before = e.session_arc();
-                io::import_midi_bytes(e, bytes, at, true).map(|_| {
+                io::import_midi_bytes(e, &bytes, at, true).map(|_| {
                     e.push_undo("Import MIDI", before);
                     format!("Imported {name}")
                 })
@@ -205,7 +197,7 @@ impl eframe::App for WebShell {
         self.take_path_prompt(ctx);
         let pending = std::mem::take(&mut *self.inbox.borrow_mut());
         for (purpose, name, bytes) in pending {
-            self.handle(purpose, &name, &bytes);
+            self.handle(purpose, name, bytes);
         }
         let samples = std::mem::take(&mut *self.samples.borrow_mut());
         for (read, bytes) in samples {

crates/ui-egui/src/dialogs.rs

@@ -34,6 +34,8 @@ pub struct Dialogs {
     pub open: Option<Dialog>,
     /// Runs once the session has been saved from `SaveChanges`.
     after_save: Option<(String, Value)>,
+    /// A session the browser read while the open one had changes, waiting for `SaveChanges`.
+    pending_session: Option<(String, Vec<u8>)>,
 }
 
 impl Dialogs {
@@ -155,7 +157,7 @@ impl Dialogs {
 /// When `id` would close a session with unsaved changes, ask first; true when the question is
 /// open instead. A take in progress is stopped first, so it counts as a change.
 pub fn ask_to_save(app: &mut SoundApp, id: &str, params: &Value) -> bool {
-    if !matches!(id, "session.new" | "session.open" | "session.close" | "session.revert" | "app.quit")
+    if !matches!(id, "session.new" | "session.open" | "session.close" | "session.revert" | "app.quit" | OPEN_PENDING)
         || soundcraft_engine::find_command(id).is_some_and(|c| (c.enabled)(&app.engine).is_err())
     {
         return false;
@@ -178,6 +180,34 @@ pub fn run_or_ask(app: &mut SoundApp, id: &str, params: Value) -> Result<Value,
     app.run(id, params)
 }
 
+/// What `SaveChanges` carries on with to open the session the browser read.
+const OPEN_PENDING: &str = "ui.open_pending_session";
+
+/// Opens a session the browser read as bytes (picked or dropped), asking first when the open one
+/// has changes. The read finishes later than the pick, so the question is asked only now.
+pub fn open_session_bytes(app: &mut SoundApp, name: String, bytes: Vec<u8>) {
+    if crate::bounce::refuses_session_switch(app) {
+        return;
+    }
+    app.dialogs.pending_session = Some((name, bytes));
+    if !ask_to_save(app, OPEN_PENDING, &json!({})) {
+        open_pending_session(app);
+    }
+}
+
+fn open_pending_session(app: &mut SoundApp) {
+    let Some((name, bytes)) = app.dialogs.pending_session.take() else { return };
+    if crate::bounce::refuses_session_switch(app) {
+        return;
+    }
+    let _ = app.run("transport.stop", json!({}));
+    app.ui.status = match soundcraft_engine::io::open_session_bytes(&mut app.engine, &name, &bytes) {
+        Ok(missing) if missing.is_empty() => format!("Opened {name}"),
+        Ok(missing) => format!("Opened {name} ({} missing media)", missing.len()),
+        Err(e) => e.to_string(),
+    };
+}
+
 /// A path prompt for any command whose parameters start with a required `path` or `dir` (plus
 /// Print Score, whose path is optional), with a default file name from the command.
 pub fn takes_path(id: &str) -> bool {
@@ -594,6 +624,7 @@ pub fn show(app: &mut SoundApp, ctx: &egui::Context) {
     if esc || ctx.memory(|m| m.data.get_temp::<bool>(egui::Id::new("dlg_cancel")).unwrap_or(false)) {
         keep = false;
         app.dialogs.after_save = None;
+        app.dialogs.pending_session = None;
         ctx.memory_mut(|m| m.data.remove::<bool>(egui::Id::new("dlg_cancel")));
     }
     if let Some(then) = save_first {
@@ -613,7 +644,7 @@ pub fn show(app: &mut SoundApp, ctx: &egui::Context) {
     let saving = action.as_ref().is_some_and(|(cmd, _)| cmd.starts_with("session.save"));
     if let Some((cmd, params)) = action {
         keep = false;
-        match app.run(&cmd, params) {
+        match run_then(app, &cmd, params) {
             Ok(v) if cmd == "file.export_clips" => {
                 app.ui.status = format!("Exported {} clip(s) to {}", v["written"], v["dir"].as_str().unwrap_or_default());
             }
@@ -624,7 +655,7 @@ pub fn show(app: &mut SoundApp, ctx: &egui::Context) {
     if saving
         && let Some((then, params)) = app.dialogs.after_save.take()
         && !app.engine.is_dirty()
-        && let Err(e) = app.run(&then, params)
+        && let Err(e) = run_then(app, &then, params)
     {
         app.ui.status = e;
     }
@@ -633,6 +664,15 @@ pub fn show(app: &mut SoundApp, ctx: &egui::Context) {
     }
 }
 
+/// Runs a dialog's command, or what `SaveChanges` carries on with.
+fn run_then(app: &mut SoundApp, cmd: &str, params: Value) -> Result<Value, String> {
+    if cmd == OPEN_PENDING {
+        open_pending_session(app);
+        return Ok(Value::Null);
+    }
+    app.run(cmd, params)
+}
+
 /// Cancel/OK row; returns true when OK was pressed (or Enter).
 fn buttons(ui: &mut egui::Ui, ok: &str, enter: bool) -> bool {
     let mut pressed = enter;

crates/ui-egui/tests/save_changes.rs

@@ -2,7 +2,7 @@ use egui_kittest::Harness;
 use egui_kittest::kittest::Queryable;
 use serde_json::json;
 use soundcraft_ui_egui::dialogs::Dialog;
-use soundcraft_ui_egui::{Services, SoundApp, menus};
+use soundcraft_ui_egui::{Services, SoundApp, dialogs, menus};
 
 fn harness() -> Harness<'static, SoundApp> {
     let app = SoundApp::new(soundcraft_engine::demo::demo_engine(), None, Services::default());
@@ -95,3 +95,59 @@ fn save_in_the_question_saves_before_closing() {
     assert!(!h.state().engine.session().track_by_name("Kick").unwrap().mixer.mute);
     let _ = std::fs::remove_dir_all(&dir);
 }
+
+/// A session the browser read: an empty one, so opening it is easy to see.
+fn session_bytes() -> Vec<u8> {
+    soundcraft_engine::Engine::default().session().to_json().unwrap().into_bytes()
+}
+
+fn open_bytes(h: &mut Harness<'_, SoundApp>) {
+    dialogs::open_session_bytes(h.state_mut(), "Other.scraft".into(), session_bytes());
+    steps(h);
+}
+
+#[test]
+fn a_session_read_in_the_browser_asks_before_replacing_a_modified_one() {
+    let mut h = harness();
+    open_bytes(&mut h);
+    assert!(h.state().dialogs.open.is_none(), "an unmodified session is replaced without asking");
+    assert!(h.state().engine.session().tracks.is_empty());
+
+    let mut h = harness();
+    let tracks = h.state().engine.session().tracks.len();
+    mute_kick(&mut h);
+    open_bytes(&mut h);
+    assert_eq!(h.state().dialogs.open_name(), Some("save_changes"));
+    click(&mut h, "Cancel");
+    assert!(h.state().dialogs.open.is_none());
+    assert_eq!(h.state().engine.session().tracks.len(), tracks);
+    h.state_mut().run("edit.undo", json!({})).unwrap();
+    assert!(!h.state().engine.session().track_by_name("Kick").unwrap().mixer.mute, "Cancel keeps the undo history");
+
+    mute_kick(&mut h);
+    open_bytes(&mut h);
+    click(&mut h, "Don't Save");
+    assert!(h.state().engine.session().tracks.is_empty());
+}
+
+#[test]
+fn save_in_the_question_saves_before_opening_a_session_read_in_the_browser() {
+    let dir = std::env::temp_dir().join(format!("soundcraft-save-changes-bytes-{}", std::process::id()));
+    let _ = std::fs::remove_dir_all(&dir);
+    let path = dir.join("Song.scraft").to_string_lossy().into_owned();
+
+    let mut h = harness();
+    mute_kick(&mut h);
+    open_bytes(&mut h);
+    click(&mut h, "Save");
+    match &mut h.state_mut().dialogs.open {
+        Some(Dialog::PathPrompt { cmd, path: p, .. }) if cmd == "session.save_as" => p.clone_from(&path),
+        other => panic!("expected Save As, got {other:?}"),
+    }
+    steps(&mut h);
+    click(&mut h, "OK");
+    assert!(h.state().engine.session().tracks.is_empty());
+    h.state_mut().run("session.open", json!({"path": path})).unwrap();
+    assert!(h.state().engine.session().track_by_name("Kick").unwrap().mixer.mute);
+    let _ = std::fs::remove_dir_all(&dir);
+}