From 9245f8d23e1ed1476dd3219ae275e57e3a696abe Mon Sep 17 00:00:00 2001 From: Codex-ytr83r Date: Sat, 12 Sep 2026 19:41:48 -0700 Subject: [PATCH] fix: preserve migration context and reject unsupported SVG exports --- .../src/source_project/source_migration.rs | 5 + crates/oa-desktop/src/app.rs | 92 ++++++++++++- crates/oa-desktop/src/diagram.rs | 122 ++++++++++++++++-- .../src/diagram/view_options_tests.rs | 18 ++- 4 files changed, 219 insertions(+), 18 deletions(-) diff --git a/crates/oa-core/src/source_project/source_migration.rs b/crates/oa-core/src/source_project/source_migration.rs index 05ce86d1..dc8243b9 100644 --- a/crates/oa-core/src/source_project/source_migration.rs +++ b/crates/oa-core/src/source_project/source_migration.rs @@ -68,6 +68,11 @@ pub(super) fn upgrade( &actual.fields, Some(&actual.fields), &next.fields, + if project.diagrams.contains_key(&id) { + EditContext::Diagram + } else { + EditContext::Other + }, edits.entry(path.clone()).or_default(), ); } diff --git a/crates/oa-desktop/src/app.rs b/crates/oa-desktop/src/app.rs index 992bf5eb..cd3ca660 100644 --- a/crates/oa-desktop/src/app.rs +++ b/crates/oa-desktop/src/app.rs @@ -3693,6 +3693,14 @@ impl App { } } Message::Export => { + let Some(diagram) = self.active_diagram else { + self.status = "Create a diagram first.".into(); + return Task::none(); + }; + if !self.project().diagrams.contains_key(&diagram) { + self.status = crate::diagram::ExportError::MissingDiagram(diagram).to_string(); + return Task::none(); + } if self.apply() { return self.choose_file(FileOperation::Export); } @@ -4138,7 +4146,13 @@ impl App { self.status = "Create a diagram first.".into(); return Task::none(); }; - let svg = crate::diagram::export_svg(self.project(), diagram); + let svg = match crate::diagram::export_svg(self.project(), diagram) { + Ok(svg) => svg, + Err(error) => { + self.status = error.to_string(); + return Task::none(); + } + }; self.busy = true; Task::perform( async move { @@ -5657,6 +5671,82 @@ mod saved_scenario_dispatch_tests { (app, architecture, saved, f.owner) } + #[test] + fn rejected_export_schedules_no_write_and_preserves_draft_history_and_busy_state() { + let (mut app, _, saved, _) = fixture(); + let _ = app.update(Message::Field("/name".into(), "Keep export draft".into())); + assert!(app.draft_dirty()); + let path = + std::env::temp_dir().join(format!("arcadia-rejected-export-{}.svg", Id::new_v4())); + std::fs::write(&path, b"existing export").unwrap(); + for target in [Some(saved), Some(Id::new_v4()), None] { + for busy in [false, true] { + app.active_diagram = target; + app.busy = busy; + let before = app.document.bundle().clone(); + let history = ( + app.document.revision(), + app.document.can_undo(), + app.document.can_redo(), + ); + let view = ( + app.page, + app.selected, + app.active_diagram, + app.zoom, + app.pan, + ); + let task = app.export_to(path.clone()); + assert_eq!(task.units(), 0, "rejection must not schedule a write"); + assert_eq!(app.busy, busy); + assert_eq!(app.document.bundle(), &before); + assert_eq!( + ( + app.document.revision(), + app.document.can_undo(), + app.document.can_redo() + ), + history + ); + assert_eq!( + ( + app.page, + app.selected, + app.active_diagram, + app.zoom, + app.pan + ), + view + ); + assert!(app.draft_dirty()); + assert!(app.file_prompt.is_none()); + assert_eq!(std::fs::read(&path).unwrap(), b"existing export"); + } + } + std::fs::remove_file(path).unwrap(); + } + + #[test] + fn missing_export_target_does_not_apply_pending_properties_or_open_dialog() { + for target in [None, Some(Id::new_v4())] { + let (mut app, _, _, _) = fixture(); + let _ = app.update(Message::Field( + "/name".into(), + "Unapplied export draft".into(), + )); + app.active_diagram = target; + let before = app.document.bundle().clone(); + let revision = app.document.revision(); + let task = app.handle(Message::Export); + assert_eq!(task.units(), 0); + assert_eq!(app.document.bundle(), &before); + assert_eq!(app.document.revision(), revision); + assert!(app.draft_dirty()); + assert!(app.file_prompt.is_none()); + assert!(!app.busy); + } + } + #[test] fn unsupported_saved_picker_and_reveal_do_not_apply_pending_properties() { for reveal in [false, true] { diff --git a/crates/oa-desktop/src/diagram.rs b/crates/oa-desktop/src/diagram.rs index a7c3a98f..e90831fc 100644 --- a/crates/oa-desktop/src/diagram.rs +++ b/crates/oa-desktop/src/diagram.rs @@ -1594,11 +1594,32 @@ fn hex(c: Color) -> String { (c.b * 255.) as u8 ) } -pub fn export_svg(p: &Project, diagram: Id) -> String { +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ExportError { + MissingDiagram(Id), + UnsupportedScenario(Id), +} +impl std::fmt::Display for ExportError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::MissingDiagram(id) => write!(f, "Cannot export: diagram {id} no longer exists."), + Self::UnsupportedScenario(_) => { + f.write_str("SVG export of saved Scenario views is not available yet.") + } + } + } +} +impl std::error::Error for ExportError {} + +pub fn export_svg(p: &Project, diagram: Id) -> Result { + let d = p + .diagrams + .get(&diagram) + .ok_or(ExportError::MissingDiagram(diagram))?; + if d.projection.scenario().is_some() { + return Err(ExportError::UnsupportedScenario(diagram)); + } let view = project_view(p, diagram); - let Some(d) = p.diagrams.get(&diagram) else { - return String::new(); - }; let routed = route_diagram(p, d, &view); let left = routed.bounds.position.x - 30.; let top = routed.bounds.position.y - 30.; @@ -1737,7 +1758,7 @@ pub fn export_svg(p: &Project, diagram: Id) -> String { svg.push_str(""); } svg.push_str(""); - svg + Ok(svg) } #[cfg(test)] @@ -1747,6 +1768,83 @@ mod pointer_tests; #[cfg(test)] mod tests { use super::*; + + #[test] + fn svg_rejects_saved_scenario_with_absent_or_authored_layout_without_mutation() { + let f = crate::behavior::scenario::direct_tests::fixture(); + let mut p = f.project; + let diagram = p.ensure_diagram(Layer::Oa); + let d = p.diagrams.get_mut(&diagram).unwrap(); + d.projection = ProjectionKind::Scenario { scenario: f.owner }; + d.nodes.clear(); + d.routes.clear(); + for layout in [ + None, + Some(DiagramLayout::Scenario(ScenarioLayout { + lanes: vec![ScenarioLanePosition { + lifeline: f.sender, + x: 240., + }], + messages: vec![ScenarioMessagePosition { + address: OccurrenceEndpoint { + references: vec![], + occurrence: f.call, + }, + send_y: 180., + }], + })), + ] { + p.diagrams.get_mut(&diagram).unwrap().layout = layout; + let admitted = execute_batch(&p, &[]).expect("valid saved Scenario fixture"); + assert_eq!(admitted, p); + let before = p.clone(); + assert_eq!( + export_svg(&p, diagram), + Err(ExportError::UnsupportedScenario(diagram)) + ); + assert_eq!(p, before); + } + } + + #[test] + fn svg_rejects_missing_diagram_without_mutation() { + let p = Project::demo(); + let missing = Id::new_v4(); + let before = p.clone(); + assert_eq!( + export_svg(&p, missing), + Err(ExportError::MissingDiagram(missing)) + ); + assert_eq!(p, before); + } + + #[test] + fn svg_accepts_populated_and_empty_architecture_views() { + let mut p = Project::demo(); + let diagram = p + .diagrams + .values() + .find(|d| d.projection.is_architecture() && !d.nodes.is_empty()) + .unwrap() + .id; + let before = p.clone(); + let svg = export_svg(&p, diagram).expect("populated architecture"); + assert!(svg.starts_with("")); + assert_eq!(p, before); + let d = p.diagrams.get_mut(&diagram).unwrap(); + d.projection = ProjectionKind::Overview; + d.nodes.clear(); + d.routes.clear(); + assert_eq!(execute_batch(&p, &[]).expect("valid empty architecture"), p); + let before = p.clone(); + let svg = export_svg(&p, diagram).expect("empty architecture is supported"); + assert!(svg.contains("viewBox=\"-30 -30 60 60\"")); + assert!(svg.ends_with("")); + assert_eq!(p, before); + } + #[test] fn typed_function_glyphs_and_unoriented_standard_connections_have_distinct_svg() { let p = Project::demo(); @@ -1780,7 +1878,7 @@ mod tests { .find(|d| d.layer == Layer::Sa && d.projection == ProjectionKind::Allocation) .unwrap() .id; - let svg = export_svg(&p, diagram); + let svg = export_svg(&p, diagram).expect("supported diagram"); let title = format!("{}", escape(&boundary.name)); let edge_path = svg .split(&title) @@ -1854,7 +1952,7 @@ mod tests { p.place(diagram, id, Position { x: 60., y: 60. }); } let p = execute_batch(&p, &[Command::Layout(diagram)]).unwrap(); - let svg = export_svg(&p, diagram); + let svg = export_svg(&p, diagram).expect("supported diagram"); let group = svg .split("") .find(|g| g.contains(">Grouped work")) @@ -1944,7 +2042,7 @@ mod tests { .unwrap() .id; let view = project_view(&p, diagram); - let svg = export_svg(&p, diagram); + let svg = export_svg(&p, diagram).expect("supported diagram"); for edge in &view.edges { assert!(svg.contains(&format!(">{}", escape(&short(p.named(edge.id), 32))))); } @@ -1959,7 +2057,7 @@ mod tests { p.place(d, a, Position { x: 0., y: 0. }); p.place(d, b, Position { x: 400., y: 0. }); p.relate(RelationKind::CapabilityInclude, "Uses", a, b); - let svg = export_svg(&p, d); + let svg = export_svg(&p, d).expect("supported diagram"); assert!(svg.contains("«include»")); assert!(svg.contains("stroke-dasharray=\"6 4\"")); } @@ -2046,7 +2144,7 @@ mod tests { .unwrap(); assert_eq!(moved.position.x, port.position.x + 100.); assert_eq!(moved.position.y, port.position.y + 70.); - let svg = export_svg(&p, diagram); + let svg = export_svg(&p, diagram).expect("supported diagram"); for port in &view.ports { assert!(svg.contains(&format!("{}", escape(p.named(port.element))))); } @@ -2503,7 +2601,7 @@ mod tests { }); let view = project_view(&p, diagram); let geometry = route_diagram(&p, &p.diagrams[&diagram], &view); - let svg = export_svg(&p, diagram); + let svg = export_svg(&p, diagram).expect("supported diagram"); let route = &geometry.edges[&edge]; let path = route .points @@ -2563,7 +2661,7 @@ mod tests { let routed = route_diagram(&p, &p.diagrams[&diagram], &projected); let ends = routing::association_end_geometry(&p.relations[&edge], &routed.edges[&edge]); assert_eq!(ends.len(), 2); - let svg = export_svg(&p, diagram); + let svg = export_svg(&p, diagram).expect("supported diagram"); assert!(svg.contains("parent [0..1]")); assert!(svg.contains("children [0..*]")); assert!(!svg.contains("data-navigation=\"source\"")); diff --git a/crates/oa-desktop/src/diagram/view_options_tests.rs b/crates/oa-desktop/src/diagram/view_options_tests.rs index 458dbd2a..d4775681 100644 --- a/crates/oa-desktop/src/diagram/view_options_tests.rs +++ b/crates/oa-desktop/src/diagram/view_options_tests.rs @@ -27,7 +27,7 @@ fn communication_svg_uses_saved_visibility_labels_and_direction() { validate_candidate(&p).unwrap(); let initial = p.clone(); let path_key = format!("