Merge pull request #4841 from hashicorp/f-ui-error-handling-on-all-pages

UI: Error handling on all pages

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

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;
   },
 });

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);
   },
 });

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'),

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'),

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,

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'),

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'),

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'),

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'),

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');
+  });
+});

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');
+  });
+});

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',

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');
+  });
+});

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');
+  });
+});

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');
+  });
+});

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(),
 });

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(),
 });

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(),
 });

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(),
 });

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]'),
   },

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(),
 });