ui: refactor job client summary and related job details components

hashicorp · Dec 16, 2021 · e5e6fea · e5e6fea
1 parent dde13e7
commit e5e6fea
Show file tree

Hide file tree

Showing 14 changed files with 80 additions and 85 deletions.
diff --git a/ui/app/components/job-page/parameterized-child.js b/ui/app/components/job-page/parameterized-child.js
@@ -1,14 +1,11 @@
 import { computed } from '@ember/object';
 import { alias } from '@ember/object/computed';
-import { inject as service } from '@ember/service';
 import PeriodicChildJobPage from './periodic-child';
 import classic from 'ember-classic-decorator';
-import jobClientStatus from 'nomad-ui/utils/properties/job-client-status';
 
 @classic
 export default class ParameterizedChild extends PeriodicChildJobPage {
   @alias('job.decodedPayload') payload;
-  @service store;
 
   @computed('payload')
   get payloadJSON() {
@@ -20,10 +17,4 @@ export default class ParameterizedChild extends PeriodicChildJobPage {
     }
     return json;
   }
-
-  @jobClientStatus('nodes', 'job') jobClientStatus;
-
-  get nodes() {
-    return this.store.peekAll('node');
-  }
 }
diff --git a/ui/app/components/job-page/parts/job-client-status-summary.js b/ui/app/components/job-page/parts/job-client-status-summary.js
@@ -2,12 +2,13 @@ import Component from '@ember/component';
 import { action, computed } from '@ember/object';
 import { classNames } from '@ember-decorators/component';
 import classic from 'ember-classic-decorator';
