-
Notifications
You must be signed in to change notification settings - Fork 2k
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
[ui] "Clone and Edit" functionality for versions #24168
[ui] "Clone and Edit" functionality for versions #24168
Conversation
e2f0119
to
eaed41d
Compare
Ember Test Audit comparison
|
@philrenaud when this is merged, we also need to update the screenshots, and possibly text, in the job versions guide. |
d180359
to
a5d6f63
Compare
a100fba
to
af31960
Compare
af31960
to
97d3f8c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
{{#if (and this.sourceString (not this.disregardNameWarning))}} | ||
<Hds::Alert @type="inline" @color="warning" data-test-job-name-warning as |A|> | ||
<A.Title>Don't forget to change the job name!</A.Title> | ||
<A.Description>Since you're cloning a job version's source as a new job, you'll want to change the job name. Otherwise, this will appear as a new version of the original job, rather than a new job.</A.Description> | ||
</Hds::Alert> | ||
{{/if}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be nice to swap out the job name with an empty string to prevent accidental overwrites. Unfortunately we don't a JS parser for HCL2. My first pass at a regexp was something like ^job\s+"(.*)"\s+\{
but some quick testing finds some corner cases, so maybe that's not such a good idea after all. If you've got an obvious way to do this, I'd go for it, otherwise we can skip.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really wanted to do this, too — but a loud warning is better than nothing!
3d532cf
to
c7f92d6
Compare
…rroring on sourceless clone
…e one you're editing, suggest they might want to run a new one instead
c7f92d6
to
34a566a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Adds "Clone and Edit" alongside "Revert Version" on the Versions page:
There are two functional paths here:
In each of these cases, we also have a split on "fetchRawDefinition" (fetches the submitted HCL or JSON jobspec) or "fetchRawSpecification" (the pre-1.5 Nomad way of getting job details, which the UI falls back to if submission/definition is not available). A lot of the duplicate-looking code in this PR is to address both paths.
Similarly, there are some safeguards included in this PR for things like ensuring users who clone to a new job change the job's name, lest it adds it as a new version unintentionally.
Because we're passing template strings via query params, this means you can use
?sourceString=
on the jobs run page to send someone a readily-editable job spec, using a URL like this.This also adds a few UX conveniences like prompting a user to "run as a new job instead" if they try updating a job definition and changing its name (which fails a plan request), and reminding them to change the job name in the event they try to clone as a new one
Resolves #24156