Skip to content

Commit a1d3cff

Browse files
committed
[1547] Fix Delete from diagram on elements inside a Package
Bug: #1547 Signed-off-by: Axel RICHARD <axel.richard@obeo.fr>
1 parent 96b278d commit a1d3cff

6 files changed

Lines changed: 79 additions & 11 deletions

File tree

CHANGELOG.adoc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ Now it has been fixed in Sirius Web 2025.8.4, `SysONUpdateLibraryExecutor` has b
8484
- https://github.com/eclipse-syson/syson/issues/1545[#1545] [diagrams] Add _interconnection_ compartment to `PartDefinition` nodes in the standard diagrams.
8585
- https://github.com/eclipse-syson/syson/issues/1546[#1546] [diagrams] Fix drag and drop of a graphical node from the diagram background into a _Package_ graphical node.
8686
- https://github.com/eclipse-syson/syson/issues/1550[#1550] [diagrams] Fix an issue where the _action flow_ compartment was not revealed when the _New Start Action_ was executed.
87+
- https://github.com/eclipse-syson/syson/issues/1547[#1547] [diagrams] Fix an issue where the _Delete from diagram_ action did nothing on elements inside a _Package_ in diagrams.
8788

8889
=== Improvements
8990

backend/application/syson-application/src/test/java/org/eclipse/syson/application/controllers/diagrams/general/view/GVViewUsageExposedElementsTests.java

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
package org.eclipse.syson.application.controllers.diagrams.general.view;
1414

1515
import static org.assertj.core.api.Assertions.assertThat;
16+
import static org.eclipse.sirius.components.diagrams.tests.DiagramEventPayloadConsumer.assertRefreshedDiagramThat;
1617
import static org.junit.jupiter.api.Assertions.assertEquals;
1718

1819
import java.time.Duration;
@@ -36,6 +37,7 @@
3637
import org.eclipse.sirius.components.diagrams.Diagram;
3738
import org.eclipse.sirius.components.diagrams.Node;
3839
import org.eclipse.sirius.components.diagrams.tests.graphql.LayoutDiagramMutationRunner;
40+
import org.eclipse.sirius.components.diagrams.tests.navigation.DiagramNavigator;
3941
import org.eclipse.sirius.components.graphql.tests.ExecuteEditingContextFunctionInput;
4042
import org.eclipse.sirius.components.graphql.tests.ExecuteEditingContextFunctionRunner;
4143
import org.eclipse.sirius.components.graphql.tests.ExecuteEditingContextFunctionSuccessPayload;
@@ -213,7 +215,7 @@ public void addExistingElementsToolShouldUpdateExposedElements() {
213215
this.verifier.then(semanticChecker);
214216
}
215217

