diff --git a/app/web/src/store/components.store.ts b/app/web/src/store/components.store.ts index 4b299874b0..8669f11115 100644 --- a/app/web/src/store/components.store.ts +++ b/app/web/src/store/components.store.ts @@ -1719,8 +1719,17 @@ export const useComponentsStore = (forceChangeSetId?: ChangeSetId) => { // don't update if (data.changeSetId !== changeSetId) return; this.rawComponentsById[data.component.id] = data.component; - if (this.selectedComponentId === data.component.id) - this.FETCH_COMPONENT_DEBUG_VIEW(data.component.id); + if (this.selectedComponentId === data.component.id) { + const component = this.rawComponentsById[data.component.id]; + if (component && component.changeStatus !== "deleted") + this.FETCH_COMPONENT_DEBUG_VIEW(data.component.id); + else { + const idx = this.selectedComponentIds.findIndex( + (cId) => cId === data.component.id, + ); + if (idx !== -1) this.selectedComponentIds.slice(idx, 1); + } + } }, }, { diff --git a/lib/dal/src/component.rs b/lib/dal/src/component.rs index 1b2a89a803..036dd95fc0 100644 --- a/lib/dal/src/component.rs +++ b/lib/dal/src/component.rs @@ -2575,11 +2575,6 @@ impl Component { ctx.workspace_snapshot()?.remove_node_by_id(id).await?; - WsEvent::component_deleted(ctx, id) - .await? - .publish_on_commit(ctx) - .await?; - Ok(()) } @@ -3978,6 +3973,23 @@ impl Component { } } + pub async fn exists_on_head( + ctx: &DalContext, + component_ids: Vec, + ) -> ComponentResult> { + let mut components = HashMap::new(); + let base_change_set_ctx = ctx.clone_with_base().await?; + for component_id in component_ids { + let maybe_component = + Component::try_get_by_id(&base_change_set_ctx, component_id).await?; + match maybe_component { + Some(_) => components.insert(component_id, true), + _ => components.insert(component_id, false), + }; + } + Ok(components) + } + pub async fn into_frontend_type( &self, ctx: &DalContext, @@ -4087,7 +4099,7 @@ impl Component { schema_variant_id: schema_variant.id().into(), schema_variant_name: schema_variant.version().to_owned(), schema_category: schema_variant.category().to_owned(), - display_name: dbg!(self.name(ctx).await?), + display_name: self.name(ctx).await?, position, size, component_type: self.get_type(ctx).await?.to_string(), diff --git a/lib/dal/src/diagram.rs b/lib/dal/src/diagram.rs index 4815a3c70d..e362ddac64 100644 --- a/lib/dal/src/diagram.rs +++ b/lib/dal/src/diagram.rs @@ -1,13 +1,13 @@ use serde::{Deserialize, Serialize}; use si_data_pg::PgError; -use std::collections::{hash_map, HashMap, HashSet}; +use std::collections::{HashMap, HashSet}; use std::num::{ParseFloatError, ParseIntError}; use strum::{AsRefStr, Display, EnumIter, EnumString}; use telemetry::prelude::*; use thiserror::Error; use crate::attribute::prototype::argument::{ - AttributePrototypeArgument, AttributePrototypeArgumentError, AttributePrototypeArgumentId, + AttributePrototypeArgumentError, AttributePrototypeArgumentId, }; use crate::attribute::value::AttributeValueError; use crate::change_status::ChangeStatus; @@ -252,7 +252,6 @@ impl Diagram { components.iter().cloned().map(|c| (c.id(), c)).collect(); let mut component_views: Vec = Vec::with_capacity(components.len()); - dbg!(components.len()); let new_component_ids: HashSet = ctx .workspace_snapshot()? .components_added_relative_to_base(ctx) @@ -350,7 +349,7 @@ impl Diagram { let base_change_set_component = Component::get_by_id(base_change_set_ctx, removed_component_id).await?; let mut summary_diagram_component = base_change_set_component - .into_frontend_type(ctx, ChangeStatus::Deleted) + .into_frontend_type(base_change_set_ctx, ChangeStatus::Deleted) .await?; summary_diagram_component.from_base_change_set = true; virtual_and_real_components_by_id @@ -361,74 +360,23 @@ impl Diagram { // We need to bring in any AttributePrototypeArguments for incoming & outgoing // connections that have been removed. - let removed_attribute_prototype_argument_ids: Vec = - vec![]; - // = ctx - // .workspace_snapshot()? - // .socket_edges_removed_relative_to_base(ctx) - // .await?; - let mut incoming_connections_by_component_id: HashMap< - ComponentId, - Vec, - > = HashMap::new(); - - for removed_attribute_prototype_argument_id in &removed_attribute_prototype_argument_ids - { - let attribute_prototype_argument = AttributePrototypeArgument::get_by_id( - base_change_set_ctx, - *removed_attribute_prototype_argument_id, - ) + let removed_incoming_connections: Vec = ctx + .workspace_snapshot()? + .socket_edges_removed_relative_to_base(ctx) .await?; - // This should always be Some as - // `WorkspaceSnapshot::socket_edges_removed_relative_to_base` only returns the - // IDs of arguments that have targets. - if let Some(targets) = attribute_prototype_argument.targets() { - if let hash_map::Entry::Vacant(vacant_entry) = - incoming_connections_by_component_id.entry(targets.destination_component_id) - { - let removed_component = Component::get_by_id( - base_change_set_ctx, - targets.destination_component_id, - ) - .await?; - let mut incoming_connections = removed_component - .incoming_connections(base_change_set_ctx) - .await?; - // We only care about connections going to the Component in the base that - // are *ALSO* ones that we are considered to have removed. - incoming_connections.retain(|connection| { - removed_attribute_prototype_argument_ids - .contains(&connection.attribute_prototype_argument_id) - }); - vacant_entry.insert(incoming_connections); - } - } - } - - for incoming_connections in incoming_connections_by_component_id.values() { - for incoming_connection in incoming_connections { - let from_component = virtual_and_real_components_by_id - .get(&incoming_connection.from_component_id) - .cloned() - .ok_or(ComponentError::NotFound( - incoming_connection.from_component_id, - ))?; - let to_component = virtual_and_real_components_by_id - .get(&incoming_connection.to_component_id) - .ok_or(ComponentError::NotFound( - incoming_connection.to_component_id, - ))?; - let mut summary_diagram_edge = SummaryDiagramEdge::assemble( - incoming_connection.clone(), - &from_component, - to_component, - ChangeStatus::Deleted, - )?; - summary_diagram_edge.from_base_change_set = true; - - diagram_edges.push(summary_diagram_edge); - } + for removed_incoming_connection in &removed_incoming_connections { + diagram_edges.push(SummaryDiagramEdge { + from_component_id: removed_incoming_connection.from_component_id, + from_socket_id: removed_incoming_connection.from_output_socket_id, + to_component_id: removed_incoming_connection.to_component_id, + to_socket_id: removed_incoming_connection.to_input_socket_id, + change_status: ChangeStatus::Deleted, + created_info: serde_json::to_value(&removed_incoming_connection.created_info)?, + deleted_info: serde_json::to_value(&removed_incoming_connection.deleted_info)?, + to_delete: true, + from_base_change_set: true, + }); } } diff --git a/lib/dal/src/job/definition/action.rs b/lib/dal/src/job/definition/action.rs index ee62961015..21c9fdc09d 100644 --- a/lib/dal/src/job/definition/action.rs +++ b/lib/dal/src/job/definition/action.rs @@ -155,6 +155,10 @@ async fn inner_run( if let Some(component) = component { if component.allowed_to_be_removed(ctx).await? { Component::remove(ctx, component.id()).await?; + WsEvent::component_deleted(ctx, component.id()) + .await? + .publish_on_commit(ctx) + .await?; did_remove = true; } } @@ -261,6 +265,10 @@ async fn process_and_record_execution( if component.to_delete() { Component::remove(&ctx, component.id()).await?; + WsEvent::component_deleted(&ctx, component.id()) + .await? + .publish_on_commit(&ctx) + .await?; to_remove_nodes.push(component.id().into()); } else { let summary = component diff --git a/lib/dal/src/workspace_snapshot.rs b/lib/dal/src/workspace_snapshot.rs index 5dcdf07544..c06b4960f7 100644 --- a/lib/dal/src/workspace_snapshot.rs +++ b/lib/dal/src/workspace_snapshot.rs @@ -34,7 +34,7 @@ use graph::correct_transforms::correct_transforms; use graph::detect_updates::Update; use graph::{RebaseBatch, WorkspaceSnapshotGraph}; use node_weight::traits::CorrectTransformsError; -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::sync::atomic::AtomicBool; use std::sync::Arc; @@ -56,6 +56,7 @@ use crate::attribute::prototype::argument::{ AttributePrototypeArgument, AttributePrototypeArgumentError, AttributePrototypeArgumentId, }; use crate::change_set::{ChangeSetError, ChangeSetId}; +use crate::component::IncomingConnection; use crate::slow_rt::{self, SlowRuntimeError}; use crate::workspace_snapshot::content_address::ContentAddressDiscriminants; use crate::workspace_snapshot::edge_weight::{ @@ -65,7 +66,8 @@ use crate::workspace_snapshot::graph::LineageId; use crate::workspace_snapshot::node_weight::category_node_weight::CategoryNodeKind; use crate::workspace_snapshot::node_weight::NodeWeight; use crate::{ - pk, AttributeValueId, Component, ComponentError, ComponentId, Workspace, WorkspaceError, + pk, AttributeValueId, Component, ComponentError, ComponentId, OutputSocketId, Workspace, + WorkspaceError, }; use crate::{ workspace_snapshot::{graph::WorkspaceSnapshotGraphError, node_weight::NodeWeightError}, @@ -1627,9 +1629,7 @@ impl WorkspaceSnapshot { pub async fn socket_edges_removed_relative_to_base( &self, ctx: &DalContext, - ) -> WorkspaceSnapshotResult> { - let mut removed_attribute_prototype_argument_ids = Vec::new(); - + ) -> WorkspaceSnapshotResult> { // Even though the default change set for a workspace can have a base change set, we don't // want to consider anything as new/modified/removed when looking at the default change // set. @@ -1642,105 +1642,65 @@ impl WorkspaceSnapshot { .await .map_err(Box::new)?; if workspace.default_change_set_id() == ctx.change_set_id() { - return Ok(removed_attribute_prototype_argument_ids); + return Ok(Vec::new()); } let base_change_set_ctx = ctx.clone_with_base().await?; let base_change_set_ctx = &base_change_set_ctx; - // * For each Component being removed (all edges to/from removed components should also - // show as removed): - let removed_component_ids: HashSet = self - .components_removed_relative_to_base(ctx) - .await? - .iter() - .copied() - .collect(); - let remaining_component_ids: HashSet = Component::list(ctx) + let base_components = Component::list(base_change_set_ctx) .await - .map_err(Box::new)? - .iter() - .map(Component::id) - .collect(); - for removed_component_id in &removed_component_ids { - let base_change_set_component = - Component::get_by_id(base_change_set_ctx, *removed_component_id) - .await - .map_err(Box::new)?; - - // * Get incoming edges - for incoming_connection in base_change_set_component + .map_err(Box::new)?; + #[derive(Hash, Clone, PartialEq, Eq)] + struct UniqueEdge { + to_component_id: ComponentId, + from_component_id: ComponentId, + from_socket_id: OutputSocketId, + } + let mut base_incoming_edges = HashSet::new(); + let mut base_incoming = HashMap::new(); + for base_component in base_components { + let incoming_edges = base_component .incoming_connections(base_change_set_ctx) .await - .map_err(Box::new)? - { - //* Interested in: - // * Edge is coming from a Component being removed - // * Edge is coming from a Component that exists in current change set - if removed_component_ids.contains(&incoming_connection.from_component_id) - || remaining_component_ids.contains(&incoming_connection.from_component_id) - { - removed_attribute_prototype_argument_ids - .push(incoming_connection.attribute_prototype_argument_id); - } + .map_err(Box::new)?; + + for conn in incoming_edges { + let hash = UniqueEdge { + to_component_id: conn.to_component_id, + from_socket_id: conn.from_output_socket_id, + from_component_id: conn.from_component_id, + }; + base_incoming_edges.insert(hash.clone()); + base_incoming.insert(hash, conn); } + } - //* Get outgoing edges - for outgoing_connection in base_change_set_component - .outgoing_connections(base_change_set_ctx) + let current_components = Component::list(ctx).await.map_err(Box::new)?; + let mut current_incoming_edges = HashSet::new(); + for current_component in current_components { + let incoming_edges: Vec = current_component + .incoming_connections(ctx) .await .map_err(Box::new)? - { - // * Interested in: - // * Edge is going to a Component being removed - // * Edge is going to a Component that exists in current change set - if removed_component_ids.contains(&outgoing_connection.to_component_id) - || remaining_component_ids.contains(&outgoing_connection.to_component_id) - { - removed_attribute_prototype_argument_ids - .push(outgoing_connection.attribute_prototype_argument_id); - } - } + .into_iter() + .map(|conn| UniqueEdge { + to_component_id: conn.to_component_id, + from_socket_id: conn.from_output_socket_id, + from_component_id: conn.from_component_id, + }) + .collect(); + current_incoming_edges.extend(incoming_edges); } - // * For each removed AttributePrototypeArgument (removed edge connects two Components - // that have not been removed): - let base_snapshot = base_change_set_ctx.workspace_snapshot()?; - let updates = base_snapshot - .read_only_graph - .detect_updates(&self.read_only_graph); - - for update in updates { - match update { - Update::ReplaceNode { .. } | Update::NewEdge { .. } | Update::NewNode { .. } => { - /* Updates unused for determining if a connection between sockets has been removed */ - } - Update::RemoveEdge { - source: _, - destination, - edge_kind, - } => { - if edge_kind != EdgeWeightKindDiscriminants::PrototypeArgument { - continue; - } - let attribute_prototype_argument = AttributePrototypeArgument::get_by_id( - base_change_set_ctx, - AttributePrototypeArgumentId::from(Ulid::from(destination.id)), - ) - .await - .map_err(Box::new)?; - - // * Interested in all of them that have targets (connecting two Components - // via sockets). - if attribute_prototype_argument.targets().is_some() { - removed_attribute_prototype_argument_ids - .push(attribute_prototype_argument.id()); - } - } + let difference = base_incoming_edges.difference(¤t_incoming_edges); + let mut differences = vec![]; + for diff in difference { + if let Some(edge) = base_incoming.get(diff) { + differences.push(edge.clone()); } } - - Ok(removed_attribute_prototype_argument_ids) + Ok(differences) } /// Returns whether or not any Actions were dispatched. diff --git a/lib/sdf-server/src/server/service/diagram/delete_component.rs b/lib/sdf-server/src/server/service/diagram/delete_component.rs index 4604b6a9d8..34a5e0e5a5 100644 --- a/lib/sdf-server/src/server/service/diagram/delete_component.rs +++ b/lib/sdf-server/src/server/service/diagram/delete_component.rs @@ -32,6 +32,9 @@ pub async fn delete_components( let mut ctx = builder.build(request_ctx.build(request.visibility)).await?; let force_change_set_id = ChangeSet::force_new(&mut ctx).await?; + let components_existing_on_head = + Component::exists_on_head(&ctx, request.component_ids.clone()).await?; + let base_change_set_ctx = ctx.clone_with_base().await?; let mut components = HashMap::new(); for component_id in request.component_ids { @@ -46,6 +49,9 @@ pub async fn delete_components( .await?; components.insert(component_id, component_still_exists); + let exists_on_head = components_existing_on_head + .get(&component_id) + .unwrap_or(&false); if component_still_exists { // to_delete=True let component: Component = Component::get_by_id(&ctx, component_id).await?; @@ -56,7 +62,22 @@ pub async fn delete_components( .await? .publish_on_commit(&ctx) .await?; - } // component_deleted called further down the stack + } else if *exists_on_head { + let component: Component = + Component::get_by_id(&base_change_set_ctx, component_id).await?; + let payload = component + .into_frontend_type(&base_change_set_ctx, ChangeStatus::Deleted) + .await?; + WsEvent::component_updated(&ctx, payload) + .await? + .publish_on_commit(&ctx) + .await?; + } else { + WsEvent::component_deleted(&ctx, component_id) + .await? + .publish_on_commit(&ctx) + .await?; + } } ctx.commit().await?;