From 55fca36beafae3e8862e78b0679d5aed320d53cc Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 21 Aug 2018 12:06:43 -0700 Subject: [PATCH 01/18] New job edit page --- ui/app/router.js | 1 + ui/app/templates/jobs/job/definition.hbs | 1 + 2 files changed, 2 insertions(+) diff --git a/ui/app/router.js b/ui/app/router.js index 7a4015c28..2c298fa71 100644 --- a/ui/app/router.js +++ b/ui/app/router.js @@ -16,6 +16,7 @@ Router.map(function() { this.route('deployments'); this.route('evaluations'); this.route('allocations'); + this.route('edit'); }); }); diff --git a/ui/app/templates/jobs/job/definition.hbs b/ui/app/templates/jobs/job/definition.hbs index 2e4280af2..44b3515aa 100644 --- a/ui/app/templates/jobs/job/definition.hbs +++ b/ui/app/templates/jobs/job/definition.hbs @@ -1,5 +1,6 @@ {{partial "jobs/job/subnav"}}
+ {{#link-to "jobs.job.edit" job class="button is-primary"}}Edit{{/link-to}}
{{json-viewer data-test-definition-view json=model.definition}} From b3c2538c68038df52828320f34ba312755a879c6 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 21 Aug 2018 12:07:45 -0700 Subject: [PATCH 02/18] Move the bulk of the new job page into a new job editor component --- ui/app/components/job-editor.js | 70 ++++++++++++++++ ui/app/controllers/jobs/run.js | 68 +--------------- ui/app/templates/components/job-editor.hbs | 92 +++++++++++++++++++++ ui/app/templates/jobs/run.hbs | 93 +--------------------- 4 files changed, 167 insertions(+), 156 deletions(-) create mode 100644 ui/app/components/job-editor.js create mode 100644 ui/app/templates/components/job-editor.hbs diff --git a/ui/app/components/job-editor.js b/ui/app/components/job-editor.js new file mode 100644 index 000000000..7464742cf --- /dev/null +++ b/ui/app/components/job-editor.js @@ -0,0 +1,70 @@ +import Component from '@ember/component'; +import { inject as service } from '@ember/service'; +import { computed } from '@ember/object'; +import { task } from 'ember-concurrency'; +import messageFromAdapterError from 'nomad-ui/utils/message-from-adapter-error'; +import localStorageProperty from 'nomad-ui/utils/properties/local-storage'; + +export default Component.extend({ + store: service(), + + job: null, + onSubmit() {}, + + parseError: null, + planError: null, + runError: null, + + planOutput: null, + + showPlanMessage: localStorageProperty('nomadMessageJobPlan', true), + showEditorMessage: localStorageProperty('nomadMessageJobEditor', true), + + stage: computed('planOutput', function() { + return this.get('planOutput') ? 'plan' : 'editor'; + }), + + plan: task(function*() { + this.reset(); + + try { + yield this.get('job').parse(); + } catch (err) { + const error = messageFromAdapterError(err) || 'Could not parse input'; + this.set('parseError', error); + return; + } + + try { + yield this.get('job').plan(); + const plan = this.get('store').peekRecord('job-plan', this.get('job.id')); + this.set('planOutput', plan); + } catch (err) { + const error = messageFromAdapterError(err) || 'Could not plan job'; + this.set('planError', error); + } + }).drop(), + + submit: task(function*() { + try { + yield this.get('job').run(); + + const id = this.get('job.plainId'); + const namespace = this.get('job.namespace.name') || 'default'; + + this.reset(); + + // Treat the job as ephemeral and only provide ID parts. + this.get('onSubmit')(id, namespace); + } catch (err) { + const error = messageFromAdapterError(err) || 'Could not submit job'; + this.set('runError', error); + } + }), + + reset() { + this.set('planOutput', null); + this.set('planError', null); + this.set('parseError', null); + }, +}); diff --git a/ui/app/controllers/jobs/run.js b/ui/app/controllers/jobs/run.js index 79525e460..baaf84183 100644 --- a/ui/app/controllers/jobs/run.js +++ b/ui/app/controllers/jobs/run.js @@ -1,69 +1,9 @@ import Controller from '@ember/controller'; -import { inject as service } from '@ember/service'; -import { computed } from '@ember/object'; -import { task } from 'ember-concurrency'; -import messageFromAdapterError from 'nomad-ui/utils/message-from-adapter-error'; -import localStorageProperty from 'nomad-ui/utils/properties/local-storage'; export default Controller.extend({ - store: service(), - - parseError: null, - planError: null, - runError: null, - - planOutput: null, - - showPlanMessage: localStorageProperty('nomadMessageJobPlan', true), - showEditorMessage: localStorageProperty('nomadMessageJobEditor', true), - - stage: computed('planOutput', function() { - return this.get('planOutput') ? 'plan' : 'editor'; - }), - - plan: task(function*() { - this.reset(); - - try { - yield this.get('model').parse(); - } catch (err) { - const error = messageFromAdapterError(err) || 'Could not parse input'; - this.set('parseError', error); - return; - } - - try { - yield this.get('model').plan(); - const plan = this.get('store').peekRecord('job-plan', this.get('model.id')); - this.set('planOutput', plan); - } catch (err) { - const error = messageFromAdapterError(err) || 'Could not plan job'; - this.set('planError', error); - } - }).drop(), - - submit: task(function*() { - try { - yield this.get('model').run(); - - const id = this.get('model.plainId'); - const namespace = this.get('model.namespace.name') || 'default'; - - this.reset(); - - // navigate to the new job page - this.transitionToRoute('jobs.job', id, { - queryParams: { jobNamespace: namespace }, - }); - } catch (err) { - const error = messageFromAdapterError(err) || 'Could not submit job'; - this.set('runError', error); - } - }), - - reset() { - this.set('planOutput', null); - this.set('planError', null); - this.set('parseError', null); + onSubmit(id, namespace) { + this.transitionToRoute('jobs.job', id, { + queryParams: { jobNamespace: namespace }, + }); }, }); diff --git a/ui/app/templates/components/job-editor.hbs b/ui/app/templates/components/job-editor.hbs new file mode 100644 index 000000000..250934bc7 --- /dev/null +++ b/ui/app/templates/components/job-editor.hbs @@ -0,0 +1,92 @@ +{{#if parseError}} +
+

Parse Error

+

{{parseError}}

+
+{{/if}} +{{#if planError}} +
+

Plan Error

+

{{planError}}

+
+{{/if}} +{{#if runError}} +
+

Run Error

+

{{runError}}

+
+{{/if}} + +{{#if (eq stage "editor")}} + {{#if showEditorMessage}} +
+
+
+

Run a Job

+

Paste or author HCL or JSON to submit to your cluster. A plan will be requested before the job is submitted.

+
+
+ +
+
+
+ {{/if}} +
+
+ Job Definition +
+
+ {{ivy-codemirror + data-test-editor + value=(or job._newDefinition jobSpec) + valueUpdated=(action (mut job._newDefinition)) + options=(hash + mode="javascript" + theme="hashi" + tabSize=2 + lineNumbers=true + )}} +
+
+
+ +
+{{/if}} + +{{#if (eq stage "plan")}} + {{#if showPlanMessage}} +
+
+
+

Job Plan

+

This is the impact running this job will have on your cluster.

+
+
+ +
+
+
+ {{/if}} +
+
Job Plan
+
+ {{job-diff data-test-plan-output diff=planOutput.diff verbose=false}} +
+
+
+
Scheduler dry-run
+
+ {{#if planOutput.failedTGAllocs}} + {{#each planOutput.failedTGAllocs as |placementFailure|}} + {{placement-failure failedTGAlloc=placementFailure}} + {{/each}} + {{else}} + All tasks successfully allocated. + {{/if}} +
+
+
+ + +
+{{/if}} diff --git a/ui/app/templates/jobs/run.hbs b/ui/app/templates/jobs/run.hbs index 9039cae7f..52400b413 100644 --- a/ui/app/templates/jobs/run.hbs +++ b/ui/app/templates/jobs/run.hbs @@ -1,94 +1,3 @@
- {{#if parseError}} -
-

Parse Error

-

{{parseError}}

-
- {{/if}} - {{#if planError}} -
-

Plan Error

-

{{planError}}

-
- {{/if}} - {{#if runError}} -
-

Run Error

-

{{runError}}

-
- {{/if}} - - {{#if (eq stage "editor")}} - {{#if showEditorMessage}} -
-
-
-

Run a Job

-

Paste or author HCL or JSON to submit to your cluster. A plan will be requested before the job is submitted.

-
-
- -
-
-
- {{/if}} -
-
- Job Definition -
-
- {{ivy-codemirror - data-test-editor - value=(or model._newDefinition jobSpec) - valueUpdated=(action (mut model._newDefinition)) - options=(hash - mode="javascript" - theme="hashi" - tabSize=2 - lineNumbers=true - )}} -
-
-
- -
- {{/if}} - - {{#if (eq stage "plan")}} - {{#if showPlanMessage}} -
-
-
-

Job Plan

-

This is the impact running this job will have on your cluster.

-
-
- -
-
-
- {{/if}} -
-
Job Plan
-
- {{job-diff data-test-plan-output diff=planOutput.diff verbose=false}} -
-
-
-
Scheduler dry-run
-
- {{#if planOutput.failedTGAllocs}} - {{#each planOutput.failedTGAllocs as |placementFailure|}} - {{placement-failure failedTGAlloc=placementFailure}} - {{/each}} - {{else}} - All tasks successfully allocated. - {{/if}} -
-
-
- - -
- {{/if}} + {{job-editor job=model onSubmit=(action onSubmit)}}
From c81ba192dcd5b9ba9ed4a15e09245516bc394683 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 21 Aug 2018 13:47:01 -0700 Subject: [PATCH 03/18] Fix a blocking queries bug The lowest valid blocking query index is 1, but the API will return 0 if there has yet to be an index set (no response). This in conjunction with that 0 being stored as a string made the "fallback to 1" guard not work. --- ui/app/services/watch-list.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/app/services/watch-list.js b/ui/app/services/watch-list.js index 51154d6bd..4387c40c4 100644 --- a/ui/app/services/watch-list.js +++ b/ui/app/services/watch-list.js @@ -18,6 +18,6 @@ export default Service.extend({ }, setIndexFor(url, value) { - list[url] = value; + list[url] = +value; }, }); From cd6d4e41e5dbc57a7aa59838bc0beda999cd53d7 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 21 Aug 2018 16:37:36 -0700 Subject: [PATCH 04/18] Fix multiple highlight bug in the distribution-bar component Caused by the re-indexing that occurs to remove zero-value bars. --- ui/app/templates/components/distribution-bar.hbs | 2 +- ui/app/templates/components/job-page/parts/summary.hbs | 2 +- ui/app/templates/jobs/job/task-group.hbs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ui/app/templates/components/distribution-bar.hbs b/ui/app/templates/components/distribution-bar.hbs index 4db55477f..c5a5a546a 100644 --- a/ui/app/templates/components/distribution-bar.hbs +++ b/ui/app/templates/components/distribution-bar.hbs @@ -15,7 +15,7 @@
    {{#each _data as |datum index|}} -
  1. +
  2. {{datum.label}} diff --git a/ui/app/templates/components/job-page/parts/summary.hbs b/ui/app/templates/components/job-page/parts/summary.hbs index 38e9a1b68..0bacf5c52 100644 --- a/ui/app/templates/components/job-page/parts/summary.hbs +++ b/ui/app/templates/components/job-page/parts/summary.hbs @@ -39,7 +39,7 @@ class="split-view" as |chart|}}
      {{#each chart.data as |datum index|}} -
    1. +
    2. {{datum.value}} diff --git a/ui/app/templates/jobs/job/task-group.hbs b/ui/app/templates/jobs/job/task-group.hbs index 1c20a03e7..b9ead72d8 100644 --- a/ui/app/templates/jobs/job/task-group.hbs +++ b/ui/app/templates/jobs/job/task-group.hbs @@ -27,7 +27,7 @@ {{#allocation-status-bar allocationContainer=model.summary class="split-view" as |chart|}}
        {{#each chart.data as |datum index|}} -
      1. +
      2. {{datum.value}} From 33499658a471e0e53f33e7e3b5c7db7f49f9c8b5 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 21 Aug 2018 16:38:35 -0700 Subject: [PATCH 05/18] Use the same urlForFindRecord logic for urlForUpdateRecord --- ui/app/adapters/application.js | 59 ++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/ui/app/adapters/application.js b/ui/app/adapters/application.js index b1504ff03..13a24d0f2 100644 --- a/ui/app/adapters/application.js +++ b/ui/app/adapters/application.js @@ -73,32 +73,35 @@ export default RESTAdapter.extend({ // // This is the original implementation of _buildURL // without the pluralization of modelName - urlForFindRecord(id, modelName) { - let path; - let url = []; - let host = get(this, 'host'); - let prefix = this.urlPrefix(); - - if (modelName) { - path = modelName.camelize(); - if (path) { - url.push(path); - } - } - - if (id) { - url.push(encodeURIComponent(id)); - } - - if (prefix) { - url.unshift(prefix); - } - - url = url.join('/'); - if (!host && url && url.charAt(0) !== '/') { - url = '/' + url; - } - - return url; - }, + urlForFindRecord: urlForRecord, + urlForUpdateRecord: urlForRecord, }); + +function urlForRecord(id, modelName) { + let path; + let url = []; + let host = get(this, 'host'); + let prefix = this.urlPrefix(); + + if (modelName) { + path = modelName.camelize(); + if (path) { + url.push(path); + } + } + + if (id) { + url.push(encodeURIComponent(id)); + } + + if (prefix) { + url.unshift(prefix); + } + + url = url.join('/'); + if (!host && url && url.charAt(0) !== '/') { + url = '/' + url; + } + + return url; +} From 05a3238c37d8cc082415c62866cc3eac939dca1a Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 21 Aug 2018 16:39:47 -0700 Subject: [PATCH 06/18] Support job update in the adapter --- ui/app/adapters/job.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/ui/app/adapters/job.js b/ui/app/adapters/job.js index a8c09ce09..8ad0e5df2 100644 --- a/ui/app/adapters/job.js +++ b/ui/app/adapters/job.js @@ -33,6 +33,12 @@ export default Watchable.extend({ return associateNamespace(url, namespace); }, + urlForUpdateRecord(id, type, hash) { + const [name, namespace] = JSON.parse(id); + let url = this._super(name, type, hash); + return associateNamespace(url, namespace); + }, + xhrKey(url, method, options = {}) { const plainKey = this._super(...arguments); const namespace = options.data && options.data.namespace; @@ -92,6 +98,15 @@ export default Watchable.extend({ }, }); }, + + update(job) { + const url = this.urlForUpdateRecord(job.get('id'), 'job'); + return this.ajax(this.urlForUpdateRecord(job.get('id'), 'job'), 'POST', { + data: { + Job: job.get('_newDefinitionJSON'), + }, + }); + }, }); function associateNamespace(url, namespace) { From 527b5d5e97f2a80f63c099f88d3417faa17f8bc6 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 21 Aug 2018 16:41:05 -0700 Subject: [PATCH 07/18] Support different contexts for the job editor --- ui/app/components/job-editor.js | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/ui/app/components/job-editor.js b/ui/app/components/job-editor.js index 7464742cf..1e240dfd1 100644 --- a/ui/app/components/job-editor.js +++ b/ui/app/components/job-editor.js @@ -7,10 +7,25 @@ import localStorageProperty from 'nomad-ui/utils/properties/local-storage'; export default Component.extend({ store: service(), + config: service(), job: null, onSubmit() {}, + context: computed({ + get() { + return this.get('_context'); + }, + set(key, value) { + const allowedValues = ['new', 'edit']; + if (!allowedValues.includes(value)) { + throw new Error(`context must be one of: ${allowedValues.join(', ')}`); + } + this.set('_context', value); + return value; + }, + }), + _context: null, parseError: null, planError: null, runError: null, @@ -32,6 +47,7 @@ export default Component.extend({ } catch (err) { const error = messageFromAdapterError(err) || 'Could not parse input'; this.set('parseError', error); + this.scrollToError(); return; } @@ -42,12 +58,17 @@ export default Component.extend({ } catch (err) { const error = messageFromAdapterError(err) || 'Could not plan job'; this.set('planError', error); + this.scrollToError(); } }).drop(), submit: task(function*() { try { - yield this.get('job').run(); + if (this.get('context') === 'new') { + yield this.get('job').run(); + } else { + yield this.get('job').update(); + } const id = this.get('job.plainId'); const namespace = this.get('job.namespace.name') || 'default'; @@ -59,6 +80,8 @@ export default Component.extend({ } catch (err) { const error = messageFromAdapterError(err) || 'Could not submit job'; this.set('runError', error); + this.set('planOutput', null); + this.scrollToError(); } }), @@ -66,5 +89,12 @@ export default Component.extend({ this.set('planOutput', null); this.set('planError', null); this.set('parseError', null); + this.set('runError', null); + }, + + scrollToError() { + if (!this.get('config.isTest')) { + window.scrollTo(0, 0); + } }, }); From 902ec3e734990e7e6aae3b3c968e95d889512db2 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 21 Aug 2018 16:43:30 -0700 Subject: [PATCH 08/18] fixup-adapter --- ui/app/adapters/job.js | 1 - 1 file changed, 1 deletion(-) diff --git a/ui/app/adapters/job.js b/ui/app/adapters/job.js index 8ad0e5df2..b9dc7aa09 100644 --- a/ui/app/adapters/job.js +++ b/ui/app/adapters/job.js @@ -100,7 +100,6 @@ export default Watchable.extend({ }, update(job) { - const url = this.urlForUpdateRecord(job.get('id'), 'job'); return this.ajax(this.urlForUpdateRecord(job.get('id'), 'job'), 'POST', { data: { Job: job.get('_newDefinitionJSON'), From a7cd7b39d67364281b1c6da4f46c6c4b9ceb2efd Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 21 Aug 2018 16:43:46 -0700 Subject: [PATCH 09/18] fixup-job-editor --- ui/app/components/job-editor.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ui/app/components/job-editor.js b/ui/app/components/job-editor.js index 1e240dfd1..7cb6ff201 100644 --- a/ui/app/components/job-editor.js +++ b/ui/app/components/job-editor.js @@ -1,4 +1,5 @@ import Component from '@ember/component'; +import { assert } from '@ember/debug'; import { inject as service } from '@ember/service'; import { computed } from '@ember/object'; import { task } from 'ember-concurrency'; @@ -17,9 +18,9 @@ export default Component.extend({ }, set(key, value) { const allowedValues = ['new', 'edit']; - if (!allowedValues.includes(value)) { - throw new Error(`context must be one of: ${allowedValues.join(', ')}`); - } + + assert(`context must be one of: ${allowedValues.join(', ')}`, allowedValues.includes(value)); + this.set('_context', value); return value; }, From ed763ad14f81e0557eafd8e045537eedcad65d4c Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 21 Aug 2018 16:44:31 -0700 Subject: [PATCH 10/18] Handle update job in the model --- ui/app/models/job.js | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/ui/app/models/job.js b/ui/app/models/job.js index fc9dfd555..6db6ef676 100644 --- a/ui/app/models/job.js +++ b/ui/app/models/job.js @@ -203,6 +203,11 @@ export default Model.extend({ return this.store.adapterFor('job').run(this); }, + update() { + assert('A job must be parsed before updated', this.get('_newDefinitionJSON')); + return this.store.adapterFor('job').update(this); + }, + parse() { const definition = this.get('_newDefinition'); let promise; @@ -211,7 +216,12 @@ export default Model.extend({ // If the definition is already JSON then it doesn't need to be parsed. const json = JSON.parse(definition); this.set('_newDefinitionJSON', json); - this.setIDByPayload(json); + + // You can't set the ID of a record that already exists + if (this.get('isNew')) { + this.setIdByPayload(json); + } + promise = RSVP.resolve(definition); } catch (err) { // If the definition is invalid JSON, assume it is HCL. If it is invalid @@ -221,14 +231,14 @@ export default Model.extend({ .parse(this.get('_newDefinition')) .then(response => { this.set('_newDefinitionJSON', response); - this.setIDByPayload(response); + this.setIdByPayload(response); }); } return promise; }, - setIDByPayload(payload) { + setIdByPayload(payload) { const namespace = payload.Namespace || 'default'; const id = payload.Name; @@ -241,6 +251,10 @@ export default Model.extend({ } }, + resetId() { + this.set('id', JSON.stringify([this.get('plainId'), this.get('namespace.name') || 'default'])); + }, + statusClass: computed('status', function() { const classMap = { pending: 'is-pending', From 772731ccd47763582d152b808ad335d7466b3ac0 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 21 Aug 2018 16:44:59 -0700 Subject: [PATCH 11/18] Fix bug where scrolling wasn't using the document Instead it was using the page-layout is-right div --- ui/app/styles/components/page-layout.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/app/styles/components/page-layout.scss b/ui/app/styles/components/page-layout.scss index 294f041ff..26bcf732e 100644 --- a/ui/app/styles/components/page-layout.scss +++ b/ui/app/styles/components/page-layout.scss @@ -1,5 +1,5 @@ .page-layout { - height: 100%; + min-height: 100%; display: flex; flex-direction: column; From 3ca2237b29bac4612e0858887c17f1ca0ebc4394 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 21 Aug 2018 16:45:53 -0700 Subject: [PATCH 12/18] Support cancellation of editing in the job-editor --- ui/app/templates/components/job-editor.hbs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ui/app/templates/components/job-editor.hbs b/ui/app/templates/components/job-editor.hbs index 250934bc7..c0c2df862 100644 --- a/ui/app/templates/components/job-editor.hbs +++ b/ui/app/templates/components/job-editor.hbs @@ -34,6 +34,9 @@
        Job Definition + {{#if cancelable}} + + {{/if}}
        {{ivy-codemirror From 1f40ab5da51326551eaf299472877686548ddc9a Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 21 Aug 2018 16:46:24 -0700 Subject: [PATCH 13/18] Introduce job editing to the job definition page --- ui/app/controllers/jobs/job/definition.js | 18 ++++++++++++++++++ ui/app/routes/jobs/job/definition.js | 9 +++++++++ ui/app/templates/jobs/job/definition.hbs | 22 +++++++++++++++++----- ui/app/templates/jobs/run.hbs | 5 ++++- 4 files changed, 48 insertions(+), 6 deletions(-) diff --git a/ui/app/controllers/jobs/job/definition.js b/ui/app/controllers/jobs/job/definition.js index 7efede0aa..ff03ba643 100644 --- a/ui/app/controllers/jobs/job/definition.js +++ b/ui/app/controllers/jobs/job/definition.js @@ -4,4 +4,22 @@ import { alias } from '@ember/object/computed'; export default Controller.extend(WithNamespaceResetting, { job: alias('model.job'), + definition: alias('model.definition'), + + isEditing: false, + + edit() { + this.get('job').set('_newDefinition', JSON.stringify(this.get('definition'), null, 2)); + this.set('isEditing', true); + }, + + onCancel() { + this.set('isEditing', false); + }, + + onSubmit(id, namespace) { + this.transitionToRoute('jobs.job', id, { + queryParams: { jobNamespace: namespace }, + }); + }, }); diff --git a/ui/app/routes/jobs/job/definition.js b/ui/app/routes/jobs/job/definition.js index 8730f83b8..f6294ebf5 100644 --- a/ui/app/routes/jobs/job/definition.js +++ b/ui/app/routes/jobs/job/definition.js @@ -8,4 +8,13 @@ export default Route.extend({ definition, })); }, + + resetController(controller, isExiting) { + if (isExiting) { + const job = controller.get('job'); + job.rollbackAttributes(); + job.resetId(); + controller.set('isEditing', false); + } + }, }); diff --git a/ui/app/templates/jobs/job/definition.hbs b/ui/app/templates/jobs/job/definition.hbs index 44b3515aa..5d3813fb2 100644 --- a/ui/app/templates/jobs/job/definition.hbs +++ b/ui/app/templates/jobs/job/definition.hbs @@ -1,9 +1,21 @@ {{partial "jobs/job/subnav"}}
        - {{#link-to "jobs.job.edit" job class="button is-primary"}}Edit{{/link-to}} -
        -
        - {{json-viewer data-test-definition-view json=model.definition}} + {{#unless isEditing}} +
        +
        + Job Definition + +
        +
        + {{json-viewer data-test-definition-view json=definition}} +
        -
        + {{else}} + {{job-editor + job=job + cancelable=true + context="edit" + onCancel=(action onCancel) + onSubmit=(action onSubmit)}} + {{/unless}}
        diff --git a/ui/app/templates/jobs/run.hbs b/ui/app/templates/jobs/run.hbs index 52400b413..2fa8f850f 100644 --- a/ui/app/templates/jobs/run.hbs +++ b/ui/app/templates/jobs/run.hbs @@ -1,3 +1,6 @@
        - {{job-editor job=model onSubmit=(action onSubmit)}} + {{job-editor + job=model + context="new" + onSubmit=(action onSubmit)}}
        From e60b16a7e0f0e770451a5dea230c384338561486 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 22 Aug 2018 17:34:25 -0700 Subject: [PATCH 14/18] Since registerHelper doesn't work in integration tests a new way is needed This exports a function that returns the pertinent function when given a container. This way it works in registerHelper and in integration tests beforeEach hooks --- ui/tests/helpers/codemirror.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/ui/tests/helpers/codemirror.js b/ui/tests/helpers/codemirror.js index 87db0c814..247eb7699 100644 --- a/ui/tests/helpers/codemirror.js +++ b/ui/tests/helpers/codemirror.js @@ -4,9 +4,9 @@ const invariant = (truthy, error) => { if (!truthy) throw new Error(error); }; -export default function registerCodeMirrorHelpers() { - registerHelper('getCodeMirrorInstance', function(app, selector) { - const cmService = app.__container__.lookup('service:code-mirror'); +export function getCodeMirrorInstance(container) { + return function(selector) { + const cmService = container.lookup('service:code-mirror'); const element = document.querySelector(selector); invariant(element, `Selector ${selector} matched no elements`); @@ -15,5 +15,12 @@ export default function registerCodeMirrorHelpers() { invariant(cm, `No registered CodeMirror instance for ${selector}`); return cm; + }; +} + +export default function registerCodeMirrorHelpers() { + registerHelper('getCodeMirrorInstance', function(app, selector) { + const helper = getCodeMirrorInstance(app.__container__); + return helper(selector); }); } From b5b4a979f94495e3a2cb1e88f48d41c65b27e2be Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 22 Aug 2018 17:36:04 -0700 Subject: [PATCH 15/18] Test coverage for the job-editor component Most of this was ported over from the existing job run acceptance tests --- ui/app/components/job-editor.js | 2 + ui/app/templates/components/job-editor.hbs | 4 +- ui/mirage/config.js | 8 + ui/tests/integration/job-editor-test.js | 492 +++++++++++++++++++++ ui/tests/pages/components/job-editor.js | 49 ++ ui/tests/pages/jobs/run.js | 44 +- 6 files changed, 556 insertions(+), 43 deletions(-) create mode 100644 ui/tests/integration/job-editor-test.js create mode 100644 ui/tests/pages/components/job-editor.js diff --git a/ui/app/components/job-editor.js b/ui/app/components/job-editor.js index 7cb6ff201..d72c97a8a 100644 --- a/ui/app/components/job-editor.js +++ b/ui/app/components/job-editor.js @@ -10,6 +10,8 @@ export default Component.extend({ store: service(), config: service(), + 'data-test-job-editor': true, + job: null, onSubmit() {}, context: computed({ diff --git a/ui/app/templates/components/job-editor.hbs b/ui/app/templates/components/job-editor.hbs index c0c2df862..8f85ebc58 100644 --- a/ui/app/templates/components/job-editor.hbs +++ b/ui/app/templates/components/job-editor.hbs @@ -18,7 +18,7 @@ {{/if}} {{#if (eq stage "editor")}} - {{#if showEditorMessage}} + {{#if (and showEditorMessage (eq context "new"))}}
        @@ -35,7 +35,7 @@
        Job Definition {{#if cancelable}} - + {{/if}}
        diff --git a/ui/mirage/config.js b/ui/mirage/config.js index d3b2a9778..d573ed6de 100644 --- a/ui/mirage/config.js +++ b/ui/mirage/config.js @@ -116,6 +116,14 @@ export default function() { }) ); + this.post('/job/:id', function(schema, req) { + const body = JSON.parse(req.requestBody); + + if (!body.Job) return new Response(400, {}, 'Job is a required field on the request payload'); + + return okEmpty(); + }); + this.get( '/job/:id/summary', withBlockingSupport(function({ jobSummaries }, { params }) { diff --git a/ui/tests/integration/job-editor-test.js b/ui/tests/integration/job-editor-test.js new file mode 100644 index 000000000..9513a650e --- /dev/null +++ b/ui/tests/integration/job-editor-test.js @@ -0,0 +1,492 @@ +import { getOwner } from '@ember/application'; +import { assign } from '@ember/polyfills'; +import { run } from '@ember/runloop'; +import { test, moduleForComponent } from 'ember-qunit'; +import wait from 'ember-test-helpers/wait'; +import hbs from 'htmlbars-inline-precompile'; +import { create } from 'ember-cli-page-object'; +import sinon from 'sinon'; +import { startMirage } from 'nomad-ui/initializers/ember-cli-mirage'; +import { getCodeMirrorInstance } from 'nomad-ui/tests/helpers/codemirror'; +import jobEditor from 'nomad-ui/tests/pages/components/job-editor'; +import { initialize as fragmentSerializerInitializer } from 'nomad-ui/initializers/fragment-serializer'; + +const Editor = create(jobEditor()); + +moduleForComponent('job-editor', 'Integration | Component | job-editor', { + integration: true, + beforeEach() { + window.localStorage.clear(); + + fragmentSerializerInitializer(getOwner(this)); + + // Normally getCodeMirrorInstance is a registered test helper, + // but those registered test helpers only work in acceptance tests. + window._getCodeMirrorInstance = window.getCodeMirrorInstance; + window.getCodeMirrorInstance = getCodeMirrorInstance(getOwner(this)); + + this.store = getOwner(this).lookup('service:store'); + this.server = startMirage(); + + // Required for placing allocations (a result of creating jobs) + this.server.create('node'); + + Editor.setContext(this); + }, + afterEach() { + this.server.shutdown(); + Editor.removeContext(); + window.getCodeMirrorInstance = window._getCodeMirrorInstance; + delete window._getCodeMirrorInstance; + }, +}); + +const newJobName = 'new-job'; +const newJobTaskGroupName = 'redis'; +const jsonJob = overrides => { + return JSON.stringify( + assign( + {}, + { + Name: newJobName, + Namespace: 'default', + Datacenters: ['dc1'], + Priority: 50, + TaskGroups: [ + { + Name: newJobTaskGroupName, + Tasks: [ + { + Name: 'redis', + Driver: 'docker', + }, + ], + }, + ], + }, + overrides + ), + null, + 2 + ); +}; + +const hclJob = () => ` +job "${newJobName}" { + namespace = "default" + datacenters = ["dc1"] + + task "${newJobTaskGroupName}" { + driver = "docker" + } +} +`; + +const commonTemplate = hbs` + {{job-editor + job=job + context=context + onSubmit=onSubmit}} +`; + +const cancelableTemplate = hbs` + {{job-editor + job=job + context=context + cancelable=true + onSubmit=onSubmit + onCancel=onCancel}} +`; + +const renderNewJob = (component, job) => () => { + component.setProperties({ job, onSubmit: sinon.spy(), context: 'new' }); + component.render(commonTemplate); + return wait(); +}; + +const renderEditJob = (component, job) => () => { + component.setProperties({ job, onSubmit: sinon.spy(), onCancel: sinon.spy(), context: 'edit' }); + component.render(cancelableTemplate); +}; + +const planJob = spec => () => { + Editor.editor.fillIn(spec); + return wait().then(() => { + Editor.plan(); + return wait(); + }); +}; + +test('the default state is an editor with an explanation popup', function(assert) { + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderNewJob(this, job)) + .then(() => { + assert.ok(Editor.editorHelp.isPresent, 'Editor explanation popup is present'); + assert.ok(Editor.editor.isPresent, 'Editor is present'); + }); +}); + +test('the explanation popup can be dismissed', function(assert) { + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderNewJob(this, job)) + .then(() => { + Editor.editorHelp.dismiss(); + return wait(); + }) + .then(() => { + assert.notOk(Editor.editorHelp.isPresent, 'Editor explanation popup is gone'); + assert.equal( + window.localStorage.nomadMessageJobEditor, + 'false', + 'Dismissal is persisted in localStorage' + ); + }); +}); + +test('the explanation popup is not shown once the dismissal state is set in localStorage', function(assert) { + window.localStorage.nomadMessageJobEditor = 'false'; + + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderNewJob(this, job)) + .then(() => { + assert.notOk(Editor.editorHelp.isPresent, 'Editor explanation popup is gone'); + }); +}); + +test('submitting a json job skips the parse endpoint', function(assert) { + const spec = jsonJob(); + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderNewJob(this, job)) + .then(planJob(spec)) + .then(() => { + const requests = this.server.pretender.handledRequests.mapBy('url'); + assert.notOk(requests.includes('/v1/jobs/parse'), 'JSON job spec is not parsed'); + assert.ok(requests.includes(`/v1/job/${newJobName}/plan`), 'JSON job spec is still planned'); + }); +}); + +test('submitting an hcl job requires the parse endpoint', function(assert) { + const spec = hclJob(); + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderNewJob(this, job)) + .then(planJob(spec)) + .then(() => { + const requests = this.server.pretender.handledRequests.mapBy('url'); + assert.ok(requests.includes('/v1/jobs/parse'), 'HCL job spec is parsed first'); + assert.ok(requests.includes(`/v1/job/${newJobName}/plan`), 'HCL job spec is planned'); + assert.ok( + requests.indexOf('/v1/jobs/parse') < requests.indexOf(`/v1/job/${newJobName}/plan`), + 'Parse comes before Plan' + ); + }); +}); + +test('when a job is successfully parsed and planned, the plan is shown to the user', function(assert) { + const spec = hclJob(); + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderNewJob(this, job)) + .then(planJob(spec)) + .then(() => { + assert.ok(Editor.planOutput, 'The plan is outputted'); + assert.notOk(Editor.editor.isPresent, 'The editor is replaced with the plan output'); + assert.ok(Editor.planHelp.isPresent, 'The plan explanation popup is shown'); + }); +}); + +test('from the plan screen, the cancel button goes back to the editor with the job still in tact', function(assert) { + const spec = hclJob(); + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderNewJob(this, job)) + .then(planJob(spec)) + .then(() => { + Editor.cancel(); + return wait(); + }) + .then(() => { + assert.ok(Editor.editor.isPresent, 'The editor is shown again'); + assert.equal( + Editor.editor.contents, + spec, + 'The spec that was planned is still in the editor' + ); + }); +}); + +test('when parse fails, the parse error message is shown', function(assert) { + const spec = hclJob(); + const errorMessage = 'Parse Failed!! :o'; + + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + this.server.pretender.post('/v1/jobs/parse', () => [400, {}, errorMessage]); + + return wait() + .then(renderNewJob(this, job)) + .then(planJob(spec)) + .then(() => { + assert.notOk(Editor.planError.isPresent, 'Plan error is not shown'); + assert.notOk(Editor.runError.isPresent, 'Run error is not shown'); + + assert.ok(Editor.parseError.isPresent, 'Parse error is shown'); + assert.equal( + Editor.parseError.message, + errorMessage, + 'The error message from the server is shown in the error in the UI' + ); + }); +}); + +test('when plan fails, the plan error message is shown', function(assert) { + const spec = hclJob(); + const errorMessage = 'Plan Failed!! :o'; + + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + this.server.pretender.post(`/v1/job/${newJobName}/plan`, () => [400, {}, errorMessage]); + + return wait() + .then(renderNewJob(this, job)) + .then(planJob(spec)) + .then(() => { + assert.notOk(Editor.parseError.isPresent, 'Parse error is not shown'); + assert.notOk(Editor.runError.isPresent, 'Run error is not shown'); + + assert.ok(Editor.planError.isPresent, 'Plan error is shown'); + assert.equal( + Editor.planError.message, + errorMessage, + 'The error message from the server is shown in the error in the UI' + ); + }); +}); + +test('when run fails, the run error message is shown', function(assert) { + const spec = hclJob(); + const errorMessage = 'Run Failed!! :o'; + + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + this.server.pretender.post('/v1/jobs', () => [400, {}, errorMessage]); + + return wait() + .then(renderNewJob(this, job)) + .then(planJob(spec)) + .then(() => { + Editor.run(); + return wait(); + }) + .then(() => { + assert.notOk(Editor.planError.isPresent, 'Plan error is not shown'); + assert.notOk(Editor.parseError.isPresent, 'Parse error is not shown'); + + assert.ok(Editor.runError.isPresent, 'Run error is shown'); + assert.equal( + Editor.runError.message, + errorMessage, + 'The error message from the server is shown in the error in the UI' + ); + }); +}); + +test('when the scheduler dry-run has warnings, the warnings are shown to the user', function(assert) { + const spec = jsonJob({ Unschedulable: true }); + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderNewJob(this, job)) + .then(planJob(spec)) + .then(() => { + assert.ok( + Editor.dryRunMessage.errored, + 'The scheduler dry-run message is in the warning state' + ); + assert.notOk( + Editor.dryRunMessage.succeeded, + 'The success message is not shown in addition to the warning message' + ); + assert.ok( + Editor.dryRunMessage.body.includes(newJobTaskGroupName), + 'The scheduler dry-run message includes the warning from send back by the API' + ); + }); +}); + +test('when the scheduler dry-run has no warnings, a success message is shown to the user', function(assert) { + const spec = hclJob(); + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderNewJob(this, job)) + .then(planJob(spec)) + .then(() => { + assert.ok( + Editor.dryRunMessage.succeeded, + 'The scheduler dry-run message is in the success state' + ); + assert.notOk( + Editor.dryRunMessage.errored, + 'The warning message is not shown in addition to the success message' + ); + }); +}); + +test('when a job is submitted in the edit context, a POST request is made to the update job endpoint', function(assert) { + const spec = hclJob(); + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderEditJob(this, job)) + .then(planJob(spec)) + .then(() => { + Editor.run(); + }) + .then(() => { + const requests = this.server.pretender.handledRequests + .filterBy('method', 'POST') + .mapBy('url'); + assert.ok(requests.includes(`/v1/job/${newJobName}`), 'A request was made to job update'); + assert.notOk(requests.includes('/v1/jobs'), 'A request was not made to job create'); + }); +}); + +test('when a job is submitted in the new context, a POST request is made to the create job endpoint', function(assert) { + const spec = hclJob(); + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderNewJob(this, job)) + .then(planJob(spec)) + .then(() => { + Editor.run(); + }) + .then(() => { + const requests = this.server.pretender.handledRequests + .filterBy('method', 'POST') + .mapBy('url'); + assert.ok(requests.includes('/v1/jobs'), 'A request was made to job create'); + assert.notOk( + requests.includes(`/v1/job/${newJobName}`), + 'A request was not made to job update' + ); + }); +}); + +test('when a job is successfully submitted, the onSubmit hook is called', function(assert) { + const spec = hclJob(); + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderNewJob(this, job)) + .then(planJob(spec)) + .then(() => { + Editor.run(); + return wait(); + }) + .then(() => { + assert.ok( + this.get('onSubmit').calledWith(newJobName, 'default'), + 'The onSubmit hook was called with the correct arguments' + ); + }); +}); + +test('when the job-editor cancelable flag is false, there is no cancel button in the header', function(assert) { + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderNewJob(this, job)) + .then(() => { + assert.notOk(Editor.cancelEditingIsAvailable, 'No way to cancel editing'); + }); +}); + +test('when the job-editor cancelable flag is true, there is a cancel button in the header', function(assert) { + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderEditJob(this, job)) + .then(() => { + assert.ok(Editor.cancelEditingIsAvailable, 'Cancel editing button exists'); + }); +}); + +test('when the job-editor cancel button is clicked, the onCancel hook is called', function(assert) { + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderEditJob(this, job)) + .then(() => { + Editor.cancelEditing(); + }) + .then(() => { + assert.ok(this.get('onCancel').calledOnce, 'The onCancel hook was called'); + }); +}); diff --git a/ui/tests/pages/components/job-editor.js b/ui/tests/pages/components/job-editor.js new file mode 100644 index 000000000..c9de367c5 --- /dev/null +++ b/ui/tests/pages/components/job-editor.js @@ -0,0 +1,49 @@ +import { clickable, hasClass, isPresent, text } from 'ember-cli-page-object'; +import { codeFillable, code } from 'nomad-ui/tests/pages/helpers/codemirror'; + +import error from 'nomad-ui/tests/pages/components/error'; + +export default () => ({ + scope: '[data-test-job-editor]', + + planError: error('data-test-plan-error'), + parseError: error('data-test-parse-error'), + runError: error('data-test-run-error'), + + plan: clickable('[data-test-plan]'), + cancel: clickable('[data-test-cancel]'), + run: clickable('[data-test-run]'), + + cancelEditing: clickable('[data-test-cancel-editing]'), + cancelEditingIsAvailable: isPresent('[data-test-cancel-editing]'), + + planOutput: text('[data-test-plan-output]'), + + planHelp: { + isPresent: isPresent('[data-test-plan-help-title]'), + title: text('[data-test-plan-help-title]'), + message: text('[data-test-plan-help-message]'), + dismiss: clickable('[data-test-plan-help-dismiss]'), + }, + + editorHelp: { + isPresent: isPresent('[data-test-editor-help-title]'), + title: text('[data-test-editor-help-title]'), + message: text('[data-test-editor-help-message]'), + dismiss: clickable('[data-test-editor-help-dismiss]'), + }, + + editor: { + isPresent: isPresent('[data-test-editor]'), + contents: code('[data-test-editor]'), + fillIn: codeFillable('[data-test-editor]'), + }, + + dryRunMessage: { + scope: '[data-test-dry-run-message]', + title: text('[data-test-dry-run-title]'), + body: text('[data-test-dry-run-body]'), + errored: hasClass('is-warning'), + succeeded: hasClass('is-primary'), + }, +}); diff --git a/ui/tests/pages/jobs/run.js b/ui/tests/pages/jobs/run.js index 08c1bcb7c..03c24ef14 100644 --- a/ui/tests/pages/jobs/run.js +++ b/ui/tests/pages/jobs/run.js @@ -1,46 +1,8 @@ -import { clickable, create, hasClass, isPresent, text, visitable } from 'ember-cli-page-object'; -import { codeFillable, code } from 'nomad-ui/tests/pages/helpers/codemirror'; +import { create, visitable } from 'ember-cli-page-object'; -import error from 'nomad-ui/tests/pages/components/error'; +import jobEditor from 'nomad-ui/tests/pages/components/job-editor'; export default create({ visit: visitable('/jobs/run'), - - planError: error('data-test-plan-error'), - parseError: error('data-test-parse-error'), - runError: error('data-test-run-error'), - - plan: clickable('[data-test-plan]'), - cancel: clickable('[data-test-cancel]'), - run: clickable('[data-test-run]'), - - planOutput: text('[data-test-plan-output]'), - - planHelp: { - isPresent: isPresent('[data-test-plan-help-title]'), - title: text('[data-test-plan-help-title]'), - message: text('[data-test-plan-help-message]'), - dismiss: clickable('[data-test-plan-help-dismiss]'), - }, - - editorHelp: { - isPresent: isPresent('[data-test-editor-help-title]'), - title: text('[data-test-editor-help-title]'), - message: text('[data-test-editor-help-message]'), - dismiss: clickable('[data-test-editor-help-dismiss]'), - }, - - editor: { - isPresent: isPresent('[data-test-editor]'), - contents: code('[data-test-editor]'), - fillIn: codeFillable('[data-test-editor]'), - }, - - dryRunMessage: { - scope: '[data-test-dry-run-message]', - title: text('[data-test-dry-run-title]'), - body: text('[data-test-dry-run-body]'), - errored: hasClass('is-warning'), - succeeded: hasClass('is-primary'), - }, + editor: jobEditor(), }); From 4484c28e072b0dbc57c200d4dc9ca04824010a61 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 23 Aug 2018 09:00:47 -0700 Subject: [PATCH 16/18] Rewrite the job run acceptance tests to be about routing --- ui/tests/acceptance/job-run-test.js | 283 +--------------------------- 1 file changed, 7 insertions(+), 276 deletions(-) diff --git a/ui/tests/acceptance/job-run-test.js b/ui/tests/acceptance/job-run-test.js index 5b56eafcb..8d9f99047 100644 --- a/ui/tests/acceptance/job-run-test.js +++ b/ui/tests/acceptance/job-run-test.js @@ -35,17 +35,6 @@ const jsonJob = overrides => { ); }; -const hclJob = () => ` -job "${newJobName}" { - namespace = "default" - datacenters = ["dc1"] - - task "${newJobTaskGroupName}" { - driver = "docker" - } -} -`; - moduleForAcceptance('Acceptance | job run', { beforeEach() { // Required for placing allocations (a result of creating jobs) @@ -61,234 +50,25 @@ test('visiting /jobs/run', function(assert) { }); }); -test('the page has an editor and an explanation popup', function(assert) { - JobRun.visit(); - - andThen(() => { - assert.ok(JobRun.editorHelp.isPresent, 'Editor explanation popup is present'); - assert.ok(JobRun.editor.isPresent, 'Editor is present'); - }); -}); - -test('the explanation popup can be dismissed', function(assert) { - JobRun.visit(); - - andThen(() => { - JobRun.editorHelp.dismiss(); - }); - - andThen(() => { - assert.notOk(JobRun.editorHelp.isPresent, 'Editor explanation popup is gone'); - assert.equal( - window.localStorage.nomadMessageJobEditor, - 'false', - 'Dismissal is persisted in localStorage' - ); - }); -}); - -test('the explanation popup is not shown once the dismissal state is set in localStorage', function(assert) { - window.localStorage.nomadMessageJobEditor = 'false'; - - JobRun.visit(); - - andThen(() => { - assert.notOk(JobRun.editorHelp.isPresent, 'Editor explanation popup is gone'); - }); -}); - -test('submitting a json job skips the parse endpoint', function(assert) { +test('when submitting a job, the site redirects to the new job overview page', function(assert) { const spec = jsonJob(); JobRun.visit(); andThen(() => { - JobRun.editor.fillIn(spec); - JobRun.plan(); + JobRun.editor.editor.fillIn(spec); + JobRun.editor.plan(); }); andThen(() => { - const requests = server.pretender.handledRequests.mapBy('url'); - assert.notOk(requests.includes('/v1/jobs/parse'), 'JSON job spec is not parsed'); - assert.ok(requests.includes(`/v1/job/${newJobName}/plan`), 'JSON job spec is still planned'); + JobRun.editor.run(); }); -}); - -test('submitting an hcl job requires the parse endpoint', function(assert) { - const spec = hclJob(); - - JobRun.visit(); - - andThen(() => { - JobRun.editor.fillIn(spec); - JobRun.plan(); - }); - - andThen(() => { - const requests = server.pretender.handledRequests.mapBy('url'); - assert.ok(requests.includes('/v1/jobs/parse'), 'HCL job spec is parsed first'); - assert.ok(requests.includes(`/v1/job/${newJobName}/plan`), 'HCL job spec is planned'); - assert.ok( - requests.indexOf('/v1/jobs/parse') < requests.indexOf(`/v1/job/${newJobName}/plan`), - 'Parse comes before Plan' - ); - }); -}); - -test('when a job is successfully parsed and planned, the plan is shown to the user', function(assert) { - const spec = hclJob(); - - JobRun.visit(); - - andThen(() => { - JobRun.editor.fillIn(spec); - JobRun.plan(); - }); - - andThen(() => { - assert.ok(JobRun.planOutput, 'The plan is outputted'); - assert.notOk(JobRun.editor.isPresent, 'The editor is replaced with the plan output'); - assert.ok(JobRun.planHelp.isPresent, 'The plan explanation popup is shown'); - }); -}); - -test('from the plan screen, the cancel button goes back to the editor with the job still in tact', function(assert) { - const spec = hclJob(); - - JobRun.visit(); - - andThen(() => { - JobRun.editor.fillIn(spec); - JobRun.plan(); - }); - - andThen(() => { - JobRun.cancel(); - }); - - andThen(() => { - assert.ok(JobRun.editor.isPresent, 'The editor is shown again'); - assert.notOk(JobRun.planOutpu, 'The plan is gone'); - assert.equal(JobRun.editor.contents, spec, 'The spec that was planned is still in the editor'); - }); -}); - -test('from the plan screen, the submit button submits the job and redirects to the job overview page', function(assert) { - const spec = hclJob(); - - JobRun.visit(); - - andThen(() => { - JobRun.editor.fillIn(spec); - JobRun.plan(); - }); - - andThen(() => { - JobRun.run(); - }); - andThen(() => { assert.equal( currentURL(), `/jobs/${newJobName}`, `Redirected to the job overview page for ${newJobName}` ); - - const runRequest = server.pretender.handledRequests.find( - req => req.method === 'POST' && req.url === '/v1/jobs' - ); - const planRequest = server.pretender.handledRequests.find( - req => req.method === 'POST' && req.url === '/v1/jobs/parse' - ); - - assert.ok(runRequest, 'A POST request was made to run the new job'); - assert.deepEqual( - JSON.parse(runRequest.requestBody).Job, - JSON.parse(planRequest.responseText), - 'The Job payload parameter is equivalent to the result of the parse request' - ); - }); -}); - -test('when parse fails, the parse error message is shown', function(assert) { - const spec = hclJob(); - - const errorMessage = 'Parse Failed!! :o'; - server.pretender.post('/v1/jobs/parse', () => [400, {}, errorMessage]); - - JobRun.visit(); - - andThen(() => { - JobRun.editor.fillIn(spec); - JobRun.plan(); - }); - - andThen(() => { - assert.notOk(JobRun.planError.isPresent, 'Plan error is not shown'); - assert.notOk(JobRun.runError.isPresent, 'Run error is not shown'); - - assert.ok(JobRun.parseError.isPresent, 'Parse error is shown'); - assert.equal( - JobRun.parseError.message, - errorMessage, - 'The error message from the server is shown in the error in the UI' - ); - }); -}); - -test('when plan fails, the plan error message is shown', function(assert) { - const spec = hclJob(); - - const errorMessage = 'Parse Failed!! :o'; - server.pretender.post(`/v1/job/${newJobName}/plan`, () => [400, {}, errorMessage]); - - JobRun.visit(); - - andThen(() => { - JobRun.editor.fillIn(spec); - JobRun.plan(); - }); - - andThen(() => { - assert.notOk(JobRun.parseError.isPresent, 'Parse error is not shown'); - assert.notOk(JobRun.runError.isPresent, 'Run error is not shown'); - - assert.ok(JobRun.planError.isPresent, 'Plan error is shown'); - assert.equal( - JobRun.planError.message, - errorMessage, - 'The error message from the server is shown in the error in the UI' - ); - }); -}); - -test('when run fails, the run error message is shown', function(assert) { - const spec = hclJob(); - - const errorMessage = 'Parse Failed!! :o'; - server.pretender.post('/v1/jobs', () => [400, {}, errorMessage]); - - JobRun.visit(); - - andThen(() => { - JobRun.editor.fillIn(spec); - JobRun.plan(); - }); - - andThen(() => { - JobRun.run(); - }); - - andThen(() => { - assert.notOk(JobRun.planError.isPresent, 'Plan error is not shown'); - assert.notOk(JobRun.parseError.isPresent, 'Parse error is not shown'); - - assert.ok(JobRun.runError.isPresent, 'Run error is shown'); - assert.equal( - JobRun.runError.message, - errorMessage, - 'The error message from the server is shown in the error in the UI' - ); }); }); @@ -301,12 +81,12 @@ test('when submitting a job to a different namespace, the redirect to the job ov JobRun.visit(); andThen(() => { - JobRun.editor.fillIn(spec); - JobRun.plan(); + JobRun.editor.editor.fillIn(spec); + JobRun.editor.plan(); }); andThen(() => { - JobRun.run(); + JobRun.editor.run(); }); andThen(() => { assert.equal( @@ -316,52 +96,3 @@ test('when submitting a job to a different namespace, the redirect to the job ov ); }); }); - -test('when the scheduler dry-run has warnings, the warnings are shown to the user', function(assert) { - // Unschedulable is a hint to Mirage to respond with warnings from the plan endpoint - const spec = jsonJob({ Unschedulable: true }); - - JobRun.visit(); - - andThen(() => { - JobRun.editor.fillIn(spec); - JobRun.plan(); - }); - - andThen(() => { - assert.ok( - JobRun.dryRunMessage.errored, - 'The scheduler dry-run message is in the warning state' - ); - assert.notOk( - JobRun.dryRunMessage.succeeded, - 'The success message is not shown in addition to the warning message' - ); - assert.ok( - JobRun.dryRunMessage.body.includes(newJobTaskGroupName), - 'The scheduler dry-run message includes the warning from send back by the API' - ); - }); -}); - -test('when the scheduler dry-run has no warnings, a success message is shown to the user', function(assert) { - const spec = hclJob(); - - JobRun.visit(); - - andThen(() => { - JobRun.editor.fillIn(spec); - JobRun.plan(); - }); - - andThen(() => { - assert.ok( - JobRun.dryRunMessage.succeeded, - 'The scheduler dry-run message is in the success state' - ); - assert.notOk( - JobRun.dryRunMessage.errored, - 'The warning message is not shown in addition to the success message' - ); - }); -}); From 0fb7fea8250c17eec1d706e2f2b9c0641d6831b8 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 23 Aug 2018 10:26:20 -0700 Subject: [PATCH 17/18] Acceptance tests for the edit behaviors on the job definition page --- ui/app/templates/jobs/job/definition.hbs | 2 +- ui/tests/acceptance/job-definition-test.js | 53 ++++++++++++++++++++++ ui/tests/pages/components/job-editor.js | 2 + ui/tests/pages/jobs/job/definition.js | 7 ++- 4 files changed, 62 insertions(+), 2 deletions(-) diff --git a/ui/app/templates/jobs/job/definition.hbs b/ui/app/templates/jobs/job/definition.hbs index 5d3813fb2..4f30522b6 100644 --- a/ui/app/templates/jobs/job/definition.hbs +++ b/ui/app/templates/jobs/job/definition.hbs @@ -4,7 +4,7 @@
        Job Definition - +
        {{json-viewer data-test-definition-view json=definition}} diff --git a/ui/tests/acceptance/job-definition-test.js b/ui/tests/acceptance/job-definition-test.js index 65736df28..78f65541b 100644 --- a/ui/tests/acceptance/job-definition-test.js +++ b/ui/tests/acceptance/job-definition-test.js @@ -29,3 +29,56 @@ test('the job definition page requests the job to display in an unmutated form', .filter(url => url === jobURL); assert.ok(jobRequests.length === 2, 'Two requests for the job were made'); }); + +test('the job definition can be edited', function(assert) { + assert.notOk(Definition.editor.isPresent, 'Editor is not shown on load'); + + Definition.edit(); + + andThen(() => { + assert.ok(Definition.editor.isPresent, 'Editor is shown after clicking edit'); + assert.notOk(Definition.jsonViewer, 'Editor replaces the JSON viewer'); + }); +}); + +test('when in editing mode, the action can be canceled, showing the read-only definition again', function(assert) { + Definition.edit(); + + andThen(() => { + Definition.editor.cancelEditing(); + }); + + andThen(() => { + assert.ok(Definition.jsonViewer, 'The JSON Viewer is back'); + assert.notOk(Definition.editor.isPresent, 'The editor is gone'); + }); +}); + +test('when in editing mode, the editor is prepopulated with the job definition', function(assert) { + const requests = server.pretender.handledRequests; + const jobDefinition = requests.findBy('url', `/v1/job/${job.id}`).responseText; + const formattedJobDefinition = JSON.stringify(JSON.parse(jobDefinition), null, 2); + + Definition.edit(); + + andThen(() => { + assert.equal( + Definition.editor.editor.contents, + formattedJobDefinition, + 'The editor already has the job definition in it' + ); + }); +}); + +test('when changes are submitted, the site redirects to the job overview page', function(assert) { + Definition.edit(); + + andThen(() => { + Definition.editor.plan(); + Definition.editor.run(); + }); + + andThen(() => { + assert.equal(currentURL(), `/jobs/${job.id}`, 'Now on the job overview page'); + }); +}); diff --git a/ui/tests/pages/components/job-editor.js b/ui/tests/pages/components/job-editor.js index c9de367c5..387cd95a1 100644 --- a/ui/tests/pages/components/job-editor.js +++ b/ui/tests/pages/components/job-editor.js @@ -6,6 +6,8 @@ import error from 'nomad-ui/tests/pages/components/error'; export default () => ({ scope: '[data-test-job-editor]', + isPresent: isPresent(), + planError: error('data-test-plan-error'), parseError: error('data-test-parse-error'), runError: error('data-test-run-error'), diff --git a/ui/tests/pages/jobs/job/definition.js b/ui/tests/pages/jobs/job/definition.js index 789d0cabc..b015019b7 100644 --- a/ui/tests/pages/jobs/job/definition.js +++ b/ui/tests/pages/jobs/job/definition.js @@ -1,7 +1,12 @@ -import { create, isPresent, visitable } from 'ember-cli-page-object'; +import { create, isPresent, visitable, clickable } from 'ember-cli-page-object'; + +import jobEditor from 'nomad-ui/tests/pages/components/job-editor'; export default create({ visit: visitable('/jobs/:id/definition'), jsonViewer: isPresent('[data-test-definition-view]'), + editor: jobEditor(), + + edit: clickable('[data-test-edit-job]'), }); From 715e2ac82ac1deb83428667d623168d62f98c2e2 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 23 Aug 2018 15:40:42 -0700 Subject: [PATCH 18/18] Simplify the data control flow around job.plan() --- ui/app/adapters/job.js | 10 +++++++--- ui/app/components/job-editor.js | 3 +-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/ui/app/adapters/job.js b/ui/app/adapters/job.js index b9dc7aa09..efc18efea 100644 --- a/ui/app/adapters/job.js +++ b/ui/app/adapters/job.js @@ -77,15 +77,19 @@ export default Watchable.extend({ }, plan(job) { - const url = addToPath(this.urlForFindRecord(job.get('id'), 'job'), '/plan'); + const jobId = job.get('id'); + const store = this.get('store'); + const url = addToPath(this.urlForFindRecord(jobId, 'job'), '/plan'); + return this.ajax(url, 'POST', { data: { Job: job.get('_newDefinitionJSON'), Diff: true, }, }).then(json => { - json.ID = job.get('id'); - this.get('store').pushPayload('job-plan', { jobPlans: [json] }); + json.ID = jobId; + store.pushPayload('job-plan', { jobPlans: [json] }); + return store.peekRecord('job-plan', jobId); }); }, diff --git a/ui/app/components/job-editor.js b/ui/app/components/job-editor.js index d72c97a8a..2620954dc 100644 --- a/ui/app/components/job-editor.js +++ b/ui/app/components/job-editor.js @@ -55,8 +55,7 @@ export default Component.extend({ } try { - yield this.get('job').plan(); - const plan = this.get('store').peekRecord('job-plan', this.get('job.id')); + const plan = yield this.get('job').plan(); this.set('planOutput', plan); } catch (err) { const error = messageFromAdapterError(err) || 'Could not plan job';