diff --git a/src/app/automation.rs b/src/app/automation.rs index b5ff3f74d..1c71bc55d 100644 --- a/src/app/automation.rs +++ b/src/app/automation.rs @@ -541,6 +541,116 @@ mod tests { ); } + #[test] + fn start_page_runs_tools_that_need_no_drawing_but_still_refuses_the_rest() { + // The welcome page's own buttons (Donate / Send Feedback / OCS Web) and + // Manage > About route through RibbonToolClick, so a blanket is_start + // refusal killed them outright — by definition they are only ever + // clickable while is_start holds. `dispatch_command` owns the list of + // commands that stand alone; this door must not shadow it. (#388, #389) + use crate::app::Message; + use crate::modules::ModuleEvent; + + // Fresh app = welcome tab, no drawing. + let mut app = OpenCADStudio::new_for_test(); + assert!( + app.tabs[app.active_tab].is_start, + "test needs the welcome tab" + ); + + // ABOUT is safe to drive: it opens a modal. DONATE / WEBVERSION / REPORT + // take the same path but shell out to a browser, so they are covered by + // the allowlist assertion below rather than by dispatching them here. + let start = app.command_line.history.len(); + let _ = app.update(Message::RibbonToolClick { + tool_id: "ABOUT".to_string(), + event: ModuleEvent::Command("ABOUT".to_string()), + }); + let out: String = app.command_line.history[start..] + .iter() + .map(|e| e.text.as_str()) + .collect::>() + .join("\n"); + assert!( + !out.contains("No drawing open"), + "ABOUT needs no drawing and must not be refused on the welcome page: {out:?}" + ); + + // …but a tool that does need a drawing is still turned away (#299). + let start = app.command_line.history.len(); + let _ = app.update(Message::RibbonToolClick { + tool_id: "LINE".to_string(), + event: ModuleEvent::Command("LINE".to_string()), + }); + let out: String = app.command_line.history[start..] + .iter() + .map(|e| e.text.as_str()) + .collect::>() + .join("\n"); + assert!( + out.contains("No drawing open"), + "LINE must still be refused on the welcome page: {out:?}" + ); + assert!( + app.tabs[app.active_tab].active_cmd.is_none(), + "LINE must not have started" + ); + + // A non-command tool event touches the scene, so it stays inert too. + let start = app.command_line.history.len(); + let _ = app.update(Message::RibbonToolClick { + tool_id: "LAYERS".to_string(), + event: ModuleEvent::ToggleLayers, + }); + let out: String = app.command_line.history[start..] + .iter() + .map(|e| e.text.as_str()) + .collect::>() + .join("\n"); + assert!( + out.contains("No drawing open"), + "a scene-touching event must stay inert on the welcome page: {out:?}" + ); + + // Every welcome-page button must also clear dispatch's own gate, or the + // fix above just moves the refusal one door down. Asserted on the source + // rather than by dispatching: DONATE / REPORT / WEBVERSION shell out to + // a real browser, which a test must not do. + let dispatch_src = include_str!("commands/mod.rs"); + let gate = dispatch_src + .split("is_start") + .nth(1) + .and_then(|s| s.split('{').next()) + .expect("the is_start gate moved — re-point this test"); + // DONATE/REPORT/WEBVERSION are the welcome page's own buttons; the rest + // are ribbon tools that configure the application, not a drawing. + let standalone = [ + "DONATE", + "REPORT", + "WEBVERSION", + "ABOUT", + "CHANGELOG", + "CUI", + "ALIASEDIT", + ]; + for cmd in standalone { + assert!( + gate.contains(&format!("\"{cmd}\"")), + "{cmd} needs no drawing but is missing from dispatch's standalone \ + list, so it is refused on the welcome page" + ); + } + // …and each must actually have somewhere to land. + let view_src = include_str!("commands/view.rs"); + for cmd in standalone { + assert!( + view_src.contains(&format!("\"{cmd}\" =>")) + || view_src.contains(&format!("\"{cmd}\" |")), + "{cmd} has no dispatch arm" + ); + } + } + #[test] fn handing_over_an_already_open_drawing_switches_to_its_tab() { // Double-clicking a drawing that is already open should land on the tab diff --git a/src/app/commands/mod.rs b/src/app/commands/mod.rs index 06b69a936..864365498 100644 --- a/src/app/commands/mod.rs +++ b/src/app/commands/mod.rs @@ -103,6 +103,11 @@ impl OpenCADStudio { // command would silently do nothing. Allow only the commands that // make sense there (create / open a document, or quit) and tell the // user otherwise instead of running a no-op. See #96. + // Anything that acts on the application rather than a drawing belongs + // here: document lifecycle, the links, and the app-wide configuration + // editors (shortcuts, aliases) — none of them read the scene. This is + // the single place that decides; `on_ribbon_tool_click` defers to it + // rather than keeping a second, blunter copy (#388, #389). if self.tabs[i].is_start && !matches!( cmd, @@ -117,6 +122,11 @@ impl OpenCADStudio { | "PLUGINMANAGER" | "DONATE" | "WEBVERSION" + | "HELP" + | "CUI" + | "ALIASEDIT" + | "CUILOAD" + | "CUIIMPORT" ) { self.command_line diff --git a/src/app/update/dialog.rs b/src/app/update/dialog.rs index afdffd887..edede88c6 100644 --- a/src/app/update/dialog.rs +++ b/src/app/update/dialog.rs @@ -91,12 +91,18 @@ impl OpenCADStudio { pub(super) fn on_ribbon_tool_click(&mut self, tool_id: String, event: ModuleEvent) -> Task { - // On the Start page there is no drawing to act on — ribbon - // commands are inert. Point the user at New / Open instead of - // running a command into the empty welcome tab (#299). The - // quick-access New / Open / Save buttons use a separate path and - // stay available. - if self.tabs[self.active_tab].is_start { + // On the Start page there is no drawing to act on, so a tool that + // touches the scene is inert — point the user at New / Open + // instead of running it into the empty welcome tab (#299). + // + // Commands are exempt: `dispatch_command` already decides which + // ones stand alone (About, Donate, Report, the web link…) and + // reports the rest. Refusing them here shadowed that list and + // killed the welcome page's own buttons, which by definition can + // only ever be clicked while `is_start` holds (#388, #389). + // Keep the policy in one place — this door must not second-guess + // it. Every other event below mutates the scene or its panels. + if self.tabs[self.active_tab].is_start && !matches!(event, ModuleEvent::Command(_)) { self.ribbon.close_dropdown(); self.command_line .push_info("No drawing open — use New or Open first.");