216-
@DisplayName("GIVEN a GV diagram on a ViewUsage, WHEN Add existing element(s) (recursive) tool is executed, THEN a the ViewUsage#exposedElements is updated with partA and partB")
218+
@DisplayName("GIVEN a GV diagram on a ViewUsage, WHEN Add existing element(s) (recursive) tool is executed, THEN the ViewUsage#exposedElements is updated with partA and partB")
217219
@Sql(scripts = { ViewUsageExposedElementsTestProjectData.SCRIPT_PATH }, executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD,
218220
config = @SqlConfig(transactionMode = SqlConfig.TransactionMode.ISOLATED))
219221
@Sql(scripts = { "/scripts/cleanup.sql" }, executionPhase = Sql.ExecutionPhase.AFTER_TEST_METHOD, config = @SqlConfig(transactionMode = SqlConfig.TransactionMode.ISOLATED))
@@ -303,4 +305,47 @@ public void updateExposedElementsShouldUpdateTheDiagram() {
303305

304306
this.diagramCheckerService.checkDiagram(diagramChecker, this.diagram, this.verifier);
305307
}
308+
309+
@DisplayName("GIVEN a GV diagram on a ViewUsage, WHEN an element is created inside a Package, THEN the element should only be visible inside the Package")
310+
@Sql(scripts = { ViewUsageExposedElementsTestProjectData.SCRIPT_PATH }, executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD,
311+
config = @SqlConfig(transactionMode = SqlConfig.TransactionMode.ISOLATED))
312+
@Sql(scripts = { "/scripts/cleanup.sql" }, executionPhase = Sql.ExecutionPhase.AFTER_TEST_METHOD, config = @SqlConfig(transactionMode = SqlConfig.TransactionMode.ISOLATED))
313+
@Test
314+
public void exposePackageChildShouldDisplayChildOnlyinPAckage() {
315+
var newPackageToolId = this.diagramDescriptionIdProvider.getDiagramCreationToolId("New Package");
316+
assertThat(newPackageToolId).as("The tool 'New Package' should exist on the diagram").isNotNull();
317+
var newInterfaceToolId = this.diagramDescriptionIdProvider.getNodeCreationToolId(this.descriptionNameGenerator.getNodeName(SysmlPackage.eINSTANCE.getPackage()), "New Interface");
318+
assertThat(newInterfaceToolId).as("The tool 'New Interface' should exist on the Package").isNotNull();
319+
320+
Runnable newPackageTool = () -> this.toolTester.invokeTool(ViewUsageExposedElementsTestProjectData.EDITING_CONTEXT_ID, this.diagram, newPackageToolId);
321+
322+
var packageNodeId = new AtomicReference<String>();
323+
324+
Consumer<Object> updatedDiagramWithPackage = assertRefreshedDiagramThat(diag -> {
325+
int diagramRootNodesCount = diag.getNodes().size();
326+
assertThat(diagramRootNodesCount).isEqualTo(1);
327+
var packageNode = new DiagramNavigator(diag).nodeWithLabel("Package1").getNode();
328+
assertThat(packageNode).isNotNull();
329+
assertThat(packageNode.getChildNodes()).hasSize(0);
330+
packageNodeId.set(packageNode.getId());
331+
});
332+
333+
Runnable newInterfaceTool = () -> this.toolTester.invokeTool(ViewUsageExposedElementsTestProjectData.EDITING_CONTEXT_ID, this.diagram.get().getId(), packageNodeId.get(), newInterfaceToolId,
334+
List.of());
335+
336+
Consumer<Object> updatedDiagramWithInterface = assertRefreshedDiagramThat(diag -> {
337+
int diagramRootNodesCount = diag.getNodes().size();
338+
assertThat(diagramRootNodesCount).isEqualTo(1);
339+
var packageNode = new DiagramNavigator(diag).nodeWithLabel("Package1").getNode();
340+
assertThat(packageNode.getChildNodes()).hasSize(1);
341+
var interfaceNode = new DiagramNavigator(diag).nodeWithLabel("Package1").childNodeWithLabel("\u00ABinterface\u00BB\ninterface1").getNode();
342+
assertThat(interfaceNode).isNotNull();
343+
});
344+
345+
this.verifier
346+
.then(newPackageTool)
347+
.consumeNextWith(updatedDiagramWithPackage)
348+
.then(newInterfaceTool)
349+
.consumeNextWith(updatedDiagramWithInterface);
350+
}
306351
}

backend/services/syson-services/src/main/java/org/eclipse/syson/services/ToolService.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,6 @@ public Element expose(Element element, IEditingContext editingContext, DiagramCo
133133
var membershipExpose = SysmlFactory.eINSTANCE.createMembershipExpose();
134134
membershipExpose.setImportedMembership(element.getOwningMembership());
135135
viewUsage.getOwnedRelationship().add(membershipExpose);
136-
if (element instanceof Package) {
137-
membershipExpose.setIsRecursive(true);
138-
}
139136
}
140137
}
141138
return element;

