diff --git a/ui/app/routes/allocations/allocation.js b/ui/app/routes/allocations/allocation.js index 7cdc67537..ec07efe25 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 c33797290..d06a7b978 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 399258f21..3b5ad2312 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 9e2493030..8af737503 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'), diff --git a/ui/app/routes/jobs/job/allocations.js b/ui/app/routes/jobs/job/allocations.js index da59c95a4..9b222ed87 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 f6294ebf5..90d9e6b0b 100644 --- a/ui/app/routes/jobs/job/definition.js +++ b/ui/app/routes/jobs/job/definition.js @@ -3,6 +3,8 @@ import Route from '@ember/routing/route'; export default Route.extend({ model() { const job = this.modelFor('jobs.job'); + if (!job) return; + return job.fetchRawDefinition().then(definition => ({ job, definition, diff --git a/ui/app/routes/jobs/job/deployments.js b/ui/app/routes/jobs/job/deployments.js index 9f4e97410..e5e92e46a 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 640809db3..e146abd3f 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/app/routes/jobs/job/task-group.js b/ui/app/routes/jobs/job/task-group.js index da17981c5..ae7bca406 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) { @@ -21,27 +24,42 @@ export default Route.extend(WithWatchers, { }, model({ name }) { + const job = this.modelFor('jobs.job'); + + // 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. - 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/app/routes/jobs/job/versions.js b/ui/app/routes/jobs/job/versions.js index 4e24e6a7e..0e1c9de45 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-allocations-test.js b/ui/tests/acceptance/job-allocations-test.js index 202104599..cf58acc2a 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-definition-test.js b/ui/tests/acceptance/job-definition-test.js index 78f65541b..18dbb3982 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/acceptance/job-deployments-test.js b/ui/tests/acceptance/job-deployments-test.js index 407317b37..ba334ebba 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 45c1d651e..271896914 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 f046e1be7..067787c48 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 versions 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/acceptance/task-group-detail-test.js b/ui/tests/acceptance/task-group-detail-test.js index cccdd67d7..13f2d71ba 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/allocations.js b/ui/tests/pages/jobs/job/allocations.js index 65ca46f5b..adb48aa70 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 b015019b7..6bf8b5b64 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 } 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'), @@ -9,4 +10,6 @@ export default create({ editor: jobEditor(), edit: clickable('[data-test-edit-job]'), + + error: error(), }); diff --git a/ui/tests/pages/jobs/job/deployments.js b/ui/tests/pages/jobs/job/deployments.js index fe7d2ed4c..3a75faf32 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 7d6a30ce7..79200460a 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(), }); diff --git a/ui/tests/pages/jobs/job/task-group.js b/ui/tests/pages/jobs/job/task-group.js index d20537e5a..b3e5c9342 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,6 +38,8 @@ export default create({ isEmpty: isPresent('[data-test-empty-allocations-list]'), + 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 be0553962..1b58b9557 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(), });