Skip to content

Commit

Permalink
ui: add error handling and form validation to job create template (#1…
Browse files Browse the repository at this point in the history
…5767)

* ui: handle server side errors

* ui: show error to prevent duplicate

* refact: conditional namespace

* ui: save as template flow (#15787)
  • Loading branch information
ChaiWithJai authored Jan 17, 2023
1 parent 14d3fd6 commit 771e58d
Show file tree
Hide file tree
Showing 10 changed files with 214 additions and 23 deletions.
1 change: 1 addition & 0 deletions ui/app/components/job-editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export default class JobEditor extends Component {

job = null;
onSubmit() {}
handleSaveAsTemplate() {}

@computed('_context')
get context() {
Expand Down
12 changes: 12 additions & 0 deletions ui/app/controllers/jobs/run.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import Controller from '@ember/controller';
import { action } from '@ember/object';
import { tracked } from '@glimmer/tracking';

export default class JobsRunTemplatesController extends Controller {
@tracked jsonTemplate = null;

@action
setTemplate(template) {
this.jsonTemplate = template;
}
}
9 changes: 9 additions & 0 deletions ui/app/controllers/jobs/run/index.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,20 @@
import { getOwner } from '@ember/application';
import Controller from '@ember/controller';
import { action } from '@ember/object';
import { inject as service } from '@ember/service';

export default class RunController extends Controller {
@service router;

queryParams = ['template'];

@action
handleSaveAsTemplate() {
getOwner(this)
.lookup('controller:jobs.run')
.setTemplate(this.model._newDefinition);
}

onSubmit(id, namespace) {
this.router.transitionTo('jobs.job', `${id}@${namespace || 'default'}`);
}
Expand Down
43 changes: 34 additions & 9 deletions ui/app/controllers/jobs/run/templates/new.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@ import Controller from '@ember/controller';
import { inject as service } from '@ember/service';
import { action } from '@ember/object';
import { tracked } from '@glimmer/tracking';
import { trimPath } from '../../../../helpers/trim-path';

export default class JobsRunTemplatesController extends Controller {
@service router;
@service store;
@service system;
@tracked templateName = null;
@tracked templateNamespace = 'default';

Expand All @@ -19,6 +22,17 @@ export default class JobsRunTemplatesController extends Controller {
return namespaces;
}

get isDuplicateTemplate() {
const templates = this.store.peekAll('variable');
const templateName = trimPath([`nomad/job-templates/${this.templateName}`]);

return !!templates
.without(this.model)
.find(
(v) => v.path === templateName && v.namespace === this.templateNamespace
);
}

@action
updateKeyValue(key, value) {
if (this.keyValues.find((kv) => kv.key === key)) {
Expand Down Expand Up @@ -46,16 +60,27 @@ export default class JobsRunTemplatesController extends Controller {
this.model.set('keyValues', this.keyValues);
this.model.set('path', `nomad/job-templates/${this.templateName}`);
this.model.setAndTrimPath();
await this.model.save({ adapterOptions: { overwrite } });

this.flashMessages.add({
title: 'Job template saved',
message: `${this.templateName} successfully saved`,
type: 'success',
destroyOnClick: false,
timeout: 5000,
});
try {
await this.model.save({ adapterOptions: { overwrite } });

this.router.transitionTo('jobs.run.templates');
this.flashMessages.add({
title: 'Job template saved',
message: `${this.templateName} successfully saved`,
type: 'success',
destroyOnClick: false,
timeout: 5000,
});

this.router.transitionTo('jobs.run.templates');
} catch (e) {
this.flashMessages.add({
title: 'Job template cannot be saved.',
message: e,
type: 'error',
destroyOnClick: false,
timeout: 5000,
});
}
}
}
16 changes: 15 additions & 1 deletion ui/app/routes/jobs/run/templates/new.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { getOwner } from '@ember/application';
import Route from '@ember/routing/route';
import { inject as service } from '@ember/service';
import notifyForbidden from 'nomad-ui/utils/notify-forbidden';
Expand All @@ -22,8 +23,21 @@ export default class RunRoute extends Route {
try {
// When variables are created with a namespace attribute, it is verified against
// available namespaces to prevent redirecting to a non-existent namespace.
await this.store.findAll('namespace');
await Promise.all([
this.store.query('variable', {
prefix: 'nomad/job-templates',
namespace: '*',
}),
this.store.findAll('namespace'),
]);

// When navigating from jobs.run.index using "Save as Template"
const json = getOwner(this).lookup('controller:jobs.run').jsonTemplate;
if (json) {
return this.store.createRecord('variable', {
keyValues: [{ key: 'template', value: json }],
});
}
return this.store.createRecord('variable');
} catch (e) {
notifyForbidden(this)(e);
Expand Down
10 changes: 9 additions & 1 deletion ui/app/styles/core/forms.scss
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,19 @@
.new-job-template {
& > div {
margin-bottom: 1rem;
align-items: end;
}

.path-input {
height: 2.25em;
&.error {
color: $red;
border-color: $red;
}

+ p {
position: relative;
animation: slide-in 0.3s ease-out;
}
}

.input-dropdown-row {
Expand Down
1 change: 1 addition & 0 deletions ui/app/templates/components/job-editor.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
<input type="file" onchange={{action this.uploadJobSpec}} accept=".hcl,.json,.nomad" />
</label>
{{#if (can "write variable" path="*" namespace="*")}}
<Hds::Button @icon="file-plus" @text="Save as template" @color="tertiary" @route="jobs.run.templates.new" {{on "click" this.handleSaveAsTemplate}} data-test-save-as-template />
<Hds::Button @icon="file-text" @text="Choose from a template" @color="tertiary" @route="jobs.run.templates" data-test-choose-template />
{{/if}}
</Hds::ButtonSet>
Expand Down
2 changes: 1 addition & 1 deletion ui/app/templates/jobs/run/index.hbs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<Breadcrumb @crumb={{hash label="Run" args=(array "jobs.run")}} />
{{page-title "Run a job"}}
<section class="section">
<JobEditor @job={{this.model}} @context="new" @onSubmit={{action this.onSubmit}} />
<JobEditor @job={{this.model}} @context="new" @onSubmit={{action this.onSubmit}} @handleSaveAsTemplate={{this.handleSaveAsTemplate}} />
</section>
32 changes: 21 additions & 11 deletions ui/app/templates/jobs/run/templates/new.hbs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{{page-title "Create a custom template"}}
<section class="section">
<form class="new-job-template" autocomplete="off">
<div class={{if this.namespaceOptions "input-dropdown-row"}}>
<div class={{if this.system.shouldShowNamespaces "input-dropdown-row"}}>
<label>
<span>
Template name
Expand All @@ -10,27 +10,37 @@
@type="text"
@value={{mut this.templateName}}
placeholder="your-template-name-here"
class="input path-input"
class="input path-input {{if this.isDuplicateTemplate "error"}}"
{{autofocus}}
data-test-template-name
/>
{{#if this.isDuplicateTemplate}}
<p class="help is-danger" data-test-duplicate-error>
There is already a templated named {{this.templateName}}.
</p>
{{/if}}
</label>
{{#if this.namespaceOptions.length}}
<SingleSelectDropdown
data-test-namespace-facet
@label="Namespace"
@options={{this.namespaceOptions}}
@selection={{this.templateNamespace}}
@onSelect={{fn (mut this.templateNamespace)}}
/>
{{#if this.system.shouldShowNamespaces}}
<label>
<span>
Namespace
</span>
<SingleSelectDropdown
data-test-namespace-facet
@label="Namespace"
@options={{this.namespaceOptions}}
@selection={{this.templateNamespace}}
@onSelect={{fn (mut this.templateNamespace)}}
/>
</label>
{{/if}}
</div>
<VariableForm::JobTemplateEditor
@keyValues={{this.model.keyValues}}
@updateKeyValue={{this.updateKeyValue}}
/>
<footer class="button-group">
<Hds::Button @text="Save" @route="jobs.run.templates" disabled={{is-empty this.templateName}} {{on "click" this.save}} data-test-save-template />
<Hds::Button @text="Save" disabled={{is-empty this.templateName}} {{on "click" this.save}} data-test-save-template />
<Hds::Button @text="Cancel" @route="jobs.run.templates" @color="critical" data-test-cancel-template />
</footer>
</form>
Expand Down
111 changes: 111 additions & 0 deletions ui/tests/acceptance/job-run-test.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import AdapterError from '@ember-data/adapter/error';
import {
click,
currentRouteName,
Expand All @@ -6,6 +7,10 @@ import {
} from '@ember/test-helpers';
import { assign } from '@ember/polyfills';
import { module, test } from 'qunit';
import {
selectChoose,
clickTrigger,
} from 'ember-power-select/test-support/helpers';
import { setupApplicationTest } from 'ember-qunit';
import { setupMirage } from 'ember-cli-mirage/test-support';
import a11yAudit from 'nomad-ui/tests/helpers/a11y-audit';
Expand Down Expand Up @@ -310,5 +315,111 @@ module('Acceptance | job run', function (hooks) {
.dom('[data-test-template-card=foo]')
.exists('The newly created template appears in the list.');
});

test('a toast notification alerts the user if there is an error saving the newly created job template', async function (assert) {
assert.expect(5);
// Arrange
await JobRun.visit();
await click('[data-test-choose-template]');

// Assert
assert
.dom('[data-test-empty-templates-list-headline]')
.exists('No templates are listed if none have been created.');

await click('[data-test-create-inline]');
assert.equal(currentRouteName(), 'jobs.run.templates.new');
assert
.dom('[data-test-save-template]')
.isDisabled('the save button should be disabled if no path is set');

await fillIn('[data-test-template-name]', 'try@');
await fillIn('[data-test-template-description]', 'foo-bar-baz');
const codeMirror = getCodeMirrorInstance('[data-test-template-json]');
codeMirror.setValue(jsonJob());

server.put('/var/:varId?cas=0', function () {
return new AdapterError({
detail: `invalid path "nomad/job-templates/try@"`,
status: 500,
});
});

await click('[data-test-save-template]');
assert.equal(
currentRouteName(),
'jobs.run.templates.new',
'We do not navigate away from the page if an error is returned by the API.'
);
assert
.dom('.flash-message.alert-error')
.exists('A toast error message pops up.');
});

test('a user cannot create a job template if one with the same name and namespace already exists', async function (assert) {
assert.expect(4);
// Arrange
await JobRun.visit();
await click('[data-test-choose-template]');
server.create('variable', {
path: 'nomad/job-templates/foo',
namespace: 'default',
id: 'nomad/job-templates/foo',
});
server.create('namespace', { id: 'test' });

// Assert
assert
.dom('[data-test-empty-templates-list-headline]')
.exists('No templates are listed if none have been created.');

await click('[data-test-create-inline]');
assert.equal(currentRouteName(), 'jobs.run.templates.new');

await fillIn('[data-test-template-name]', 'foo');
assert
.dom('[data-test-duplicate-error]')
.exists('an error message alerts the user');

await clickTrigger('[data-test-namespace-facet]');
await selectChoose('[data-test-namespace-facet]', 'test');

assert
.dom('[data-test-duplicate-error]')
.doesNotExist(
'an error disappears when name or namespace combination is unique'
);
});

test('a user can save code from the editor as a template', async function (assert) {
assert.expect(4);
// Arrange
await JobRun.visit();
await JobRun.editor.editor.fillIn(jsonJob());

await click('[data-test-save-as-template]');
assert.equal(
currentRouteName(),
'jobs.run.templates.new',
'We navigate template creation page.'
);

// Assert
assert
.dom('[data-test-template-name]')
.hasNoText('No template name is prefilled.');
assert
.dom('[data-test-template-description]')
.hasNoText('No template description is prefilled.');

const codeMirror = getCodeMirrorInstance('[data-test-template-json]');
const json = codeMirror.getValue();

assert.equal(
json,
jsonJob(),
'Template is filled out with text from the editor.'
);
});
});
});

0 comments on commit 771e58d

Please sign in to comment.