backend/views/syson-diagram-common-view/src/main/java/org/eclipse/syson/diagram/common/view/services/ViewNodeService.java

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
import org.eclipse.syson.sysml.Type;
5757
import org.eclipse.syson.sysml.ViewDefinition;
5858
import org.eclipse.syson.sysml.ViewUsage;
59+
import org.eclipse.syson.sysml.helper.EMFUtils;
5960
import org.eclipse.syson.sysml.util.ElementUtil;
6061
import org.eclipse.syson.util.NodeFinder;
6162
import org.slf4j.Logger;
@@ -512,21 +513,21 @@ public boolean showAnnotatingNode(Element element, DiagramContext diagramContext
512513
*/
513514
protected Set<Element> getDirectExposedElements(ViewUsage viewUsage) {
514515
var directExposedElements = new HashSet<Element>();
515-
List<Expose> exposes = viewUsage.getOwnedRelationship().stream()
516+
List<Expose> exposedElements = viewUsage.getOwnedRelationship().stream()
516517
.filter(Expose.class::isInstance)
517518
.map(Expose.class::cast)
518519
.toList();
519-
for (Expose expose : exposes) {
520-
Element importedElement = expose.getImportedElement();
520+
for (Expose exposedElement : exposedElements) {
521+
Element importedElement = exposedElement.getImportedElement();
521522
if (importedElement instanceof Package) {
522523
directExposedElements.add(importedElement);
523-
} else if (expose instanceof MembershipExpose membershipExpose) {
524+
} else if (exposedElement instanceof MembershipExpose membershipExpose) {
524525
var importedMembership = membershipExpose.getImportedMembership();
525526
if (importedMembership != null) {
526527
var memberElement = importedMembership.getMemberElement();
527-
if (memberElement != null) {
528+
if (memberElement != null && !this.isChildOfExposedPackage(memberElement, exposedElements)) {
528529
directExposedElements.add(memberElement);
529-
if (expose.isIsRecursive()) {
530+
if (exposedElement.isIsRecursive()) {
530531
directExposedElements.addAll(this.getRecursiveContents(memberElement));
531532
}
532533
}
@@ -537,6 +538,29 @@ protected Set<Element> getDirectExposedElements(ViewUsage viewUsage) {
537538
return directExposedElements;
538539
}
539540

541+
/**
542+
* Check if the given {@link Element} is a child of an exposed Package. In this case, we don't want to display it on
543+
* the diagram background, or as a ViewUsage child node. It will be displayed inside another node.
544+
*
545+
* @param element
546+
* the element that could be a child of an exposed element
547+
* @param exposedElements
548+
* the list of exposed elements
549+
* @return <code>true</code> if the given {@link Element} is a child of an exposed element, <code>false</code>
550+
* otherwise.
551+
*/
552+
protected boolean isChildOfExposedPackage(Element element, List<Expose> exposedElements) {
553+
boolean isChildOfExposedElement = false;
554+
for (Expose exposedElement : exposedElements) {
555+
Element importedElement = exposedElement.getImportedElement();
556+
if (importedElement instanceof Package && !Objects.equals(element, importedElement) && EMFUtils.isAncestor(importedElement, element)) {
557+
isChildOfExposedElement = true;
558+
break;
559+
}
560+
}
561+
return isChildOfExposedElement;
562+
}
563+
540564
protected List<Element> getRecursiveContents(Element element) {
541565
var contents = new ArrayList<Element>();
542566
var ownedElements = element.getOwnedElement().stream().filter(Objects::nonNull).toList();

backend/views/syson-diagram-common-view/src/main/java/org/eclipse/syson/diagram/common/view/services/ViewToolService.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ public Element addToExposedElements(Element element, boolean recursive, IEditing
154154
var membershipExpose = SysmlFactory.eINSTANCE.createMembershipExpose();
155155
membershipExpose.setImportedMembership(childElement.getOwningMembership());
156156
viewUsage.getOwnedRelationship().add(membershipExpose);
157-
if (childElement instanceof Package || recursive) {
157+
if (recursive) {
158158
membershipExpose.setIsRecursive(true);
159159
}
160160
}

doc/content/modules/user-manual/pages/release-notes/2025.10.0.adoc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ Now it is only available on SysML model elements.
7575
- Fix an issue where diagrams' edges were blinking when refreshing a diagram.
7676
- Fix drag and drop of a graphical node from the diagram background into a _Package_ graphical node.
7777
- Fix an issue where the _action flow_ compartment was not revealed when the _New Start Action_ was executed.
78+
- Fix an issue where the _Delete from diagram_ action did nothing on elements inside a _Package_ in diagrams.
7879

7980
== Improvements
8081

0 commit comments

Comments
 (0)