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

Fixes issues with extra var prompting in workflow nodes #4525

Merged
merged 3 commits into from
Aug 23, 2019

Conversation

mabashian
Copy link
Member

SUMMARY

link #4293

The easiest way to reproduce the pipe in the extra vars was to:

  1. Create a JT that prompted for extra vars with no default values
  2. Create a WFJT and select the JT from step 1 as a node
  3. While creating the node in step 2, click on the prompt button and go through the prompts without selecting anything
  4. Save the WFJT
  5. Open the WFJT back up and edit the node
  6. Click the prompt button and navigate to the extra vars prompt section

Here's a gif of part of that process:

extra_vars

This bug got introduced by #3669 which was a fix for a bug introduced by #3359.

The short of it is that params.currentValues.extra_data can be an object (json) or a string (yaml) when being processed and we need to be able to handle both scenarios.

I went back and doubled checked that yaml comments were still being presented to the user on launch (see #3359 (comment) for more info on that).

I also dropped a commit in here to address the prompt tab spacing issues seen when launching a job from the job template form.

Before:
Screen Shot 2019-08-20 at 9 48 17 AM

After:
Screen Shot 2019-08-20 at 9 48 40 AM

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME
  • UI
AWX VERSION

awx: 6.1.0

ADDITIONAL INFORMATION

In addition to testing out the cases listed in #4293 and #3359 I've done some manual regression testing in the other places where extra variables are prompted. Schedules is one place that can be impacted by prompting changes. I double checked to make sure that promptable extra vars display properly when prompted for a schedule (both add and edit).

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@mabashian
Copy link
Member Author

mabashian commented Aug 20, 2019

Non-workflow scenarios locally tested:

  1. Launch a JT with no prompts
  2. Launch a JT with survey questions
  3. Create a schedule for JT with survey questions
  4. Edit a schedule for JT with survey questions
  5. Launch a JT with extra var prompting
  6. Create a schedule for JT with extra var prompting
  7. Edit a schedule for JT with extra var prompting
  8. Launch a JT with extra var and survey prompting
  9. Create a schedule for JT with extra var prompting
  10. Edit a schedule for JT with extra var prompting

I did come across #4293 (comment)

The way I reproduced was by creating a JT with default and promptable extra vars that look like:

Screen Shot 2019-08-20 at 11 40 33 AM

When creating the workflow node I changed the variables to look like:

Screen Shot 2019-08-20 at 11 40 41 AM

Editing the node results in the extra vars looking like:

Screen Shot 2019-08-20 at 11 41 01 AM

I'll work on including a fix for ^^ in this PR

@mabashian mabashian changed the title Fixes issues with extra var prompting in workflow nodes [WIP] Fixes issues with extra var prompting in workflow nodes Aug 20, 2019
@mabashian
Copy link
Member Author

Workflow scenarios tested:

  1. Create a node with a JT with no prompts
  2. Create a node with a JT with survey
  3. Edit node from step 2
  4. Create a node with a JT with extra var prompting
  5. Edit node from step 4
  6. Create a node with a JT with survey and extra var prompting
  7. Edit node from step 6

…orm. Small bug fixes for extra var corner cases in workflow nodes.
@mabashian mabashian changed the title [WIP] Fixes issues with extra var prompting in workflow nodes Fixes issues with extra var prompting in workflow nodes Aug 21, 2019
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

Copy link
Contributor

@jlmitch5 jlmitch5 left a comment

Choose a reason for hiding this comment

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

this all looks good so far as I understand it

@mabashian
Copy link
Member Author

recheck

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 4516e64 into ansible:devel Aug 23, 2019
@mabashian mabashian deleted the 4293-vars branch September 27, 2019 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants