opensource/galaxy
3
0
Fork 0
mirror of https://github.com/galaxyproject/galaxy.git synced 2026-09-24 16:30:27 +08:00
Code Issues Packages Projects Releases Wiki Activity

Fix workflow editor bug related state updates for collections attached to multiple input data parameters.

Browse Source 
Detail bug report from Michael Crusoe here : https://trello.com/c/0mdGCx4P.

This also fixes a test case added in 289e48b which was both attempting to assert something wrong and was incorrectly implemented. Augmenting the workflow editor test suite with some actually valid test cases that assert the correct behaviors.

For a longer explaination - workflow input terminals have two related concepts 'canAccept' and 'attachable'. An output terminal is 'attachable' if in the abstract it could be attached to the input regardless of whether the input is already filled or not. 'canAccept' is more stateful in that an output terminal is 'canAccept'able if the input terminal is not filled ('_inputFilled') and it is 'attachable'.

So - the problem was 'attachable' was not correctly defined for a multiple input data parameters. 'attachable' was asserting that any connected input terminal could not be 'attachable' by an collection output terminal - so on these asynchronous node state changes collections attached to multiple input data parameters were being wiped out. The more percise/correct distinction is that if a multiple input data parameter has single inputs connected to it - it cannot also have a collection connected to it (yet anyway). This fixes the mentioned bug.

The problematic test case was conflating 'attachable' and 'canAccept'able - I have fixed the test case to verify the correct 'attachable' logic and added newer, higher level test cases to test the canAccept logic and the actual behavior the end user would observe (of the connector being destroy).
This commit is contained in:
John Chilton
2014-09-02 11:20:44 -04:00
parent 7a5fed6208
commit 24e914f57b
2 changed files with 64 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files
static/scripts/galaxy.workflow_editor.canvas.js
+24 -13
View File
@@ -302,19 +302,12 @@ var BaseInputTerminal = Terminal.extend( {
inputFilled = false;
} else {
if( this.multiple ) {
if( ! this.connected() ) {
inputFilled = false;
if(this._collectionAttached()) {
// Can only attach one collection to multiple input
// data parameter.
inputsFilled = true;
} else {
var firstOutput = this.connectors[ 0 ].handle1;
if( ! firstOutput ){
inputFilled = false;
} else {
if( firstOutput.isDataCollectionInput || firstOutput.isMappedOver() || firstOutput.datatypes.indexOf( "input_collection" ) > 0 ) {
inputFilled = true;
} else {
inputFilled = false;
}
}
inputFilled = false;
}
} else {
inputFilled = true;
@@ -322,6 +315,22 @@ var BaseInputTerminal = Terminal.extend( {
}
return inputFilled;
},
_collectionAttached: function( ) {
if( ! this.connected() ) {
return false;
} else {
var firstOutput = this.connectors[ 0 ].handle1;
if( ! firstOutput ){
return false;
} else {
if( firstOutput.isDataCollectionInput || firstOutput.isMappedOver() || firstOutput.datatypes.indexOf( "input_collection" ) > 0 ) {
return true;
} else {
return false;
}
}
}
},
_mappingConstraints: function( ) {
// If this is a connected terminal, return list of collection types
// other terminals connected to node are constraining mapping to.
@@ -407,7 +416,9 @@ var InputTerminal = BaseInputTerminal.extend( {
var thisMapOver = this.mapOver();
if( otherCollectionType.isCollection ) {
if( this.multiple ) {
if( this.connected() ) {
if( this.connected() && ! this._collectionAttached() ) {
// if single inputs attached, cannot also attach a
// collection (yet...)
return false;
}
if( otherCollectionType.rank == 1 ) {
test/qunit/tests/workflow_editor_tests.js
+40 -4
View File
@@ -91,7 +91,9 @@ define([
},
test_accept: function( other ) {
other = other || { node: {}, datatypes: [ "txt" ] };
other.mapOver = function() { return NULL_COLLECTION_TYPE_DESCRIPTION; };
if( ! other.mapOver ) {
other.mapOver = function() { return NULL_COLLECTION_TYPE_DESCRIPTION; };
}
return this.input_terminal.canAccept( other );
},
pja_change_datatype_node: function( output_name, newtype ) {
@@ -230,6 +232,22 @@ define([
ok( self.test_accept() );
} );
test( "can accept list collection for empty multiple inputs", function() {
var other = { node: {}, datatypes: [ "tabular" ], mapOver: function() { return new CollectionTypeDescription( "list" ) } };
var self = this;
this.multiple();
ok( self.test_accept( other ) );
} );
test( "cannot accept list collection for multiple input if collection already connected", function() {
var other = { node: {}, datatypes: [ "tabular" ], mapOver: function() { return new CollectionTypeDescription( "list" ) } };
var self = this;
this.multiple();
this.with_test_connector( function() {
ok( ! self.test_accept( other ) );
} );
} );
module( "Connector test", {
} );
@@ -483,6 +501,17 @@ define([
return c;
},
connectAttachedMultiInputTerminal: function( inputType, outputType ) {
this.view.addDataInput( { name: "TestName", extensions: [ inputType ], multiple: true } );
var terminal = this.view.node.input_terminals[ "TestName" ];
var outputTerminal = new OutputTerminal( { name: "TestOuptut", datatypes: [ "txt" ] } );
outputTerminal.node = { markChanged: function() {}, post_job_actions: [], hasMappedOverInputTerminals: function() { return false; }, hasConnectedOutputTerminals: function() { return true; } };
outputTerminal.terminalMapping = { disableMapOver: function() {}, mapOver: new CollectionTypeDescription( "list" ) };
var c = new Connector( outputTerminal, terminal );
return c;
},
connectAttachedMappedOutput: function( ) {
this.view.addDataInput( { name: "TestName", extensions: [ "txt" ], input_type: "dataset_collection" } );
var terminal = this.view.node.input_terminals[ "TestName" ];
@@ -530,6 +559,14 @@ define([
ok( connector.handle2 === terminal );
} );
test( "replacing terminal on data multiple input update preserves collection connections", function() {
var connector = this.connectAttachedMultiInputTerminal( "txt", "txt" );
var connector_destroy_spy = sinon.spy( connector, "destroy" );
var newElement = $("<div class='inputs'></div>");
this.view.addDataInput( { name: "TestName", extensions: ["txt"], multiple: true }, newElement );
ok( ! connector_destroy_spy.called );
} );
test( "replacing mapped terminal on data collection input update preserves connections", function() {
var connector = this.connectAttachedMappedOutput();
var newElement = $("<div class='inputs'></div>");
@@ -922,12 +959,11 @@ define([
this.verifyAttachable( inputTerminal1, "list" );
} );
test( "connected multiple input cannot be connected to collections", function() {
test( "multiple input attachable by collections", function() {
var inputTerminal1 = this.newInputTerminal( null, { multiple: true } );
var connectedInput1 = this.addConnectedInput( inputTerminal1 );
this.addConnectedOutput( connectedInput1 );
// Normally could do this reduction, but cannot because input already connected.
this.verifyNotAttachable( connectedInput1, "list" );
this.verifyAttachable( inputTerminal1, "list" );
} );
test( "unconnected multiple inputs cannot be connected to rank > 1 collections (yet...)", function() {