This is an automated email from the ASF dual-hosted git repository.
mattcasters pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/hop.git
The following commit(s) were added to refs/heads/main by this push:
new fefa41f87b Fixes #8736 : Keep the Problems tab on detach, list
whole-file remarks first and leave submenus out of the actions view (#8741)
fefa41f87b is described below
commit fefa41f87ba331866b9866f610630e2c6334cae8
Author: Bart Maertens <[email protected]>
AuthorDate: Mon Oct 5 21:11:22 2026 +0200
Fixes #8736 : Keep the Problems tab on detach, list whole-file remarks
first and leave submenus out of the actions view (#8741)
---
.../apache/hop/ui/hopgui/file/ProblemsTabTest.java | 173 +++++++++++++++++++++
.../ui/hopgui/context/menu/MenuContextHandler.java | 6 +
.../delegates/HopGuiPipelineCheckDelegate.java | 32 +++-
.../delegates/HopGuiWorkflowCheckDelegate.java | 32 +++-
.../ui/hopgui/messages/messages_en_US.properties | 2 +
.../context/menu/MenuContextHandlerTest.java | 67 ++++++++
6 files changed, 310 insertions(+), 2 deletions(-)
diff --git
a/rcp/src/test/java/org/apache/hop/ui/hopgui/file/ProblemsTabTest.java
b/rcp/src/test/java/org/apache/hop/ui/hopgui/file/ProblemsTabTest.java
new file mode 100644
index 0000000000..7ebd1dbdeb
--- /dev/null
+++ b/rcp/src/test/java/org/apache/hop/ui/hopgui/file/ProblemsTabTest.java
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hop.ui.hopgui.file;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.hop.core.CheckResult;
+import org.apache.hop.core.ICheckResult;
+import org.apache.hop.core.ICheckResultSource;
+import org.apache.hop.pipeline.PipelineMeta;
+import org.apache.hop.pipeline.transform.TransformMeta;
+import org.apache.hop.ui.hopgui.file.pipeline.HopGuiPipelineGraph;
+import
org.apache.hop.ui.hopgui.file.pipeline.delegates.HopGuiPipelineCheckDelegate;
+import org.apache.hop.ui.hopgui.file.workflow.HopGuiWorkflowGraph;
+import
org.apache.hop.ui.hopgui.file.workflow.delegates.HopGuiWorkflowCheckDelegate;
+import org.apache.hop.ui.testing.SwtBotTestBase;
+import org.apache.hop.workflow.WorkflowMeta;
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.custom.CTabFolder;
+import org.eclipse.swt.layout.FillLayout;
+import org.eclipse.swt.widgets.Composite;
+import org.eclipse.swt.widgets.Control;
+import org.eclipse.swt.widgets.Tree;
+import org.eclipse.swt.widgets.TreeItem;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+
+/**
+ * The Problems tab of a pipeline or workflow.
+ *
+ * <p>Detaching the execution results builds the tab again, and it came back
empty. Remarks about
+ * the pipeline as a whole were listed between two transforms' groups, as if
they belonged to one.
+ *
+ * @see <a href="https://github.com/apache/hop/issues/8736";>#8736</a>
+ */
+@Tag("uitest")
+class ProblemsTabTest extends SwtBotTestBase {
+
+ private static TransformMeta transform(String name) {
+ TransformMeta transformMeta = new TransformMeta();
+ transformMeta.setName(name);
+ transformMeta.setTransformPluginId("Dummy");
+ return transformMeta;
+ }
+
+ private static ICheckResultSource action(String name) {
+ ICheckResultSource source = mock(ICheckResultSource.class);
+ when(source.getName()).thenReturn(name);
+ return source;
+ }
+
+ private static ICheckResult warning(String text, ICheckResultSource source) {
+ return new CheckResult(ICheckResult.TYPE_RESULT_WARNING, text, source);
+ }
+
+ /** The top-level rows of the tab, with the first child of each, as "group >
first remark". */
+ private static List<String> rows(Composite parent) {
+ List<String> rows = new ArrayList<>();
+ Tree tree = findTree(parent);
+ for (TreeItem item : tree.getItems()) {
+ rows.add(
+ item.getItemCount() == 0
+ ? item.getText()
+ : item.getText() + " > " + item.getItem(0).getText());
+ }
+ return rows;
+ }
+
+ private static Tree findTree(Composite parent) {
+ for (Control child : parent.getChildren()) {
+ if (child instanceof Tree tree && !tree.isDisposed()) {
+ return tree;
+ }
+ if (child instanceof Composite composite) {
+ Tree tree = findTree(composite);
+ if (tree != null) {
+ return tree;
+ }
+ }
+ }
+ return null;
+ }
+
+ @Test
+ void pipelineRemarksAreOnTopAndSurviveADetach() {
+ PipelineMeta pipelineMeta = new PipelineMeta();
+ pipelineMeta.setName("load-customers");
+ HopGuiPipelineGraph graph = mock(HopGuiPipelineGraph.class);
+ when(graph.getPipelineMeta()).thenReturn(pipelineMeta);
+ List<ICheckResult> remarks =
+ List.of(
+ warning("No description", transform("Dummy (do nothing) 2")),
+ warning("[STRUCT-003] The pipeline has disabled hops", null),
+ warning("No description", transform("Dummy (do nothing)")));
+ List<String> expected =
+ List.of(
+ "load-customers > [STRUCT-003] The pipeline has disabled hops",
+ "Dummy (do nothing) 2 > No description",
+ "Dummy (do nothing) > No description");
+
+ List<List<String>> seen = new ArrayList<>();
+ withScene(
+ shell -> {
+ shell.setLayout(new FillLayout());
+ graph.extraViewTabFolder = new CTabFolder(shell, SWT.NONE);
+ HopGuiPipelineCheckDelegate delegate = new
HopGuiPipelineCheckDelegate(null, graph);
+ delegate.addPipelineCheck();
+ delegate.refresh(remarks);
+ seen.add(rows(shell));
+
+ graph.extraViewTabFolder.dispose();
+ graph.extraViewTabFolder = new CTabFolder(shell, SWT.NONE);
+ delegate.addPipelineCheck();
+ seen.add(rows(shell));
+ },
+ bot -> {});
+
+ assertEquals(expected, seen.get(0), "as listed");
+ assertEquals(expected, seen.get(1), "after the results view was detached");
+ }
+
+ @Test
+ void workflowRemarksAreOnTopAndSurviveADetach() {
+ WorkflowMeta workflowMeta = new WorkflowMeta();
+ workflowMeta.setName("nightly");
+ HopGuiWorkflowGraph graph = mock(HopGuiWorkflowGraph.class);
+ when(graph.getWorkflowMeta()).thenReturn(workflowMeta);
+ List<ICheckResult> remarks =
+ List.of(
+ warning("No server", action("Mail")),
+ warning("[STRUCT-003] The workflow has disabled hops", null));
+ List<String> expected =
+ List.of("nightly > [STRUCT-003] The workflow has disabled hops", "Mail
> No server");
+
+ List<List<String>> seen = new ArrayList<>();
+ withScene(
+ shell -> {
+ shell.setLayout(new FillLayout());
+ graph.extraViewTabFolder = new CTabFolder(shell, SWT.NONE);
+ HopGuiWorkflowCheckDelegate delegate = new
HopGuiWorkflowCheckDelegate(null, graph);
+ delegate.addWorkflowCheck();
+ delegate.refresh(remarks);
+ seen.add(rows(shell));
+
+ graph.extraViewTabFolder.dispose();
+ graph.extraViewTabFolder = new CTabFolder(shell, SWT.NONE);
+ delegate.addWorkflowCheck();
+ seen.add(rows(shell));
+ },
+ bot -> {});
+
+ assertEquals(expected, seen.get(0), "as listed");
+ assertEquals(expected, seen.get(1), "after the results view was detached");
+ }
+}
diff --git
a/ui/src/main/java/org/apache/hop/ui/hopgui/context/menu/MenuContextHandler.java
b/ui/src/main/java/org/apache/hop/ui/hopgui/context/menu/MenuContextHandler.java
index 38096cff14..91751e8ef8 100644
---
a/ui/src/main/java/org/apache/hop/ui/hopgui/context/menu/MenuContextHandler.java
+++
b/ui/src/main/java/org/apache/hop/ui/hopgui/context/menu/MenuContextHandler.java
@@ -70,6 +70,12 @@ public class MenuContextHandler implements
IGuiContextHandler {
continue;
}
+ // An item with children is a submenu: the menu opens it, and as an
action it did nothing.
+ // Its children are listed under its label as their category.
+ if (!registry.findChildGuiMenuItems(rootMenuId, item.getId()).isEmpty())
{
+ continue;
+ }
+
String parentId = item.getParentId();
if (parentId != null) {
GuiMenuItem parentMenuItem = registry.findGuiMenuItem(rootMenuId,
parentId);
diff --git
a/ui/src/main/java/org/apache/hop/ui/hopgui/file/pipeline/delegates/HopGuiPipelineCheckDelegate.java
b/ui/src/main/java/org/apache/hop/ui/hopgui/file/pipeline/delegates/HopGuiPipelineCheckDelegate.java
index 56ef579fe1..d9db46aab8 100644
---
a/ui/src/main/java/org/apache/hop/ui/hopgui/file/pipeline/delegates/HopGuiPipelineCheckDelegate.java
+++
b/ui/src/main/java/org/apache/hop/ui/hopgui/file/pipeline/delegates/HopGuiPipelineCheckDelegate.java
@@ -30,6 +30,7 @@ import org.apache.hop.core.Props;
import org.apache.hop.core.SwtUniversalImage;
import org.apache.hop.core.gui.plugin.GuiPlugin;
import org.apache.hop.core.gui.plugin.toolbar.GuiToolbarElement;
+import org.apache.hop.core.util.Utils;
import org.apache.hop.i18n.BaseMessages;
import org.apache.hop.pipeline.PipelineMeta;
import org.apache.hop.pipeline.transform.TransformMeta;
@@ -71,6 +72,12 @@ public class HopGuiPipelineCheckDelegate {
@Getter private GuiToolbarWidgets toolBarWidgets;
private Tree wTree;
+ /**
+ * What the tab shows. Detaching or docking the results view builds the tab
again, empty; the
+ * remarks are put back from here.
+ */
+ private List<ICheckResult> shownRemarks = List.of();
+
/**
* Check pipeline and transforms
*
@@ -151,6 +158,10 @@ public class HopGuiPipelineCheckDelegate {
fdTree.bottom = new FormAttachment(100, 0);
wTree.setLayoutData(fdTree);
wTree.addListener(SWT.DefaultSelection, this::edit);
+
+ if (!shownRemarks.isEmpty()) {
+ refresh(shownRemarks);
+ }
}
@GuiToolbarElement(
@@ -257,9 +268,13 @@ public class HopGuiPipelineCheckDelegate {
* @param remarks the remarks to show
*/
public void refresh(List<ICheckResult> remarks) {
+ shownRemarks = List.copyOf(remarks);
wTree.setRedraw(false);
wTree.removeAll();
+ // Remarks about the pipeline as a whole sit together at the top, under
its name. Added at the
+ // top level in arrival order, they landed between two groups and read as
belonging to one.
+ TreeItem pipelineItem = null;
Map<ICheckResultSource, TreeItem> mapSourceItems = new HashMap<>();
for (ICheckResult cr : remarks) {
// Ignore OK result
@@ -268,7 +283,12 @@ public class HopGuiPipelineCheckDelegate {
ICheckResultSource source = cr.getSourceInfo();
TreeItem item = mapSourceItems.get(source);
if (source == null) {
- item = new TreeItem(wTree, SWT.NONE);
+ if (pipelineItem == null) {
+ pipelineItem = new TreeItem(wTree, SWT.NONE, 0);
+ pipelineItem.setText(pipelineLabel());
+ pipelineItem.setImage(GuiResource.getInstance().getImagePipeline());
+ }
+ item = new TreeItem(pipelineItem, SWT.NONE);
} else if (item == null) {
TreeItem parentItem = new TreeItem(wTree, SWT.NONE);
parentItem.setText(source.getName());
@@ -297,9 +317,19 @@ public class HopGuiPipelineCheckDelegate {
item.setImage(image);
}
}
+ if (pipelineItem != null) {
+ pipelineItem.setExpanded(true);
+ }
wTree.setRedraw(true);
}
+ private String pipelineLabel() {
+ String name = pipelineGraph.getPipelineMeta().getName();
+ return Utils.isEmpty(name)
+ ? BaseMessages.getString(PKG, "PipelineGraph.Check.PipelineRemarks")
+ : name;
+ }
+
private Image getImage(ICheckResult cr) {
return switch (cr.getType()) {
case ICheckResult.TYPE_RESULT_OK ->
GuiResource.getInstance().getImageTrue();
diff --git
a/ui/src/main/java/org/apache/hop/ui/hopgui/file/workflow/delegates/HopGuiWorkflowCheckDelegate.java
b/ui/src/main/java/org/apache/hop/ui/hopgui/file/workflow/delegates/HopGuiWorkflowCheckDelegate.java
index 98c77b93f8..cc8fc28df1 100644
---
a/ui/src/main/java/org/apache/hop/ui/hopgui/file/workflow/delegates/HopGuiWorkflowCheckDelegate.java
+++
b/ui/src/main/java/org/apache/hop/ui/hopgui/file/workflow/delegates/HopGuiWorkflowCheckDelegate.java
@@ -29,6 +29,7 @@ import org.apache.hop.core.Props;
import org.apache.hop.core.SwtUniversalImage;
import org.apache.hop.core.gui.plugin.GuiPlugin;
import org.apache.hop.core.gui.plugin.toolbar.GuiToolbarElement;
+import org.apache.hop.core.util.Utils;
import org.apache.hop.i18n.BaseMessages;
import org.apache.hop.ui.core.PropsUi;
import org.apache.hop.ui.core.dialog.ErrorDialog;
@@ -71,6 +72,12 @@ public class HopGuiWorkflowCheckDelegate {
@Getter private GuiToolbarWidgets toolBarWidgets;
private Tree wTree;
+ /**
+ * What the tab shows. Detaching or docking the results view builds the tab
again, empty; the
+ * remarks are put back from here.
+ */
+ private List<ICheckResult> shownRemarks = List.of();
+
/**
* Check workflow and actions
*
@@ -152,6 +159,10 @@ public class HopGuiWorkflowCheckDelegate {
wTree.addListener(SWT.DefaultSelection, this::edit);
workflowCheckTab.setControl(checkComposite);
+
+ if (!shownRemarks.isEmpty()) {
+ refresh(shownRemarks);
+ }
}
@GuiToolbarElement(
@@ -231,9 +242,13 @@ public class HopGuiWorkflowCheckDelegate {
* @param remarks the remarks to show
*/
public void refresh(List<ICheckResult> remarks) {
+ shownRemarks = List.copyOf(remarks);
wTree.setRedraw(false);
wTree.removeAll();
+ // Remarks about the workflow as a whole sit together at the top, under
its name. Added at the
+ // top level in arrival order, they landed between two groups and read as
belonging to one.
+ TreeItem workflowItem = null;
Map<ICheckResultSource, TreeItem> mapSourceItems = new HashMap<>();
for (ICheckResult cr : remarks) {
// Ignore OK result
@@ -242,7 +257,12 @@ public class HopGuiWorkflowCheckDelegate {
ICheckResultSource source = cr.getSourceInfo();
TreeItem item = mapSourceItems.get(source);
if (source == null) {
- item = new TreeItem(wTree, SWT.NONE);
+ if (workflowItem == null) {
+ workflowItem = new TreeItem(wTree, SWT.NONE, 0);
+ workflowItem.setText(workflowLabel());
+ workflowItem.setImage(GuiResource.getInstance().getImageWorkflow());
+ }
+ item = new TreeItem(workflowItem, SWT.NONE);
} else if (item == null) {
TreeItem parentItem = new TreeItem(wTree, SWT.NONE);
parentItem.setText(source.getName());
@@ -271,9 +291,19 @@ public class HopGuiWorkflowCheckDelegate {
}
}
+ if (workflowItem != null) {
+ workflowItem.setExpanded(true);
+ }
wTree.setRedraw(true);
}
+ private String workflowLabel() {
+ String name = workflowGraph.getWorkflowMeta().getName();
+ return Utils.isEmpty(name)
+ ? BaseMessages.getString(PKG, "WorkflowGraph.Check.WorkflowRemarks")
+ : name;
+ }
+
private Image getImage(ICheckResult cr) {
return switch (cr.getType()) {
case ICheckResult.TYPE_RESULT_OK ->
GuiResource.getInstance().getImageTrue();
diff --git
a/ui/src/main/resources/org/apache/hop/ui/hopgui/messages/messages_en_US.properties
b/ui/src/main/resources/org/apache/hop/ui/hopgui/messages/messages_en_US.properties
index 83ee807799..37e33ec483 100644
---
a/ui/src/main/resources/org/apache/hop/ui/hopgui/messages/messages_en_US.properties
+++
b/ui/src/main/resources/org/apache/hop/ui/hopgui/messages/messages_en_US.properties
@@ -195,6 +195,7 @@ LogBrowser.Filter.CaseSensitive.Tooltip=Match the filter
text with case sensitiv
LogBrowser.Filter.Exclude.Label=Exclude
LogBrowser.Filter.Exclude.Tooltip=Hide log lines that match the filter text
(exclude from the log view)
PipelineGraph.Check.Tab.Name=Problems
+PipelineGraph.Check.PipelineRemarks=Pipeline
PipelineGraph.Check.ErrorCheckingPipeline.Exception=Error checking pipeline:
{0}
PipelineGraph.Check.ErrorCheckingPipeline.Message=Error checking pipeline
PipelineGraph.ContextualActionDialog.Hop.Header=Select the hop action to take:
@@ -355,6 +356,7 @@ PipelineLog.System.ERROR2=ERROR
PipelineLog.System.EXCEPTION=EXCEPTION
PipelineLog.System.EXCEPTION2=EXCEPTION
WorkflowGraph.Check.Tab.Name=Problems
+WorkflowGraph.Check.WorkflowRemarks=Workflow
WorkflowGraph.Check.ErrorCheckingWorkflow.Exception=Error checking workflow:
{0}
WorkflowGraph.Check.ErrorCheckingWorkflow.Message=Error checking workflow
WorkflowGraph.Dialog.HopCausesLoop.Message=This hop causes a loop. This is not
allowed.
diff --git
a/ui/src/test/java/org/apache/hop/ui/hopgui/context/menu/MenuContextHandlerTest.java
b/ui/src/test/java/org/apache/hop/ui/hopgui/context/menu/MenuContextHandlerTest.java
new file mode 100644
index 0000000000..082f4fcefb
--- /dev/null
+++
b/ui/src/test/java/org/apache/hop/ui/hopgui/context/menu/MenuContextHandlerTest.java
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hop.ui.hopgui.context.menu;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.util.List;
+import org.apache.hop.core.gui.plugin.GuiRegistry;
+import org.apache.hop.core.gui.plugin.action.GuiAction;
+import org.apache.hop.core.gui.plugin.menu.GuiMenuElementType;
+import org.apache.hop.core.gui.plugin.menu.GuiMenuItem;
+import org.apache.hop.ui.core.gui.GuiMenuWidgets;
+import org.junit.jupiter.api.Test;
+
+/**
+ * The menu as actions, in the searchable actions view.
+ *
+ * <p>A submenu was listed as an action of its own that did nothing when
clicked, next to the
+ * actions it contains.
+ *
+ * @see <a href="https://github.com/apache/hop/issues/8736";>#8736</a>
+ */
+class MenuContextHandlerTest {
+
+ private static final String ROOT = "MenuContextHandlerTest-Menu";
+
+ private static void addItem(String id, String parentId, String label) {
+ GuiMenuItem item = new GuiMenuItem();
+ item.setRoot(ROOT);
+ item.setId(id);
+ item.setParentId(parentId);
+ item.setLabel(label);
+ item.setType(GuiMenuElementType.MENU_ITEM);
+ item.setClassLoader(MenuContextHandlerTest.class.getClassLoader());
+ GuiRegistry.getInstance().addGuiMenuItem(ROOT, item);
+ }
+
+ @Test
+ void aSubmenuIsNotAnAction() {
+ addItem("10000-tools", ROOT, "Tools");
+ addItem("10100-tools-export", "10000-tools", "Export");
+ addItem("10110-tools-export-csv", "10100-tools-export", "Export to CSV");
+ addItem("10200-tools-search", "10000-tools", "Search");
+
+ List<GuiAction> actions =
+ new MenuContextHandler(ROOT, new
GuiMenuWidgets()).getSupportedActions();
+
+ assertEquals(
+ List.of("10110-tools-export-csv", "10200-tools-search"),
+ actions.stream().map(GuiAction::getId).toList());
+ assertEquals("Export", actions.get(0).getCategory(), "the submenu names
its children's group");
+ }
+}
