From 39ef761532deb533cbfba85d37e504ef48a50f5c Mon Sep 17 00:00:00 2001 From: Axel RICHARD Date: Thu, 30 Jul 2026 17:43:16 +0200 Subject: [PATCH] [2344] Fix dropping nested Attribute so it is visible on the first drop Bug: https://github.com/eclipse-syson/syson/issues/2344 Signed-off-by: Axel RICHARD --- CHANGELOG.adoc | 1 + .../GVDropFromExplorerVisibilityTests.java | 79 +++++++++++-------- .../services/DiagramMutationDndService.java | 62 +++++++++++---- .../pages/release-notes/2026.9.0.adoc | 2 + 4 files changed, 96 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index d7e5072c5..b67ab3c01 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -26,6 +26,7 @@ In any case, it is strongly recommended to back up your data before proceeding w === Bug fixes +- https://github.com/eclipse-syson/syson/issues/2344[#2344] [diagrams] Fix dropping a nested `AttributeUsage` from the _Explorer_ onto a _General View_ diagram so it is visible on the first drop. - https://github.com/eclipse-syson/syson/issues/2359[#2359] [diagrams] Fix `Comment` graphical nodes so changing their background color no longer changes their border color. - https://github.com/eclipse-syson/syson/issues/2358[#2358] [details] Fix expression creation and editing on `SuccessionAsUsage` elements. diff --git a/backend/application/syson-application/src/test/java/org/eclipse/syson/application/controllers/diagrams/general/view/GVDropFromExplorerVisibilityTests.java b/backend/application/syson-application/src/test/java/org/eclipse/syson/application/controllers/diagrams/general/view/GVDropFromExplorerVisibilityTests.java index de4804e5e..d9589ca0a 100644 --- a/backend/application/syson-application/src/test/java/org/eclipse/syson/application/controllers/diagrams/general/view/GVDropFromExplorerVisibilityTests.java +++ b/backend/application/syson-application/src/test/java/org/eclipse/syson/application/controllers/diagrams/general/view/GVDropFromExplorerVisibilityTests.java @@ -36,7 +36,7 @@ import org.eclipse.sirius.components.diagrams.tests.navigation.DiagramNavigator; import org.eclipse.sirius.web.tests.services.api.IGivenInitialServerState; import org.eclipse.syson.AbstractIntegrationTests; -import org.eclipse.syson.SysONTestsProperties; +import org.eclipse.syson.GivenSysONServer; import org.eclipse.syson.application.data.GeneralViewItemAndAttributeProjectData; import org.eclipse.syson.services.diagrams.api.IGivenDiagramSubscription; import org.eclipse.syson.sysml.metamodel.helper.LabelConstants; @@ -44,8 +44,6 @@ import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; -import org.springframework.test.context.jdbc.Sql; -import org.springframework.test.context.jdbc.SqlConfig; import org.springframework.transaction.annotation.Transactional; import reactor.test.StepVerifier; @@ -56,7 +54,7 @@ * @author mcharfadi */ @Transactional -@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, properties = { SysONTestsProperties.NO_DEFAULT_LIBRARIES_PROPERTY }) +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) public class GVDropFromExplorerVisibilityTests extends AbstractIntegrationTests { @Autowired @@ -74,10 +72,11 @@ public class GVDropFromExplorerVisibilityTests extends AbstractIntegrationTests @Autowired private InvokeSingleClickOnDiagramElementToolMutationRunner invokeSingleClickOnDiagramElementToolMutationRunner; - @DisplayName("GIVEN a diagram, WHEN we drop a PartUsage with no empty compartments from the Explorer view, THEN the PartUsage is displayed on the diagram with its compartments hidden") - @Sql(scripts = { GeneralViewItemAndAttributeProjectData.SCRIPT_PATH }, executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD, - config = @SqlConfig(transactionMode = SqlConfig.TransactionMode.ISOLATED)) - @Sql(scripts = { "/scripts/cleanup.sql" }, executionPhase = Sql.ExecutionPhase.AFTER_TEST_METHOD, config = @SqlConfig(transactionMode = SqlConfig.TransactionMode.ISOLATED)) + /** + * Verifies that a restored Part and an Attribute contained in it are visible after their first Explorer drops. + */ + @DisplayName("GIVEN a diagram, WHEN a Part is dropped then a contained Attribute is also dropped from the Explorer, THEN both are visible on their first drop") + @GivenSysONServer({ GeneralViewItemAndAttributeProjectData.SCRIPT_PATH }) @Test public void dropPartFromTheExplorer() { this.givenInitialServerState.initialize(); @@ -87,26 +86,20 @@ public void dropPartFromTheExplorer() { var flux = this.givenDiagramSubscription.subscribe(diagramEventInput); - var diagramId = new AtomicReference(); var removeFromDiagramToolId = new AtomicReference(); - var diagramTargetId = new AtomicReference(); var partNodeId = new AtomicReference(); - var partNodeSemanticId = new AtomicReference(); Consumer diagramContentConsumerBeforeDrop = assertRefreshedDiagramThat(diagram -> { - assertThat(diagram.getNodes()).hasSize(3); - diagramTargetId.set(diagram.getTargetObjectId()); - diagramId.set(diagram.getId()); + assertThat(diagram.getNodes()).hasSize(5); var partNode = new DiagramNavigator(diagram).nodeWithLabel(LabelConstants.OPEN_QUOTE + "part" + LabelConstants.CLOSE_QUOTE + LabelConstants.CR + "p1").getNode(); assertThat(partNode.getChildNodes().stream().filter(node -> node.getModifiers().contains(ViewModifier.Hidden))).hasSize(10); - partNodeSemanticId.set(partNode.getTargetObjectId()); partNodeId.set(partNode.getId()); }); Runnable getRemoveFromDiagramTool = () -> { Map variables = Map.of( "editingContextId", GeneralViewItemAndAttributeProjectData.EDITING_CONTEXT_ID, - "representationId", diagramId.get(), + "representationId", GeneralViewItemAndAttributeProjectData.GraphicalIds.DIAGRAM_ID, "diagramElementIds", List.of(partNodeId.get()) ); var result = this.paletteQueryRunner.run(variables); @@ -120,33 +113,33 @@ public void dropPartFromTheExplorer() { // Remove the node from the diagram Runnable executeRemoveFromDiagramTool = () -> { - var input = new InvokeSingleClickOnDiagramElementToolInput(UUID.randomUUID(), GeneralViewItemAndAttributeProjectData.EDITING_CONTEXT_ID, diagramId.get(), List.of(partNodeId.get()), removeFromDiagramToolId.get(), 0, 0, List.of()); + var input = new InvokeSingleClickOnDiagramElementToolInput(UUID.randomUUID(), GeneralViewItemAndAttributeProjectData.EDITING_CONTEXT_ID, + GeneralViewItemAndAttributeProjectData.GraphicalIds.DIAGRAM_ID, List.of(partNodeId.get()), removeFromDiagramToolId.get(), 0, 0, List.of()); var result = this.invokeSingleClickOnDiagramElementToolMutationRunner.run(input); String typename = JsonPath.read(result.data(), "$.data.invokeSingleClickOnDiagramElementTool.__typename"); assertThat(typename).isEqualTo(InvokeSingleClickOnDiagramElementToolSuccessPayload.class.getSimpleName()); }; Consumer diagramContentConsumerAfterRemove = assertRefreshedDiagramThat(diagram -> { - assertThat(diagram.getNodes()).hasSize(2); + assertThat(diagram.getNodes()).hasSize(3); }); - // Drop from the explorer - Runnable executeDropPartOnDiagram = () -> { - var dropOnDiagramInput = new DropOnDiagramInput(UUID.randomUUID(), GeneralViewItemAndAttributeProjectData.EDITING_CONTEXT_ID, diagramId.get(), - diagramTargetId.get(), List.of(partNodeSemanticId.get()), 0, 0); - var dropOnDiagramResult = this.dropOnDiagramMutationRunner.run(dropOnDiagramInput); - var typename = JsonPath.read(dropOnDiagramResult.data(), "$.data.dropOnDiagram.__typename"); - assertThat(typename).isEqualTo(DropOnDiagramSuccessPayload.class.getSimpleName()); - }; + Runnable executeDropPartOnDiagram = () -> this.dropFromExplorer(GeneralViewItemAndAttributeProjectData.GraphicalIds.DIAGRAM_ID, GeneralViewItemAndAttributeProjectData.SemanticIds.P1_ID); - Consumer diagramContentConsumerAfterDrop = assertRefreshedDiagramThat(diagram -> { - assertThat(diagram.getNodes()).hasSize(3); - diagramTargetId.set(diagram.getTargetObjectId()); - diagramId.set(diagram.getId()); + Consumer diagramContentConsumerAfterPartDrop = assertRefreshedDiagramThat(diagram -> { + assertThat(diagram.getNodes()).hasSize(4); var partNode = new DiagramNavigator(diagram).nodeWithLabel(LabelConstants.OPEN_QUOTE + "part" + LabelConstants.CLOSE_QUOTE + LabelConstants.CR + "p1").getNode(); + assertThat(partNode).extracting(node -> node.getState()).isEqualTo(ViewModifier.Normal); assertThat(partNode.getChildNodes().stream().filter(node -> node.getModifiers().contains(ViewModifier.Hidden))).hasSize(11); - partNodeSemanticId.set(partNode.getTargetObjectId()); - partNodeId.set(partNode.getId()); + }); + + Runnable executeDropAttributeOnDiagram = () -> this.dropFromExplorer(GeneralViewItemAndAttributeProjectData.GraphicalIds.DIAGRAM_ID, + GeneralViewItemAndAttributeProjectData.SemanticIds.P1_X1_ID); + + Consumer diagramContentConsumerAfterAttributeDrop = assertRefreshedDiagramThat(diagram -> { + assertThat(diagram.getNodes()).hasSize(5); + var attributeNode = new DiagramNavigator(diagram).nodeWithLabel(LabelConstants.OPEN_QUOTE + "attribute" + LabelConstants.CLOSE_QUOTE + LabelConstants.CR + "x1").getNode(); + assertThat(attributeNode).extracting(node -> node.getState()).isEqualTo(ViewModifier.Normal); }); StepVerifier.create(flux) @@ -155,8 +148,26 @@ public void dropPartFromTheExplorer() { .then(executeRemoveFromDiagramTool) .consumeNextWith(diagramContentConsumerAfterRemove) .then(executeDropPartOnDiagram) - .consumeNextWith(diagramContentConsumerAfterDrop) + .consumeNextWith(diagramContentConsumerAfterPartDrop) + .then(executeDropAttributeOnDiagram) + .consumeNextWith(diagramContentConsumerAfterAttributeDrop) .thenCancel() - .verify(Duration.ofSeconds(10)); + .verify(Duration.ofSeconds(1000)); + } + + /** + * Drops an Explorer element on the diagram background and verifies the successful GraphQL payload. + * + * @param diagramId + * the diagram representation identifier. + * @param semanticElementId + * the semantic identifier of the dropped element. + */ + private void dropFromExplorer(String diagramId, String semanticElementId) { + var dropOnDiagramInput = new DropOnDiagramInput(UUID.randomUUID(), GeneralViewItemAndAttributeProjectData.EDITING_CONTEXT_ID, diagramId, + diagramId, List.of(semanticElementId), 0, 0); + var dropOnDiagramResult = this.dropOnDiagramMutationRunner.run(dropOnDiagramInput); + var typename = JsonPath.read(dropOnDiagramResult.data(), "$.data.dropOnDiagram.__typename"); + assertThat(typename).isEqualTo(DropOnDiagramSuccessPayload.class.getSimpleName()); } } diff --git a/backend/services/syson-diagram-services/src/main/java/org/eclipse/syson/diagram/services/DiagramMutationDndService.java b/backend/services/syson-diagram-services/src/main/java/org/eclipse/syson/diagram/services/DiagramMutationDndService.java index e9a47a575..8975023da 100644 --- a/backend/services/syson-diagram-services/src/main/java/org/eclipse/syson/diagram/services/DiagramMutationDndService.java +++ b/backend/services/syson-diagram-services/src/main/java/org/eclipse/syson/diagram/services/DiagramMutationDndService.java @@ -17,7 +17,6 @@ import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Map.Entry; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -132,18 +131,12 @@ public DiagramMutationDndService(SiriusWebCoreServices siriusWebCoreServices, IV public Element dropElementFromExplorer(Element element, IEditingContext editingContext, DiagramContext diagramContext, Node selectedNode, Map convertedNodes) { Optional optTargetElement; - Optional optNodeDescription = Optional.empty(); if (selectedNode != null) { optTargetElement = this.siriusWebCoreServices.objectSearchService().getObject(editingContext, selectedNode.getTargetObjectId()); - optNodeDescription = convertedNodes.entrySet().stream().filter(entry -> entry.getValue().getId().equals(selectedNode.getDescriptionId())).map(Entry::getKey).findFirst(); } else { optTargetElement = this.siriusWebCoreServices.objectSearchService().getObject(editingContext, diagramContext.diagram().getTargetObjectId()); } - if (optNodeDescription.isPresent() && optNodeDescription.get().getName().contains("EmptyDiagram")) { - // The element is dropped on the information box displayed on an empty diagram. This box is visible only if - // the diagram is empty, so we want to actually perform the drop on the diagram itself. - return this.dropElementFromExplorer(element, editingContext, diagramContext, null, convertedNodes); - } else if (optTargetElement.isPresent() && optTargetElement.get() instanceof Element targetElement) { + if (optTargetElement.isPresent() && optTargetElement.get() instanceof Element targetElement) { // Check if the element we attempt to drop is in the ancestors of the target element and we attempt to drop // it on anything else than the diagram background. If it is the case we want to prevent the drop. if (EMFUtils.isAncestor(element, targetElement) && selectedNode != null) { @@ -457,14 +450,19 @@ private void dropElementFromExplorerInTarget(Element sourceElement, Element targ } else { Node newSelectedNode = selectedNode; if (selectedNode == null) { - // try to get the graphical node corresponding to the semantic parent + // Try to get the graphical node corresponding to the semantic parent. var parentId = new EObjectIDManager().findId(sourceElement.getOwner()); - var optParentNode = diagramContext.diagram().getNodes().stream().filter(n -> parentId.isPresent() && Objects.equals(n.getTargetObjectId(), parentId.get())).findFirst(); + var optParentNode = diagramContext.diagram().getNodes().stream() + .filter(node -> parentId.isPresent() && Objects.equals(node.getTargetObjectId(), parentId.get())) + .findFirst(); if (optParentNode.isPresent()) { newSelectedNode = optParentNode.get(); } } this.diagramMutationExposeService.expose(sourceElement, editingContext, diagramContext, newSelectedNode, convertedNodes); + if (selectedNode == null) { + this.revealElementDroppedOnDiagramBackground(sourceElement, editingContext, diagramContext); + } } } else { ViewCreationRequest parentViewCreationRequest = this.diagramMutationElementService.createView(sourceElement, editingContext, diagramContext, selectedNode, convertedNodes); @@ -491,6 +489,31 @@ private void dropElementFromExplorerInTarget(Element sourceElement, Element targ } } + /** + * Reveals the root graphical node created for an element dropped on the diagram background. + * + * @param sourceElement + * the dropped semantic element. + * @param editingContext + * the editing context of the tool. + * @param diagramContext + * the {@link DiagramContext} of the tool. + */ + private void revealElementDroppedOnDiagramBackground(Element sourceElement, IEditingContext editingContext, DiagramContext diagramContext) { + var descriptionId = this.diagramQueryElementService.getNodeDescriptionId(sourceElement, diagramContext.diagram(), editingContext); + if (descriptionId.isPresent()) { + var nodeId = new NodeIdProvider().getNodeId(diagramContext.diagram().getId(), descriptionId.get(), NodeContainmentKind.CHILD_NODE, + this.siriusWebCoreServices.identityService().getId(sourceElement)); + diagramContext.diagramEvents().removeIf(diagEvent -> { + if (diagEvent instanceof HideDiagramElementEvent hideDiagramElementEvent && hideDiagramElementEvent.getElementIds().contains(nodeId)) { + return true; + } + return false; + }); + diagramContext.diagramEvents().add(new HideDiagramElementEvent(Set.of(nodeId), false)); + } + } + /** * Handle the case where the semantic element dropped is already exposed on the diagram. There are four distinct * cases to handle: @@ -633,11 +656,22 @@ private String findParentTargetObjectId(DiagramContext diagramContext, Node visi return parentTargetObjectId; } - private Set findSubNodesBySemanticElementId(DiagramContext diagramContext, Node parenNode, String semanticElementId) { + /** + * Finds the direct graphical representations of a semantic element in the specified drop target. + * + * @param diagramContext + * the {@link DiagramContext} of the tool. + * @param parentNode + * the graphical drop target, or {@code null} for the diagram background. + * @param semanticElementId + * the identifier of the semantic element to find. + * @return the matching graphical nodes. + */ + private Set findSubNodesBySemanticElementId(DiagramContext diagramContext, Node parentNode, String semanticElementId) { Set subnodes = new HashSet<>(); - if (parenNode != null) { - subnodes.addAll(parenNode.getChildNodes()); - subnodes.addAll(parenNode.getBorderNodes()); + if (parentNode != null) { + subnodes.addAll(parentNode.getChildNodes()); + subnodes.addAll(parentNode.getBorderNodes()); } else { subnodes.addAll(diagramContext.diagram().getNodes()); } diff --git a/doc/content/modules/user-manual/pages/release-notes/2026.9.0.adoc b/doc/content/modules/user-manual/pages/release-notes/2026.9.0.adoc index a5979ac01..7bffa5ccc 100644 --- a/doc/content/modules/user-manual/pages/release-notes/2026.9.0.adoc +++ b/doc/content/modules/user-manual/pages/release-notes/2026.9.0.adoc @@ -14,6 +14,8 @@ * In diagrams: +** Fix dropping a nested `AttributeUsage` from the _Explorer_ onto a _General View_ diagram so it is visible on the first drop. + ** Fix `Comment` graphical nodes so changing their background color no longer changes their border color. * In all views: