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

[ui] "Clone and Edit" functionality for versions #24168

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
3 changes: 3 additions & 0 deletions .changelog/24168.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
ui: Add an Edit From Version button as an option when reverting from an older job version
```
42 changes: 37 additions & 5 deletions ui/app/adapters/job.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,48 @@ export default class JobAdapter extends WatchableNamespaceIDs {
summary: '/summary',
};

fetchRawDefinition(job) {
const url = this.urlForFindRecord(job.get('id'), 'job');
return this.ajax(url, 'GET');
/**
* Gets the JSON definition of a job.
* Prior to Nomad 1.6, this was the only way to get job definition data.
* Now, this is included as a stringified JSON object when fetching raw specification (under .Source).
* This method is still important for backwards compatibility with older job versions, as well as a fallback
* for when fetching raw specification fails.
* @param {import('../models/job').default} job
* @param {number} version
*/
async fetchRawDefinition(job, version) {
if (version == null) {
const url = this.urlForFindRecord(job.get('id'), 'job');
return this.ajax(url, 'GET');
}

// For specific versions, we need to fetch from versions endpoint,
// and then find the specified version info from the response.
const versionsUrl = addToPath(
this.urlForFindRecord(job.get('id'), 'job', null, 'versions')
);

const response = await this.ajax(versionsUrl, 'GET');
const versionInfo = response.Versions.find((v) => v.Version === version);

if (!versionInfo) {
throw new Error(`Version ${version} not found`);
}

return versionInfo;
}

fetchRawSpecification(job) {
/**
* Gets submission info for a job, including (if available) the raw HCL or JSON spec used to run it,
* including variable flags and literals.
* @param {import('../models/job').default} job
* @param {number} version
*/
fetchRawSpecification(job, version) {
const url = addToPath(
this.urlForFindRecord(job.get('id'), 'job', null, 'submission'),
'',
'version=' + job.get('version')
'version=' + (version || job.get('version'))
);
return this.ajax(url, 'GET');
}
Expand Down
59 changes: 59 additions & 0 deletions ui/app/components/job-version.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
* SPDX-License-Identifier: BUSL-1.1
*/

// @ts-check

import Component from '@glimmer/component';
import { action, computed } from '@ember/object';
import { alias } from '@ember/object/computed';
Expand Down Expand Up @@ -80,6 +82,11 @@ export default class JobVersion extends Component {
this.isOpen = !this.isOpen;
}

/**
* @type {'idle' | 'confirming'}
*/
@tracked cloneButtonStatus = 'idle';

@task(function* () {
try {
const versionBeforeReversion = this.version.get('job.version');
Expand Down Expand Up @@ -108,6 +115,58 @@ export default class JobVersion extends Component {
})
revertTo;

@action async cloneAsNewVersion() {
try {
this.router.transitionTo(
'jobs.job.definition',
this.version.get('job.idWithNamespace'),
{
queryParams: {
isEditing: true,
version: this.version.number,
},
}
);
} catch (e) {
this.args.handleError({
level: 'danger',
title: 'Could Not Edit from Version',
});
}
}

@action async cloneAsNewJob() {
const job = await this.version.get('job');
try {
const specification = await job.fetchRawSpecification(
this.version.number
);
this.router.transitionTo('jobs.run', {
queryParams: {
sourceString: specification.Source,
},
});
return;
} catch (specError) {
try {
// If submission info is not available, try to fetch the raw definition
const definition = await job.fetchRawDefinition(this.version.number);
this.router.transitionTo('jobs.run', {
queryParams: {
sourceString: JSON.stringify(definition, null, 2),
},
});
} catch (defError) {
// Both methods failed, show error
this.args.handleError({
level: 'danger',
title: 'Could Not Clone as New Job',
description: messageForError(defError),
});
}
}
}

@action
handleKeydown(event) {
if (event.key === 'Escape') {
Expand Down
2 changes: 1 addition & 1 deletion ui/app/controllers/jobs/run/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { inject as service } from '@ember/service';
export default class RunController extends Controller {
@service router;

queryParams = ['template'];
queryParams = ['template', 'sourceString', 'disregardNameWarning'];

@action
handleSaveAsTemplate() {
Expand Down
8 changes: 4 additions & 4 deletions ui/app/models/job.js
Original file line number Diff line number Diff line change
Expand Up @@ -537,12 +537,12 @@ export default class Job extends Model {
return undefined;
}

fetchRawDefinition() {
return this.store.adapterFor('job').fetchRawDefinition(this);
fetchRawDefinition(version) {
return this.store.adapterFor('job').fetchRawDefinition(this, version);
}

fetchRawSpecification() {
return this.store.adapterFor('job').fetchRawSpecification(this);
fetchRawSpecification(version) {
return this.store.adapterFor('job').fetchRawSpecification(this, version);
}

forcePeriodic() {
Expand Down
34 changes: 30 additions & 4 deletions ui/app/routes/jobs/job/definition.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,29 +5,55 @@

// @ts-check
import Route from '@ember/routing/route';

import { inject as service } from '@ember/service';
/**
* Route for fetching and displaying a job's definition and specification.
*/
export default class DefinitionRoute extends Route {
@service notifications;

queryParams = {
version: {
refreshModel: true,
},
};

/**
* Fetch the job's definition, specification, and variables from the API.
*
* @returns {Promise<Object>} A promise that resolves to an object containing the job, definition, format,
* specification, variableFlags, and variableLiteral.
*/
async model() {
async model({ version }) {
version = version ? +version : undefined; // query parameter is a string; convert to number
/** @type {import('../../../models/job').default} */
const job = this.modelFor('jobs.job');
if (!job) return;

const definition = await job.fetchRawDefinition();
let definition;

if (version !== undefined) {
// Not (!version) because version can be 0
try {
definition = await job.fetchRawDefinition(version);
} catch (e) {
console.error('error fetching job version definition', e);
this.notifications.add({
title: 'Error Fetching Job Version Definition',
message: `There was an error fetching the versions for this job: ${e.message}`,
color: 'critical',
});
}
} else {
definition = await job.fetchRawDefinition();
}

let format = 'json'; // default to json in network request errors
let specification;
let variableFlags;
let variableLiteral;
try {
const specificationResponse = await job.fetchRawSpecification();
const specificationResponse = await job.fetchRawSpecification(version);
specification = specificationResponse?.Source ?? null;
variableFlags = specificationResponse?.VariableFlags ?? null;
variableLiteral = specificationResponse?.Variables ?? null;
Expand Down
11 changes: 10 additions & 1 deletion ui/app/routes/jobs/run/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ export default class JobsRunIndexRoute extends Route {
template: {
refreshModel: true,
},
sourceString: {
refreshModel: true,
},
};

beforeModel(transition) {
Expand All @@ -33,7 +36,7 @@ export default class JobsRunIndexRoute extends Route {
}
}

async model({ template }) {
async model({ template, sourceString }) {
try {
// When jobs are created with a namespace attribute, it is verified against
// available namespaces to prevent redirecting to a non-existent namespace.
Expand All @@ -45,6 +48,10 @@ export default class JobsRunIndexRoute extends Route {
return this.store.createRecord('job', {
_newDefinition: templateRecord.items.template,
});
} else if (sourceString) {
return this.store.createRecord('job', {
_newDefinition: sourceString,
});
} else {
return this.store.createRecord('job');
}
Expand Down Expand Up @@ -72,6 +79,8 @@ export default class JobsRunIndexRoute extends Route {
if (isExiting) {
controller.model?.deleteRecord();
controller.set('template', null);
controller.set('sourceString', null);
controller.set('disregardNameWarning', null);
}
}
}
4 changes: 3 additions & 1 deletion ui/app/templates/components/job-editor/alert.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
<Hds::Alert @type="inline" @color="critical" data-test-error={{@data.error.type}} as |A|>
<A.Title data-test-error-title>{{conditionally-capitalize @data.error.type true}}</A.Title>
<A.Description data-test-error-message>{{@data.error.message}}</A.Description>
{{#if (eq @data.error.message "Job ID does not match")}}
<A.Button @text="Run as a new job instead" @color="primary" @route="jobs.run" @query={{hash [email protected]._newDefinition disregardNameWarning=true}} />
{{/if}}
</Hds::Alert>
{{/if}}
{{#if (and (eq @data.stage "read") @data.hasVariables (eq @data.view "job-spec"))}}
Expand All @@ -31,4 +34,3 @@
</Hds::Alert>
{{/if}}
</div>

67 changes: 52 additions & 15 deletions ui/app/templates/components/job-version.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
~}}

{{did-update this.versionsDidUpdate this.diff}}
<section class="job-version">
<section class="job-version" data-test-job-version={{this.version.number}}>
<div class="boxed-section {{if this.version.versionTag "tagged"}}" data-test-tagged-version={{if this.version.versionTag "true" "false"}}>
<header class="boxed-section-head is-light inline-definitions">
Version #{{this.version.number}}
Expand Down Expand Up @@ -81,20 +81,57 @@
<div class="version-options">
{{#unless this.isCurrent}}
{{#if (can "run job" namespace=this.version.job.namespace)}}
<TwoStepButton
data-test-revert-to
@classes={{hash
idleButton="is-warning is-outlined"
confirmButton="is-warning"}}
@fadingBackground={{true}}
@idleText="Revert Version"
@cancelText="Cancel"
@confirmText="Yes, Revert Version"
@confirmationMessage="Are you sure you want to revert to this version?"
@inlineText={{true}}
@size="small"
@awaitingConfirmation={{this.revertTo.isRunning}}
@onConfirm={{perform this.revertTo}} />
{{#if (eq this.cloneButtonStatus 'idle')}}
<Hds::Button
data-test-clone-and-edit
@text="Clone and Edit"
@color="secondary"
@size="small"
@isInline={{true}}
{{on "click" (action (mut this.cloneButtonStatus) "confirming")}}
/>

<TwoStepButton
data-test-revert-to
@classes={{hash
idleButton="is-warning is-outlined"
confirmButton="is-warning"}}
@fadingBackground={{true}}
@idleText="Revert Version"
@cancelText="Cancel"
@confirmText="Yes, Revert Version"
@confirmationMessage="Are you sure you want to revert to this version?"
@inlineText={{true}}
@size="small"
@awaitingConfirmation={{this.revertTo.isRunning}}
@onConfirm={{perform this.revertTo}}
/>
{{else if (eq this.cloneButtonStatus 'confirming')}}
<Hds::Button
data-test-cancel-clone
@text="Cancel"
@color="secondary"
@size="small"
@isInline={{true}}
{{on "click" (action (mut this.cloneButtonStatus) "idle")}}
/>
<Hds::Button
data-test-clone-as-new-version
@text="Clone as New Version of {{this.version.job.name}}"
@color="secondary"
@size="small"
@isInline={{true}}
{{on "click" (action this.cloneAsNewVersion)}}
/>
<Hds::Button
data-test-clone-as-new-job
@text="Clone as New Job"
@color="secondary"
@size="small"
@isInline={{true}}
{{on "click" (action this.cloneAsNewJob)}}
/>
{{/if}}
{{else}}
<Hds::Button
data-test-revert-to
Expand Down
8 changes: 7 additions & 1 deletion ui/app/templates/jobs/run/index.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,11 @@
<Breadcrumb @crumb={{hash label="Run" args=(array "jobs.run")}} />
{{page-title "Run a job"}}
<section class="section">
{{#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}}
Comment on lines +9 to +14
Copy link
Member

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.

Copy link
Contributor Author

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!

<JobEditor @job={{this.model}} @context="new" @onSubmit={{action this.onSubmit}} @handleSaveAsTemplate={{this.handleSaveAsTemplate}} data-test-job-editor />
</section>
</section>
Loading
Loading