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

Show yaml comments when possible on launch prompt #3359

Merged
merged 2 commits into from
Mar 7, 2019

Conversation

mabashian
Copy link
Member

SUMMARY

When the JT launch prompt modal was overhauled we added a Preview tab. When we added this tab, we added some logic to aggregate prompted extra vars and survey questions into a single block of
extra vars to give the user an idea of exactly what extra vars will be used during that JT run. In order to achieve this we have to convert the existing extra vars from YAML to JSON. This allows us to merge the two objects easily. When the extra vars are converted though, the comments are stripped out. We could support maintaining the comments but this would require a dependency change that I don't think we're ready to commit to at this point.

This PR ensures that YAML comments are shown to the user during launch when possible.

Scenarios (in all scenarios the JT has comments in extra vars):

  1. JT has no survey - comments shown in extra vars prompt codemirror and Preview extra vars codemirror
  2. JT has survey - comments shown in extra vars prompt codemirror but not shown in Preview extra vars codemirror

This PR also impacts schedules. When the user is adding a new schedule the comments should appear in the codemirror. Once the schedule is saved the api will have stripped out all the comments and stored the extra vars as JSON so they will no longer appear in the UI. This is expected behavior.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME
  • UI
ADDITIONAL INFORMATION

The local testing that I've done has consisted of setting up all the different permutations of job template configurations and going through the launch process to ensure that a) comments are displayed when expected and b) extra vars are passed to job correctly.

  1. No default vars, no var prompting, no survey
  2. No default vars, no var prompting, survey
  3. No default vars, var prompting, no survey
  4. No default vars, var prompting, survey
  5. Default vars, no var prompting, no survey
  6. Default vars, no var prompting, survey
  7. Default vars, var prompting, no survey
  8. Default vars, var prompting, survey

I also went over and took a look at schedules to make sure that those still behaved as expected.

Workflow node prompting is also worth looking at. The rules that apply to jt launch should apply to workflow nodes as well. The variables should be shown when possible and will be wiped out when a survey is present (as well as on save).

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

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.

Code looks good

@jakemcdermott
Copy link
Contributor

recheck

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

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.

4 participants