From 007cafac63e0c3984338053352ccadfd3b396a5d Mon Sep 17 00:00:00 2001
From: Michael Lange <dingoeatingfuzz@gmail.com>
Date: Tue, 24 Oct 2017 12:35:29 -0700
Subject: [PATCH] Don't show the deployments tab for non-service jobs

They can't have deployments, so the link makes no sense.
---
 ui/app/models/job.js                   |  2 ++
 ui/app/templates/jobs/job/subnav.hbs   |  4 +++-
 ui/tests/acceptance/job-detail-test.js | 30 +++++++++++++++++++++-----
 3 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/ui/app/models/job.js b/ui/app/models/job.js
index 4e1e3a8e3..d44f88e00 100644
--- a/ui/app/models/job.js
+++ b/ui/app/models/job.js
@@ -54,6 +54,8 @@ export default Model.extend({
   deployments: hasMany('deployments'),
   namespace: belongsTo('namespace'),
 
+  supportsDeployments: computed.equal('type', 'service'),
+
   runningDeployment: computed('deployments.@each.status', function() {
     return this.get('deployments').findBy('status', 'running');
   }),
diff --git a/ui/app/templates/jobs/job/subnav.hbs b/ui/app/templates/jobs/job/subnav.hbs
index 77e44a2f1..63e712a6c 100644
--- a/ui/app/templates/jobs/job/subnav.hbs
+++ b/ui/app/templates/jobs/job/subnav.hbs
@@ -3,6 +3,8 @@
     <li>{{#link-to "jobs.job.index" job activeClass="is-active"}}Overview{{/link-to}}</li>
     <li>{{#link-to "jobs.job.definition" job activeClass="is-active"}}Definition{{/link-to}}</li>
     <li>{{#link-to "jobs.job.versions" job activeClass="is-active"}}Versions{{/link-to}}</li>
-    <li>{{#link-to "jobs.job.deployments" job activeClass="is-active"}}Deployments{{/link-to}}</li>
+    {{#if job.supportsDeployments}}
+      <li>{{#link-to "jobs.job.deployments" job activeClass="is-active"}}Deployments{{/link-to}}</li>
+    {{/if}}
   </ul>
 </div>
diff --git a/ui/tests/acceptance/job-detail-test.js b/ui/tests/acceptance/job-detail-test.js
index 140267d6a..b09633969 100644
--- a/ui/tests/acceptance/job-detail-test.js
+++ b/ui/tests/acceptance/job-detail-test.js
@@ -12,8 +12,7 @@ let job;
 moduleForAcceptance('Acceptance | job detail', {
   beforeEach() {
     server.create('node');
-    server.create('job');
-    job = server.db.jobs[0];
+    job = server.create('job', { type: 'service' });
     visit(`/jobs/${job.id}`);
   },
 });
@@ -36,6 +35,27 @@ test('breadcrumbs includes job name and link back to the jobs list', function(as
   });
 });
 
+test('the subnav includes links to definition, versions, and deployments when type = service', function(
+  assert
+) {
+  const subnavLabels = findAll('.tabs.is-subnav a').map(anchor => anchor.textContent);
+  assert.ok(subnavLabels.some(label => label === 'Definition'), 'Definition link');
+  assert.ok(subnavLabels.some(label => label === 'Versions'), 'Versions link');
+  assert.ok(subnavLabels.some(label => label === 'Deployments'), 'Deployments link');
+});
+
+test('the subnav includes links to definition and versions when type != service', function(assert) {
+  job = server.create('job', { type: 'batch' });
+  visit(`/jobs/${job.id}`);
+
+  andThen(() => {
+    const subnavLabels = findAll('.tabs.is-subnav a').map(anchor => anchor.textContent);
+    assert.ok(subnavLabels.some(label => label === 'Definition'), 'Definition link');
+    assert.ok(subnavLabels.some(label => label === 'Versions'), 'Versions link');
+    assert.notOk(subnavLabels.some(label => label === 'Deployments'), 'Deployments link');
+  });
+});
+
 test('the job detail page should contain basic information about the job', function(assert) {
   assert.ok(findAll('.title .tag')[0].textContent.includes(job.status), 'Status');
   assert.ok(findAll('.job-stats span')[0].textContent.includes(job.type), 'Type');
@@ -136,7 +156,7 @@ test('there is no active deployment section when the job has no active deploymen
 ) {
   // TODO: it would be better to not visit two different job pages in one test, but this
   // way is much more convenient.
-  job = server.create('job', { noActiveDeployment: true });
+  job = server.create('job', { noActiveDeployment: true, type: 'service' });
   visit(`/jobs/${job.id}`);
 
   andThen(() => {
@@ -147,7 +167,7 @@ test('there is no active deployment section when the job has no active deploymen
 test('the active deployment section shows up for the currently running deployment', function(
   assert
 ) {
-  job = server.create('job', { activeDeployment: true });
+  job = server.create('job', { activeDeployment: true, type: 'service' });
   const deployment = server.db.deployments.where({ jobId: job.id })[0];
   const taskGroupSummaries = server.db.deploymentTaskGroupSummaries.where({
     deploymentId: deployment.id,
@@ -232,7 +252,7 @@ test('the active deployment section shows up for the currently running deploymen
 test('the active deployment section can be expanded to show task groups and allocations', function(
   assert
 ) {
-  job = server.create('job', { activeDeployment: true });
+  job = server.create('job', { activeDeployment: true, type: 'service' });
   visit(`/jobs/${job.id}`);
 
   andThen(() => {