ui: Misc changes for Catalog area (#9414)

* Use DataLoader errors for Service Detail and Service Instance * uiCfg > config use the repo-like async interface where possible * Clean up node show * Make sure you can put `=` in dev cookie values * Never default to default * Tweak chain variable * Remove env service * Pass chain through to the template for the tempalte to clean it up * Delete controller tests * Remove cleanup in Nodes show as this is still being used in another tab * Use dc.Local
2020-12-17 16:35:01 +00:00 · 2020-12-17 16:35:01 +00:00 · ae049feeab
parent 325bca338b
commit ae049feeab
15 changed files with 210 additions and 245 deletions
--- a/ui/packages/consul-ui/app/components/consul/service/notifications/index.hbs
+++ b/ui/packages/consul-ui/app/components/consul/service/notifications/index.hbs
@ -1,6 +0,0 @@
-{{#if (eq @type 'update')}}
-  {{#if (eq @status 'warning') }}
-    This service has been deregistered and no longer exists in the catalog.
-  {{else}}
-  {{/if}}
-{{/if}}
--- a/ui/packages/consul-ui/app/controllers/dc/services/instance.js
+++ b/ui/packages/consul-ui/app/controllers/dc/services/instance.js
@ -1,28 +0,0 @@
-import { inject as service } from '@ember/service';
-import Controller from '@ember/controller';
-import { get, action } from '@ember/object';
-
-export default class InstanceController extends Controller {
-  @service('flashMessages')
-  notify;
-
-  @action
-  error(e) {
-    if (e.target.readyState === 1) {
-      // OPEN
-      if (get(e, 'error.errors.firstObject.status') === '404') {
-        this.notify.add({
-          destroyOnClick: false,
-          sticky: true,
-          type: 'warning',
-          action: 'update',
-        });
-        [e.target, this.proxy].forEach(function(item) {
-          if (item && typeof item.close === 'function') {
-            item.close();
-          }
-        });
-      }
-    }
-  }
-}
--- a/ui/packages/consul-ui/app/controllers/dc/services/show.js
+++ b/ui/packages/consul-ui/app/controllers/dc/services/show.js
@ -1,27 +0,0 @@
-import { inject as service } from '@ember/service';
-import Controller from '@ember/controller';
-import { get, action } from '@ember/object';
-
-export default class ShowController extends Controller {
-  @service('flashMessages') notify;
-
-  @action
-  error(e) {
-    if (e.target.readyState === 1) {
-      // OPEN
-      if (get(e, 'error.errors.firstObject.status') === '404') {
-        this.notify.add({
-          destroyOnClick: false,
-          sticky: true,
-          type: 'warning',
-          action: 'update',
-        });
-      }
-      [e.target, this.proxies].forEach(function(item) {
-        if (item && typeof item.close === 'function') {
-          item.close();
-        }
-      });
-    }
-  }
-}
--- a/ui/packages/consul-ui/app/routes/dc/services/instance.js
+++ b/ui/packages/consul-ui/app/routes/dc/services/instance.js
@ -7,7 +7,7 @@ export default class InstanceRoute extends Route {

  async model(params, transition) {
    const dc = this.modelFor('dc').dc.Name;
-    const nspace = this.modelFor('nspace').nspace.substr(1) || 'default';
+    const nspace = this.modelFor('nspace').nspace.substr(1);

    const item = await this.data.source(
      uri => uri`/${nspace}/${dc}/service-instance/${params.id}/${params.node}/${params.name}`
--- a/ui/packages/consul-ui/app/routes/dc/services/show.js
+++ b/ui/packages/consul-ui/app/routes/dc/services/show.js
@ -11,9 +11,10 @@ export default class ShowRoute extends Route {
    const nspace = this.modelFor('nspace').nspace.substr(1);
    const slug = params.name;

+    let chain;
    let proxies = [];

-    const urls = this.config.get().dashboard_url_templates;
+    const urls = await this.config.findByPath('dashboard_url_templates');
    const items = await this.data.source(
      uri => uri`/${nspace}/${dc.Name}/service-instances/for-service/${params.name}`
    );
@ -30,9 +31,7 @@ export default class ShowRoute extends Route {
      // use that endpoint here. Eventually if we have an endpoint specific to
      // a dc that gives us more DC specific info we can use that instead
      // higher up the routing hierarchy instead.
-      let chain = this.data.source(
-        uri => uri`/${nspace}/${dc.Name}/discovery-chain/${params.name}`
-      );
+      chain = this.data.source(uri => uri`/${nspace}/${dc.Name}/discovery-chain/${params.name}`);
      [chain, proxies] = await Promise.all([chain, proxies]);
      // we close the chain for now, if you enter the routing tab before the
      // EventSource comes around to request again, this one will just be
@ -45,6 +44,7 @@ export default class ShowRoute extends Route {
      slug,
      items,
      urls,
+      chain,
      proxies,
    };
  }
--- a/ui/packages/consul-ui/app/routes/dc/services/show/topology.js
+++ b/ui/packages/consul-ui/app/routes/dc/services/show/topology.js
@ -4,7 +4,6 @@ import { get, action } from '@ember/object';

 export default class TopologyRoute extends Route {
  @service('ui-config') config;
-  @service('env') env;
  @service('data-source/service') data;
  @service('repository/intention') repo;

@ -32,19 +31,20 @@ export default class TopologyRoute extends Route {
    const nspace = get(model, 'nspace');

    const item = get(model, 'items.firstObject');
-    if (get(item, 'IsMeshOrigin')) {
    let kind = get(item, 'Service.Kind');
    if (typeof kind === 'undefined') {
      kind = '';
    }
-      model.topology = await this.data.source(
+    const topology = await this.data.source(
      uri => uri`/${nspace}/${dc.Name}/topology/${model.slug}/${kind}`
    );
-    }
+    let hasMetricsProvider = await this.config.findByPath('metrics_provider');
+    hasMetricsProvider = !!hasMetricsProvider;
+
    return {
      ...model,
-      hasMetricsProvider: !!this.config.get().metrics_provider,
-      isRemoteDC: this.env.var('CONSUL_DATACENTER_LOCAL') !== this.modelFor('dc').dc.Name,
+      topology,
+      hasMetricsProvider,
    };
  }

--- a/ui/packages/consul-ui/app/services/repository/metrics.js
+++ b/ui/packages/consul-ui/app/services/repository/metrics.js
@ -5,7 +5,7 @@ import RepositoryService from 'consul-ui/services/repository';
 // metrics provider

 export default class MetricsService extends RepositoryService {
-  @service('ui-config') cfg;
+  @service('ui-config') config;
  @service('env') env;
  @service('client/http') client;

@ -13,16 +13,16 @@ export default class MetricsService extends RepositoryService {

  init() {
    super.init(...arguments);
-    const uiCfg = this.cfg.get();
+    const config = this.config.get();
    // Inject whether or not the proxy is enabled as an option into the opaque
    // JSON options the user provided.
-    const opts = uiCfg.metrics_provider_options || {};
-    opts.metrics_proxy_enabled = uiCfg.metrics_proxy_enabled;
+    const opts = config.metrics_provider_options || {};
+    opts.metrics_proxy_enabled = config.metrics_proxy_enabled;
    // Inject a convenience function for dialing through the metrics proxy.
    opts.fetch = (path, params) =>
      this.client.fetchWithToken(`/v1/internal/ui/metrics-proxy${path}`, params);
    // Inject the base app URL
-    const provider = uiCfg.metrics_provider || 'prometheus';
+    const provider = config.metrics_provider || 'prometheus';

    try {
      this.provider = window.consul.getMetricsProvider(provider, opts);
--- a/ui/packages/consul-ui/app/templates/dc/nodes/show.hbs
+++ b/ui/packages/consul-ui/app/templates/dc/nodes/show.hbs
@ -1,17 +1,17 @@
 {{page-title item.Node}}
-<DataLoader as |api|>
+<DataLoader as |loader|>

  <BlockSlot @name="data">
+    <EventSource @src={{item}} @onerror={{action loader.dispatchError}} />
    <EventSource @src={{tomography}} />
-    <EventSource @src={{item}} @onerror={{queue (action api.dispatchError)}} />
  </BlockSlot>

  <BlockSlot @name="error">
-    <AppError @error={{api.error}} />
+    <AppError @error={{loader.error}} />
  </BlockSlot>

  <BlockSlot @name="disconnected" as |Notification|>
-    {{#if (eq api.error.status "404")}}
+    {{#if (eq loader.error.status "404")}}
      <Notification @sticky={{true}}>
        <p data-notification role="alert" class="warning notification-update">
          <strong>Warning!</strong>
--- a/ui/packages/consul-ui/app/templates/dc/services/index.hbs
+++ b/ui/packages/consul-ui/app/templates/dc/services/index.hbs
@ -21,12 +21,6 @@
 as |sort filters items|}}

  <AppView>
-    <BlockSlot @name="notification" as |status type|>
-      <Consul::Service::Notifications
-        @type={{type}}
-        @status={{status}}
-      />
-    </BlockSlot>
    <BlockSlot @name="header">
        <h1>
          Services <em>{{format-number items.length}} total</em>
--- a/ui/packages/consul-ui/app/templates/dc/services/instance.hbs
+++ b/ui/packages/consul-ui/app/templates/dc/services/instance.hbs
@ -1,14 +1,38 @@
 {{page-title item.Service.ID}}
-<EventSource @src={{item}} @onerror={{action "error"}} />
+  <DataLoader as |loader|>
+
+    <BlockSlot @name="data">
+        <EventSource @src={{item}} @onerror={{action loader.dispatchError}} />
+      {{#if (not loader.error)}}
        <EventSource @src={{proxy}} />
        <EventSource @src={{proxyMeta}} />
-<AppView>
-    <BlockSlot @name="notification" as |status type|>
-      <Consul::Service::Notifications
-        @type={{type}}
-        @status={{status}}
-      />
+      {{/if}}
    </BlockSlot>
+
+    <BlockSlot @name="error">
+      <AppError @error={{loader.error}} />
+    </BlockSlot>
+
+    <BlockSlot @name="disconnected" as |Notification|>
+      {{#if (eq loader.error.status "404")}}
+        <Notification @sticky={{true}}>
+          <p data-notification role="alert" class="warning notification-update">
+            <strong>Warning!</strong>
+            This service has been deregistered and no longer exists in the catalog.
+          </p>
+        </Notification>
+      {{else}}
+        <Notification @sticky={{true}}>
+          <p data-notification role="alert" class="warning notification-update">
+            <strong>Warning!</strong>
+            An error was returned whilst loading this data, refresh to try again.
+          </p>
+        </Notification>
+      {{/if}}
+    </BlockSlot>
+
+    <BlockSlot @name="loaded">
+      <AppView>
          <BlockSlot @name="breadcrumbs">
              <ol>
                  <li><a data-test-back href={{href-to 'dc.services'}}>All Services</a></li>
@ -62,3 +86,5 @@
            </Outlet>
          </BlockSlot>
      </AppView>
+    </BlockSlot>
+</DataLoader>
--- a/ui/packages/consul-ui/app/templates/dc/services/show.hbs
+++ b/ui/packages/consul-ui/app/templates/dc/services/show.hbs
@ -1,14 +1,39 @@
-<EventSource @src={{items}} @onerror={{action "error"}} />
-<EventSource @src={{proxies}} />
 {{#let items.firstObject as |item|}}
 {{page-title item.Service.Service}}
-  <AppView>
-    <BlockSlot @name="notification" as |status type|>
-      <Consul::Service::Notifications
-        @type={{type}}
-        @status={{status}}
-      />
+  <DataLoader as |loader|>
+
+    <BlockSlot @name="data">
+      <EventSource @src={{items}} @onerror={{action loader.dispatchError}} />
+      {{#if (not loader.error)}}
+        <EventSource @src={{proxies}} />
+        <EventSource @src={{chain}} />
+      {{/if}}
    </BlockSlot>
+
+    <BlockSlot @name="error">
+      <AppError @error={{loader.error}} />
+    </BlockSlot>
+
+    <BlockSlot @name="disconnected" as |Notification|>
+      {{#if (eq loader.error.status "404")}}
+        <Notification @sticky={{true}}>
+          <p data-notification role="alert" class="warning notification-update">
+            <strong>Warning!</strong>
+            This service has been deregistered and no longer exists in the catalog.
+          </p>
+        </Notification>
+      {{else}}
+        <Notification @sticky={{true}}>
+          <p data-notification role="alert" class="warning notification-update">
+            <strong>Warning!</strong>
+            An error was returned whilst loading this data, refresh to try again.
+          </p>
+        </Notification>
+      {{/if}}
+    </BlockSlot>
+
+    <BlockSlot @name="loaded">
+      <AppView>
        <BlockSlot @name="breadcrumbs">
          <ol>
              <li><a data-test-back href={{href-to 'dc.services'}}>All Services</a></li>
@ -70,4 +95,6 @@
          </Outlet>
        </BlockSlot>
      </AppView>
+    </BlockSlot>
+  </DataLoader>
 {{/let}}
--- a/ui/packages/consul-ui/app/templates/dc/services/show/topology.hbs
+++ b/ui/packages/consul-ui/app/templates/dc/services/show/topology.hbs
@ -32,7 +32,7 @@
        Datacenter=dc.Name
        Service=items.firstObject
      )}}
-      @isRemoteDC={{isRemoteDC}}
+      @isRemoteDC={{not dc.Local}}
      @hasMetricsProvider={{hasMetricsProvider}}
      @oncreate={{route-action 'createIntention'}}
    />
--- a/ui/packages/consul-ui/app/utils/get-environment.js
+++ b/ui/packages/consul-ui/app/utils/get-environment.js
@ -23,7 +23,10 @@ export default function(config = {}, win = window, doc = document) {
    return str
      .split(';')
      .filter(item => item !== '')
-      .map(item => item.trim().split('='));
+      .map(item => {
+        const [key, ...rest] = item.trim().split('=');
+        return [key, rest.join('=')];
+      });
  };
  const user = function(str) {
    const item = win.localStorage.getItem(str);
--- a/ui/packages/consul-ui/tests/unit/controllers/dc/services/instance-test.js
+++ b/ui/packages/consul-ui/tests/unit/controllers/dc/services/instance-test.js
@ -1,12 +0,0 @@
-import { module, test } from 'qunit';
-import { setupTest } from 'ember-qunit';
-
-module('Unit | Controller | dc/services/instance', function(hooks) {
-  setupTest(hooks);
-
-  // Replace this with your real tests.
-  test('it exists', function(assert) {
-    let controller = this.owner.lookup('controller:dc/services/instance');
-    assert.ok(controller);
-  });
-});
--- a/ui/packages/consul-ui/tests/unit/controllers/dc/services/show-test.js
+++ b/ui/packages/consul-ui/tests/unit/controllers/dc/services/show-test.js
@ -1,12 +0,0 @@
-import { module, test } from 'qunit';
-import { setupTest } from 'ember-qunit';
-
-module('Unit | Controller | dc/services/show', function(hooks) {
-  setupTest(hooks);
-
-  // Replace this with your real tests.
-  test('it exists', function(assert) {
-    let controller = this.owner.lookup('controller:dc/services/show');
-    assert.ok(controller);
-  });
-});