From d5e8c1ca8cfb6eedb569721c0e0eb970819f2ad2 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 20 May 2020 14:13:41 -0700 Subject: [PATCH 01/10] Force the use of fetch despite jquery still being integrated --- ui/app/adapters/application.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ui/app/adapters/application.js b/ui/app/adapters/application.js index 3b82d526d..60a7fdacb 100644 --- a/ui/app/adapters/application.js +++ b/ui/app/adapters/application.js @@ -8,6 +8,10 @@ import { default as NoLeaderError, NO_LEADER } from '../utils/no-leader-error'; export const namespace = 'v1'; export default RESTAdapter.extend({ + // TODO: This can be removed once jquery-integration is turned off for + // the entire app. + useFetch: true, + namespace, system: service(), From 899d8266f42af93f0aeea6d02c18577410ba00ab Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 20 May 2020 14:14:26 -0700 Subject: [PATCH 02/10] Instrument jquery ajax method to see if it is getting called anywhere --- ui/app/initializers/app-env.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/ui/app/initializers/app-env.js b/ui/app/initializers/app-env.js index 0d5284f2d..c5fa7542b 100644 --- a/ui/app/initializers/app-env.js +++ b/ui/app/initializers/app-env.js @@ -4,6 +4,15 @@ export function initialize() { // Provides the app config to all templates application.inject('controller', 'config', 'service:config'); application.inject('component', 'config', 'service:config'); + + const jQuery = window.jQuery; + + jQuery.__ajax = jQuery.ajax; + jQuery.ajax = function() { + // eslint-disable-next-line + console.log('jQuery.ajax called:', ...arguments); + return jQuery.__ajax(...arguments); + }; } export default { From 20f209c4fbb8d66d5163655f472d9373695d92be Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 20 May 2020 14:58:50 -0700 Subject: [PATCH 03/10] Use the AbortController provided by the fetch polyfill --- ui/app/components/task-log.js | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/ui/app/components/task-log.js b/ui/app/components/task-log.js index 773ef1072..108a9b300 100644 --- a/ui/app/components/task-log.js +++ b/ui/app/components/task-log.js @@ -4,12 +4,7 @@ import { computed } from '@ember/object'; import RSVP from 'rsvp'; import { logger } from 'nomad-ui/utils/classes/log'; import timeout from 'nomad-ui/utils/timeout'; - -class MockAbortController { - abort() { - /* noop */ - } -} +import { AbortController } from 'fetch'; export default Component.extend({ token: service(), @@ -52,8 +47,7 @@ export default Component.extend({ // If the log request can't settle in one second, the client // must be unavailable and the server should be used instead - // AbortControllers don't exist in IE11, so provide a mock if it doesn't exist - const aborter = window.AbortController ? new AbortController() : new MockAbortController(); + const aborter = new AbortController(); const timing = this.useServer ? this.serverTimeout : this.clientTimeout; // Capture the state of useServer at logger create time to avoid a race From de73b9539d13e0b7be6b51ecbf8fee699bbe1c08 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 20 May 2020 16:18:23 -0700 Subject: [PATCH 04/10] Replace XHRToken with AbortController --- ui/app/adapters/watchable.js | 31 ++++++++----------------- ui/app/utils/classes/xhr-token.js | 11 --------- ui/app/utils/properties/watch.js | 29 ++++++++++++----------- ui/tests/unit/adapters/job-test.js | 33 +++++++++++++++------------ ui/tests/unit/adapters/volume-test.js | 8 +++---- 5 files changed, 47 insertions(+), 65 deletions(-) delete mode 100644 ui/app/utils/classes/xhr-token.js diff --git a/ui/app/adapters/watchable.js b/ui/app/adapters/watchable.js index e069ea37e..92da7f56a 100644 --- a/ui/app/adapters/watchable.js +++ b/ui/app/adapters/watchable.js @@ -23,19 +23,6 @@ export default ApplicationAdapter.extend({ delete ajaxOptions.data[key]; }); - const abortToken = (options || {}).abortToken; - if (abortToken) { - delete options.abortToken; - - const previousBeforeSend = ajaxOptions.beforeSend; - ajaxOptions.beforeSend = function(jqXHR) { - abortToken.capture(jqXHR); - if (previousBeforeSend) { - previousBeforeSend(...arguments); - } - }; - } - return ajaxOptions; }, @@ -64,9 +51,9 @@ export default ApplicationAdapter.extend({ params.index = this.watchList.getIndexFor(url); } - const abortToken = get(snapshotRecordArray || {}, 'adapterOptions.abortToken'); + const signal = get(snapshotRecordArray || {}, 'adapterOptions.abortController.signal'); return this.ajax(url, 'GET', { - abortToken, + signal, data: params, }); }, @@ -79,9 +66,9 @@ export default ApplicationAdapter.extend({ params.index = this.watchList.getIndexFor(url); } - const abortToken = get(snapshot || {}, 'adapterOptions.abortToken'); + const signal = get(snapshot || {}, 'adapterOptions.abortController.signal'); return this.ajax(url, 'GET', { - abortToken, + signal, data: params, }).catch(error => { if (error instanceof AbortError) { @@ -102,9 +89,9 @@ export default ApplicationAdapter.extend({ params.index = this.watchList.getIndexFor(`${url}?${queryString.stringify(query)}`); } - const abortToken = get(options, 'adapterOptions.abortToken'); + const signal = get(options, 'adapterOptions.abortController.signal'); return this.ajax(url, 'GET', { - abortToken, + signal, data: params, }).then(payload => { const adapter = store.adapterFor(type.modelName); @@ -133,8 +120,8 @@ export default ApplicationAdapter.extend({ }); }, - reloadRelationship(model, relationshipName, options = { watch: false, abortToken: null }) { - const { watch, abortToken } = options; + reloadRelationship(model, relationshipName, options = { watch: false, abortController: null }) { + const { watch, abortController } = options; const relationship = model.relationshipFor(relationshipName); if (relationship.kind !== 'belongsTo' && relationship.kind !== 'hasMany') { throw new Error( @@ -158,7 +145,7 @@ export default ApplicationAdapter.extend({ } return this.ajax(url, 'GET', { - abortToken, + signal: abortController && abortController.signal, data: params, }).then( json => { diff --git a/ui/app/utils/classes/xhr-token.js b/ui/app/utils/classes/xhr-token.js deleted file mode 100644 index 20b6386b3..000000000 --- a/ui/app/utils/classes/xhr-token.js +++ /dev/null @@ -1,11 +0,0 @@ -export default class XHRToken { - capture(xhr) { - this._xhr = xhr; - } - - abort() { - if (this._xhr) { - this._xhr.abort(); - } - } -} diff --git a/ui/app/utils/properties/watch.js b/ui/app/utils/properties/watch.js index 209b42746..f4aac8d32 100644 --- a/ui/app/utils/properties/watch.js +++ b/ui/app/utils/properties/watch.js @@ -3,9 +3,9 @@ import { get } from '@ember/object'; import { assert } from '@ember/debug'; import RSVP from 'rsvp'; import { task } from 'ember-concurrency'; +import { AbortController } from 'fetch'; import wait from 'nomad-ui/utils/wait'; import Watchable from 'nomad-ui/adapters/watchable'; -import XHRToken from 'nomad-ui/utils/classes/xhr-token'; import config from 'nomad-ui/config/environment'; const isEnabled = config.APP.blockingQueries !== false; @@ -16,7 +16,7 @@ export function watchRecord(modelName) { 'To watch a record, the record adapter MUST extend Watchable', this.store.adapterFor(modelName) instanceof Watchable ); - const token = new XHRToken(); + const controller = new AbortController(); if (typeof id === 'object') { id = get(id, 'id'); } @@ -25,7 +25,7 @@ export function watchRecord(modelName) { yield RSVP.all([ this.store.findRecord(modelName, id, { reload: true, - adapterOptions: { watch: true, abortToken: token }, + adapterOptions: { watch: true, abortController: controller }, }), wait(throttle), ]); @@ -33,7 +33,7 @@ export function watchRecord(modelName) { yield e; break; } finally { - token.abort(); + controller.abort(); } } }).drop(); @@ -45,20 +45,23 @@ export function watchRelationship(relationshipName) { 'To watch a relationship, the adapter of the model provided to the watchRelationship task MUST extend Watchable', this.store.adapterFor(model.constructor.modelName) instanceof Watchable ); - const token = new XHRToken(); + const controller = new AbortController(); while (isEnabled && !Ember.testing) { try { yield RSVP.all([ this.store .adapterFor(model.constructor.modelName) - .reloadRelationship(model, relationshipName, { watch: true, abortToken: token }), + .reloadRelationship(model, relationshipName, { + watch: true, + abortController: controller, + }), wait(throttle), ]); } catch (e) { yield e; break; } finally { - token.abort(); + controller.abort(); } } }).drop(); @@ -70,13 +73,13 @@ export function watchAll(modelName) { 'To watch all, the respective adapter MUST extend Watchable', this.store.adapterFor(modelName) instanceof Watchable ); - const token = new XHRToken(); + const controller = new AbortController(); while (isEnabled && !Ember.testing) { try { yield RSVP.all([ this.store.findAll(modelName, { reload: true, - adapterOptions: { watch: true, abortToken: token }, + adapterOptions: { watch: true, abortController: controller }, }), wait(throttle), ]); @@ -84,7 +87,7 @@ export function watchAll(modelName) { yield e; break; } finally { - token.abort(); + controller.abort(); } } }).drop(); @@ -96,13 +99,13 @@ export function watchQuery(modelName) { 'To watch a query, the adapter for the type being queried MUST extend Watchable', this.store.adapterFor(modelName) instanceof Watchable ); - const token = new XHRToken(); + const controller = new AbortController(); while (isEnabled && !Ember.testing) { try { yield RSVP.all([ this.store.query(modelName, params, { reload: true, - adapterOptions: { watch: true, abortToken: token }, + adapterOptions: { watch: true, abortController: controller }, }), wait(throttle), ]); @@ -110,7 +113,7 @@ export function watchQuery(modelName) { yield e; break; } finally { - token.abort(); + controller.abort(); } } }).drop(); diff --git a/ui/tests/unit/adapters/job-test.js b/ui/tests/unit/adapters/job-test.js index d2710f9be..37029a63a 100644 --- a/ui/tests/unit/adapters/job-test.js +++ b/ui/tests/unit/adapters/job-test.js @@ -4,7 +4,7 @@ import { settled } from '@ember/test-helpers'; import { setupTest } from 'ember-qunit'; import { module, test } from 'qunit'; import { startMirage } from 'nomad-ui/initializers/ember-cli-mirage'; -import XHRToken from 'nomad-ui/utils/classes/xhr-token'; +import { AbortController } from 'fetch'; module('Unit | Adapter | Job', function(hooks) { setupTest(hooks); @@ -261,14 +261,14 @@ module('Unit | Adapter | Job', function(hooks) { await this.initializeUI(); const { pretender } = this.server; - const token = new XHRToken(); + const controller = new AbortController(); pretender.get('/v1/jobs', () => [200, {}, '[]'], true); this.subject() .findAll(null, { modelName: 'job' }, null, { reload: true, - adapterOptions: { watch: true, abortToken: token }, + adapterOptions: { watch: true, abortController: controller }, }) .catch(() => {}); @@ -277,7 +277,7 @@ module('Unit | Adapter | Job', function(hooks) { // Schedule the cancelation before waiting run.next(() => { - token.abort(); + controller.abort(); }); await settled(); @@ -289,13 +289,13 @@ module('Unit | Adapter | Job', function(hooks) { const { pretender } = this.server; const jobId = JSON.stringify(['job-1', 'default']); - const token = new XHRToken(); + const controller = new AbortController(); pretender.get('/v1/job/:id', () => [200, {}, '{}'], true); this.subject().findRecord(null, { modelName: 'job' }, jobId, { reload: true, - adapterOptions: { watch: true, abortToken: token }, + adapterOptions: { watch: true, abortController: controller }, }); const { request: xhr } = pretender.requestReferences[0]; @@ -303,7 +303,7 @@ module('Unit | Adapter | Job', function(hooks) { // Schedule the cancelation before waiting run.next(() => { - token.abort(); + controller.abort(); }); await settled(); @@ -315,18 +315,21 @@ module('Unit | Adapter | Job', function(hooks) { const { pretender } = this.server; const plainId = 'job-1'; - const token = new XHRToken(); + const controller = new AbortController(); const mockModel = makeMockModel(plainId); pretender.get('/v1/job/:id/summary', () => [200, {}, '{}'], true); - this.subject().reloadRelationship(mockModel, 'summary', { watch: true, abortToken: token }); + this.subject().reloadRelationship(mockModel, 'summary', { + watch: true, + abortController: controller, + }); const { request: xhr } = pretender.requestReferences[0]; assert.equal(xhr.status, 0, 'Request is still pending'); // Schedule the cancelation before waiting run.next(() => { - token.abort(); + controller.abort(); }); await settled(); @@ -338,19 +341,19 @@ module('Unit | Adapter | Job', function(hooks) { const { pretender } = this.server; const jobId = JSON.stringify(['job-1', 'default']); - const token1 = new XHRToken(); - const token2 = new XHRToken(); + const controller1 = new AbortController(); + const controller2 = new AbortController(); pretender.get('/v1/job/:id', () => [200, {}, '{}'], true); this.subject().findRecord(null, { modelName: 'job' }, jobId, { reload: true, - adapterOptions: { watch: true, abortToken: token1 }, + adapterOptions: { watch: true, abortController: controller1 }, }); this.subject().findRecord(null, { modelName: 'job' }, jobId, { reload: true, - adapterOptions: { watch: true, abortToken: token2 }, + adapterOptions: { watch: true, abortController: controller2 }, }); const { request: xhr } = pretender.requestReferences[0]; @@ -365,7 +368,7 @@ module('Unit | Adapter | Job', function(hooks) { // Schedule the cancelation and resolution before waiting run.next(() => { - token1.abort(); + controller1.abort(); pretender.resolve(xhr2); }); diff --git a/ui/tests/unit/adapters/volume-test.js b/ui/tests/unit/adapters/volume-test.js index fcb3d04fa..fdb3606c9 100644 --- a/ui/tests/unit/adapters/volume-test.js +++ b/ui/tests/unit/adapters/volume-test.js @@ -3,7 +3,7 @@ import { settled } from '@ember/test-helpers'; import { setupTest } from 'ember-qunit'; import { module, test } from 'qunit'; import { startMirage } from 'nomad-ui/initializers/ember-cli-mirage'; -import XHRToken from 'nomad-ui/utils/classes/xhr-token'; +import { AbortController } from 'fetch'; module('Unit | Adapter | Volume', function(hooks) { setupTest(hooks); @@ -113,14 +113,14 @@ module('Unit | Adapter | Volume', function(hooks) { await this.initializeUI(); const { pretender } = this.server; - const token = new XHRToken(); + const controller = new AbortController(); pretender.get('/v1/volumes', () => [200, {}, '[]'], true); this.subject() .query(this.store, { modelName: 'volume' }, { type: 'csi' }, null, { reload: true, - adapterOptions: { watch: true, abortToken: token }, + adapterOptions: { watch: true, abortController: controller }, }) .catch(() => {}); @@ -129,7 +129,7 @@ module('Unit | Adapter | Volume', function(hooks) { // Schedule the cancelation before waiting run.next(() => { - token.abort(); + controller.abort(); }); await settled(); From 07a604033f1f39c21d4c73e809c6ada238a38d06 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 20 May 2020 16:23:03 -0700 Subject: [PATCH 05/10] Adjust header case, which changed with the migration to fetch --- ui/tests/unit/adapters/job-test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/tests/unit/adapters/job-test.js b/ui/tests/unit/adapters/job-test.js index 37029a63a..92c4f0a13 100644 --- a/ui/tests/unit/adapters/job-test.js +++ b/ui/tests/unit/adapters/job-test.js @@ -133,7 +133,7 @@ module('Unit | Adapter | Job', function(hooks) { await settled(); assert.notOk( - pretender.handledRequests.mapBy('requestHeaders').some(headers => headers['X-Nomad-Token']), + pretender.handledRequests.mapBy('requestHeaders').some(headers => headers['x-nomad-token']), 'No token header present on either job request' ); }); @@ -152,7 +152,7 @@ module('Unit | Adapter | Job', function(hooks) { assert.ok( pretender.handledRequests .mapBy('requestHeaders') - .every(headers => headers['X-Nomad-Token'] === secret), + .every(headers => headers['x-nomad-token'] === secret), 'The token header is present on both job requests' ); }); From e1f3086ba32838783b83c7c5dc9472b48e28ae0b Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 20 May 2020 17:46:07 -0700 Subject: [PATCH 06/10] Don't double query params for findQuery --- ui/app/adapters/watchable.js | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/ui/app/adapters/watchable.js b/ui/app/adapters/watchable.js index 92da7f56a..983c87b8b 100644 --- a/ui/app/adapters/watchable.js +++ b/ui/app/adapters/watchable.js @@ -10,22 +10,6 @@ export default ApplicationAdapter.extend({ watchList: service(), store: service(), - ajaxOptions(url, type, options) { - const ajaxOptions = this._super(url, type, options); - - // Since ajax has been changed to include query params in the URL, - // we have to remove query params that are in the URL from the data - // object so they don't get passed along twice. - const [newUrl, params] = ajaxOptions.url.split('?'); - const queryParams = queryString.parse(params); - ajaxOptions.url = !params ? newUrl : `${newUrl}?${queryString.stringify(queryParams)}`; - Object.keys(queryParams).forEach(key => { - delete ajaxOptions.data[key]; - }); - - return ajaxOptions; - }, - // Overriding ajax is not advised, but this is a minimal modification // that sets off a series of events that results in query params being // available in handleResponse below. Unfortunately, this is the only @@ -40,6 +24,11 @@ export default ApplicationAdapter.extend({ const params = { ...options.data }; delete params.index; + // Options data gets appended as query params as part of ajaxOptions. + // In order to prevent doubling params, data should only include index + // at this point since everything else is added to the URL in advance. + options.data = options.data.index ? { index: options.data.index } : {}; + return this._super(`${url}?${queryString.stringify(params)}`, type, options); }, @@ -80,17 +69,17 @@ export default ApplicationAdapter.extend({ query(store, type, query, snapshotRecordArray, options, additionalParams = {}) { const url = this.buildURL(type.modelName, null, null, 'query', query); - let [, params] = url.split('?'); + let [urlPath, params] = url.split('?'); params = assign(queryString.parse(params) || {}, this.buildQuery(), additionalParams, query); if (get(options, 'adapterOptions.watch')) { // The intended query without additional blocking query params is used // to track the appropriate query index. - params.index = this.watchList.getIndexFor(`${url}?${queryString.stringify(query)}`); + params.index = this.watchList.getIndexFor(`${urlPath}?${queryString.stringify(query)}`); } const signal = get(options, 'adapterOptions.abortController.signal'); - return this.ajax(url, 'GET', { + return this.ajax(urlPath, 'GET', { signal, data: params, }).then(payload => { From 388bb138c29dd7d6d0d6fe3a6f7ca98b67337c01 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 20 May 2020 17:46:29 -0700 Subject: [PATCH 07/10] Always lower case headers now --- ui/mirage/config.js | 8 ++++---- ui/tests/acceptance/token-test.js | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/ui/mirage/config.js b/ui/mirage/config.js index 367dc67ba..994135eef 100644 --- a/ui/mirage/config.js +++ b/ui/mirage/config.js @@ -44,7 +44,7 @@ export default function() { // Annotate the response with the index if (response instanceof Response) { - response.headers['X-Nomad-Index'] = index; + response.headers['x-nomad-index'] = index; return response; } return new Response(200, { 'x-nomad-index': index }, response); @@ -316,7 +316,7 @@ export default function() { }); this.get('/acl/token/self', function({ tokens }, req) { - const secret = req.requestHeaders['X-Nomad-Token']; + const secret = req.requestHeaders['x-nomad-token']; const tokenForSecret = tokens.findBy({ secretId: secret }); // Return the token if it exists @@ -330,7 +330,7 @@ export default function() { this.get('/acl/token/:id', function({ tokens }, req) { const token = tokens.find(req.params.id); - const secret = req.requestHeaders['X-Nomad-Token']; + const secret = req.requestHeaders['x-nomad-token']; const tokenForSecret = tokens.findBy({ secretId: secret }); // Return the token only if the request header matches the token @@ -345,7 +345,7 @@ export default function() { this.get('/acl/policy/:id', function({ policies, tokens }, req) { const policy = policies.find(req.params.id); - const secret = req.requestHeaders['X-Nomad-Token']; + const secret = req.requestHeaders['x-nomad-token']; const tokenForSecret = tokens.findBy({ secretId: secret }); if (req.params.id === 'anonymous') { diff --git a/ui/tests/acceptance/token-test.js b/ui/tests/acceptance/token-test.js index c66c8f8e7..cba90e6a2 100644 --- a/ui/tests/acceptance/token-test.js +++ b/ui/tests/acceptance/token-test.js @@ -39,7 +39,7 @@ module('Acceptance | tokens', function(hooks) { }); // TODO: unskip once store.unloadAll reliably waits for in-flight requests to settle - skip('the X-Nomad-Token header gets sent with requests once it is set', async function(assert) { + skip('the x-nomad-token header gets sent with requests once it is set', async function(assert) { const { secretId } = managementToken; await JobDetail.visit({ id: job.id }); @@ -48,7 +48,7 @@ module('Acceptance | tokens', function(hooks) { assert.ok(server.pretender.handledRequests.length > 1, 'Requests have been made'); server.pretender.handledRequests.forEach(req => { - assert.notOk(getHeader(req, 'X-Nomad-Token'), `No token for ${req.url}`); + assert.notOk(getHeader(req, 'x-nomad-token'), `No token for ${req.url}`); }); const requestPosition = server.pretender.handledRequests.length; @@ -64,7 +64,7 @@ module('Acceptance | tokens', function(hooks) { // Cross-origin requests can't have a token newRequests.forEach(req => { - assert.equal(getHeader(req, 'X-Nomad-Token'), secretId, `Token set for ${req.url}`); + assert.equal(getHeader(req, 'x-nomad-token'), secretId, `Token set for ${req.url}`); }); }); From 1f82e19e2d69f4e7c688902666861a5fdafeb42e Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 20 May 2020 22:28:23 -0700 Subject: [PATCH 08/10] Type-check the ForbiddenError rather than ducktype it Going off of the error message being "Forbidden" was brittle to begin with and no longer works with Fetch due to the error message coming from jquery underpinnings that were unobserved by Ember Data's attempted recreation. --- ui/app/components/job-page/parts/latest-deployment.js | 5 ++++- ui/app/components/job-page/parts/title.js | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/ui/app/components/job-page/parts/latest-deployment.js b/ui/app/components/job-page/parts/latest-deployment.js index c9d2f3873..dacfd7497 100644 --- a/ui/app/components/job-page/parts/latest-deployment.js +++ b/ui/app/components/job-page/parts/latest-deployment.js @@ -1,5 +1,6 @@ import Component from '@ember/component'; import { task } from 'ember-concurrency'; +import { ForbiddenError } from '@ember-data/adapter/error'; import messageFromAdapterError from 'nomad-ui/utils/message-from-adapter-error'; export default Component.extend({ @@ -15,9 +16,11 @@ export default Component.extend({ yield this.get('job.latestDeployment.content').promote(); } catch (err) { let message = messageFromAdapterError(err); - if (!message || message === 'Forbidden') { + + if (err instanceof ForbiddenError) { message = 'Your ACL token does not grant permission to promote deployments.'; } + this.handleError({ title: 'Could Not Promote Deployment', description: message, diff --git a/ui/app/components/job-page/parts/title.js b/ui/app/components/job-page/parts/title.js index 2e65dfc25..b13702d64 100644 --- a/ui/app/components/job-page/parts/title.js +++ b/ui/app/components/job-page/parts/title.js @@ -1,5 +1,6 @@ import Component from '@ember/component'; import { task } from 'ember-concurrency'; +import { ForbiddenError } from '@ember-data/adapter/error'; import messageFromAdapterError from 'nomad-ui/utils/message-from-adapter-error'; export default Component.extend({ @@ -38,7 +39,8 @@ export default Component.extend({ job.set('status', 'running'); } catch (err) { let message = messageFromAdapterError(err); - if (!message || message === 'Forbidden') { + + if (err instanceof ForbiddenError) { message = 'Your ACL token does not grant permission to stop jobs.'; } From 6274282948facf2deecc17e20d6be054e4ad8077 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 20 May 2020 22:29:59 -0700 Subject: [PATCH 09/10] Fix latent race condition in client detail test Adding this settled makes this test pass now that Ember Data is using fetch instead of jquery. The test was presumably always incorrect but never flaked. --- ui/tests/acceptance/client-detail-test.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ui/tests/acceptance/client-detail-test.js b/ui/tests/acceptance/client-detail-test.js index 5c0834401..104429cb6 100644 --- a/ui/tests/acceptance/client-detail-test.js +++ b/ui/tests/acceptance/client-detail-test.js @@ -1,4 +1,4 @@ -import { currentURL, waitUntil } from '@ember/test-helpers'; +import { currentURL, waitUntil, settled } from '@ember/test-helpers'; import { assign } from '@ember/polyfills'; import { module, test } from 'qunit'; import { setupApplicationTest } from 'ember-qunit'; @@ -569,6 +569,8 @@ module('Acceptance | client detail', function(hooks) { assert.ok(ClientDetail.eligibilityToggle.isDisabled); server.pretender.resolve(server.pretender.requestReferences[0].request); + await settled(); + assert.notOk(ClientDetail.eligibilityToggle.isActive); assert.notOk(ClientDetail.eligibilityToggle.isDisabled); From 262c558e1c2fe2b607341b5fbe64e4777b019621 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 20 May 2020 22:31:12 -0700 Subject: [PATCH 10/10] Replace nulls with empty strings to have an empty response body --- ui/tests/integration/job-page/periodic-test.js | 6 +++--- ui/tests/integration/job-page/service-test.js | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ui/tests/integration/job-page/periodic-test.js b/ui/tests/integration/job-page/periodic-test.js index 06a648200..9c9ccfc28 100644 --- a/ui/tests/integration/job-page/periodic-test.js +++ b/ui/tests/integration/job-page/periodic-test.js @@ -83,7 +83,7 @@ module('Integration | Component | job-page/periodic', function(hooks) { }); test('Clicking force launch without proper permissions shows an error message', async function(assert) { - this.server.pretender.post('/v1/job/:id/periodic/force', () => [403, {}, null]); + this.server.pretender.post('/v1/job/:id/periodic/force', () => [403, {}, '']); this.server.create('job', 'periodic', { id: 'parent', @@ -138,7 +138,7 @@ module('Integration | Component | job-page/periodic', function(hooks) { }); test('Stopping a job without proper permissions shows an error message', async function(assert) { - this.server.pretender.delete('/v1/job/:id', () => [403, {}, null]); + this.server.pretender.delete('/v1/job/:id', () => [403, {}, '']); const mirageJob = this.server.create('job', 'periodic', { childrenCount: 0, @@ -175,7 +175,7 @@ module('Integration | Component | job-page/periodic', function(hooks) { }); test('Starting a job without proper permissions shows an error message', async function(assert) { - this.server.pretender.post('/v1/job/:id', () => [403, {}, null]); + this.server.pretender.post('/v1/job/:id', () => [403, {}, '']); const mirageJob = this.server.create('job', 'periodic', { childrenCount: 0, diff --git a/ui/tests/integration/job-page/service-test.js b/ui/tests/integration/job-page/service-test.js index 84082389e..abc218aa1 100644 --- a/ui/tests/integration/job-page/service-test.js +++ b/ui/tests/integration/job-page/service-test.js @@ -69,7 +69,7 @@ module('Integration | Component | job-page/service', function(hooks) { }); test('Stopping a job without proper permissions shows an error message', async function(assert) { - this.server.pretender.delete('/v1/job/:id', () => [403, {}, null]); + this.server.pretender.delete('/v1/job/:id', () => [403, {}, '']); const mirageJob = makeMirageJob(this.server); await this.store.findAll('job'); @@ -97,7 +97,7 @@ module('Integration | Component | job-page/service', function(hooks) { }); test('Starting a job without proper permissions shows an error message', async function(assert) { - this.server.pretender.post('/v1/job/:id', () => [403, {}, null]); + this.server.pretender.post('/v1/job/:id', () => [403, {}, '']); const mirageJob = makeMirageJob(this.server, { status: 'dead' }); await this.store.findAll('job'); @@ -189,7 +189,7 @@ module('Integration | Component | job-page/service', function(hooks) { }); test('When promoting the active deployment fails, an error is shown', async function(assert) { - this.server.pretender.post('/v1/deployment/promote/:id', () => [403, {}, null]); + this.server.pretender.post('/v1/deployment/promote/:id', () => [403, {}, '']); this.server.create('node'); const mirageJob = makeMirageJob(this.server, { activeDeployment: true });