+import jobClientStatus from 'nomad-ui/utils/properties/job-client-status';
 
 @classic
 @classNames('boxed-section')
 export default class JobClientStatusSummary extends Component {
   job = null;
-  jobClientStatus = null;
+  nodes = null;
   forceCollapsed = false;
   gotoClients() {}
 
@@ -19,6 +20,8 @@ export default class JobClientStatusSummary extends Component {
     return storageValue != null ? JSON.parse(storageValue) : true;
   }
 
+  @jobClientStatus('nodes', 'job') jobClientStatus;
+
   @action
   onSliceClick(ev, slice) {
     this.gotoClients([slice.className.camelize()]);

diff --git a/ui/app/components/job-page/periodic-child.js b/ui/app/components/job-page/periodic-child.js
@@ -1,13 +1,9 @@
 import AbstractJobPage from './abstract';
 import { computed } from '@ember/object';
-import { inject as service } from '@ember/service';
 import classic from 'ember-classic-decorator';
-import jobClientStatus from 'nomad-ui/utils/properties/job-client-status';
 
 @classic
 export default class PeriodicChild extends AbstractJobPage {
-  @service store;
-
   @computed('job.{name,id}', 'job.parent.{name,id}')
   get breadcrumbs() {
     const job = this.job;
@@ -25,10 +21,4 @@ export default class PeriodicChild extends AbstractJobPage {
       },
     ];
   }
-
-  @jobClientStatus('nodes', 'job') jobClientStatus;
-
-  get nodes() {
-    return this.store.peekAll('node');
-  }
 }
diff --git a/ui/app/components/job-page/sysbatch.js b/ui/app/components/job-page/sysbatch.js
@@ -1,15 +1,5 @@
 import AbstractJobPage from './abstract';
 import classic from 'ember-classic-decorator';
-import { inject as service } from '@ember/service';
-import jobClientStatus from 'nomad-ui/utils/properties/job-client-status';
 
 @classic
-export default class Sysbatch extends AbstractJobPage {
-  @service store;
-
-  @jobClientStatus('nodes', 'job') jobClientStatus;
-
-  get nodes() {
-    return this.store.peekAll('node');
-  }
-}
+export default class Sysbatch extends AbstractJobPage {}
diff --git a/ui/app/components/job-page/system.js b/ui/app/components/job-page/system.js
@@ -1,15 +1,5 @@
 import AbstractJobPage from './abstract';
 import classic from 'ember-classic-decorator';
-import { inject as service } from '@ember/service';
-import jobClientStatus from 'nomad-ui/utils/properties/job-client-status';
 
 @classic
-export default class System extends AbstractJobPage {
-  @service store;
-
-  @jobClientStatus('nodes', 'job') jobClientStatus;
-
-  get nodes() {
-    return this.store.peekAll('node');
-  }
-}
+export default class System extends AbstractJobPage {}
diff --git a/ui/app/controllers/jobs/job/index.js b/ui/app/controllers/jobs/job/index.js
@@ -1,5 +1,5 @@
 import { inject as service } from '@ember/service';
-import { alias } from '@ember/object/computed';
+import { computed } from '@ember/object';
 import Controller from '@ember/controller';
 import WithNamespaceResetting from 'nomad-ui/mixins/with-namespace-resetting';
 import { action } from '@ember/object';
@@ -23,7 +23,15 @@ export default class IndexController extends Controller.extend(WithNamespaceRese
 
   currentPage = 1;
 
-  @alias('model') job;
+  @computed('model.job')
+  get job() {
+    return this.model.job;
+  }
+
+  @computed('model.nodes')
+  get nodes() {
+    return this.model.nodes;
+  }
 
   sortProperty = 'name';
   sortDescending = false;

diff --git a/ui/app/routes/jobs/job/index.js b/ui/app/routes/jobs/job/index.js
@@ -11,42 +11,48 @@ import WithWatchers from 'nomad-ui/mixins/with-watchers';
 
 export default class IndexRoute extends Route.extend(WithWatchers) {
   @service can;
+  @service store;
 
   async model() {
-    return this.modelFor('jobs.job');
-  }
+    const job = this.modelFor('jobs.job');
+    if (!job) {
+      return { job, nodes: [] };
+    }
 
-  async afterModel(model) {
     // Optimizing future node look ups by preemptively loading all nodes if
     // necessary and allowed.
-    if (model && model.get('hasClientStatus') && this.can.can('read client')) {
+    if (this.can.can('read client') && job.get('hasClientStatus')) {
       await this.store.findAll('node');
     }
+    const nodes = this.store.peekAll('node');
+    return { job, nodes };
   }
 
   startWatchers(controller, model) {
-    if (!model) {
+    if (!model.job) {
       return;
     }
     controller.set('watchers', {
-      model: this.watch.perform(model),
-      summary: this.watchSummary.perform(model.get('summary')),
-      allocations: this.watchAllocations.perform(model),
-      evaluations: this.watchEvaluations.perform(model),
+      model: this.watch.perform(model.job),
+      summary: this.watchSummary.perform(model.job.get('summary')),
+      allocations: this.watchAllocations.perform(model.job),
+      evaluations: this.watchEvaluations.perform(model.job),
       latestDeployment:
-        model.get('supportsDeployments') && this.watchLatestDeployment.perform(model),
+        model.job.get('supportsDeployments') && this.watchLatestDeployment.perform(model.job),
       list:
-        model.get('hasChildren') &&
-        this.watchAllJobs.perform({ namespace: model.namespace.get('name') }),
+        model.job.get('hasChildren') &&
+        this.watchAllJobs.perform({ namespace: model.job.namespace.get('name') }),
       nodes:
-        this.can.can('read client') && model.get('hasClientStatus') && this.watchNodes.perform(),
+        this.can.can('read client') &&
+        model.job.get('hasClientStatus') &&
+        this.watchNodes.perform(),
     });
   }
 
   setupController(controller, model) {
     // Parameterized and periodic detail pages, which list children jobs,
     // should sort by submit time.
-    if (model && ['periodic', 'parameterized'].includes(model.templateType)) {
+    if (model.job && ['periodic', 'parameterized'].includes(model.job.templateType)) {
       controller.setProperties({
         sortProperty: 'submitTime',
         sortDescending: true,

diff --git a/ui/app/templates/components/job-page/parameterized-child.hbs b/ui/app/templates/components/job-page/parameterized-child.hbs
@@ -17,7 +17,7 @@
   {{#if this.job.hasClientStatus}}
     <JobPage::Parts::JobClientStatusSummary
       @job={{this.job}}
-      @jobClientStatus={{if this.shouldDisplayClientInformation this.jobClientStatus}}
+      @nodes={{this.nodes}}
       @forceCollapsed={{not this.shouldDisplayClientInformation}}
       @gotoClients={{this.gotoClients}} />
   {{/if}}

diff --git a/ui/app/templates/components/job-page/periodic-child.hbs b/ui/app/templates/components/job-page/periodic-child.hbs
@@ -17,7 +17,7 @@
   {{#if this.job.hasClientStatus}}
     <JobPage::Parts::JobClientStatusSummary
       @job={{this.job}}
-      @jobClientStatus={{if this.shouldDisplayClientInformation this.jobClientStatus}}
+      @nodes={{this.nodes}}
       @forceCollapsed={{not this.shouldDisplayClientInformation}}
       @gotoClients={{this.gotoClients}} />
   {{/if}}

diff --git a/ui/app/templates/components/job-page/sysbatch.hbs b/ui/app/templates/components/job-page/sysbatch.hbs
@@ -7,7 +7,7 @@
 
   <JobPage::Parts::JobClientStatusSummary
     @job={{this.job}}
-    @jobClientStatus={{if this.shouldDisplayClientInformation this.jobClientStatus}}
+    @nodes={{this.nodes}}
     @forceCollapsed={{not this.shouldDisplayClientInformation}}
     @gotoClients={{this.gotoClients}} />
 

diff --git a/ui/app/templates/components/job-page/system.hbs b/ui/app/templates/components/job-page/system.hbs
@@ -13,7 +13,7 @@
 
   <JobPage::Parts::JobClientStatusSummary
     @job={{this.job}}
-    @jobClientStatus={{if this.shouldDisplayClientInformation this.jobClientStatus}}
+    @nodes={{this.nodes}}
     @forceCollapsed={{not this.shouldDisplayClientInformation}}
     @gotoClients={{this.gotoClients}} />
 

diff --git a/ui/app/templates/jobs/job/index.hbs b/ui/app/templates/jobs/job/index.hbs
@@ -1,6 +1,7 @@
-{{page-title "Job " this.model.name}}
-{{component (concat "job-page/" this.model.templateType)
-  job=this.model
+{{page-title "Job " this.job.name}}
+{{component (concat "job-page/" this.job.templateType)
+  job=this.job
+  nodes=this.nodes
   sortProperty=this.sortProperty
   sortDescending=this.sortDescending
   currentPage=this.currentPage

diff --git a/ui/app/utils/properties/job-client-status.js b/ui/app/utils/properties/job-client-status.js
@@ -35,7 +35,7 @@ export default function jobClientStatus(nodesKey, jobKey) {
       // Group the job allocations by the ID of the client that is running them.
       const allocsByNodeID = {};
       job.allocations.forEach(a => {
-        const nodeId = a.node.get('id');
+        const nodeId = a.belongsTo('node').id();
         if (!allocsByNodeID[nodeId]) {
           allocsByNodeID[nodeId] = [];
         }

diff --git a/ui/tests/unit/utils/job-client-status-test.js b/ui/tests/unit/utils/job-client-status-test.js
@@ -35,14 +35,30 @@ class NodeMock {
   }
 }
 
+class AllocationMock {
+  constructor(node, clientStatus) {
+    this.node = node;
+    this.clientStatus = clientStatus;
+  }
+
+  belongsTo() {
+    const self = this;
+    return {
+      id() {
+        return self.node.id;
+      },
+    };
+  }
+}
+
 module('Unit | Util | JobClientStatus', function() {
   test('it handles the case where all nodes are running', async function(assert) {
     const node = new NodeMock('node-1', 'dc1');
     const nodes = [node];
     const job = {
       datacenters: ['dc1'],
       status: 'running',
-      allocations: [{ node, clientStatus: 'running' }],
+      allocations: [new AllocationMock(node, 'running')],
       taskGroups: [{}],
     };
     const expected = {
@@ -75,9 +91,9 @@ module('Unit | Util | JobClientStatus', function() {
       datacenters: ['dc1'],
       status: 'running',
       allocations: [
-        { node, clientStatus: 'running' },
-        { node, clientStatus: 'failed' },
-        { node, clientStatus: 'running' },
+        new AllocationMock(node, 'running'),
+        new AllocationMock(node, 'failed'),
+        new AllocationMock(node, 'running'),
       ],
       taskGroups: [{}, {}, {}],
     };
@@ -111,9 +127,9 @@ module('Unit | Util | JobClientStatus', function() {
       datacenters: ['dc1'],
       status: 'running',
       allocations: [
-        { node, clientStatus: 'lost' },
-        { node, clientStatus: 'lost' },
-        { node, clientStatus: 'lost' },
+        new AllocationMock(node, 'lost'),
+        new AllocationMock(node, 'lost'),
+        new AllocationMock(node, 'lost'),
       ],
       taskGroups: [{}, {}, {}],
     };
@@ -147,9 +163,9 @@ module('Unit | Util | JobClientStatus', function() {
       datacenters: ['dc1'],
       status: 'running',
       allocations: [
-        { node, clientStatus: 'failed' },
-        { node, clientStatus: 'failed' },
-        { node, clientStatus: 'failed' },
+        new AllocationMock(node, 'failed'),
+        new AllocationMock(node, 'failed'),
+        new AllocationMock(node, 'failed'),
       ],
       taskGroups: [{}, {}, {}],
     };
@@ -183,9 +199,9 @@ module('Unit | Util | JobClientStatus', function() {
       datacenters: ['dc1'],
       status: 'running',
       allocations: [
-        { node, clientStatus: 'running' },
-        { node, clientStatus: 'running' },
-        { node, clientStatus: 'running' },
+        new AllocationMock(node, 'running'),
+        new AllocationMock(node, 'running'),
+        new AllocationMock(node, 'running'),
       ],
       taskGroups: [{}, {}, {}, {}],
     };
@@ -251,9 +267,9 @@ module('Unit | Util | JobClientStatus', function() {
       datacenters: ['dc1'],
       status: 'pending',
       allocations: [
-        { node, clientStatus: 'starting' },
-        { node, clientStatus: 'starting' },
-        { node, clientStatus: 'starting' },
+        new AllocationMock(node, 'starting'),
+        new AllocationMock(node, 'starting'),
+        new AllocationMock(node, 'starting'),
       ],
       taskGroups: [{}, {}, {}, {}],
     };
@@ -288,9 +304,9 @@ module('Unit | Util | JobClientStatus', function() {
       datacenters: ['dc1'],
       status: 'running',
       allocations: [
-        { node: node1, clientStatus: 'running' },
-        { node: node2, clientStatus: 'failed' },
-        { node: node1, clientStatus: 'running' },
+        new AllocationMock(node1, 'running'),
+        new AllocationMock(node2, 'failed'),
+        new AllocationMock(node1, 'running'),
       ],
       taskGroups: [{}, {}],
     };