Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplified Workflow Run Form #9151

Merged
merged 1 commit into from
Aug 14, 2020
Merged

Conversation

jmchilton
Copy link
Member

@jmchilton jmchilton commented Dec 19, 2019

Builds on refactoring the workflow run form landing and orchestration into Vue (#9147).

Implements #9111 - part of https://github.com/galaxyproject/galaxy/projects/15 and outlined in http://bit.ly/simplified-workflow-ui.

What is in the PR:

  • Add new Galaxy configuration option - simplified_workflow_run_ui set to off by default.
  • If instead the admin has set this parameter to prefer, on the workflow run page scan the workflow run request and if three conditions are met show a simplified tool form. These conditions are:
    • No workflow "resource parameters" - pretty niche, not documented, I don't think anyone outside of Canada is using them and even them I'm not sure if they ever got to production.
    • No "open" tools (i.e. tools with disconnected runtime inputs).
    • No "replacement parameters" defined outside PJAs. I'm calling #{} inside PJA and ${} inside tool steps both "replacement parameters" because the user populates them the same way in the GUI. The API only handles them inside the context of the PJA - it seems the GUI is responsible for replacing them at runtime in the traditional form.
  • The simplified workflow form:
    • Drops tool and subworkflow steps from rendering all together and instead just renders the inputs. These are not rendered as separate "steps" or forms - but as just different inputs the same clean, simple form (more the like tool GUI). Labels (or step indexes as a fallback) are used at the input name, step annotations are used as the input help text.
    • Simplify the workflow request made by the client - send only replacement parameters and inputs - do not serialize tool state for each module. I think this makes what we are tracking more structured and should be more performant as well. Prevents storing HDA IDs in unstructured JSON blobs in the database as well.
    • Drops history target option to simplify the UI - simplified_workflow_run_ui_target_history can be set to either 'current' (default) or 'new' to adjust this parameter Galaxy-wide for the simplified form.
    • Drops job caching option to simplify the UI - simplified_workflow_run_ui_job_cache can be set to 'off' (default) or 'on' to change this parameter Galaxy-wide for the simplified form.
    • Drops resource parameters - we've already verified there are none for simplified workflows.

What is in the PR:

Screenshots

Original Form:

Screen Shot 2019-12-19 at 4 11 11 PM

Simplified Form:

Screen Shot 2019-12-19 at 4 10 59 PM

Clicking to open advanced form from simplified one:

inaction

@nsoranzo
Copy link
Member

The cleanliness and simplicity of the UI is amazing, thanks @jmchilton !
@TKlingstrom will love it!

jmchilton added a commit to jmchilton/galaxy that referenced this pull request Jan 30, 2020
jmchilton added a commit to jmchilton/galaxy that referenced this pull request Feb 18, 2020
jmchilton added a commit to jmchilton/galaxy that referenced this pull request Mar 20, 2020
jmchilton added a commit to jmchilton/galaxy that referenced this pull request Apr 20, 2020
@jmchilton jmchilton added this to In progress in Simplified Workflow-centric User Experience via automation May 21, 2020
@bgruening
Copy link
Member

Would it be possible to show the simplified form even when simplified_workflow_run_ui = off but some GET parameter is sent? I think this would be nice for integration from Dockstore or workflowhub.eu.

@jmchilton jmchilton force-pushed the simplified_run branch 5 times, most recently from 440835c to a2c43f3 Compare August 11, 2020 00:22
@dannon
Copy link
Member

dannon commented Aug 11, 2020

What's the argument for putting this behind a default-off configuration option? From what the end-user sees, it's a very simple 'just don't render all the steps if I don't have to touch them', with a toggle to go to the full mode. And there are no migration considerations for an admin, right?

@jmchilton
Copy link
Member Author

@dannon

Just me being conservative (and before my work today this not all the options on the screen worked - batch parameters were not working). At worst giving people a release to test it out and form opinions. I'm happy to make this the default I think though. Arguments against it might be more clicks to get to get to certain options and a jarring user experience when best practice workflows render the new way and other workflows render the old way.

@bgruening

It can be overridden now with simplified_workflow_run_ui=prefer in the query string.

@dannon
Copy link
Member

dannon commented Aug 11, 2020

@jmchilton Well, it would definitely be best if it worked, but yeah -- I think this becoming the default with a simple toggle (could even set a user preference for this very easily, to allow individuals to prefer simple or advanced) to show the whole form is totally fine.

@guerler
Copy link
Contributor

guerler commented Aug 11, 2020

This is great, thank you so much for simplifying the execution form. While looking into it I noticed that the lazy loading for steps seems to be impeded, maybe we can fix this here too. ref #10034

@jmchilton jmchilton force-pushed the simplified_run branch 2 times, most recently from 646a4fa to 2f01417 Compare August 11, 2020 16:04
@jmchilton jmchilton force-pushed the simplified_run branch 3 times, most recently from 9fd5ad2 to 7c0b28c Compare August 12, 2020 03:59
@jmchilton jmchilton changed the title [WIP] Simplified Workflow Run Form Simplified Workflow Run Form Aug 12, 2020
@galaxybot galaxybot added this to the 20.09 milestone Aug 12, 2020
Copy link
Member

@bgruening bgruening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Thanks John!

future 'force' may be added an option for Galaskio-style servers that should
only render simplified workflows.

simplified_workflow_run_ui_target_history:
Copy link
Member

@dannon dannon Aug 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and the setting below really feel like they should be user preferences, not galaxy-wide configuration settings for the sake of simplifying a page. For more context, this notion is probably stemming from my work separating the client from the server, but this more tightly couples the displayed interface with the server and its configuration.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This project stemmed from a desire to present a completely different, simplified UI around workflows - including one that doesn't even need to present the concept of a history to users. If you wanted to deploy such an app you really wouldn't want to make this a user setting.

I'm all for decoupling - the corresponding attributes on Vue components do have defaults. They should work stand-alone in other scenarios. So they are decoupled properly in my mind. Just because we don't want to force the developer using the components to define these doesn't mean the admins shouldn't be allowed to pick defaults. Likewise, just because it would likely be good to allow user preferences in the abstract for more typical uses doesn't mean we shouldn't allow admins to be able to control how the app is configured as a default.

Having one part of the UI depend on Galaxy's concept of a user preference tab and the backend for storing and fetching those - is in fact more coupling to my mind than this. Again, the components would work without those so it seems like a fine sort of optional coupling. We have independent ways to configure these things and the component internals don't depend on how those are being configured - that would seem to be the goal to me.

Anyway, while I agree with some of this, adding the user preferences seems like outside the scope of the initial implementation. If you insist on my adding setting I will consider it to get this done, but even in that scenario I still think the defaults should be configurable by the deployer. Though I'm totally open to there being a different way for the deployer to configure the client than Galaxy's YAML.

Copy link
Member

@dannon dannon Aug 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you insist

@jmchilton I really wasn't insisting on anything, sorry if you thought as much.

I think this component should be seen as a simple, togglable level of detail view rather than a different, server-set preference. In my mind the second pass, the actual workflow-centric interface addition, that's what would add these extra properties (however they end up) and pass them to this view. This is nice progress as a straightforward level of detail toggle on the existing workflow interface. That's what I was trying to get at above, with the configuration defaults -- it doesn't serve anyone to force one view over the other with just this piece, I don't think, and I suppose a user could just flip the interface one way or another if they wanted anyway by swapping a bit of client-side data.

Anyway, for now, it's fair enough that until we have a way to configure more (and default) client(s), that this can live in the core galaxy configuration. If you want this configurable at this point, then it's fine.

@jmchilton
Copy link
Member Author

This is ancient code at this point 😂 - I’ll clean up the jquery and underscore today - thanks for the review.

Builds on refactoring the workflow run form landing and orchestration into Vue (galaxyproject#9147).

Implements galaxyproject#9111 - part of https://github.com/galaxyproject/galaxy/projects/15 and outlined in http://bit.ly/simplified-workflow-ui.

What is in the PR:

- Add new Galaxy configuration option - ``simplified_workflow_run_ui`` set to ``off`` by default.
- If instead the admin has set this parameter to ``prefer``, on the workflow run page scan the workflow run request and if three conditions are met show a simplfied tool form. These conditions are:
  - No workflow "resource parameters" - pretty niche, not documented, I don't think anyone outside of Canada is using them and even them I'm not sure if they ever got to production.
  - No "open" tools (i.e. tools with disconnected runtime inputs).
  - No "replacement parameters" defined outside PJAs. I'm calling #{} inside PJA and ${} inside tool steps both "replacement parameters" because the user populates them the same way in the GUI. The API only handles them inside the context of the PJA - it seems the GUI is responsible for replacing them at runtime in the traditional form.
- The simplified workflow form:
   - Drops tool and subworkflow steps from rendering all together and instead just renders the inputs. These are not rendered as separate "steps" or forms - but as just different inputs the same clean, simple form (more the like tool GUI). Labels (or step indexes as a fallback) are used at the input name, step annotations are used as the input help text.
   - Simplify the workflow request made by the client - send only replacement parameters and inputs - do not serialize tool state for each module. I think this makes what we are tracking more structured and should be more performant as well. Prevents storing HDA IDs in unstructured JSON blobs in the database as well.
   - Drops history target option to simplify the UI - simplified_workflow_run_ui_target_history can be set to either 'current' or 'new' to adjust this parameter Galaxy-wide for the simplified form.
   - Drops job caching option to simplify the UI - simplified_workflow_run_ui_cache can be set to 'off' or 'on' to change this parameter Galaxy-wide for the simplified form.
   - Drops resource parameters - we've already verified there are none for simplified workflows.
@dannon dannon merged commit 246603a into galaxyproject:dev Aug 14, 2020
Simplified Workflow-centric User Experience automation moved this from In progress to Done Aug 14, 2020
@bgruening
Copy link
Member

Awesome! Yeah!!!!

@TKlingstrom
Copy link

TKlingstrom commented Aug 14, 2020

Wow thanks Jon for doing this and other people commenting, I really look forward to testing things.

(Also hugely sorry for disappearing and not doing anything for half a year, I do not control time in my my work life nor my private life as much as I would like but it's moving in the right direction at least).

@afgane
Copy link
Contributor

afgane commented Aug 14, 2020

This is indeed excellent. Nonetheless, 3 comments:

  • The config values are not included in galaxy.yml.sample
  • If there is a link to switch to the full workflow form, it feels it should be a toggle so you can switch back to the simplified view
  • It would be great to include the workflow annotation text at the top of the run form

@bgruening
Copy link
Member

@afgane can you create a new issue with those ideas, please. I think we can iterate here with many other small things. We also discussed briefly to include maybe a small image of the workflow etc.

@mvdbeek
Copy link
Member

mvdbeek commented Aug 20, 2020

Looks like the input step parameters are not actually sent in the simplified form:

{"replacement_dict":{},"inputs":{"0":{"values":[{"hid":1,"id":"db210f2338fbfc8b","keep":false,"name":"Pasted Entry","src":"hda","tags":[]}],"batch":false}},"inputs_by":"step_index","batch":true,"use_cached_job":false,"history_id":"6169422af4f6057a"}

Screenshot 2020-08-20 at 14 22 58

@jmchilton do you know of the top of your head what's going on here ?

@mvdbeek
Copy link
Member

mvdbeek commented Aug 20, 2020

Got it, https://github.com/galaxyproject/galaxy/blob/dev/client/src/components/Workflow/Run/WorkflowRunFormSimple.vue#L88 doesn't include parameter_input, I'll have a fix in a minute

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

10 participants