From 92abbbacc1b795a2056951ee837404c6d15a7ccd Mon Sep 17 00:00:00 2001
From: Kenia <19161242+kaxcode@users.noreply.github.com>
Date: Fri, 23 Oct 2020 09:45:10 -0400
Subject: [PATCH] ui: Prevent redirect to topology url and hide Topology tab if
 service has no services (#9008)

* Prevent redirect to topology url and hide Topology tab if service has no proxies

* Remove unused computed function from topology model

* Fix up tests

* Remove use of Exists computed function

* Add tests for hiding topology tab
---
 ui/packages/consul-ui/app/models/topology.js  |  4 --
 .../app/routes/dc/services/show/index.js      | 36 +++++++++--------
 .../app/templates/dc/services/show.hbs        |  2 +-
 .../dc/services/show-topology.feature         | 40 +++++++++++++++++++
 .../steps/dc/services/show-topology-steps.js  | 10 +++++
 .../consul-ui/tests/pages/dc/services/show.js |  1 +
 6 files changed, 72 insertions(+), 21 deletions(-)
 create mode 100644 ui/packages/consul-ui/tests/acceptance/dc/services/show-topology.feature
 create mode 100644 ui/packages/consul-ui/tests/acceptance/steps/dc/services/show-topology-steps.js

diff --git a/ui/packages/consul-ui/app/models/topology.js b/ui/packages/consul-ui/app/models/topology.js
index 63c2d9e08b..5f8d3f3b08 100644
--- a/ui/packages/consul-ui/app/models/topology.js
+++ b/ui/packages/consul-ui/app/models/topology.js
@@ -1,6 +1,5 @@
 import Model from 'ember-data/model';
 import attr from 'ember-data/attr';
-import { computed } from '@ember/object';
 
 export const PRIMARY_KEY = 'uid';
 export const SLUG_KEY = 'ServiceName';
@@ -13,7 +12,4 @@ export default Model.extend({
   Downstreams: attr(),
   Protocol: attr(),
   meta: attr(),
-  Exists: computed(function() {
-    return true;
-  }),
 });
diff --git a/ui/packages/consul-ui/app/routes/dc/services/show/index.js b/ui/packages/consul-ui/app/routes/dc/services/show/index.js
index 5b9171d8e7..6d1e6c381f 100644
--- a/ui/packages/consul-ui/app/routes/dc/services/show/index.js
+++ b/ui/packages/consul-ui/app/routes/dc/services/show/index.js
@@ -11,25 +11,29 @@ export default Route.extend({
     // so check the length here.
     let to = 'topology';
     const parentModel = this.modelFor(parent);
-
+    const hasProxy = get(parentModel, 'proxies');
     const kind = get(parentModel, 'items.firstObject.Service.Kind');
 
-    switch (kind) {
-      case 'ingress-gateway':
-        if (!get(parentModel, 'topology.Exists')) {
-          to = 'upstreams';
-        }
-        break;
-      case 'terminating-gateway':
-        to = 'services';
-        break;
-      case 'mesh-gateway':
-        to = 'instances';
-        break;
-      default:
-        if (!get(parentModel, 'topology.Exists')) {
+    if (hasProxy.length === 0) {
+      to = 'instances';
+    } else {
+      switch (kind) {
+        case 'ingress-gateway':
+          if (!get(parentModel, 'topology.Datacenter')) {
+            to = 'upstreams';
+          }
+          break;
+        case 'terminating-gateway':
+          to = 'services';
+          break;
+        case 'mesh-gateway':
           to = 'instances';
-        }
+          break;
+        default:
+          if (!get(parentModel, 'topology.Datacenter')) {
+            to = 'instances';
+          }
+      }
     }
 
     this.replaceWith(`${parent}.${to}`, parentModel);
diff --git a/ui/packages/consul-ui/app/templates/dc/services/show.hbs b/ui/packages/consul-ui/app/templates/dc/services/show.hbs
index e3c11aa323..0eb0aacb1f 100644
--- a/ui/packages/consul-ui/app/templates/dc/services/show.hbs
+++ b/ui/packages/consul-ui/app/templates/dc/services/show.hbs
@@ -29,7 +29,7 @@
     <TabNav @items={{
       compact
           (array
-(if topology.Datacenter
+(if (and topology.Datacenter (gt proxies.length 0))
             (hash label="Topology" href=(href-to "dc.services.show.topology") selected=(is-href "dc.services.show.topology"))
 '')
 (if (eq item.Service.Kind 'terminating-gateway')
diff --git a/ui/packages/consul-ui/tests/acceptance/dc/services/show-topology.feature b/ui/packages/consul-ui/tests/acceptance/dc/services/show-topology.feature
new file mode 100644
index 0000000000..54c7bd2ed4
--- /dev/null
+++ b/ui/packages/consul-ui/tests/acceptance/dc/services/show-topology.feature
@@ -0,0 +1,40 @@
+@setupApplicationTest
+Feature: dc / services / show-topology: Show Topology tab for Service
+  Scenario: Given a service, the Topology tab should display
+    Given 1 datacenter model with the value "dc1"
+    And 1 node models
+    And 1 service model from yaml
+    ---
+    - Service:
+        Name: service-0
+        ID: service-0-with-id
+    ---
+    When I visit the service page for yaml
+    ---
+      dc: dc1
+      service: service-0
+    ---
+    And I see topology on the tabs
+    Then the url should be /dc1/services/service-0/topology
+  Scenario: Given connect is disabled, the Topology tab should not display or error
+    Given 1 datacenter model with the value "dc1"
+    And 1 node models
+    And 1 service model from yaml
+    ---
+    - Service:
+        Name: service-0
+        ID: service-0-with-id
+    ---
+    And the url "/v1/discovery-chain/service-0?dc=dc1&ns=@namespace" responds with from yaml
+    ---
+    status: 500
+    body: "Connect must be enabled in order to use this endpoint"
+    ---
+    When I visit the service page for yaml
+    ---
+      dc: dc1
+      service: service-0
+    ---
+    And I don't see topology on the tabs
+    Then the url should be /dc1/services/service-0/instances
+
diff --git a/ui/packages/consul-ui/tests/acceptance/steps/dc/services/show-topology-steps.js b/ui/packages/consul-ui/tests/acceptance/steps/dc/services/show-topology-steps.js
new file mode 100644
index 0000000000..ba1093295f
--- /dev/null
+++ b/ui/packages/consul-ui/tests/acceptance/steps/dc/services/show-topology-steps.js
@@ -0,0 +1,10 @@
+import steps from '../../steps';
+
+// step definitions that are shared between features should be moved to the
+// tests/acceptance/steps/steps.js file
+
+export default function(assert) {
+  return steps(assert).then('I should find a file', function() {
+    assert.ok(true, this.step);
+  });
+}
diff --git a/ui/packages/consul-ui/tests/pages/dc/services/show.js b/ui/packages/consul-ui/tests/pages/dc/services/show.js
index 0630edbe97..6e5313c00d 100644
--- a/ui/packages/consul-ui/tests/pages/dc/services/show.js
+++ b/ui/packages/consul-ui/tests/pages/dc/services/show.js
@@ -8,6 +8,7 @@ export default function(visitable, attribute, collection, text, intentions, filt
       href: attribute('href', '[data-test-dashboard-anchor]'),
     },
     tabs: tabs('tab', [
+      'topology',
       'instances',
       'linked-services',
       'upstreams',