From 1e00702624f4d5b9809dc1c5d93940ea9eb90674 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 2 Nov 2018 18:24:33 -0700 Subject: [PATCH 1/5] Handle the job 404 case in task group routes --- ui/app/routes/jobs/job/task-group.js | 48 ++++++++++++++----- ui/tests/acceptance/task-group-detail-test.js | 32 +++++++++++++ ui/tests/pages/jobs/job/task-group.js | 7 +++ 3 files changed, 74 insertions(+), 13 deletions(-) diff --git a/ui/app/routes/jobs/job/task-group.js b/ui/app/routes/jobs/job/task-group.js index a219f252f75..afdce06b324 100644 --- a/ui/app/routes/jobs/job/task-group.js +++ b/ui/app/routes/jobs/job/task-group.js @@ -1,8 +1,11 @@ import Route from '@ember/routing/route'; import { collect } from '@ember/object/computed'; +import EmberError from '@ember/error'; +import { resolve } from 'rsvp'; import { watchRecord, watchRelationship } from 'nomad-ui/utils/properties/watch'; import WithWatchers from 'nomad-ui/mixins/with-watchers'; import { qpBuilder } from 'nomad-ui/utils/classes/query-params'; +import notifyError from 'nomad-ui/utils/notify-error'; export default Route.extend(WithWatchers, { breadcrumbs(model) { @@ -20,27 +23,46 @@ export default Route.extend(WithWatchers, { }, model({ name }) { + const job = this.modelFor('jobs.job'); + + // If there is no job, then there is no task group, so handle this as a 404 + if (!job) { + const err = new EmberError(`Job for task group ${name} not found`); + err.code = '404'; + this.controllerFor('application').set('error', err); + return; + } + // If the job is a partial (from the list request) it won't have task // groups. Reload the job to ensure task groups are present. - return this.modelFor('jobs.job') - .reload() - .then(job => { + const reload = job.get('isPartial') ? job.reload() : resolve(); + return reload + .then(() => { + const taskGroup = job.get('taskGroups').findBy('name', name); + if (!taskGroup) { + const err = new EmberError(`Task group ${name} for job ${job.get('name')} not found`); + err.code = '404'; + throw err; + } + + // Refresh job allocations before-hand (so page sort works on load) return job .hasMany('allocations') .reload() - .then(() => { - return job.get('taskGroups').findBy('name', name); - }); - }); + .then(() => taskGroup); + }) + .catch(notifyError(this)); }, startWatchers(controller, model) { - const job = model.get('job'); - controller.set('watchers', { - job: this.get('watchJob').perform(job), - summary: this.get('watchSummary').perform(job.get('summary')), - allocations: this.get('watchAllocations').perform(job), - }); + if (model) { + const job = model.get('job'); + controller.set('watchers', { + job: this.get('watchJob').perform(job), + summary: this.get('watchSummary').perform(job.get('summary')), + allocations: this.get('watchAllocations').perform(job), + }); + } }, watchJob: watchRecord('job'), diff --git a/ui/tests/acceptance/task-group-detail-test.js b/ui/tests/acceptance/task-group-detail-test.js index cccdd67d71e..13f2d71badf 100644 --- a/ui/tests/acceptance/task-group-detail-test.js +++ b/ui/tests/acceptance/task-group-detail-test.js @@ -224,3 +224,35 @@ test('when the allocation has reschedule events, the allocation row is denoted w assert.ok(rescheduleRow.rescheduled, 'Reschedule row has a reschedule icon'); assert.notOk(normalRow.rescheduled, 'Normal row has no reschedule icon'); }); + +test('when the job for the task group is not found, an error message is shown, but the URL persists', function(assert) { + TaskGroup.visit({ id: 'not-a-real-job', name: 'not-a-real-task-group' }); + + andThen(() => { + assert.equal( + server.pretender.handledRequests.findBy('status', 404).url, + '/v1/job/not-a-real-job', + 'A request to the nonexistent job is made' + ); + assert.equal(currentURL(), '/jobs/not-a-real-job/not-a-real-task-group', 'The URL persists'); + assert.ok(TaskGroup.error.isPresent, 'Error message is shown'); + assert.equal(TaskGroup.error.title, 'Not Found', 'Error message is for 404'); + }); +}); + +test('when the task group is not found on the job, an error message is shown, but the URL persists', function(assert) { + TaskGroup.visit({ id: job.id, name: 'not-a-real-task-group' }); + + andThen(() => { + assert.ok( + server.pretender.handledRequests + .filterBy('status', 200) + .mapBy('url') + .includes(`/v1/job/${job.id}`), + 'A request to the job is made and succeeds' + ); + assert.equal(currentURL(), `/jobs/${job.id}/not-a-real-task-group`, 'The URL persists'); + assert.ok(TaskGroup.error.isPresent, 'Error message is shown'); + assert.equal(TaskGroup.error.title, 'Not Found', 'Error message is for 404'); + }); +}); diff --git a/ui/tests/pages/jobs/job/task-group.js b/ui/tests/pages/jobs/job/task-group.js index d20537e5a24..2830ece3715 100644 --- a/ui/tests/pages/jobs/job/task-group.js +++ b/ui/tests/pages/jobs/job/task-group.js @@ -37,6 +37,13 @@ export default create({ isEmpty: isPresent('[data-test-empty-allocations-list]'), + error: { + isPresent: isPresent('[data-test-error]'), + title: text('[data-test-error-title]'), + message: text('[data-test-error-message]'), + seekHelp: clickable('[data-test-error-message] a'), + }, + emptyState: { headline: text('[data-test-empty-allocations-list-headline]'), }, From 1226a079f91c193b68805ba3cf3e86a5c5fbf730 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Mon, 5 Nov 2018 15:20:31 -0800 Subject: [PATCH 2/5] Handle the job 404 case in the job definition route --- ui/app/routes/jobs/job/definition.js | 10 ++++++++++ ui/tests/acceptance/job-definition-test.js | 15 +++++++++++++++ ui/tests/pages/jobs/job/definition.js | 9 ++++++++- 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/ui/app/routes/jobs/job/definition.js b/ui/app/routes/jobs/job/definition.js index f6294ebf5cc..4be568a11dd 100644 --- a/ui/app/routes/jobs/job/definition.js +++ b/ui/app/routes/jobs/job/definition.js @@ -1,8 +1,18 @@ import Route from '@ember/routing/route'; +import EmberError from '@ember/error'; export default Route.extend({ model() { const job = this.modelFor('jobs.job'); + + // If there is no job, then there is no task group, so handle this as a 404 + if (!job) { + const err = new EmberError(`Job for task group ${name} not found`); + err.code = '404'; + this.controllerFor('application').set('error', err); + return; + } + return job.fetchRawDefinition().then(definition => ({ job, definition, diff --git a/ui/tests/acceptance/job-definition-test.js b/ui/tests/acceptance/job-definition-test.js index 78f65541bb0..18dbb3982a2 100644 --- a/ui/tests/acceptance/job-definition-test.js +++ b/ui/tests/acceptance/job-definition-test.js @@ -82,3 +82,18 @@ test('when changes are submitted, the site redirects to the job overview page', assert.equal(currentURL(), `/jobs/${job.id}`, 'Now on the job overview page'); }); }); + +test('when the job for the definition is not found, an error message is shown, but the URL persists', function(assert) { + Definition.visit({ id: 'not-a-real-job' }); + + andThen(() => { + assert.equal( + server.pretender.handledRequests.findBy('status', 404).url, + '/v1/job/not-a-real-job', + 'A request to the nonexistent job is made' + ); + assert.equal(currentURL(), '/jobs/not-a-real-job/definition', 'The URL persists'); + assert.ok(Definition.error.isPresent, 'Error message is shown'); + assert.equal(Definition.error.title, 'Not Found', 'Error message is for 404'); + }); +}); diff --git a/ui/tests/pages/jobs/job/definition.js b/ui/tests/pages/jobs/job/definition.js index b015019b714..16272040be8 100644 --- a/ui/tests/pages/jobs/job/definition.js +++ b/ui/tests/pages/jobs/job/definition.js @@ -1,4 +1,4 @@ -import { create, isPresent, visitable, clickable } from 'ember-cli-page-object'; +import { create, isPresent, visitable, clickable, text } from 'ember-cli-page-object'; import jobEditor from 'nomad-ui/tests/pages/components/job-editor'; @@ -9,4 +9,11 @@ export default create({ editor: jobEditor(), edit: clickable('[data-test-edit-job]'), + + error: { + isPresent: isPresent('[data-test-error]'), + title: text('[data-test-error-title]'), + message: text('[data-test-error-message]'), + seekHelp: clickable('[data-test-error-message] a'), + }, }); From cb5bd66dd6667f05d802b673d2be2ef56aa5ff1f Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Mon, 5 Nov 2018 15:42:04 -0800 Subject: [PATCH 3/5] Handle the job 404 error case in versions --- ui/app/routes/jobs/job/task-group.js | 10 +++------- ui/app/routes/jobs/job/versions.js | 6 ++++-- ui/tests/acceptance/job-versions-test.js | 15 +++++++++++++++ ui/tests/pages/jobs/job/definition.js | 8 ++------ ui/tests/pages/jobs/job/task-group.js | 8 ++------ ui/tests/pages/jobs/job/versions.js | 4 ++++ 6 files changed, 30 insertions(+), 21 deletions(-) diff --git a/ui/app/routes/jobs/job/task-group.js b/ui/app/routes/jobs/job/task-group.js index afdce06b324..42ea9090211 100644 --- a/ui/app/routes/jobs/job/task-group.js +++ b/ui/app/routes/jobs/job/task-group.js @@ -25,13 +25,9 @@ export default Route.extend(WithWatchers, { model({ name }) { const job = this.modelFor('jobs.job'); - // If there is no job, then there is no task group, so handle this as a 404 - if (!job) { - const err = new EmberError(`Job for task group ${name} not found`); - err.code = '404'; - this.controllerFor('application').set('error', err); - return; - } + // If there is no job, then there is no task group. + // Let the job route handle the 404. + if (!job) return; // If the job is a partial (from the list request) it won't have task // groups. Reload the job to ensure task groups are present. diff --git a/ui/app/routes/jobs/job/versions.js b/ui/app/routes/jobs/job/versions.js index 4e24e6a7ec9..0e1c9de4589 100644 --- a/ui/app/routes/jobs/job/versions.js +++ b/ui/app/routes/jobs/job/versions.js @@ -6,11 +6,13 @@ import WithWatchers from 'nomad-ui/mixins/with-watchers'; export default Route.extend(WithWatchers, { model() { const job = this.modelFor('jobs.job'); - return job.get('versions').then(() => job); + return job && job.get('versions').then(() => job); }, startWatchers(controller, model) { - controller.set('watcher', this.get('watchVersions').perform(model)); + if (model) { + controller.set('watcher', this.get('watchVersions').perform(model)); + } }, watchVersions: watchRelationship('versions'), diff --git a/ui/tests/acceptance/job-versions-test.js b/ui/tests/acceptance/job-versions-test.js index f046e1be78d..c98de902539 100644 --- a/ui/tests/acceptance/job-versions-test.js +++ b/ui/tests/acceptance/job-versions-test.js @@ -28,3 +28,18 @@ test('each version mentions the version number, the stability, and the submitted assert.equal(versionRow.stability, version.stable.toString(), 'Stability'); assert.equal(versionRow.submitTime, formattedSubmitTime, 'Submit time'); }); + +test('when the job for the definition is not found, an error message is shown, but the URL persists', function(assert) { + Versions.visit({ id: 'not-a-real-job' }); + + andThen(() => { + assert.equal( + server.pretender.handledRequests.findBy('status', 404).url, + '/v1/job/not-a-real-job', + 'A request to the nonexistent job is made' + ); + assert.equal(currentURL(), '/jobs/not-a-real-job/versions', 'The URL persists'); + assert.ok(Versions.error.isPresent, 'Error message is shown'); + assert.equal(Versions.error.title, 'Not Found', 'Error message is for 404'); + }); +}); diff --git a/ui/tests/pages/jobs/job/definition.js b/ui/tests/pages/jobs/job/definition.js index 16272040be8..b8529e233a2 100644 --- a/ui/tests/pages/jobs/job/definition.js +++ b/ui/tests/pages/jobs/job/definition.js @@ -1,6 +1,7 @@ import { create, isPresent, visitable, clickable, text } from 'ember-cli-page-object'; import jobEditor from 'nomad-ui/tests/pages/components/job-editor'; +import error from 'nomad-ui/tests/pages/components/error'; export default create({ visit: visitable('/jobs/:id/definition'), @@ -10,10 +11,5 @@ export default create({ edit: clickable('[data-test-edit-job]'), - error: { - isPresent: isPresent('[data-test-error]'), - title: text('[data-test-error-title]'), - message: text('[data-test-error-message]'), - seekHelp: clickable('[data-test-error-message] a'), - }, + error: error(), }); diff --git a/ui/tests/pages/jobs/job/task-group.js b/ui/tests/pages/jobs/job/task-group.js index 2830ece3715..b3e5c9342ec 100644 --- a/ui/tests/pages/jobs/job/task-group.js +++ b/ui/tests/pages/jobs/job/task-group.js @@ -10,6 +10,7 @@ import { } from 'ember-cli-page-object'; import allocations from 'nomad-ui/tests/pages/components/allocations'; +import error from 'nomad-ui/tests/pages/components/error'; export default create({ pageSize: 10, @@ -37,12 +38,7 @@ export default create({ isEmpty: isPresent('[data-test-empty-allocations-list]'), - error: { - isPresent: isPresent('[data-test-error]'), - title: text('[data-test-error-title]'), - message: text('[data-test-error-message]'), - seekHelp: clickable('[data-test-error-message] a'), - }, + error: error(), emptyState: { headline: text('[data-test-empty-allocations-list-headline]'), diff --git a/ui/tests/pages/jobs/job/versions.js b/ui/tests/pages/jobs/job/versions.js index be05539628e..1b58b9557f7 100644 --- a/ui/tests/pages/jobs/job/versions.js +++ b/ui/tests/pages/jobs/job/versions.js @@ -1,5 +1,7 @@ import { create, collection, text, visitable } from 'ember-cli-page-object'; +import error from 'nomad-ui/tests/pages/components/error'; + export default create({ visit: visitable('/jobs/:id/versions'), @@ -8,4 +10,6 @@ export default create({ stability: text('[data-test-version-stability]'), submitTime: text('[data-test-version-submit-time]'), }), + + error: error(), }); From 11ba511471e6a73ea39c267fb1b6045818178298 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Mon, 5 Nov 2018 16:06:08 -0800 Subject: [PATCH 4/5] Handle the job 404 error case on the other job sub pages --- ui/app/routes/jobs/job/allocations.js | 6 ++++-- ui/app/routes/jobs/job/definition.js | 10 +--------- ui/app/routes/jobs/job/deployments.js | 8 +++++--- ui/app/routes/jobs/job/evaluations.js | 6 ++++-- ui/tests/acceptance/job-allocations-test.js | 15 +++++++++++++++ ui/tests/acceptance/job-deployments-test.js | 15 +++++++++++++++ ui/tests/acceptance/job-evaluations-test.js | 15 +++++++++++++++ ui/tests/acceptance/job-versions-test.js | 2 +- ui/tests/pages/jobs/job/allocations.js | 3 +++ ui/tests/pages/jobs/job/definition.js | 2 +- ui/tests/pages/jobs/job/deployments.js | 3 +++ ui/tests/pages/jobs/job/evaluations.js | 4 ++++ 12 files changed, 71 insertions(+), 18 deletions(-) diff --git a/ui/app/routes/jobs/job/allocations.js b/ui/app/routes/jobs/job/allocations.js index da59c95a4e0..9b222ed8799 100644 --- a/ui/app/routes/jobs/job/allocations.js +++ b/ui/app/routes/jobs/job/allocations.js @@ -6,11 +6,13 @@ import WithWatchers from 'nomad-ui/mixins/with-watchers'; export default Route.extend(WithWatchers, { model() { const job = this.modelFor('jobs.job'); - return job.get('allocations').then(() => job); + return job && job.get('allocations').then(() => job); }, startWatchers(controller, model) { - controller.set('watchAllocations', this.get('watchAllocations').perform(model)); + if (model) { + controller.set('watchAllocations', this.get('watchAllocations').perform(model)); + } }, watchAllocations: watchRelationship('allocations'), diff --git a/ui/app/routes/jobs/job/definition.js b/ui/app/routes/jobs/job/definition.js index 4be568a11dd..90d9e6b0b52 100644 --- a/ui/app/routes/jobs/job/definition.js +++ b/ui/app/routes/jobs/job/definition.js @@ -1,17 +1,9 @@ import Route from '@ember/routing/route'; -import EmberError from '@ember/error'; export default Route.extend({ model() { const job = this.modelFor('jobs.job'); - - // If there is no job, then there is no task group, so handle this as a 404 - if (!job) { - const err = new EmberError(`Job for task group ${name} not found`); - err.code = '404'; - this.controllerFor('application').set('error', err); - return; - } + if (!job) return; return job.fetchRawDefinition().then(definition => ({ job, diff --git a/ui/app/routes/jobs/job/deployments.js b/ui/app/routes/jobs/job/deployments.js index 9f4e974106a..e5e92e46a1e 100644 --- a/ui/app/routes/jobs/job/deployments.js +++ b/ui/app/routes/jobs/job/deployments.js @@ -7,12 +7,14 @@ import WithWatchers from 'nomad-ui/mixins/with-watchers'; export default Route.extend(WithWatchers, { model() { const job = this.modelFor('jobs.job'); - return RSVP.all([job.get('deployments'), job.get('versions')]).then(() => job); + return job && RSVP.all([job.get('deployments'), job.get('versions')]).then(() => job); }, startWatchers(controller, model) { - controller.set('watchDeployments', this.get('watchDeployments').perform(model)); - controller.set('watchVersions', this.get('watchVersions').perform(model)); + if (model) { + controller.set('watchDeployments', this.get('watchDeployments').perform(model)); + controller.set('watchVersions', this.get('watchVersions').perform(model)); + } }, watchDeployments: watchRelationship('deployments'), diff --git a/ui/app/routes/jobs/job/evaluations.js b/ui/app/routes/jobs/job/evaluations.js index 640809db340..e146abd3fcb 100644 --- a/ui/app/routes/jobs/job/evaluations.js +++ b/ui/app/routes/jobs/job/evaluations.js @@ -6,11 +6,13 @@ import WithWatchers from 'nomad-ui/mixins/with-watchers'; export default Route.extend(WithWatchers, { model() { const job = this.modelFor('jobs.job'); - return job.get('evaluations').then(() => job); + return job && job.get('evaluations').then(() => job); }, startWatchers(controller, model) { - controller.set('watchEvaluations', this.get('watchEvaluations').perform(model)); + if (model) { + controller.set('watchEvaluations', this.get('watchEvaluations').perform(model)); + } }, watchEvaluations: watchRelationship('evaluations'), diff --git a/ui/tests/acceptance/job-allocations-test.js b/ui/tests/acceptance/job-allocations-test.js index 20210459906..cf58acc2ad6 100644 --- a/ui/tests/acceptance/job-allocations-test.js +++ b/ui/tests/acceptance/job-allocations-test.js @@ -108,3 +108,18 @@ test('when a search yields no results, the search box remains', function(assert) assert.ok(Allocations.hasSearchBox, 'Search box is still shown'); }); }); + +test('when the job for the allocations is not found, an error message is shown, but the URL persists', function(assert) { + Allocations.visit({ id: 'not-a-real-job' }); + + andThen(() => { + assert.equal( + server.pretender.handledRequests.findBy('status', 404).url, + '/v1/job/not-a-real-job', + 'A request to the nonexistent job is made' + ); + assert.equal(currentURL(), '/jobs/not-a-real-job/allocations', 'The URL persists'); + assert.ok(Allocations.error.isPresent, 'Error message is shown'); + assert.equal(Allocations.error.title, 'Not Found', 'Error message is for 404'); + }); +}); diff --git a/ui/tests/acceptance/job-deployments-test.js b/ui/tests/acceptance/job-deployments-test.js index 407317b37a8..ba334ebba53 100644 --- a/ui/tests/acceptance/job-deployments-test.js +++ b/ui/tests/acceptance/job-deployments-test.js @@ -237,6 +237,21 @@ test('when open, a deployment shows a list of all allocations for the deployment }); }); +test('when the job for the deployments is not found, an error message is shown, but the URL persists', function(assert) { + Deployments.visit({ id: 'not-a-real-job' }); + + andThen(() => { + assert.equal( + server.pretender.handledRequests.findBy('status', 404).url, + '/v1/job/not-a-real-job', + 'A request to the nonexistent job is made' + ); + assert.equal(currentURL(), '/jobs/not-a-real-job/deployments', 'The URL persists'); + assert.ok(Deployments.error.isPresent, 'Error message is shown'); + assert.equal(Deployments.error.title, 'Not Found', 'Error message is for 404'); + }); +}); + function classForStatus(status) { const classMap = { running: 'is-running', diff --git a/ui/tests/acceptance/job-evaluations-test.js b/ui/tests/acceptance/job-evaluations-test.js index 45c1d651ec2..271896914ad 100644 --- a/ui/tests/acceptance/job-evaluations-test.js +++ b/ui/tests/acceptance/job-evaluations-test.js @@ -45,3 +45,18 @@ test('evaluations table is sortable', function(assert) { }); }); }); + +test('when the job for the evaluations is not found, an error message is shown, but the URL persists', function(assert) { + Evaluations.visit({ id: 'not-a-real-job' }); + + andThen(() => { + assert.equal( + server.pretender.handledRequests.findBy('status', 404).url, + '/v1/job/not-a-real-job', + 'A request to the nonexistent job is made' + ); + assert.equal(currentURL(), '/jobs/not-a-real-job/evaluations', 'The URL persists'); + assert.ok(Evaluations.error.isPresent, 'Error message is shown'); + assert.equal(Evaluations.error.title, 'Not Found', 'Error message is for 404'); + }); +}); diff --git a/ui/tests/acceptance/job-versions-test.js b/ui/tests/acceptance/job-versions-test.js index c98de902539..067787c4803 100644 --- a/ui/tests/acceptance/job-versions-test.js +++ b/ui/tests/acceptance/job-versions-test.js @@ -29,7 +29,7 @@ test('each version mentions the version number, the stability, and the submitted assert.equal(versionRow.submitTime, formattedSubmitTime, 'Submit time'); }); -test('when the job for the definition is not found, an error message is shown, but the URL persists', function(assert) { +test('when the job for the versions is not found, an error message is shown, but the URL persists', function(assert) { Versions.visit({ id: 'not-a-real-job' }); andThen(() => { diff --git a/ui/tests/pages/jobs/job/allocations.js b/ui/tests/pages/jobs/job/allocations.js index 65ca46f5b92..adb48aa704e 100644 --- a/ui/tests/pages/jobs/job/allocations.js +++ b/ui/tests/pages/jobs/job/allocations.js @@ -10,6 +10,7 @@ import { } from 'ember-cli-page-object'; import allocations from 'nomad-ui/tests/pages/components/allocations'; +import error from 'nomad-ui/tests/pages/components/error'; export default create({ visit: visitable('/jobs/:id/allocations'), @@ -37,4 +38,6 @@ export default create({ .findBy('id', id) .sort(); }, + + error: error(), }); diff --git a/ui/tests/pages/jobs/job/definition.js b/ui/tests/pages/jobs/job/definition.js index b8529e233a2..6bf8b5b64df 100644 --- a/ui/tests/pages/jobs/job/definition.js +++ b/ui/tests/pages/jobs/job/definition.js @@ -1,4 +1,4 @@ -import { create, isPresent, visitable, clickable, text } 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'; import error from 'nomad-ui/tests/pages/components/error'; diff --git a/ui/tests/pages/jobs/job/deployments.js b/ui/tests/pages/jobs/job/deployments.js index fe7d2ed4ced..3a75faf320a 100644 --- a/ui/tests/pages/jobs/job/deployments.js +++ b/ui/tests/pages/jobs/job/deployments.js @@ -9,6 +9,7 @@ import { } from 'ember-cli-page-object'; import allocations from 'nomad-ui/tests/pages/components/allocations'; +import error from 'nomad-ui/tests/pages/components/error'; export default create({ visit: visitable('/jobs/:id/deployments'), @@ -51,4 +52,6 @@ export default create({ ...allocations('[data-test-deployment-allocation]'), hasAllocations: isPresent('[data-test-deployment-allocations]'), }), + + error: error(), }); diff --git a/ui/tests/pages/jobs/job/evaluations.js b/ui/tests/pages/jobs/job/evaluations.js index 7d6a30ce7fb..79200460ac2 100644 --- a/ui/tests/pages/jobs/job/evaluations.js +++ b/ui/tests/pages/jobs/job/evaluations.js @@ -1,5 +1,7 @@ import { attribute, clickable, create, collection, text, visitable } from 'ember-cli-page-object'; +import error from 'nomad-ui/tests/pages/components/error'; + export default create({ visit: visitable('/jobs/:id/evaluations'), @@ -18,4 +20,6 @@ export default create({ .findBy('id', id) .sort(); }, + + error: error(), }); From 705efadfe7f99ccdd8605e16fdbc9c14d4dfb441 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Mon, 5 Nov 2018 16:33:33 -0800 Subject: [PATCH 5/5] Clean up the remaining routes --- ui/app/routes/allocations/allocation.js | 4 +++- ui/app/routes/allocations/allocation/task.js | 13 ++++++++----- ui/app/routes/allocations/allocation/task/logs.js | 2 +- ui/app/routes/clients/client.js | 6 ++++-- 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/ui/app/routes/allocations/allocation.js b/ui/app/routes/allocations/allocation.js index 9a73e184ac4..0932852837f 100644 --- a/ui/app/routes/allocations/allocation.js +++ b/ui/app/routes/allocations/allocation.js @@ -8,7 +8,9 @@ import { jobCrumbs } from 'nomad-ui/utils/breadcrumb-utils'; export default Route.extend(WithWatchers, { startWatchers(controller, model) { - controller.set('watcher', this.get('watch').perform(model)); + if (model) { + controller.set('watcher', this.get('watch').perform(model)); + } }, // Allocation breadcrumbs extend from job / task group breadcrumbs diff --git a/ui/app/routes/allocations/allocation/task.js b/ui/app/routes/allocations/allocation/task.js index c3379729054..d06a7b978ce 100644 --- a/ui/app/routes/allocations/allocation/task.js +++ b/ui/app/routes/allocations/allocation/task.js @@ -17,16 +17,19 @@ export default Route.extend({ model({ name }) { const allocation = this.modelFor('allocations.allocation'); - if (allocation) { - const task = allocation.get('states').findBy('name', name); - if (task) { - return task; - } + // If there is no allocation, then there is no task. + // Let the allocation route handle the 404 error. + if (!allocation) return; + const task = allocation.get('states').findBy('name', name); + + if (!task) { const err = new EmberError(`Task ${name} not found for allocation ${allocation.get('id')}`); err.code = '404'; this.controllerFor('application').set('error', err); } + + return task; }, }); diff --git a/ui/app/routes/allocations/allocation/task/logs.js b/ui/app/routes/allocations/allocation/task/logs.js index 399258f2120..3b5ad231248 100644 --- a/ui/app/routes/allocations/allocation/task/logs.js +++ b/ui/app/routes/allocations/allocation/task/logs.js @@ -3,6 +3,6 @@ import Route from '@ember/routing/route'; export default Route.extend({ model() { const task = this._super(...arguments); - return task.get('allocation.node').then(() => task); + return task && task.get('allocation.node').then(() => task); }, }); diff --git a/ui/app/routes/clients/client.js b/ui/app/routes/clients/client.js index 9e249303003..8af73750342 100644 --- a/ui/app/routes/clients/client.js +++ b/ui/app/routes/clients/client.js @@ -30,8 +30,10 @@ export default Route.extend(WithWatchers, { }, startWatchers(controller, model) { - controller.set('watchModel', this.get('watch').perform(model)); - controller.set('watchAllocations', this.get('watchAllocations').perform(model)); + if (model) { + controller.set('watchModel', this.get('watch').perform(model)); + controller.set('watchAllocations', this.get('watchAllocations').perform(model)); + } }, watch: watchRecord('node'),