Idea #9043
closed
[Crunch2] [Workbench] Render CWL input selection forms
Added by Brett Smith about 8 years ago. Updated over 7 years ago.
Description
CWL input form offer significantly more options than the existing Arvados pipeline template forms. Major points include arrays and union types (where more than one type of value is acceptable), which are characterized by the user needed to edit a list or select among different types which may offer different editing controls.
For now, we will adapt (or, if needed, copy) the existing Rails-based pipeline form so it can operate on a container request that has an embedded CWL workflow (as provided by #9766).
The first version must support:- text entry
- number entry
- file selection
- directory selection (where the desired directory is an entire collection)
- checkbox
- control that wraps list of individual controls to add/reorder/remove items in arrays
- control that wraps above controls to enable/disable optional parameters
- directory selection (where the desired directory is not an entire collection)
Files
| exampleinputs.yml (899 Bytes) exampleinputs.yml | Peter Amstutz, 07/13/2016 07:16 PM | ||
| inputs-post.json (2.39 KB) inputs-post.json | Peter Amstutz, 07/13/2016 07:16 PM |
Related issues
Updated by Radhika Chippada almost 8 years ago
Checked with Peter and one of the inputs can be a file from within an arvados collection and hence we need to be able to support this.
Updated by Tom Morris almost 8 years ago
Possible starting points from SevenBridges:
https://github.com/rabix/cwl-ts
https://github.com/rabix/cottontail
Updated by Brett Smith almost 8 years ago
Stories:
- Just render the workflow, no editing
- Support editing non-compound input types
- Support editing compound input types (this could be broken out into even more sub-stories, one per type, if need be)
Updated by Peter Amstutz almost 8 years ago
- File exampleinputs.yml exampleinputs.yml added
- File inputs-post.json inputs-post.json added
See attached exampleinputs.yml for something a user might write.
Normalize this file and get JSON by using cwltool --print-pre
cwltool --print-pre exampleinputs.yml > inputs-post.json
The attached inputs-post.json is an example of the normalized form. For UI it will be more robust to run the --print-pre normalization and load the resulting JSON.
We need the following editing controls:
- text entry
- number entry
- checkbox
- file selection
- directory selection
- control that wraps list of individual controls to add/reorder/remove items in arrays
- control that wraps above controls to enable/disable optional parameters
Updated by Tom Clegg almost 8 years ago
- Category set to Workbench
- Assigned To set to Tom Clegg
Updated by Ward Vandewege almost 8 years ago
Perhaps relevant: https://github.com/mrsweaters/mithril-rails
Updated by Tom Morris almost 8 years ago
- Target version set to Arvados Future Sprints
Updated by Peter Amstutz over 7 years ago
- Story points deleted (
4.0)
Relevant bits of existing field component rendering:
workbench/app/views/pipeline_instances/_show_components_editable.rb
Render the pipeline template form entry by going through each element in the script parameter of the pipeline template and calling render_pipeline_component_attribute
<% component[:script_parameters].andand.each do |p, tv| %>
<tr>
<td style="border-top: none"></td>
<td style="border-top: none"></td>
<td class="property-edit-row"><%= p %></td>
<td class="property-edit-row"><%=
render_pipeline_component_attribute (editable && @object),
:components, [k, :script_parameters, p.to_sym], tv %></td>
</tr>
<% end %>
workbench/app/helpers/application_helper.rb
Renders HTML for a single editable field (with AJAX update).
May need to be tweaked/updated for CWL fields.
def render_pipeline_component_attribute(object, attr, subattr, value_info, htmloptions={})
workbench/app/controllers/pipeline_instances_controller.rb
Accepts AJAX PUT from browser to update a field (the corresponding server side for above code).
Will need to be copied in some form to container_requests:
def update
@updates ||= params[@object.class.to_s.underscore.singularize.to_sym]
if (components = @updates[:components])
components.each do |cname, component|
if component[:script_parameters]
component[:script_parameters].each do |param, value_info|
...
end
Updated by Tom Morris over 7 years ago
- Assigned To changed from Tom Clegg to Peter Amstutz
- Target version changed from Arvados Future Sprints to 2016-08-31 sprint
Updated by Radhika Chippada over 7 years ago
- Status changed from New to In Progress
Updated by Radhika Chippada over 7 years ago
Comments regarding 7700e66
- container_requests_controller.rb => show_pane_list
- This “if @object and @object.state 'Uncommitted'” can be “if @object.andand.state 'Uncommitted’”
- Should the “Graph” tab also be hidden when Log is hidden?
- Fiddlesticks when I visit the Graph tab
- container_requests_controller.rb => update
- Since most of the code is in “if input_obj” block, it might be better if you unpyramid the code and return earlier if input_obj is not found?
- same with “if input_obj.include? input_schema[:id]” - use a continue if not?
- In “c = display_value = Collection.find(re1)” : it appears that display_value is unused?
- Would you be able to define “/^([0-9a-z]{5}-([0-9a-z]{5})-[0-9a-z]{15})(\/.*)?$/“ once and reuse than repeat it?
- application_helper
- Instead of “null” can we use “optional” here to indicate an optional parameter? — required = !(input_schema[:type].include? "null")
- It appears that “id” in “inputs” is what distinguishes one from the other. If I had something as below for my inputs definition in the workflow, it still is allowed and rendered but it appears that populating it may be problematic. Do we need to some kind of validation that an id is not repeated in the workflow?
"inputs": [ { "type": "double", "id": "ex_double", "inputBinding": {"position": 1} }, { "type": "double", "id": "ex_double", "inputBinding": {"position": 1} }
- When I select “false” for a boolean input field, it’s being shown in UI as “#”
- container_requests_test
- Does “INPUT_SELECTOR” need to be all uppercase?
- The test of course is not passing, but I think you are aware of it already.
Updated by Peter Amstutz over 7 years ago
Radhika Chippada wrote:
Comments regarding 7700e66
- container_requests_controller.rb => show_pane_list
- This “if @object and @object.state 'Uncommitted'” can be “if @object.andand.state 'Uncommitted’”
Done.
- Should the “Graph” tab also be hidden when Log is hidden?
- Fiddlesticks when I visit the Graph tab
The Graph tab doesn't exist, I got rid of it.
- container_requests_controller.rb => update
- Since most of the code is in “if input_obj” block, it might be better if you unpyramid the code and return earlier if input_obj is not found?
It calls "super" at the end so I prefer not to change the flow.
- same with “if input_obj.include? input_schema[:id]” - use a continue if not?
Done.
- In “c = display_value = Collection.find(re1)” : it appears that display_value is unused?
Fixed.
- Would you be able to define “/^([0-9a-z]{5}-([0-9a-z]{5})-[0-9a-z]{15})(\/.*)?$/“ once and reuse than repeat it?
You're right, there's already a helper function for this, I changed it to use the helper.
- application_helper
- Instead of “null” can we use “optional” here to indicate an optional parameter? — required = !(input_schema[:type].include? "null")
No, "null" is how CWL represents optional inputs (an optional input is defined as one where "null" is a permitted value).
- It appears that “id” in “inputs” is what distinguishes one from the other. If I had something as below for my inputs definition in the workflow, it still is allowed and rendered but it appears that populating it may be problematic. Do we need to some kind of validation that an id is not repeated in the workflow?
[...]
That's an error in the workflow, I'm not sure where the validation should happen.
- When I select “false” for a boolean input field, it’s being shown in UI as “#”
Ok, I think I fixed it. I keep having trouble because both Ruby and Javascript have notions about "false == null".
- container_requests_test
- Does “INPUT_SELECTOR” need to be all uppercase?
I copied that from the pipeline_instances test.
- The test of course is not passing, but I think you are aware of it already.
Yes, for some reason it's not able to find the "ok" button to submit the value, I haven't looked into it.
Updated by Radhika Chippada over 7 years ago
Thanks for addressing those.
- There is still one more occurrence of /^([0-9a-z]{5}-([0-9a-z]{5})-[0-9a-z]{15})(\/.*)?$/ in application_helper
- Regarding "That's an error in the workflow, I'm not sure where the validation should happen" : I think it would be desirable that we add this validation to workflow objects. In any case, would you please add ticket about this for future consideration? Thanks.
Updated by Peter Amstutz over 7 years ago
Radhika Chippada wrote:
Thanks for addressing those.
- There is still one more occurrence of /^([0-9a-z]{5}-([0-9a-z]{5})-[0-9a-z]{15})(\/.*)?$/ in application_helper
Fixed, thanks.
- Regarding "That's an error in the workflow, I'm not sure where the validation should happen" : I think it would be desirable that we add this validation to workflow objects. In any case, would you please add ticket about this for future consideration? Thanks.
This validation should really happen as part of CWL document loading & validation, I filed #9853 which has a link to a schema salad issue.
Updated by Radhika Chippada over 7 years ago
Branch 9043-test-edit-container-request at commit e93aaa7
- Updated application_helper to address the issue where “number” was not allowing decimal numbers to be entered for floats and doubles. Set to text in these cases
- added tests to add and verify input for double, float, int, int_opt, long, string, string_opt, boolean, enum, directory, file typed inputs
- added test to verify that the Run button is enabled and works as expected when all required inputs are provided
Updated by Radhika Chippada over 7 years ago
Couple more comments / questions about containers_request.yml updates at 3bdb80d
- Both uncommitted and uncommitted_ready_to_run have ex_opt_string and ex_string_opt. Can we remove the second one in these fixtures?
- Since we do not yet support array types, do we want to remove ex_opt_file_array from the fixtures?
Updated by Peter Amstutz over 7 years ago
Radhika Chippada wrote:
Couple more comments / questions about containers_request.yml updates at 3bdb80d
- Both uncommitted and uncommitted_ready_to_run have ex_opt_string and ex_string_opt. Can we remove the second one in these fixtures?
Yes, that was a mistake, I think I added the 2nd optional string not realizing the first one was already there.
- Since we do not yet support array types, do we want to remove ex_opt_file_array from the fixtures?
Yes.
Updated by Peter Amstutz over 7 years ago
- Status changed from In Progress to Resolved
- % Done changed from 0 to 100
Applied in changeset arvados|commit:7497b21c937bb6e8451f16047945b7cfc9081a53.