Skip to content

Commit

Permalink
Merge de96c34 into backport/24156-ui-versions-revert-version-improvem…
Browse files Browse the repository at this point in the history
…ents/entirely-thorough-mosquito
  • Loading branch information
hc-github-team-nomad-core authored Nov 29, 2024
2 parents d435e1f + de96c34 commit 3061ff7
Show file tree
Hide file tree
Showing 12 changed files with 446 additions and 32 deletions.
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 sourceString=@data.job._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}}
<JobEditor @job={{this.model}} @context="new" @onSubmit={{action this.onSubmit}} @handleSaveAsTemplate={{this.handleSaveAsTemplate}} data-test-job-editor />
</section>
</section>
Loading

0 comments on commit 3061ff7

Please sign in to comment.