From d2515ed409c9a06fbf68466068e9e765abe7110f Mon Sep 17 00:00:00 2001
From: John Cowen <johncowen@users.noreply.github.com>
Date: Tue, 16 Jun 2020 14:13:29 +0100
Subject: [PATCH] ui: Move healthcheck ordering to use new comparators (#8096)

---
 .../app/components/healthcheck-list/index.hbs | 93 ++++++++++---------
 .../app/components/healthcheck-list/index.js  | 33 +------
 .../components/healthcheck-output/index.hbs   |  8 --
 .../components/healthcheck-output/index.js    |  6 --
 ui-v2/app/controllers/dc/nodes/show.js        | 30 ------
 ui-v2/app/initializers/sort.js                |  2 +
 ui-v2/app/sort/comparators/check.js           | 42 +++++++++
 .../templates/dc/nodes/show/healthchecks.hbs  |  2 +-
 .../dc/services/instance/healthchecks.hbs     |  4 +-
 .../templates/dc/services/instance/proxy.hbs  |  2 +-
 .../components/healthcheck-output-test.js     | 23 -----
 11 files changed, 96 insertions(+), 149 deletions(-)
 delete mode 100644 ui-v2/app/components/healthcheck-output/index.hbs
 delete mode 100644 ui-v2/app/components/healthcheck-output/index.js
 create mode 100644 ui-v2/app/sort/comparators/check.js
 delete mode 100644 ui-v2/tests/integration/components/healthcheck-output-test.js

diff --git a/ui-v2/app/components/healthcheck-list/index.hbs b/ui-v2/app/components/healthcheck-list/index.hbs
index fb07ce39e..178581ac1 100644
--- a/ui-v2/app/components/healthcheck-list/index.hbs
+++ b/ui-v2/app/components/healthcheck-list/index.hbs
@@ -1,47 +1,48 @@
-<ul>
-{{#each (sort-by (action 'sortChecksByImportance') items) as |item| }}
-{{! TODO: this component and its child should be moved to a single component }}
-  <HealthcheckOutput class={{item.Status}} @tagName="li">
-      <BlockSlot @name="header">
-        <h3>{{item.Name}}</h3>
-      </BlockSlot>
-      <BlockSlot @name="content">
-      <dl>
-      {{#if (eq item.ServiceName "")}}
-        <dt>NodeName</dt>
-        <dd>{{item.Node}}</dd>
-      {{else}}
-        <dt>ServiceName</dt>
-        <dd>{{item.ServiceName}}</dd>
-      {{/if}}
-      </dl>
-      <dl>
-        <dt>CheckID</dt>
-        <dd>{{or item.CheckID '-'}}</dd>
-      </dl>
-      <dl>
-        <dt>Type</dt>
-        <dd>
-          {{item.Type}}
-{{#if (and exposed (contains item.Type (array 'http' 'grpc')))}}
-          <em data-test-exposed="true" data-tooltip="Expose.checks is set to true, so all registered HTTP and gRPC check paths are exposed through Envoy for the Consul agent.">Exposed</em>
-{{/if}}
-        </dd>
-      </dl>
-      <dl>
-        <dt>Notes</dt>
-        <dd>{{or item.Notes '-'}}</dd>
-      </dl>
-      <dl>
-{{#if (not-eq item.Type 'ttl')}}
-        <dt>Output</dt>
-        <dd>
-            <pre><code>{{item.Output}}</code></pre>
-            <CopyButton @value={{item.Output}} @name="output" />
-        </dd>
-{{/if}}
-      </dl>
-      </BlockSlot>
-    </HealthcheckOutput>
+<div>
+  <ul>
+{{#each items as |item| }}
+    <li class={{concat 'healthcheck-output ' item.Status}}>
+      <div>
+        <header>
+          <h3>{{item.Name}}</h3>
+        </header>
+        <dl>
+        {{#if (eq item.ServiceName "")}}
+          <dt>NodeName</dt>
+          <dd>{{item.Node}}</dd>
+        {{else}}
+          <dt>ServiceName</dt>
+          <dd>{{item.ServiceName}}</dd>
+        {{/if}}
+        </dl>
+        <dl>
+          <dt>CheckID</dt>
+          <dd>{{or item.CheckID '-'}}</dd>
+        </dl>
+        <dl>
+          <dt>Type</dt>
+          <dd>
+            {{item.Type}}
+  {{#if (and exposed (contains item.Type (array 'http' 'grpc')))}}
+            <em data-test-exposed="true" data-tooltip="Expose.checks is set to true, so all registered HTTP and gRPC check paths are exposed through Envoy for the Consul agent.">Exposed</em>
+  {{/if}}
+          </dd>
+        </dl>
+        <dl>
+          <dt>Notes</dt>
+          <dd>{{or item.Notes '-'}}</dd>
+        </dl>
+        <dl>
+  {{#if (not-eq item.Type 'ttl')}}
+          <dt>Output</dt>
+          <dd>
+              <pre><code>{{item.Output}}</code></pre>
+              <CopyButton @value={{item.Output}} @name="output" />
+          </dd>
+  {{/if}}
+        </dl>
+      </div>
+    </li>
 {{/each}}
-</ul>
+  </ul>
+</div>
diff --git a/ui-v2/app/components/healthcheck-list/index.js b/ui-v2/app/components/healthcheck-list/index.js
index 092a1aada..479865264 100644
--- a/ui-v2/app/components/healthcheck-list/index.js
+++ b/ui-v2/app/components/healthcheck-list/index.js
@@ -1,36 +1,5 @@
 import Component from '@ember/component';
-import { get } from '@ember/object';
 
 export default Component.extend({
-  // TODO: Could potentially do this on attr change
-  actions: {
-    sortChecksByImportance: function(a, b) {
-      const statusA = get(a, 'Status');
-      const statusB = get(b, 'Status');
-      switch (statusA) {
-        case 'passing':
-          // a = passing
-          // unless b is also passing then a is less important
-          return statusB === 'passing' ? 0 : 1;
-        case 'critical':
-          // a = critical
-          // unless b is also critical then a is more important
-          return statusB === 'critical' ? 0 : -1;
-        case 'warning':
-          // a = warning
-          switch (statusB) {
-            // b is passing so a is more important
-            case 'passing':
-              return -1;
-            // b is critical so a is less important
-            case 'critical':
-              return 1;
-            // a and b are both warning, therefore equal
-            default:
-              return 0;
-          }
-      }
-      return 0;
-    },
-  },
+  tagName: '',
 });
diff --git a/ui-v2/app/components/healthcheck-output/index.hbs b/ui-v2/app/components/healthcheck-output/index.hbs
deleted file mode 100644
index 789e14afa..000000000
--- a/ui-v2/app/components/healthcheck-output/index.hbs
+++ /dev/null
@@ -1,8 +0,0 @@
-{{! TODO: this component and its parent should be moved to a single component }}
-{{yield}}
-<div>
-  <header>
-    <YieldSlot @name="header">{{yield}}</YieldSlot>
-  </header>
-  <YieldSlot @name="content">{{yield}}</YieldSlot>
-</div>
\ No newline at end of file
diff --git a/ui-v2/app/components/healthcheck-output/index.js b/ui-v2/app/components/healthcheck-output/index.js
deleted file mode 100644
index 35c2e7a28..000000000
--- a/ui-v2/app/components/healthcheck-output/index.js
+++ /dev/null
@@ -1,6 +0,0 @@
-import Component from '@ember/component';
-import Slotted from 'block-slots';
-
-export default Component.extend(Slotted, {
-  classNames: ['healthcheck-output'],
-});
diff --git a/ui-v2/app/controllers/dc/nodes/show.js b/ui-v2/app/controllers/dc/nodes/show.js
index 0190caba7..0c23f88a2 100644
--- a/ui-v2/app/controllers/dc/nodes/show.js
+++ b/ui-v2/app/controllers/dc/nodes/show.js
@@ -23,34 +23,4 @@ export default Controller.extend(WithEventSource, {
       }
     }
   }),
-  actions: {
-    sortChecksByImportance: function(a, b) {
-      const statusA = get(a, 'Status');
-      const statusB = get(b, 'Status');
-      switch (statusA) {
-        case 'passing':
-          // a = passing
-          // unless b is also passing then a is less important
-          return statusB === 'passing' ? 0 : 1;
-        case 'critical':
-          // a = critical
-          // unless b is also critical then a is more important
-          return statusB === 'critical' ? 0 : -1;
-        case 'warning':
-          // a = warning
-          switch (statusB) {
-            // b is passing so a is more important
-            case 'passing':
-              return -1;
-            // b is critical so a is less important
-            case 'critical':
-              return 1;
-            // a and b are both warning, therefore equal
-            default:
-              return 0;
-          }
-      }
-      return 0;
-    },
-  },
 });
diff --git a/ui-v2/app/initializers/sort.js b/ui-v2/app/initializers/sort.js
index 3065d0fc9..6ddcc3ef8 100644
--- a/ui-v2/app/initializers/sort.js
+++ b/ui-v2/app/initializers/sort.js
@@ -1,10 +1,12 @@
 import service from 'consul-ui/sort/comparators/service';
+import check from 'consul-ui/sort/comparators/check';
 
 export function initialize(container) {
   // Service-less injection using private properties at a per-project level
   const Sort = container.resolveRegistration('service:sort');
   const comparators = {
     service: service(),
+    check: check(),
   };
   Sort.reopen({
     comparator: function(type) {
diff --git a/ui-v2/app/sort/comparators/check.js b/ui-v2/app/sort/comparators/check.js
new file mode 100644
index 000000000..c3993a903
--- /dev/null
+++ b/ui-v2/app/sort/comparators/check.js
@@ -0,0 +1,42 @@
+export default () => key => {
+  if (key.startsWith('Status:')) {
+    return function(itemA, itemB) {
+      const [, dir] = key.split(':');
+      let a, b;
+      if (dir === 'asc') {
+        a = itemA;
+        b = itemB;
+      } else {
+        b = itemA;
+        a = itemB;
+      }
+      const statusA = a.Status;
+      const statusB = b.Status;
+      switch (statusA) {
+        case 'passing':
+          // a = passing
+          // unless b is also passing then a is less important
+          return statusB === 'passing' ? 0 : 1;
+        case 'critical':
+          // a = critical
+          // unless b is also critical then a is more important
+          return statusB === 'critical' ? 0 : -1;
+        case 'warning':
+          // a = warning
+          switch (statusB) {
+            // b is passing so a is more important
+            case 'passing':
+              return -1;
+            // b is critical so a is less important
+            case 'critical':
+              return 1;
+            // a and b are both warning, therefore equal
+            default:
+              return 0;
+          }
+      }
+      return 0;
+    };
+  }
+  return key;
+};
diff --git a/ui-v2/app/templates/dc/nodes/show/healthchecks.hbs b/ui-v2/app/templates/dc/nodes/show/healthchecks.hbs
index cf44b208d..06836132e 100644
--- a/ui-v2/app/templates/dc/nodes/show/healthchecks.hbs
+++ b/ui-v2/app/templates/dc/nodes/show/healthchecks.hbs
@@ -1,7 +1,7 @@
 <div id="health-checks" class="tab-section">
   <div role="tabpanel">
     {{#if (gt item.Checks.length 0) }}
-      <HealthcheckList @items={{item.Checks}} />
+      <HealthcheckList @items={{sort-by (comparator 'check' 'Status:asc') item.Checks}} />
     {{else}}
       <p>
         This node has no health checks.
diff --git a/ui-v2/app/templates/dc/services/instance/healthchecks.hbs b/ui-v2/app/templates/dc/services/instance/healthchecks.hbs
index cb61592e0..eb0d3d50c 100644
--- a/ui-v2/app/templates/dc/services/instance/healthchecks.hbs
+++ b/ui-v2/app/templates/dc/services/instance/healthchecks.hbs
@@ -2,7 +2,7 @@
   <div role="tabpanel">
     {{#if (gt item.ServiceChecks.length 0) }}
       <section data-test-service-checks>
-        <HealthcheckList @items={{item.ServiceChecks}} @exposed={{proxy.ServiceProxy.Expose.Checks}} />
+        <HealthcheckList @items={{sort-by (comparator 'check' 'Status:asc') item.ServiceChecks}} @exposed={{proxy.ServiceProxy.Expose.Checks}} />
       </section>
     {{else}}
       <p>
@@ -11,7 +11,7 @@
     {{/if}}
     {{#if (gt item.NodeChecks.length 0) }}
       <section data-test-node-checks>
-        <HealthcheckList @items={{item.NodeChecks}} />
+        <HealthcheckList @items={{sort-by (comparator 'check' 'Status:asc') item.NodeChecks}} />
       </section>
     {{else}}
       <p>
diff --git a/ui-v2/app/templates/dc/services/instance/proxy.hbs b/ui-v2/app/templates/dc/services/instance/proxy.hbs
index 387b051c0..307963c25 100644
--- a/ui-v2/app/templates/dc/services/instance/proxy.hbs
+++ b/ui-v2/app/templates/dc/services/instance/proxy.hbs
@@ -140,7 +140,7 @@
   {{#if (or (gt proxy.ServiceChecks.length 0) (gt proxy.NodeChecks.length 0))}}
     <section>
       <h3>Proxy health</h3>
-      <HealthcheckList data-test-proxy-checks @items={{append proxy.ServiceChecks proxy.NodeChecks}} />
+      <HealthcheckList data-test-proxy-checks @items={{sort-by (comparator 'check' 'Status:asc') (append proxy.ServiceChecks proxy.NodeChecks)}} />
     </section>
   {{/if}}
   </div>
diff --git a/ui-v2/tests/integration/components/healthcheck-output-test.js b/ui-v2/tests/integration/components/healthcheck-output-test.js
deleted file mode 100644
index 8d079a08a..000000000
--- a/ui-v2/tests/integration/components/healthcheck-output-test.js
+++ /dev/null
@@ -1,23 +0,0 @@
-import { module, test } from 'qunit';
-import { setupRenderingTest } from 'ember-qunit';
-import { render, find } from '@ember/test-helpers';
-import hbs from 'htmlbars-inline-precompile';
-
-module('Integration | Component | healthcheck output', function(hooks) {
-  setupRenderingTest(hooks);
-
-  test('it renders', async function(assert) {
-    // Set any properties with this.set('myProperty', 'value');
-    // Handle any actions with this.on('myAction', function(val) { ... });
-
-    await render(hbs`{{healthcheck-output}}`);
-
-    assert.equal(find('*').textContent.trim(), '');
-
-    // Template block usage:
-    await render(hbs`
-      {{#healthcheck-output}}{{/healthcheck-output}}
-    `);
-    assert.equal(find('*').textContent.trim(), '');
-  });
-});