From ff712b24190342363d0275467e96f181872da6ad Mon Sep 17 00:00:00 2001 From: "sorokin.oleg.edu" Date: Wed, 23 Sep 2026 06:25:50 -0700 Subject: [PATCH] Preserve exact Process selection across definition navigation Keep caller identity and property subjects aligned when opening and returning from qualified contributions, and when explicitly navigating to local references. Validate camera, source, history and read-only boundaries. --- crates/oa-desktop/src/app.rs | 9 +- crates/oa-desktop/src/app/process_canvas.rs | 24 ++- .../oa-desktop/src/app/process_multiselect.rs | 6 +- .../src/app/process_multiselect/tests.rs | 199 ++++++++++++++++++ 4 files changed, 231 insertions(+), 7 deletions(-) diff --git a/crates/oa-desktop/src/app.rs b/crates/oa-desktop/src/app.rs index 01cde07b..2a9c7df9 100644 --- a/crates/oa-desktop/src/app.rs +++ b/crates/oa-desktop/src/app.rs @@ -1119,10 +1119,13 @@ impl App { previous != (self.layer, self.active_diagram) } fn select(&mut self, id: Id) { + // An exact Process preview and its defining object share a semantic ID, + // but ordinary Explorer/property navigation must still leave the use. + let process_preview = self.process_preview_primary().is_some(); if id != self.selected { self.selected_attachment = None; } - if self.selected == id { + if self.selected == id && !process_preview { return; } if !self.apply() { @@ -1131,6 +1134,10 @@ impl App { if !self.project().contains(id) { return; } + self.behavior.topology.clear_context(); + if process_preview { + self.diagram_navigation_focus = None; + } let previous_state_view = self.state_view_identity(); self.selected = id; self.selected_node = None; diff --git a/crates/oa-desktop/src/app/process_canvas.rs b/crates/oa-desktop/src/app/process_canvas.rs index 9df5a654..3d59b657 100644 --- a/crates/oa-desktop/src/app/process_canvas.rs +++ b/crates/oa-desktop/src/app/process_canvas.rs @@ -128,6 +128,12 @@ impl App { let resolved = oa_core::topology_context::TopologyIndex::new(self.project()) .resolve(context.owner, &endpoint); if let Ok(resolved) = resolved { + // The definition may be opened from a nonprimary context + // target. Remember that exact invocation, not whichever + // qualified occurrence happened to be primary beforehand. + self.behavior + .topology + .select_context(resolved.address.clone()); self.behavior.topology.remember( context.owner, endpoint @@ -137,6 +143,7 @@ impl App { .unwrap_or(endpoint.occurrence), resolved.defining_sequence, ); + self.behavior.topology.clear_context(); self.behavior.active_owner = Some(resolved.defining_sequence); self.select(endpoint.occurrence); self.canvas_epoch = self.canvas_epoch.wrapping_add(1); @@ -188,6 +195,9 @@ impl App { if let Some(target) = topology::referenced(self.project(), context.owner, occurrence) { + // Opening a local card returns to that local use, even if + // a qualified tile was the previous inspection target. + self.behavior.topology.clear_context(); if self .project() .elements @@ -212,8 +222,20 @@ impl App { self.behavior.topology = state; if let Some((owner, occurrence)) = back { self.behavior.active_owner = Some(owner); - self.select(occurrence); + // Back restores a use, whereas ordinary select navigates + // to a defining object. The validated return address owns + // the exact subject; never infer it from the outer card ID. + self.selected = self + .behavior + .topology + .selection_address() + .filter(|address| address.owner == owner) + .map_or(occurrence, |address| address.endpoint.occurrence); self.canvas_epoch = self.canvas_epoch.wrapping_add(1); + // Synchronize the exact primary and inspector before the + // returned caller is drawn or accepts property edits. + self.reconcile_process_multiselect(); + self.inspect_process_multiselect_primary(); } } mutation => { diff --git a/crates/oa-desktop/src/app/process_multiselect.rs b/crates/oa-desktop/src/app/process_multiselect.rs index 2ee9b420..3258f76e 100644 --- a/crates/oa-desktop/src/app/process_multiselect.rs +++ b/crates/oa-desktop/src/app/process_multiselect.rs @@ -54,11 +54,7 @@ impl App { .or_else(|| { address .as_ref() - .filter(|a| { - a.owner == context.owner - && (a.endpoint.occurrence == self.selected - || a.endpoint.references.first() == Some(&self.selected)) - }) + .filter(|a| a.owner == context.owner && a.endpoint.occurrence == self.selected) .map(|a| Target::ProcessOccurrence(a.endpoint.clone())) }) .or_else(|| { diff --git a/crates/oa-desktop/src/app/process_multiselect/tests.rs b/crates/oa-desktop/src/app/process_multiselect/tests.rs index a0389d7d..f29afa20 100644 --- a/crates/oa-desktop/src/app/process_multiselect/tests.rs +++ b/crates/oa-desktop/src/app/process_multiselect/tests.rs @@ -672,6 +672,111 @@ fn process_qualified_nonprimary_menu_opens_definition_from_caller_and_back_retur assert!(!f.app.document.can_undo()); } +#[test] +fn process_qualified_definition_back_restores_exact_invocation_readonly_subject_and_camera() { + for open_nonprimary in [false, true] { + let (mut f, definition, first, second, leaf) = qualified_fixture(); + // The shared fixture has already expanded both fifth-tile invocations. + let context = f.app.topology_context().unwrap(); + let _ = f.app.update(Message::TopologyCanvas(CanvasEvent::Zoom( + context, + Some(0.75), + ))); + let first_target = Target::ProcessOccurrence(OccurrenceEndpoint { + references: vec![first], + occurrence: leaf, + }); + let second_target = Target::ProcessOccurrence(OccurrenceEndpoint { + references: vec![second], + occurrence: leaf, + }); + if open_nonprimary { + // Keep the requested second path outside the current selection so + // its ordinary definition action is available in its context menu. + choose(&mut f.app, local(f.nodes[0]), Choice::Replace); + choose(&mut f.app, first_target.clone(), Choice::Add); + } else { + choose(&mut f.app, first_target.clone(), Choice::Replace); + choose(&mut f.app, second_target.clone(), Choice::Add); + } + assert_eq!(f.app.multiselect_snapshot().unwrap().targets().len(), 2); + assert_eq!( + f.app.multiselect_snapshot().unwrap().primary(), + Some(if open_nonprimary { + &first_target + } else { + &second_target + }) + ); + assert_eq!(f.app.selected, leaf); + assert!(f.app.draft.is_none()); + let original = f.app.document.snapshot(); + let source = f.app.document.bundle().clone(); + let camera = f.app.behavior.topology.camera_boundary(); + let target = second_target; + let action = if open_nonprimary { + menu(&mut f.app, target.clone(), "Open referenced definition") + } else { + let Target::ProcessOccurrence(endpoint) = &target else { + unreachable!() + }; + Message::TopologyCanvas(CanvasEvent::OpenQualified( + f.app.topology_context().unwrap(), + endpoint.clone(), + )) + }; + let _ = f.app.update(action); + assert_eq!(f.app.topology_context().unwrap().owner, definition); + assert_eq!(f.app.selected, leaf); + assert!(f.app.process_preview_primary().is_none()); + assert!(f.app.draft.is_some(), "Definition view is editable"); + let context = f.app.topology_context().unwrap(); + let _ = f + .app + .update(Message::TopologyCanvas(CanvasEvent::Back(context))); + assert_eq!(f.app.topology_context().unwrap().owner, f.owner); + assert_eq!(f.app.behavior.active_owner, Some(f.owner)); + assert_eq!( + f.app.multiselect_snapshot().unwrap().targets(), + std::slice::from_ref(&target) + ); + assert_eq!( + f.app.multiselect_snapshot().unwrap().primary(), + Some(&target) + ); + assert_eq!( + f.app.selected, leaf, + "Back must not inspect the outer reference" + ); + assert_eq!(f.app.process_preview_primary(), Some(&target)); + let Target::ProcessOccurrence(endpoint) = &target else { + unreachable!() + }; + let address = f.app.behavior.topology.selection_address().unwrap(); + assert_eq!(address.owner, f.owner); + assert_eq!(&address.endpoint, endpoint); + assert!( + f.app.multiselect_target_visible(&target), + "Caller expansions survive Back" + ); + assert!( + f.app.draft.is_none(), + "Returned qualified inspector stays read-only" + ); + assert!(f.app.behavior.topology.camera_boundary() == camera); + let _ = f.app.update(Message::Field( + "/name".into(), + "Must not edit the outer reference or definition".into(), + )); + assert!(f.app.draft.is_none()); + assert_eq!(f.app.process_preview_primary(), Some(&target)); + assert!(Arc::ptr_eq(&original, &f.app.document.snapshot())); + assert_eq!(f.app.document.bundle(), &source); + assert!(!f.app.document.can_undo() && !f.app.document.can_redo()); + source_matches(&f.app); + } +} + #[test] fn process_open_current_qualified_primary_and_catalog_navigation_restore_editable_draft() { for catalog in [false, true] { @@ -708,6 +813,100 @@ fn process_open_current_qualified_primary_and_catalog_navigation_restore_editabl } } +#[test] +fn process_explicit_object_navigation_leaves_qualified_use_even_for_same_leaf_id() { + for edit_properties in [false, true] { + for defining_leaf in [false, true] { + let (mut f, definition, first, _, leaf) = qualified_fixture(); + let qualified = Target::ProcessOccurrence(OccurrenceEndpoint { + references: vec![first], + occurrence: leaf, + }); + choose(&mut f.app, qualified.clone(), Choice::Replace); + assert_eq!(f.app.selected, leaf); + assert_eq!(f.app.process_preview_primary(), Some(&qualified)); + assert!(f.app.draft.is_none()); + let before = f.app.document.snapshot(); + let source = f.app.document.bundle().clone(); + let camera = f.app.behavior.topology.camera_boundary(); + let target = if defining_leaf { leaf } else { first }; + let expected_owner = if defining_leaf { definition } else { f.owner }; + let message = if edit_properties { + Message::EditProperties(target) + } else { + Message::Select(target) + }; + let _ = f.app.update(message); + assert_eq!(f.app.selected, target); + assert_eq!(f.app.topology_context().unwrap().owner, expected_owner); + assert_eq!(f.app.behavior.active_owner, Some(expected_owner)); + assert_eq!( + f.app.multiselect_snapshot().unwrap().targets(), + &[local(target)] + ); + assert_eq!( + f.app.multiselect_snapshot().unwrap().primary(), + Some(&local(target)) + ); + assert!(f.app.behavior.topology.selection_address().is_none()); + assert!(f.app.process_preview_primary().is_none()); + assert!( + f.app.draft.is_some(), + "Explicit model-object navigation restores its editor" + ); + assert!(!f.app.draft_dirty()); + if !defining_leaf { + assert!(f.app.behavior.topology.camera_boundary() == camera); + } + assert!(Arc::ptr_eq(&before, &f.app.document.snapshot())); + assert_eq!(f.app.document.bundle(), &source); + assert!(!f.app.document.can_undo() && !f.app.document.can_redo()); + source_matches(&f.app); + } + } +} + +#[test] +fn process_open_local_reference_after_qualified_inspection_returns_local_reference() { + let (mut f, definition, first, _, leaf) = qualified_fixture(); + choose( + &mut f.app, + Target::ProcessOccurrence(OccurrenceEndpoint { + references: vec![first], + occurrence: leaf, + }), + Choice::Replace, + ); + let before = f.app.document.snapshot(); + let source = f.app.document.bundle().clone(); + let camera = f.app.behavior.topology.camera_boundary(); + let context = f.app.topology_context().unwrap(); + let _ = f + .app + .update(Message::TopologyCanvas(CanvasEvent::Open(context, first))); + assert_eq!(f.app.topology_context().unwrap().owner, definition); + assert_eq!(f.app.selected, definition); + assert!(f.app.behavior.topology.selection_address().is_none()); + let context = f.app.topology_context().unwrap(); + let _ = f + .app + .update(Message::TopologyCanvas(CanvasEvent::Back(context))); + assert_eq!(f.app.topology_context().unwrap().owner, f.owner); + assert_eq!(f.app.selected, first); + assert_eq!( + f.app.multiselect_snapshot().unwrap().targets(), + &[local(first)] + ); + assert!(f.app.behavior.topology.selection_address().is_none()); + assert!(f.app.process_preview_primary().is_none()); + assert!(f.app.draft.is_some()); + assert!(f.app.behavior.topology.camera_boundary() == camera); + assert!(Arc::ptr_eq(&before, &f.app.document.snapshot())); + assert_eq!(f.app.document.bundle(), &source); + assert!(!f.app.document.can_undo() && !f.app.document.can_redo()); + source_matches(&f.app); +} + #[test] fn process_usage_navigation_between_two_preview_paths_keeps_exact_link_identity() { use crate::diagram_usage as usage;