From 7f0ce0ec8457ac15273cafd3b8248e2eebfd1d5d Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 20 Jan 2022 11:31:49 +0100 Subject: [PATCH 1/5] Add selenium test to check map over state for parameter steps --- .../selenium/test_workflow_editor.py | 31 +++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/lib/galaxy_test/selenium/test_workflow_editor.py b/lib/galaxy_test/selenium/test_workflow_editor.py index ca47f673b9a..854a5a41194 100644 --- a/lib/galaxy_test/selenium/test_workflow_editor.py +++ b/lib/galaxy_test/selenium/test_workflow_editor.py @@ -271,6 +271,32 @@ steps: self.workflow_editor_connect("input_int#output", "tool_exec#inttest", screenshot_partial="workflow_editor_parameter_connection_dragging") self.assert_connected("input_int#output", "tool_exec#inttest") + @selenium_test + def test_non_data_map_over_carried_through(self): + # Use auto_layout=false, which prevents placing any + # step outside of the scroll area + # xref: https://github.com/galaxyproject/galaxy/issues/13211 + self.open_in_workflow_editor(""" +class: GalaxyWorkflow +inputs: + input_collection: + type: collection + collection_type: "list" +steps: + param_value_from_file: + tool_id: param_value_from_file + in: + input1: input_collection + text_input_step: + tool_id: param_text_option + in: + text_param: param_value_from_file/text_param + collection_input: + tool_id: identifier_collection +""", auto_layout=False) + self.workflow_editor_connect("text_input_step#out_file1", "collection_input#input1") + self.assert_connected("text_input_step#out_file1", "collection_input#input1") + @selenium_test def test_existing_connections(self): self.open_in_workflow_editor(WORKFLOW_SIMPLE_CAT_TWICE) @@ -628,11 +654,12 @@ steps: source_id, sink_id = self.workflow_editor_source_sink_terminal_ids(source, sink) self.components.workflow_editor.connector_for(source_id=source_id, sink_id=sink_id).wait_for_absent() - def open_in_workflow_editor(self, yaml_content): + def open_in_workflow_editor(self, yaml_content, auto_layout=True): name = self.workflow_upload_yaml_with_random_name(yaml_content) self.workflow_index_open() self.workflow_index_open_with_name(name) - self.workflow_editor_click_option("Auto Layout") + if auto_layout: + self.workflow_editor_click_option("Auto Layout") return name def workflow_editor_source_sink_terminal_ids(self, source, sink): From dce483e7f542d5fb3464d814fc51ee85a7b88143 Mon Sep 17 00:00:00 2001 From: guerler Date: Tue, 18 Jan 2022 23:15:46 -0500 Subject: [PATCH 2/5] Call setMapOver routine in input parameter terminals to detect mapped collections --- .../Workflow/Editor/modules/terminals.js | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/client/src/components/Workflow/Editor/modules/terminals.js b/client/src/components/Workflow/Editor/modules/terminals.js index 51b39ab26c8..359c1c736bb 100644 --- a/client/src/components/Workflow/Editor/modules/terminals.js +++ b/client/src/components/Workflow/Editor/modules/terminals.js @@ -224,6 +224,15 @@ class BaseInputTerminal extends Terminal { this.datatypesMapper = attr.datatypesMapper; this.update(attr.input); // subclasses should implement this... } + setDefaultMapOver(connector) { + var other_output = connector.outputHandle; + if (other_output) { + var otherCollectionType = this._otherCollectionType(other_output); + if (otherCollectionType.isCollection) { + this.setMapOver(otherCollectionType); + } + } + } canAccept(other) { if (this._inputFilled()) { return new ConnectionAcceptable( @@ -385,14 +394,7 @@ class InputTerminal extends BaseInputTerminal { } connect(connector) { super.connect(connector); - var other_output = connector.outputHandle; - if (!other_output) { - return; - } - var otherCollectionType = this._otherCollectionType(other_output); - if (otherCollectionType.isCollection) { - this.setMapOver(otherCollectionType); - } + this.setDefaultMapOver(connector); } attachable(other) { var otherCollectionType = this._otherCollectionType(other); @@ -460,6 +462,10 @@ class InputParameterTerminal extends BaseInputTerminal { this.type = input.type; this.optional = input.optional; } + connect(connector) { + super.connect(connector); + this.setDefaultMapOver(connector); + } effectiveType(parameterType) { return parameterType == "select" ? "text" : parameterType; } From b5f35c76c26ca181cf3b5f07f82b65f052119af2 Mon Sep 17 00:00:00 2001 From: guerler Date: Tue, 18 Jan 2022 23:57:03 -0500 Subject: [PATCH 3/5] Add test to validate correct mapping of parameter input terminals --- .../qunit/tests/workflow_editor_tests.js | 42 ++++++++++++++----- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/client/tests/qunit/tests/workflow_editor_tests.js b/client/tests/qunit/tests/workflow_editor_tests.js index a55604b395f..35c92bb3000 100644 --- a/client/tests/qunit/tests/workflow_editor_tests.js +++ b/client/tests/qunit/tests/workflow_editor_tests.js @@ -889,6 +889,23 @@ QUnit.module("terminal mapping logic", { } return inputTerminal; }, + newInputParameterTerminal: function (mapOver, input, node) { + input = input || {}; + node = node || this.newNode(); + if (!("type" in input)) { + input["type"] = "text"; + } + const inputEl = $("
")[0]; + const inputTerminal = new Terminals.InputParameterTerminal({ + element: inputEl, + input: input, + }); + inputTerminal.node = node; + if (mapOver) { + inputTerminal.setMapOver(new Terminals.CollectionTypeDescription(mapOver)); + } + return inputTerminal; + }, newInputCollectionTerminal: function (input, node) { input = input || {}; node = node || this.newNode(); @@ -1023,6 +1040,15 @@ QUnit.module("terminal mapping logic", { verifyNotMappedOver: function (assert, terminal) { assert.ok(!terminal.mapOver.isCollection); }, + verifyDefaultMapOver: function(assert, inputTerminal1) { + const outputCollectionTerminal1 = this.newOutputCollectionTerminal("list"); + assert.ok(!inputTerminal1.node.mapOver); + const connector = new Connector({}, outputCollectionTerminal1, inputTerminal1); + outputCollectionTerminal1.connect(connector); + assert.ok(inputTerminal1.node.mapOver); + inputTerminal1.disconnect(connector); + assert.ok(!inputTerminal1.node.mapOver); + } }); QUnit.test("unconstrained input can be mapped over", function (assert) { @@ -1280,13 +1306,9 @@ QUnit.test("simple mapping over collection outputs works correctly", function (a this.verifyNotAttachable(assert, testTerminal1, connectedOutput); }); -QUnit.test("node mapping state over collection outputs works correctly", function (assert) { - const inputTerminal1 = this.newInputTerminal(); - const outputCollectionTerminal1 = this.newOutputCollectionTerminal("list"); - assert.ok(!inputTerminal1.node.mapOver); - const connector = new Connector({}, outputCollectionTerminal1, inputTerminal1); - outputCollectionTerminal1.connect(connector); - assert.ok(inputTerminal1.node.mapOver); - inputTerminal1.disconnect(connector); - assert.ok(!inputTerminal1.node.mapOver); -}); \ No newline at end of file +QUnit.test("node input parameter mapping state over collection outputs works correctly", function (assert) { + const inputTerminal = this.newInputTerminal(); + this.verifyDefaultMapOver(assert, inputTerminal); + const inputParameterTerminal = this.newInputParameterTerminal(); + this.verifyDefaultMapOver(assert, inputParameterTerminal); +}); From f7fb4ede5502b329b5902b7c4bc125c9a780b97c Mon Sep 17 00:00:00 2001 From: guerler Date: Wed, 19 Jan 2022 00:58:35 -0500 Subject: [PATCH 4/5] Remove currently unused options from qunit terminal creation helpers --- .../qunit/tests/workflow_editor_tests.js | 60 +++++++------------ 1 file changed, 21 insertions(+), 39 deletions(-) diff --git a/client/tests/qunit/tests/workflow_editor_tests.js b/client/tests/qunit/tests/workflow_editor_tests.js index 35c92bb3000..23317d11793 100644 --- a/client/tests/qunit/tests/workflow_editor_tests.js +++ b/client/tests/qunit/tests/workflow_editor_tests.js @@ -871,9 +871,9 @@ QUnit.test("resetMapping", function (assert) { }); QUnit.module("terminal mapping logic", { - newInputTerminal: function (mapOver, input, node) { + newInputTerminal: function (mapOver, input) { input = input || {}; - node = node || this.newNode(); + const node = this.newNode(); if (!("extensions" in input)) { input["extensions"] = ["data"]; } @@ -889,26 +889,19 @@ QUnit.module("terminal mapping logic", { } return inputTerminal; }, - newInputParameterTerminal: function (mapOver, input, node) { - input = input || {}; - node = node || this.newNode(); - if (!("type" in input)) { - input["type"] = "text"; - } + newInputParameterTerminal: function () { + const node = this.newNode(); const inputEl = $("
")[0]; const inputTerminal = new Terminals.InputParameterTerminal({ element: inputEl, - input: input, + input: {}, }); inputTerminal.node = node; - if (mapOver) { - inputTerminal.setMapOver(new Terminals.CollectionTypeDescription(mapOver)); - } return inputTerminal; }, - newInputCollectionTerminal: function (input, node) { + newInputCollectionTerminal: function (input) { input = input || {}; - node = node || this.newNode(); + const node = this.newNode(); if (!("extensions" in input)) { input["extensions"] = ["data"]; } @@ -921,16 +914,12 @@ QUnit.module("terminal mapping logic", { }); return inputTerminal; }, - newOutputTerminal: function (mapOver, output, node) { - output = output || {}; - node = node || this.newNode(); - if (!("extensions" in output)) { - output["extensions"] = ["data"]; - } + newOutputTerminal: function (mapOver) { + const node = this.newNode(); const outputEl = $("
")[0]; const outputTerminal = new Terminals.OutputTerminal({ element: outputEl, - datatypes: output.extensions, + datatypes: ["data"], node: {}, }); outputTerminal.node = node; @@ -939,24 +928,17 @@ QUnit.module("terminal mapping logic", { } return outputTerminal; }, - newOutputCollectionTerminal: function (collectionType, output, node, mapOver) { + newOutputCollectionTerminal: function (collectionType) { collectionType = collectionType || "list"; - output = output || {}; - node = node || this.newNode(); - if (!("extensions" in output)) { - output["extensions"] = ["data"]; - } + const node = this.newNode(); const outputEl = $("
")[0]; const outputTerminal = new Terminals.OutputCollectionTerminal({ element: outputEl, - datatypes: output.extensions, + datatypes: ["data"], collection_type: collectionType, node: {}, }); outputTerminal.node = node; - if (mapOver) { - outputTerminal.setMapOver(new Terminals.CollectionTypeDescription(mapOver)); - } return outputTerminal; }, newNode: function () { @@ -1040,14 +1022,14 @@ QUnit.module("terminal mapping logic", { verifyNotMappedOver: function (assert, terminal) { assert.ok(!terminal.mapOver.isCollection); }, - verifyDefaultMapOver: function(assert, inputTerminal1) { - const outputCollectionTerminal1 = this.newOutputCollectionTerminal("list"); - assert.ok(!inputTerminal1.node.mapOver); - const connector = new Connector({}, outputCollectionTerminal1, inputTerminal1); - outputCollectionTerminal1.connect(connector); - assert.ok(inputTerminal1.node.mapOver); - inputTerminal1.disconnect(connector); - assert.ok(!inputTerminal1.node.mapOver); + verifyDefaultMapOver: function(assert, terminal) { + const outputCollectionTerminal = this.newOutputCollectionTerminal("list"); + assert.ok(!terminal.node.mapOver); + const connector = new Connector({}, outputCollectionTerminal, terminal); + outputCollectionTerminal.connect(connector); + assert.ok(terminal.node.mapOver); + terminal.disconnect(connector); + assert.ok(!terminal.node.mapOver); } }); From 2a005df01873b84bbaad093d500c4165db67d43c Mon Sep 17 00:00:00 2001 From: guerler Date: Wed, 19 Jan 2022 22:16:04 -0500 Subject: [PATCH 5/5] Separate qunit test cases for data and parameter input terminals --- client/tests/qunit/tests/workflow_editor_tests.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/client/tests/qunit/tests/workflow_editor_tests.js b/client/tests/qunit/tests/workflow_editor_tests.js index 23317d11793..bf4f43b7317 100644 --- a/client/tests/qunit/tests/workflow_editor_tests.js +++ b/client/tests/qunit/tests/workflow_editor_tests.js @@ -1288,9 +1288,12 @@ QUnit.test("simple mapping over collection outputs works correctly", function (a this.verifyNotAttachable(assert, testTerminal1, connectedOutput); }); -QUnit.test("node input parameter mapping state over collection outputs works correctly", function (assert) { +QUnit.test("node input terminal mapping state over collection outputs works correctly", function (assert) { const inputTerminal = this.newInputTerminal(); this.verifyDefaultMapOver(assert, inputTerminal); +}); + +QUnit.test("node input parameter terminal mapping state over collection outputs works correctly", function (assert) { const inputParameterTerminal = this.newInputParameterTerminal(); this.verifyDefaultMapOver(assert, inputParameterTerminal); });