From 67f2fa17344b392682913a71470cea516cd168c8 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 18 Apr 2018 12:17:55 -0700 Subject: [PATCH 01/11] Add new model action for stopping a job --- ui/app/adapters/job.js | 24 +++++++++++++++++------- ui/app/models/job.js | 4 ++++ 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/ui/app/adapters/job.js b/ui/app/adapters/job.js index a868db7dc33..9845645724e 100644 --- a/ui/app/adapters/job.js +++ b/ui/app/adapters/job.js @@ -74,14 +74,24 @@ export default Watchable.extend({ forcePeriodic(job) { if (job.get('periodic')) { - const [path, params] = this.buildURL('job', job.get('id'), job, 'findRecord').split('?'); - let url = `${path}/periodic/force`; - - if (params) { - url += `?${params}`; - } - + const url = addToPath(this.urlForFindRecord(job.get('id'), 'job'), '/periodic/force'); return this.ajax(url, 'POST'); } }, + + stop(job) { + const url = this.urlForFindRecord(job.get('id'), 'job'); + return this.ajax(url, 'DELETE'); + }, }); + +function addToPath(url, extension = '') { + const [path, params] = url.split('?'); + let newUrl = `${path}${extension}`; + + if (params) { + newUrl += `?${params}`; + } + + return newUrl; +} diff --git a/ui/app/models/job.js b/ui/app/models/job.js index 8a24bcaa2de..b8ba9670f0f 100644 --- a/ui/app/models/job.js +++ b/ui/app/models/job.js @@ -162,6 +162,10 @@ export default Model.extend({ return this.store.adapterFor('job').forcePeriodic(this); }, + stop() { + return this.store.adapterFor('job').stop(this); + }, + statusClass: computed('status', function() { const classMap = { pending: 'is-pending', From f0cf06a64a1d6bbaaccac2150a900899e653357a Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 18 Apr 2018 12:19:35 -0700 Subject: [PATCH 02/11] New two-step-button For performing an action that requires confirmation --- ui/app/components/two-step-button.js | 26 +++++++++++++ ui/app/styles/components.scss | 37 ++++++++++--------- ui/app/styles/components/two-step-button.scss | 14 +++++++ .../templates/components/two-step-button.hbs | 19 ++++++++++ 4 files changed, 78 insertions(+), 18 deletions(-) create mode 100644 ui/app/components/two-step-button.js create mode 100644 ui/app/styles/components/two-step-button.scss create mode 100644 ui/app/templates/components/two-step-button.hbs diff --git a/ui/app/components/two-step-button.js b/ui/app/components/two-step-button.js new file mode 100644 index 00000000000..7016e96ec24 --- /dev/null +++ b/ui/app/components/two-step-button.js @@ -0,0 +1,26 @@ +import Component from '@ember/component'; +import { equal } from '@ember/object/computed'; + +export default Component.extend({ + classNames: ['two-step-button'], + + idleText: '', + cancelText: '', + confirmText: '', + confirmationMessage: '', + onConfirm() {}, + onCancel() {}, + + state: 'idle', + isIdle: equal('state', 'idle'), + isPendingConfirmation: equal('state', 'prompt'), + + actions: { + setToIdle() { + this.set('state', 'idle'); + }, + promptForConfirmation() { + this.set('state', 'prompt'); + }, + }, +}); diff --git a/ui/app/styles/components.scss b/ui/app/styles/components.scss index afd92f4e00f..49b51380666 100644 --- a/ui/app/styles/components.scss +++ b/ui/app/styles/components.scss @@ -1,18 +1,19 @@ -@import "./components/badge"; -@import "./components/boxed-section"; -@import "./components/cli-window"; -@import "./components/ember-power-select"; -@import "./components/empty-message"; -@import "./components/error-container"; -@import "./components/gutter"; -@import "./components/inline-definitions"; -@import "./components/job-diff"; -@import "./components/json-viewer"; -@import "./components/loading-spinner"; -@import "./components/metrics"; -@import "./components/node-status-light"; -@import "./components/page-layout"; -@import "./components/simple-list"; -@import "./components/status-text"; -@import "./components/timeline"; -@import "./components/tooltip"; +@import './components/badge'; +@import './components/boxed-section'; +@import './components/cli-window'; +@import './components/ember-power-select'; +@import './components/empty-message'; +@import './components/error-container'; +@import './components/gutter'; +@import './components/inline-definitions'; +@import './components/job-diff'; +@import './components/json-viewer'; +@import './components/loading-spinner'; +@import './components/metrics'; +@import './components/node-status-light'; +@import './components/page-layout'; +@import './components/simple-list'; +@import './components/status-text'; +@import './components/timeline'; +@import './components/tooltip'; +@import './components/two-step-button'; diff --git a/ui/app/styles/components/two-step-button.scss b/ui/app/styles/components/two-step-button.scss new file mode 100644 index 00000000000..18ed9fb01f3 --- /dev/null +++ b/ui/app/styles/components/two-step-button.scss @@ -0,0 +1,14 @@ +.two-step-button { + display: inline; + position: relative; + + .confirmation-text { + position: absolute; + left: 0; + top: -1.2em; + font-size: $body-size; + font-weight: $weight-normal; + color: darken($grey-blue, 20%); + white-space: nowrap; + } +} diff --git a/ui/app/templates/components/two-step-button.hbs b/ui/app/templates/components/two-step-button.hbs new file mode 100644 index 00000000000..d10e17681dc --- /dev/null +++ b/ui/app/templates/components/two-step-button.hbs @@ -0,0 +1,19 @@ +{{#if isIdle}} + +{{else if isPendingConfirmation}} + {{confirmationMessage}} + + +{{/if}} From 9899b15bf3cceeb2cbbe8d96d625727770c6e702 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 18 Apr 2018 13:02:58 -0700 Subject: [PATCH 03/11] Test coverage for the two-step-button component --- .../templates/components/job-page/service.hbs | 8 ++ .../templates/components/two-step-button.hbs | 8 +- ui/tests/integration/two-step-button-test.js | 111 ++++++++++++++++++ 3 files changed, 123 insertions(+), 4 deletions(-) create mode 100644 ui/tests/integration/two-step-button-test.js diff --git a/ui/app/templates/components/job-page/service.hbs b/ui/app/templates/components/job-page/service.hbs index 47343c49a97..136749f0c78 100644 --- a/ui/app/templates/components/job-page/service.hbs +++ b/ui/app/templates/components/job-page/service.hbs @@ -9,6 +9,14 @@

{{job.name}} {{job.status}} + {{#if (not (eq job.status "dead"))}} + {{two-step-button + idleText="Stop" + cancelText="Cancel" + confirmText="Yes, Stop" + confirmationMessage="Are you sure you want to stop this job?" + onConfirm=(action "stopJob")}} + {{/if}}

diff --git a/ui/app/templates/components/two-step-button.hbs b/ui/app/templates/components/two-step-button.hbs index d10e17681dc..cc2849b1dae 100644 --- a/ui/app/templates/components/two-step-button.hbs +++ b/ui/app/templates/components/two-step-button.hbs @@ -1,16 +1,16 @@ {{#if isIdle}} - {{else if isPendingConfirmation}} - {{confirmationMessage}} - - +
+ + +{{/if}} From 473814447824fc6cf9ec4004ba8711339ec4c2f4 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 19 Apr 2018 10:17:47 -0700 Subject: [PATCH 06/11] New errorMessage format --- ui/app/components/job-page/abstract.js | 12 ++++++++++++ ui/app/components/job-page/periodic.js | 11 +++++++---- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/ui/app/components/job-page/abstract.js b/ui/app/components/job-page/abstract.js index 01358cf3480..d2922c2afed 100644 --- a/ui/app/components/job-page/abstract.js +++ b/ui/app/components/job-page/abstract.js @@ -17,6 +17,9 @@ export default Component.extend({ gotoTaskGroup() {}, gotoJob() {}, + // Set to a { title, description } to surface an error + errorMessage: null, + breadcrumbs: computed('job.{name,id}', function() { const job = this.get('job'); return [ @@ -33,4 +36,13 @@ export default Component.extend({ }, ]; }), + + actions: { + clearErrorMessage() { + this.set('errorMessage', null); + }, + handleError(errorObject) { + this.set('errorMessage', errorObject); + }, + }, }); diff --git a/ui/app/components/job-page/periodic.js b/ui/app/components/job-page/periodic.js index 95be2cbb16f..a36338c7c64 100644 --- a/ui/app/components/job-page/periodic.js +++ b/ui/app/components/job-page/periodic.js @@ -4,18 +4,21 @@ import { inject as service } from '@ember/service'; export default AbstractJobPage.extend({ store: service(), - errorMessage: '', + errorMessage: null, actions: { forceLaunch() { this.get('job') .forcePeriodic() - .catch(error => { - this.set('errorMessage', `Could not force launch: ${error}`); + .catch(() => { + this.set('errorMessage', { + title: 'Could Not Force Launch', + description: 'Your ACL token does not grant permission to submit jobs.', + }); }); }, clearErrorMessage() { - this.set('errorMessage', ''); + this.set('errorMessage', null); }, }, }); From a3be7ef006f944de22b81c37dd6487e3ea9d42a2 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 19 Apr 2018 10:18:16 -0700 Subject: [PATCH 07/11] Refactor job-page components to use common title and error parts --- .../templates/components/job-page/batch.hbs | 7 +++---- .../job-page/parameterized-child.hbs | 7 +++---- .../components/job-page/parameterized.hbs | 8 +++---- .../components/job-page/periodic-child.hbs | 7 +++---- .../components/job-page/periodic.hbs | 21 ++++--------------- .../templates/components/job-page/service.hbs | 15 +++---------- .../templates/components/job-page/system.hbs | 7 +++---- 7 files changed, 23 insertions(+), 49 deletions(-) diff --git a/ui/app/templates/components/job-page/batch.hbs b/ui/app/templates/components/job-page/batch.hbs index 74b7f5c2874..d92573adda2 100644 --- a/ui/app/templates/components/job-page/batch.hbs +++ b/ui/app/templates/components/job-page/batch.hbs @@ -6,10 +6,9 @@ {{/each}} {{/global-header}} {{#job-page/parts/body job=job onNamespaceChange=onNamespaceChange}} -

- {{job.name}} - {{job.status}} -

+ {{job-page/parts/error errorMessage=errorMessage onDismiss=(action "clearErrorMessage")}} + + {{job-page/parts/title job=job handleError=(action "handleError")}}
diff --git a/ui/app/templates/components/job-page/parameterized-child.hbs b/ui/app/templates/components/job-page/parameterized-child.hbs index b01ad400f26..cd3d18f0b53 100644 --- a/ui/app/templates/components/job-page/parameterized-child.hbs +++ b/ui/app/templates/components/job-page/parameterized-child.hbs @@ -6,10 +6,9 @@ {{/each}} {{/global-header}} {{#job-page/parts/body job=job onNamespaceChange=onNamespaceChange}} -

- {{job.trimmedName}} - {{job.status}} -

+ {{job-page/parts/error errorMessage=errorMessage onDismiss=(action "clearErrorMessage")}} + + {{job-page/parts/title job=job title=job.trimmedName handleError=(action "handleError")}}
diff --git a/ui/app/templates/components/job-page/parameterized.hbs b/ui/app/templates/components/job-page/parameterized.hbs index a8e3ed47ffc..b976e73a895 100644 --- a/ui/app/templates/components/job-page/parameterized.hbs +++ b/ui/app/templates/components/job-page/parameterized.hbs @@ -6,11 +6,11 @@ {{/each}} {{/global-header}} {{#job-page/parts/body job=job onNamespaceChange=onNamespaceChange}} -

- {{job.name}} - {{job.status}} + {{job-page/parts/error errorMessage=errorMessage onDismiss=(action "clearErrorMessage")}} + + {{#job-page/parts/title job=job handleError=(action "handleError")}} Parameterized -

+ {{/job-page/parts/title}}
diff --git a/ui/app/templates/components/job-page/periodic-child.hbs b/ui/app/templates/components/job-page/periodic-child.hbs index e39aed6375c..946ce25798d 100644 --- a/ui/app/templates/components/job-page/periodic-child.hbs +++ b/ui/app/templates/components/job-page/periodic-child.hbs @@ -6,10 +6,9 @@ {{/each}} {{/global-header}} {{#job-page/parts/body job=job onNamespaceChange=onNamespaceChange}} -

- {{job.trimmedName}} - {{job.status}} -

+ {{job-page/parts/error errorMessage=errorMessage onDismiss=(action "clearErrorMessage")}} + + {{job-page/parts/title job=job title=job.trimmedName handleError=(action "handleError")}}
diff --git a/ui/app/templates/components/job-page/periodic.hbs b/ui/app/templates/components/job-page/periodic.hbs index 25fcdc102af..31e0ffe32e3 100644 --- a/ui/app/templates/components/job-page/periodic.hbs +++ b/ui/app/templates/components/job-page/periodic.hbs @@ -6,25 +6,12 @@ {{/each}} {{/global-header}} {{#job-page/parts/body job=job onNamespaceChange=onNamespaceChange}} - {{#if errorMessage}} -
-
-
-

Could Not Force Launch

-

Your ACL token does not grant permission to submit jobs.

-
-
- -
-
-
- {{/if}} -

- {{job.name}} - {{job.status}} + {{job-page/parts/error errorMessage=errorMessage onDismiss=(action "clearErrorMessage")}} + + {{#job-page/parts/title job=job title=job.trimmedName handleError=(action "handleError")}} periodic -

+ {{/job-page/parts/title}}
diff --git a/ui/app/templates/components/job-page/service.hbs b/ui/app/templates/components/job-page/service.hbs index 136749f0c78..80e2ab721a6 100644 --- a/ui/app/templates/components/job-page/service.hbs +++ b/ui/app/templates/components/job-page/service.hbs @@ -6,18 +6,9 @@ {{/each}} {{/global-header}} {{#job-page/parts/body job=job onNamespaceChange=onNamespaceChange}} -

- {{job.name}} - {{job.status}} - {{#if (not (eq job.status "dead"))}} - {{two-step-button - idleText="Stop" - cancelText="Cancel" - confirmText="Yes, Stop" - confirmationMessage="Are you sure you want to stop this job?" - onConfirm=(action "stopJob")}} - {{/if}} -

+ {{job-page/parts/error errorMessage=errorMessage onDismiss=(action "clearErrorMessage")}} + + {{job-page/parts/title job=job handleError=(action "handleError")}}
diff --git a/ui/app/templates/components/job-page/system.hbs b/ui/app/templates/components/job-page/system.hbs index 74b7f5c2874..d92573adda2 100644 --- a/ui/app/templates/components/job-page/system.hbs +++ b/ui/app/templates/components/job-page/system.hbs @@ -6,10 +6,9 @@ {{/each}} {{/global-header}} {{#job-page/parts/body job=job onNamespaceChange=onNamespaceChange}} -

- {{job.name}} - {{job.status}} -

+ {{job-page/parts/error errorMessage=errorMessage onDismiss=(action "clearErrorMessage")}} + + {{job-page/parts/title job=job handleError=(action "handleError")}}
From 396c4b041c5376ff508b113efb4acf4b5fb93d2c Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 19 Apr 2018 11:13:23 -0700 Subject: [PATCH 08/11] New tests for stop job action --- .../components/job-page/parts/error.hbs | 6 +- .../components/job-page/parts/title.hbs | 1 + ui/mirage/config.js | 6 + .../integration/job-page/periodic-test.js | 118 +++++++++++++++++- 4 files changed, 123 insertions(+), 8 deletions(-) diff --git a/ui/app/templates/components/job-page/parts/error.hbs b/ui/app/templates/components/job-page/parts/error.hbs index 168e2979c42..05ddeac1e13 100644 --- a/ui/app/templates/components/job-page/parts/error.hbs +++ b/ui/app/templates/components/job-page/parts/error.hbs @@ -2,11 +2,11 @@
-

{{errorMessage.title}}

-

{{errorMessage.description}}

+

{{errorMessage.title}}

+

{{errorMessage.description}}

- +
diff --git a/ui/app/templates/components/job-page/parts/title.hbs b/ui/app/templates/components/job-page/parts/title.hbs index 2de3064d3e8..445640d760a 100644 --- a/ui/app/templates/components/job-page/parts/title.hbs +++ b/ui/app/templates/components/job-page/parts/title.hbs @@ -4,6 +4,7 @@ {{yield}} {{#if (not (eq job.status "dead"))}} {{two-step-button + data-test-stop idleText="Stop" cancelText="Cancel" confirmText="Yes, Stop" diff --git a/ui/mirage/config.js b/ui/mirage/config.js index 6731f341cc7..689716dd937 100644 --- a/ui/mirage/config.js +++ b/ui/mirage/config.js @@ -106,6 +106,12 @@ export default function() { return new Response(200, {}, '{}'); }); + this.delete('/job/:id', function(schema, { params }) { + const job = schema.jobs.find(params.id); + job.update({ status: 'dead' }); + return new Response(204, {}, ''); + }); + this.get('/deployment/:id'); this.get('/job/:id/evaluations', function({ evaluations }, { params }) { diff --git a/ui/tests/integration/job-page/periodic-test.js b/ui/tests/integration/job-page/periodic-test.js index a9dd30bf71d..10b961fd09a 100644 --- a/ui/tests/integration/job-page/periodic-test.js +++ b/ui/tests/integration/job-page/periodic-test.js @@ -112,24 +112,132 @@ test('Clicking force launch without proper permissions shows an error message', `); return wait().then(() => { - assert.notOk(find('[data-test-force-error-title]'), 'No error message yet'); + assert.notOk(find('[data-test-job-error-title]'), 'No error message yet'); click('[data-test-force-launch]'); return wait().then(() => { assert.equal( - find('[data-test-force-error-title]').textContent, + find('[data-test-job-error-title]').textContent, 'Could Not Force Launch', 'Appropriate error is shown' ); assert.ok( - find('[data-test-force-error-body]').textContent.includes('ACL'), + find('[data-test-job-error-body]').textContent.includes('ACL'), 'The error message mentions ACLs' ); - click('[data-test-force-error-close]'); + click('[data-test-job-error-close]'); - assert.notOk(find('[data-test-force-error-title]'), 'Error message is dismissable'); + assert.notOk(find('[data-test-job-error-title]'), 'Error message is dismissable'); + }); + }); + }); +}); + +test('Stopping a job sends a delete request for the job', function(assert) { + const mirageJob = this.server.create('job', 'periodic', { + childrenCount: 0, + createAllocations: false, + }); + + this.store.findAll('job'); + + return wait().then(() => { + const job = this.store.peekAll('job').findBy('plainId', mirageJob.id); + + this.setProperties({ + job, + sortProperty: 'name', + sortDescending: true, + currentPage: 1, + gotoJob: () => {}, + }); + + this.render(hbs` + {{job-page/periodic + job=job + sortProperty=sortProperty + sortDescending=sortDescending + currentPage=currentPage + gotoJob=gotoJob}} + `); + + return wait().then(() => { + click('[data-test-stop] [data-test-idle-button]'); + return wait().then(() => { + click('[data-test-stop] [data-test-confirm-button]'); + + return wait().then(() => { + const id = job.get('plainId'); + const namespace = job.get('namespace.name') || 'default'; + let expectedURL = `/v1/job/${id}`; + if (namespace !== 'default') { + expectedURL += `?namespace=${namespace}`; + } + + assert.ok( + server.pretender.handledRequests + .filterBy('method', 'DELETE') + .find(req => req.url === expectedURL), + 'DELETE URL was made correctly' + ); + }); + }); + }); + }); +}); + +test('Stopping a job without proper permissions shows an error message', function(assert) { + server.pretender.delete('/v1/job/:id', () => [403, {}, null]); + + const mirageJob = this.server.create('job', 'periodic', { + childrenCount: 0, + createAllocations: false, + }); + + this.store.findAll('job'); + + return wait().then(() => { + const job = this.store.peekAll('job').findBy('plainId', mirageJob.id); + + this.setProperties({ + job, + sortProperty: 'name', + sortDescending: true, + currentPage: 1, + gotoJob: () => {}, + }); + + this.render(hbs` + {{job-page/periodic + job=job + sortProperty=sortProperty + sortDescending=sortDescending + currentPage=currentPage + gotoJob=gotoJob}} + `); + + return wait().then(() => { + click('[data-test-stop] [data-test-idle-button]'); + return wait().then(() => { + click('[data-test-stop] [data-test-confirm-button]'); + + return wait().then(() => { + assert.equal( + find('[data-test-job-error-title]').textContent, + 'Could Not Stop Job', + 'Appropriate error is shown' + ); + assert.ok( + find('[data-test-job-error-body]').textContent.includes('ACL'), + 'The error message mentions ACLs' + ); + + click('[data-test-job-error-close]'); + + assert.notOk(find('[data-test-job-error-title]'), 'Error message is dismissable'); + }); }); }); }); From 91c9e092d7d5ae5de650b217063bb4015700fd8c Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 19 Apr 2018 11:56:11 -0700 Subject: [PATCH 09/11] Refactor periodic job tests --- .../integration/job-page/periodic-test.js | 211 ++++++++---------- 1 file changed, 93 insertions(+), 118 deletions(-) diff --git a/ui/tests/integration/job-page/periodic-test.js b/ui/tests/integration/job-page/periodic-test.js index 10b961fd09a..76f8ede6eea 100644 --- a/ui/tests/integration/job-page/periodic-test.js +++ b/ui/tests/integration/job-page/periodic-test.js @@ -19,6 +19,23 @@ moduleForComponent('job-page/periodic', 'Integration | Component | job-page/peri }, }); +const commonTemplate = hbs` + {{job-page/periodic + job=job + sortProperty=sortProperty + sortDescending=sortDescending + currentPage=currentPage + gotoJob=gotoJob}} +`; + +const commonProperties = job => ({ + job, + sortProperty: 'name', + sortDescending: true, + currentPage: 1, + gotoJob: () => {}, +}); + test('Clicking Force Launch launches a new periodic child job', function(assert) { const childrenCount = 3; @@ -32,22 +49,9 @@ test('Clicking Force Launch launches a new periodic child job', function(assert) return wait().then(() => { const job = this.store.peekAll('job').findBy('plainId', 'parent'); - this.setProperties({ - job, - sortProperty: 'name', - sortDescending: true, - currentPage: 1, - gotoJob: () => {}, - }); - this.render(hbs` - {{job-page/periodic - job=job - sortProperty=sortProperty - sortDescending=sortDescending - currentPage=currentPage - gotoJob=gotoJob}} - `); + this.setProperties(commonProperties(job)); + this.render(commonTemplate); return wait().then(() => { const currentJobCount = server.db.jobs.length; @@ -61,12 +65,7 @@ test('Clicking Force Launch launches a new periodic child job', function(assert) click('[data-test-force-launch]'); return wait().then(() => { - const id = job.get('plainId'); - const namespace = job.get('namespace.name') || 'default'; - let expectedURL = `/v1/job/${id}/periodic/force`; - if (namespace !== 'default') { - expectedURL += `?namespace=${namespace}`; - } + const expectedURL = jobURL(job, '/periodic/force'); assert.ok( server.pretender.handledRequests @@ -88,28 +87,16 @@ test('Clicking force launch without proper permissions shows an error message', id: 'parent', childrenCount: 1, createAllocations: false, + status: 'running', }); this.store.findAll('job'); return wait().then(() => { const job = this.store.peekAll('job').findBy('plainId', 'parent'); - this.setProperties({ - job, - sortProperty: 'name', - sortDescending: true, - currentPage: 1, - gotoJob: () => {}, - }); - this.render(hbs` - {{job-page/periodic - job=job - sortProperty=sortProperty - sortDescending=sortDescending - currentPage=currentPage - gotoJob=gotoJob}} - `); + this.setProperties(commonProperties(job)); + this.render(commonTemplate); return wait().then(() => { assert.notOk(find('[data-test-job-error-title]'), 'No error message yet'); @@ -139,53 +126,23 @@ test('Stopping a job sends a delete request for the job', function(assert) { const mirageJob = this.server.create('job', 'periodic', { childrenCount: 0, createAllocations: false, + status: 'running', }); + let job; this.store.findAll('job'); - return wait().then(() => { - const job = this.store.peekAll('job').findBy('plainId', mirageJob.id); - - this.setProperties({ - job, - sortProperty: 'name', - sortDescending: true, - currentPage: 1, - gotoJob: () => {}, - }); + return wait() + .then(() => { + job = this.store.peekAll('job').findBy('plainId', mirageJob.id); - this.render(hbs` - {{job-page/periodic - job=job - sortProperty=sortProperty - sortDescending=sortDescending - currentPage=currentPage - gotoJob=gotoJob}} - `); + this.setProperties(commonProperties(job)); + this.render(commonTemplate); - return wait().then(() => { - click('[data-test-stop] [data-test-idle-button]'); - return wait().then(() => { - click('[data-test-stop] [data-test-confirm-button]'); - - return wait().then(() => { - const id = job.get('plainId'); - const namespace = job.get('namespace.name') || 'default'; - let expectedURL = `/v1/job/${id}`; - if (namespace !== 'default') { - expectedURL += `?namespace=${namespace}`; - } - - assert.ok( - server.pretender.handledRequests - .filterBy('method', 'DELETE') - .find(req => req.url === expectedURL), - 'DELETE URL was made correctly' - ); - }); - }); - }); - }); + return wait(); + }) + .then(stopJob) + .then(() => expectDeleteRequest(assert, job)); }); test('Stopping a job without proper permissions shows an error message', function(assert) { @@ -194,51 +151,69 @@ test('Stopping a job without proper permissions shows an error message', functio const mirageJob = this.server.create('job', 'periodic', { childrenCount: 0, createAllocations: false, + status: 'running', }); this.store.findAll('job'); - return wait().then(() => { - const job = this.store.peekAll('job').findBy('plainId', mirageJob.id); - - this.setProperties({ - job, - sortProperty: 'name', - sortDescending: true, - currentPage: 1, - gotoJob: () => {}, - }); + return wait() + .then(() => { + const job = this.store.peekAll('job').findBy('plainId', mirageJob.id); - this.render(hbs` - {{job-page/periodic - job=job - sortProperty=sortProperty - sortDescending=sortDescending - currentPage=currentPage - gotoJob=gotoJob}} - `); + this.setProperties(commonProperties(job)); + this.render(commonTemplate); - return wait().then(() => { - click('[data-test-stop] [data-test-idle-button]'); - return wait().then(() => { - click('[data-test-stop] [data-test-confirm-button]'); - - return wait().then(() => { - assert.equal( - find('[data-test-job-error-title]').textContent, - 'Could Not Stop Job', - 'Appropriate error is shown' - ); - assert.ok( - find('[data-test-job-error-body]').textContent.includes('ACL'), - 'The error message mentions ACLs' - ); - - click('[data-test-job-error-close]'); - - assert.notOk(find('[data-test-job-error-title]'), 'Error message is dismissable'); - }); - }); - }); - }); + return wait(); + }) + .then(stopJob) + .then(expectStopError(assert)); }); + +function expectDeleteRequest(assert, job) { + const expectedURL = jobURL(job); + + assert.ok( + server.pretender.handledRequests + .filterBy('method', 'DELETE') + .find(req => req.url === expectedURL), + 'DELETE URL was made correctly' + ); + + return wait(); +} + +function jobURL(job, path = '') { + const id = job.get('plainId'); + const namespace = job.get('namespace.name') || 'default'; + let expectedURL = `/v1/job/${id}${path}`; + if (namespace !== 'default') { + expectedURL += `?namespace=${namespace}`; + } + return expectedURL; +} + +function stopJob() { + click('[data-test-stop] [data-test-idle-button]'); + return wait().then(() => { + click('[data-test-stop] [data-test-confirm-button]'); + return wait(); + }); +} + +function expectStopError(assert) { + return () => { + assert.equal( + find('[data-test-job-error-title]').textContent, + 'Could Not Stop Job', + 'Appropriate error is shown' + ); + assert.ok( + find('[data-test-job-error-body]').textContent.includes('ACL'), + 'The error message mentions ACLs' + ); + + click('[data-test-job-error-close]'); + assert.notOk(find('[data-test-job-error-title]'), 'Error message is dismissable'); + return wait(); + }; +} From 1d19d825f2928d290878a7532ed7cf6ab64d37fa Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 19 Apr 2018 14:21:25 -0700 Subject: [PATCH 10/11] Move job-page helpers and repeat stop tests on the service type --- ui/tests/integration/job-page/helpers.js | 51 ++++++++++++ .../integration/job-page/periodic-test.js | 58 ++----------- ui/tests/integration/job-page/service-test.js | 82 +++++++++++++++++++ 3 files changed, 138 insertions(+), 53 deletions(-) create mode 100644 ui/tests/integration/job-page/helpers.js create mode 100644 ui/tests/integration/job-page/service-test.js diff --git a/ui/tests/integration/job-page/helpers.js b/ui/tests/integration/job-page/helpers.js new file mode 100644 index 00000000000..ef64e86af45 --- /dev/null +++ b/ui/tests/integration/job-page/helpers.js @@ -0,0 +1,51 @@ +import { click, find } from 'ember-native-dom-helpers'; +import wait from 'ember-test-helpers/wait'; + +export function jobURL(job, path = '') { + const id = job.get('plainId'); + const namespace = job.get('namespace.name') || 'default'; + let expectedURL = `/v1/job/${id}${path}`; + if (namespace !== 'default') { + expectedURL += `?namespace=${namespace}`; + } + return expectedURL; +} + +export function stopJob() { + click('[data-test-stop] [data-test-idle-button]'); + return wait().then(() => { + click('[data-test-stop] [data-test-confirm-button]'); + return wait(); + }); +} + +export function expectStopError(assert) { + return () => { + assert.equal( + find('[data-test-job-error-title]').textContent, + 'Could Not Stop Job', + 'Appropriate error is shown' + ); + assert.ok( + find('[data-test-job-error-body]').textContent.includes('ACL'), + 'The error message mentions ACLs' + ); + + click('[data-test-job-error-close]'); + assert.notOk(find('[data-test-job-error-title]'), 'Error message is dismissable'); + return wait(); + }; +} + +export function expectDeleteRequest(assert, server, job) { + const expectedURL = jobURL(job); + + assert.ok( + server.pretender.handledRequests + .filterBy('method', 'DELETE') + .find(req => req.url === expectedURL), + 'DELETE URL was made correctly' + ); + + return wait(); +} diff --git a/ui/tests/integration/job-page/periodic-test.js b/ui/tests/integration/job-page/periodic-test.js index 76f8ede6eea..ba93d1a51ad 100644 --- a/ui/tests/integration/job-page/periodic-test.js +++ b/ui/tests/integration/job-page/periodic-test.js @@ -4,6 +4,7 @@ import { click, find, findAll } from 'ember-native-dom-helpers'; import wait from 'ember-test-helpers/wait'; import hbs from 'htmlbars-inline-precompile'; import { startMirage } from 'nomad-ui/initializers/ember-cli-mirage'; +import { jobURL, stopJob, expectStopError, expectDeleteRequest } from './helpers'; moduleForComponent('job-page/periodic', 'Integration | Component | job-page/periodic', { integration: true, @@ -68,7 +69,7 @@ test('Clicking Force Launch launches a new periodic child job', function(assert) const expectedURL = jobURL(job, '/periodic/force'); assert.ok( - server.pretender.handledRequests + this.server.pretender.handledRequests .filterBy('method', 'POST') .find(req => req.url === expectedURL), 'POST URL was correct' @@ -81,7 +82,7 @@ test('Clicking Force Launch launches a new periodic child job', function(assert) }); test('Clicking force launch without proper permissions shows an error message', function(assert) { - server.pretender.post('/v1/job/:id/periodic/force', () => [403, {}, null]); + this.server.pretender.post('/v1/job/:id/periodic/force', () => [403, {}, null]); this.server.create('job', 'periodic', { id: 'parent', @@ -142,11 +143,11 @@ test('Stopping a job sends a delete request for the job', function(assert) { return wait(); }) .then(stopJob) - .then(() => expectDeleteRequest(assert, job)); + .then(() => expectDeleteRequest(assert, this.server, job)); }); test('Stopping a job without proper permissions shows an error message', function(assert) { - server.pretender.delete('/v1/job/:id', () => [403, {}, null]); + this.server.pretender.delete('/v1/job/:id', () => [403, {}, null]); const mirageJob = this.server.create('job', 'periodic', { childrenCount: 0, @@ -168,52 +169,3 @@ test('Stopping a job without proper permissions shows an error message', functio .then(stopJob) .then(expectStopError(assert)); }); - -function expectDeleteRequest(assert, job) { - const expectedURL = jobURL(job); - - assert.ok( - server.pretender.handledRequests - .filterBy('method', 'DELETE') - .find(req => req.url === expectedURL), - 'DELETE URL was made correctly' - ); - - return wait(); -} - -function jobURL(job, path = '') { - const id = job.get('plainId'); - const namespace = job.get('namespace.name') || 'default'; - let expectedURL = `/v1/job/${id}${path}`; - if (namespace !== 'default') { - expectedURL += `?namespace=${namespace}`; - } - return expectedURL; -} - -function stopJob() { - click('[data-test-stop] [data-test-idle-button]'); - return wait().then(() => { - click('[data-test-stop] [data-test-confirm-button]'); - return wait(); - }); -} - -function expectStopError(assert) { - return () => { - assert.equal( - find('[data-test-job-error-title]').textContent, - 'Could Not Stop Job', - 'Appropriate error is shown' - ); - assert.ok( - find('[data-test-job-error-body]').textContent.includes('ACL'), - 'The error message mentions ACLs' - ); - - click('[data-test-job-error-close]'); - assert.notOk(find('[data-test-job-error-title]'), 'Error message is dismissable'); - return wait(); - }; -} diff --git a/ui/tests/integration/job-page/service-test.js b/ui/tests/integration/job-page/service-test.js new file mode 100644 index 00000000000..5b9bf69f345 --- /dev/null +++ b/ui/tests/integration/job-page/service-test.js @@ -0,0 +1,82 @@ +import { getOwner } from '@ember/application'; +import { test, moduleForComponent } from 'ember-qunit'; +import wait from 'ember-test-helpers/wait'; +import hbs from 'htmlbars-inline-precompile'; +import { startMirage } from 'nomad-ui/initializers/ember-cli-mirage'; +import { stopJob, expectStopError, expectDeleteRequest } from './helpers'; + +moduleForComponent('job-page/service', 'Integration | Component | job-page/service', { + integration: true, + beforeEach() { + window.localStorage.clear(); + this.store = getOwner(this).lookup('service:store'); + this.server = startMirage(); + this.server.create('namespace'); + }, + afterEach() { + this.server.shutdown(); + window.localStorage.clear(); + }, +}); + +const commonTemplate = hbs` + {{job-page/service + job=job + sortProperty=sortProperty + sortDescending=sortDescending + currentPage=currentPage + gotoJob=gotoJob}} +`; + +const commonProperties = job => ({ + job, + sortProperty: 'name', + sortDescending: true, + currentPage: 1, + gotoJob() {}, +}); + +const makeMirageJob = server => + server.create('job', { + type: 'service', + createAllocations: false, + status: 'running', + }); + +test('Stopping a job sends a delete request for the job', function(assert) { + let job; + + const mirageJob = makeMirageJob(this.server); + this.store.findAll('job'); + + return wait() + .then(() => { + job = this.store.peekAll('job').findBy('plainId', mirageJob.id); + + this.setProperties(commonProperties(job)); + this.render(commonTemplate); + + return wait(); + }) + .then(stopJob) + .then(() => expectDeleteRequest(assert, this.server, job)); +}); + +test('Stopping a job without proper permissions shows an error message', function(assert) { + this.server.pretender.delete('/v1/job/:id', () => [403, {}, null]); + + const mirageJob = makeMirageJob(this.server); + this.store.findAll('job'); + + return wait() + .then(() => { + const job = this.store.peekAll('job').findBy('plainId', mirageJob.id); + + this.setProperties(commonProperties(job)); + this.render(commonTemplate); + + return wait(); + }) + .then(stopJob) + .then(expectStopError(assert)); +}); From 10c9e7ab66eb66619b788cabe3ab5ccd9e7e1527 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 20 Apr 2018 10:11:21 -0700 Subject: [PATCH 11/11] Add button type to the two-step-button buttons --- ui/app/templates/components/two-step-button.hbs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/app/templates/components/two-step-button.hbs b/ui/app/templates/components/two-step-button.hbs index cc2849b1dae..88a5097815e 100644 --- a/ui/app/templates/components/two-step-button.hbs +++ b/ui/app/templates/components/two-step-button.hbs @@ -1,10 +1,10 @@ {{#if isIdle}} - {{else if isPendingConfirmation}} {{confirmationMessage}} -