Modify Group: edit a group's name, tracks and Edit/Mix linking

A track group's name, membership and Edit/Mix flags could only be set
when it was created: nothing in the engine or the UI could change them
afterwards, so a typo in a group name was permanent.

Add track.group_modify {group: id|name, name?, tracks?, edit?, mix?}.
Omitted fields keep their value. `tracks` replaces membership only when
it is an array, so a mistyped param cannot silently swap the members for
whatever happens to be selected; it dedupes, rejects an empty group, and
rejects an empty or duplicate name, since find_group resolves by name.
The group's letter is left alone: it is the stable id in the Groups list.

Dialog::Group gains an optional group id. None creates, as before; Some
pre-fills from the group and dispatches track.group_modify, so one dialog
serves both. Reached from Modify Group... in the Groups list context menu.
OK is now disabled while no track is checked, since both commands reject
an empty group and the old OK could only raise an error.

The command has no menu path, so it adds no menu leaf and the parity
checklist is unchanged.

crates/engine/src/cmd/track.rs

@@ -19,6 +19,7 @@ pub fn specs() -> Vec<CommandSpec> {
         cmd!("track.group", "Group...", ["Track"], Some("Cmd+G"), "{name?, tracks?, edit?: true, mix?: true}", has_selection, group),
         cmd!("track.ungroup", "Delete Group", [], None, "{group: id|name}", always, delete_group),
         cmd!("track.group_toggle", "Toggle Group Active", [], None, "{group: id|name}", always, toggle_group),
+        cmd!("track.group_modify", "Modify Group", [], None, "{group: id|name, name?, tracks?, edit?, mix?}", always, modify_group),
         cmd!(
             "track.duplicate",
             "Duplicate...",
@@ -320,6 +321,55 @@ fn toggle_group(e: &mut Engine, p: &Value) -> Result<Value> {
     Ok(json!({"active": active}))
 }
 
+/// Change an existing group's name, membership and Edit/Mix linking. Omitted fields keep their value.
+fn modify_group(e: &mut Engine, p: &Value) -> Result<Value> {
+    const CMD: &str = "track.group_modify";
+    let g = find_group(e, p, CMD)?;
+    // Absent `tracks` leaves membership alone; present, it replaces it. Only an array counts, so a
+    // mistyped param can't silently swap the members for whatever happens to be selected.
+    let members = match p.get("tracks") {
+        None => None,
+        Some(Value::Array(_)) => {
+            let mut t = tracks_param(e, CMD, p)?;
+            // A track listed twice would take every linked move twice over.
+            let mut seen = std::collections::HashSet::new();
+            t.retain(|id| seen.insert(*id));
+            if t.is_empty() {
+                return Err(bad(CMD, "a group needs at least one track"));
+            }
+            Some(t)
+        }
+        Some(_) => return Err(bad(CMD, "`tracks` must be an array of track ids or names")),
+    };
+    let name = match str_param(p, "name") {
+        None => None,
+        Some(n) => {
+            let n = n.trim();
+            if n.is_empty() {
+                return Err(bad(CMD, "`name` must not be empty"));
+            }
+            // `find_group` resolves by name, so duplicates would make a group unaddressable.
+            if e.session().groups.iter().any(|x| x.name == n && x.id != g) {
+                return Err(bad(CMD, format!("a group named `{n}` already exists")));
+            }
+            let mut n = n.to_string();
+            soundcraft_model::truncate_chars(&mut n, soundcraft_model::MAX_NAME_CHARS);
+            Some(n)
+        }
+    };
+    let s = e.session_mut();
+    let Some(gr) = s.groups.iter_mut().find(|x| x.id == g) else { return Err(bad(CMD, "unknown `group`")) };
+    if let Some(n) = name {
+        gr.name = n;
+    }
+    if let Some(m) = members {
+        gr.members = m;
+    }
+    gr.edit = bool_or(p, "edit", gr.edit);
+    gr.mix = bool_or(p, "mix", gr.mix);
+    Ok(json!({"group": g, "name": gr.name, "tracks": gr.members, "edit": gr.edit, "mix": gr.mix}))
+}
+
 fn duplicate(e: &mut Engine, p: &Value) -> Result<Value> {
     let tracks = tracks_required(e, "track.duplicate", p)?;
     let (pl, auto, ins, snd) =
@@ -872,6 +922,35 @@ mod tests {
         assert!(e.execute("track.playlist_promote", &json!({"track": kick, "playlist": 1, "start": 0, "end": 10})).is_err());
     }
 
+    #[test]
+    fn group_modify_changes_name_membership_and_linking() {
+        let mut e = crate::demo::demo_engine();
+        let g = e.execute("track.group", &json!({"tracks": ["Kick", "Snare"], "name": "Drums"})).unwrap()["group"].as_u64().unwrap();
+        // Every field at once, the way the dialog submits it.
+        let r = e.execute("track.group_modify", &json!({"group": g, "name": "Rhythm", "tracks": ["Kick", "Bass"], "edit": false})).unwrap();
+        assert_eq!(r["name"], "Rhythm");
+        let gr = e.session().groups.iter().find(|x| x.id.0 == g).unwrap().clone();
+        assert_eq!(gr.name, "Rhythm");
+        assert_eq!(gr.members, vec![e.session().track_by_name("Kick").unwrap().id, e.session().track_by_name("Bass").unwrap().id]);
+        assert!(!gr.edit);
+        assert!(gr.mix, "an omitted field keeps its value");
+        assert_eq!(gr.letter, 'a', "the letter is the group's stable id and never changes");
+        // Omitting `tracks` leaves membership alone rather than falling back to the selection.
+        e.execute("edit.select", &json!({"tracks": ["Pad"], "exact": true})).unwrap();
+        e.execute("track.group_modify", &json!({"group": g, "name": "Rhythm Section"})).unwrap();
+        assert_eq!(e.session().groups.iter().find(|x| x.id.0 == g).unwrap().members, gr.members);
+        // Resolving the group by its new name keeps working, so names stay unique and non-empty.
+        e.execute("track.group", &json!({"tracks": ["Pad"], "name": "Pads"})).unwrap();
+        assert!(e.execute("track.group_modify", &json!({"group": "Pads", "name": "Rhythm Section"})).is_err());
+        assert!(e.execute("track.group_modify", &json!({"group": g, "name": "  "})).is_err());
+        assert!(e.execute("track.group_modify", &json!({"group": g, "tracks": []})).is_err());
+        assert!(e.execute("track.group_modify", &json!({"group": g, "tracks": "Kick"})).is_err(), "`tracks` must be an array");
+        assert!(e.execute("track.group_modify", &json!({"group": g, "tracks": ["Kick", "Kick"]})).is_ok());
+        assert_eq!(e.session().groups.iter().find(|x| x.id.0 == g).unwrap().members.len(), 1, "a repeated track is listed once");
+        assert!(e.execute("track.group_modify", &json!({"group": 9999, "name": "x"})).is_err());
+        assert_eq!(e.session().groups.iter().find(|x| x.id.0 == g).unwrap().name, "Rhythm Section", "a rejected call changes nothing");
+    }
+
     #[test]
     fn folder_routes_members_through_its_bus() {
         let mut e = crate::demo::demo_engine();

crates/ui-egui/src/dialogs.rs

@@ -5,7 +5,7 @@ use crate::theme::{Tokens, bold};
 use egui::{Align2, vec2};
 use serde_json::{Value, json};
 use soundcraft_engine::Engine;
-use soundcraft_model::{ClipId, Session, SourceId, TrackId};
+use soundcraft_model::{ClipId, GroupId, Session, SourceId, TrackId};
 use soundcraft_time::Samples;
 
 #[derive(Debug, Clone)]
@@ -23,7 +23,8 @@ pub enum Dialog {
     Spot(crate::spot::Spot),
     Fades { shape: String },
     StripSilence { threshold: f64, min_ms: f64, pre_ms: f64, post_ms: f64 },
-    Group { name: String, edit: bool, mix: bool, members: Vec<(u64, String, bool)> },
+    // `id: None` creates (Track › Group…); `id: Some` modifies that group (Groups list › Modify Group…).
+    Group { id: Option<GroupId>, name: String, edit: bool, mix: bool, members: Vec<(u64, String, bool)> },
     Session { frame_rate: String, bit_depth: String },
     ScoreSetup { title: String, composer: String, bars_per_system: u32, show_track_names: bool },
     SaveChanges { then: (String, Value) },
@@ -52,7 +53,8 @@ impl Dialogs {
             Dialog::Spot(_) => "spot",
             Dialog::Fades { .. } => "fades",
             Dialog::StripSilence { .. } => "strip_silence",
-            Dialog::Group { .. } => "group",
+            Dialog::Group { id: None, .. } => "group",
+            Dialog::Group { id: Some(_), .. } => "modify_group",
             Dialog::Session { .. } => "session",
             Dialog::ScoreSetup { .. } => "score_setup",
             Dialog::SaveChanges { .. } => "save_changes",
@@ -71,6 +73,16 @@ impl Dialogs {
         self.open = Some(Dialog::RenameMarker { id, name: name.to_string() });
     }
 
+    /// Groups list › Modify Group…: the creation dialog, pre-filled from an existing group. False
+    /// when the group is gone (deleted between the click and the menu).
+    pub fn open_modify_group(&mut self, e: &Engine, id: GroupId) -> bool {
+        let s = e.session();
+        let Some(g) = s.group(id) else { return false };
+        let members = s.tracks.iter().map(|t| (t.id.0, t.name.clone(), g.members.contains(&t.id))).collect();
+        self.open = Some(Dialog::Group { id: Some(id), name: g.name.clone(), edit: g.edit, mix: g.mix, members });
+        true
+    }
+
     /// Clip List › Clear: confirm before clips and files leave the session. False when there is nothing to clear.
     pub fn open_clear(&mut self, e: &Engine, clips: Vec<ClipId>, sources: Vec<SourceId>) -> bool {
         let Some(message) = clear_message(e.session(), &clips, &sources) else { return false };
@@ -131,7 +143,7 @@ impl Dialogs {
             "track.group" => {
                 let s = e.session();
                 let members = s.tracks.iter().map(|t| (t.id.0, t.name.clone(), s.edit.selected_tracks.contains(&t.id))).collect();
-                Dialog::Group { name: format!("Group {}", s.groups.len() + 1), edit: true, mix: true, members }
+                Dialog::Group { id: None, name: format!("Group {}", s.groups.len() + 1), edit: true, mix: true, members }
             }
             "setup.session" => Dialog::Session { frame_rate: e.session().frame_rate.label().into(), bit_depth: "24".into() },
             "file.score_setup" => {
@@ -283,7 +295,8 @@ pub fn show(app: &mut SoundApp, ctx: &egui::Context) {
         Dialog::Spot(_) => "Spot",
         Dialog::Fades { .. } => "Fades",
         Dialog::StripSilence { .. } => "Strip Silence",
-        Dialog::Group { .. } => "Create Group",
+        Dialog::Group { id: None, .. } => "Create Group",
+        Dialog::Group { id: Some(_), .. } => "Modify Group",
         Dialog::Session { .. } => "Session Setup",
         Dialog::ScoreSetup { .. } => "Score Setup",
         Dialog::SaveChanges { then } if then.0 == "session.revert" => "Revert to Saved",
@@ -508,7 +521,7 @@ pub fn show(app: &mut SoundApp, ctx: &egui::Context) {
                         ));
                     }
                 }
-                Dialog::Group { name, edit, mix, members } => {
+                Dialog::Group { id, name, edit, mix, members } => {
                     ui.horizontal(|ui| {
                         ui.label("Name");
                         ui.text_edit_singleline(name);
@@ -523,9 +536,18 @@ pub fn show(app: &mut SoundApp, ctx: &egui::Context) {
                             crate::widgets::checkbox(ui, on, n.as_str());
                         }
                     });
-                    if buttons(ui, "OK", enter) {
-                        let ids: Vec<u64> = members.iter().filter(|m| m.2).map(|m| m.0).collect();
-                        action = Some(("track.group".into(), json!({"name": name, "edit": edit, "mix": mix, "tracks": ids})));
+                    let ids: Vec<u64> = members.iter().filter(|m| m.2).map(|m| m.0).collect();
+                    // Both commands reject an empty group, so don't offer an OK that can only fail.
+                    if buttons_enabled(ui, "OK", enter, !ids.is_empty()) {
+                        let mut params = json!({"name": name, "edit": edit, "mix": mix, "tracks": ids});
+                        let cmd = match id {
+                            Some(g) => {
+                                params["group"] = json!(g.0);
+                                "track.group_modify"
+                            }
+                            None => "track.group",
+                        };
+                        action = Some((cmd.into(), params));
                     }
                 }
                 Dialog::Session { frame_rate, bit_depth } => {
@@ -635,13 +657,19 @@ pub fn show(app: &mut SoundApp, ctx: &egui::Context) {
 
 /// 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;
+    buttons_enabled(ui, ok, enter, true)
+}
+
+/// `buttons`, but a greyed-out OK when the dialog's values can't be submitted yet. Cancel always works.
+fn buttons_enabled(ui: &mut egui::Ui, ok: &str, enter: bool, ready: bool) -> bool {
+    let mut pressed = enter && ready;
     ui.separator();
     ui.horizontal(|ui| {
         if ui.button("Cancel").clicked() {
             ui.ctx().memory_mut(|m| m.data.insert_temp(egui::Id::new("dlg_cancel"), true));
         }
-        if ui.add(egui::Button::new(egui::RichText::new(ok).strong()).fill(Tokens::current().accent)).clicked() {
+        let b = egui::Button::new(egui::RichText::new(ok).strong()).fill(Tokens::current().accent);
+        if ui.add_enabled(ready, b).clicked() {
             pressed = true;
         }
     });

crates/ui-egui/src/panels.rs

@@ -133,6 +133,10 @@ pub fn tracks_and_groups(app: &mut SoundApp, ui: &mut Ui) {
                     let _ = app.run("track.group_toggle", json!({"group": id}));
                     ui.close();
                 }
+                if ui.button("Modify Group…").clicked() {
+                    let _ = app.dialogs.open_modify_group(&app.engine, soundcraft_model::GroupId(id));
+                    ui.close();
+                }
                 if ui.button("Delete Group").clicked() {
                     let _ = app.run("track.ungroup", json!({"group": id}));
                     ui.close();

crates/ui-egui/tests/group_dialog.rs

@@ -42,3 +42,44 @@ fn dragging_down_the_group_dialogs_track_list_adds_each_track() {
     let members: Vec<u64> = session.groups.last().map(|g| g.members.iter().map(|m| m.0).collect()).unwrap_or_default();
     assert_eq!(members, tracks[..4].iter().map(|t| t.0).collect::<Vec<_>>());
 }
+
+/// The Groups list's Modify Group… reopens the creation dialog on an existing group: it comes up
+/// pre-filled, and OK edits that group in place instead of adding a second one.
+#[test]
+fn modify_group_dialog_edits_the_existing_group() {
+    let mut app = SoundApp::new(soundcraft_engine::demo::demo_engine(), None, Services::default());
+    let tracks: Vec<(u64, String)> = app.engine.session().tracks.iter().map(|t| (t.id.0, t.name.clone())).collect();
+    assert!(tracks.len() >= 2);
+    let g = app.run("track.group", json!({"tracks": [tracks[0].0], "name": "Drums"})).unwrap()["group"].as_u64().unwrap();
+
+    assert!(app.dialogs.open_modify_group(&app.engine, soundcraft_model::GroupId(g)));
+    assert_eq!(app.dialogs.open_name(), Some("modify_group"));
+    // Pre-filled from the group, not from the track selection.
+    let Some(soundcraft_ui_egui::dialogs::Dialog::Group { id, name, edit, mix, members }) = &app.dialogs.open else {
+        panic!("Modify Group… did not open the group dialog");
+    };
+    assert_eq!((id.map(|i| i.0), name.as_str(), *edit, *mix), (Some(g), "Drums", true, true));
+    assert_eq!(members.iter().filter(|m| m.2).map(|m| m.0).collect::<Vec<_>>(), vec![tracks[0].0]);
+
+    let mut h = Harness::builder().with_size(vec2(1600.0, 1000.0)).build_ui_state(
+        |ui, app: &mut SoundApp| {
+            let ctx = ui.ctx().clone();
+            app.logic(&ctx);
+            app.ui(ui);
+        },
+        app,
+    );
+    h.run_steps(3);
+    // Add a second track and submit.
+    h.get_by_role_and_label(Role::CheckBox, &tracks[1].1).click();
+    h.run_steps(2);
+    h.get_by_role_and_label(Role::Button, "OK").click();
+    h.run_steps(2);
+
+    let session = h.state().engine.session();
+    assert_eq!(session.groups.len(), 1, "Modify edits the group in place rather than creating another");
+    let gr = &session.groups[0];
+    assert_eq!(gr.id.0, g);
+    assert_eq!(gr.name, "Drums");
+    assert_eq!(gr.members.iter().map(|m| m.0).collect::<Vec<_>>(), vec![tracks[0].0, tracks[1].0]);
+}