ui: Topology intention saving improvements

.changelog/9513.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+ui: Fixes an issue where intention description or metadata could be overwritten if saved from the topology view.
+```

ui/packages/consul-ui/app/components/topology-metrics/index.hbs

@@ -3,7 +3,7 @@
 <div
   {{did-insert (action this.calculate)}}
   {{did-update (action this.calculate) @topology.Upstreams @topology.Downstreams}}
-  class="topology-container"
+  class="topology-container consul-topology-metrics"
 >
 {{#if (gt @topology.Downstreams.length 0)}}
   <div id="downstream-container">

ui/packages/consul-ui/app/components/topology-metrics/notifications/index.hbs

@@ -0,0 +1,19 @@
+{{#if (eq @type 'create')}}
+  {{#if (eq @status 'success') }}
+    Your intention has been added.
+  {{else}}
+    There was an error adding your intention.
+  {{/if}}
+{{else if (eq @type 'update') }}
+  {{#if (eq @status 'success') }}
+    Your intention has been saved.
+  {{else}}
+    There was an error saving your intention.
+  {{/if}}
+{{/if}}
+{{#let @error.errors.firstObject as |error|}}
+  {{#if error.detail }}
+    <br />{{concat '(' (if error.status (concat error.status ': ')) error.detail ')'}}
+  {{/if}}
+{{/let}}
+

ui/packages/consul-ui/app/components/topology-metrics/popover/index.hbs

@@ -16,16 +16,25 @@
     </:header>
     <:body>
       <p>
-        Add an intention that allows these two services to connect.
+        {{#if @item.Intention.HasExact}}
+          Change the action of this intention to allow.
+        {{else}}
+          Add an intention that allows these two services to connect.
+        {{/if}}
       </p>
     </:body>
     <:actions as |Actions|>
       <Actions.Action class="action">
         <button
           {{on "click" @oncreate}}
+          data-test-confirm
           type="button"
         >
-          Create
+          {{#if @item.Intention.HasExact}}
+            Allow
+          {{else}}
+            Create
+          {{/if}}
         </button>
       </Actions.Action>
       <Actions.Action>
@@ -86,10 +95,11 @@
       )
       returns=(set this 'popoverController')
     }}
-    type="button"
     {{on 'click' (fn (optional this.popoverController.show))}}
+    type="button"
     style={{{concat 'top:' @position.y 'px;left:' @position.x 'px;'}}}
     aria-label={{if (eq @type 'deny') 'Add intention' 'View intention'}}
+    data-test-action
   >
   </button>
 </div>

ui/packages/consul-ui/app/routes/dc/services/show/topology.js

@@ -1,26 +1,62 @@
 import Route from '@ember/routing/route';
 import { inject as service } from '@ember/service';
-import { get, action } from '@ember/object';
+import { set, get, action } from '@ember/object';
 
 export default class TopologyRoute extends Route {
   @service('ui-config') config;
   @service('data-source/service') data;
   @service('repository/intention') repo;
+  @service('feedback') feedback;
 
   @action
   async createIntention(source, destination) {
-    const model = this.repo.create({
-      Datacenter: source.Datacenter,
-      SourceName: source.Name,
-      SourceNS: source.Namespace || 'default',
-      DestinationName: destination.Name,
-      DestinationNS: destination.Namespace || 'default',
-      Action: 'allow',
-    });
-    await this.repo.persist(model);
+    // begin with a create action as it makes more sense if the we can't even
+    // get a list of intentions
+    let notification = this.feedback.notification('create');
+    try {
+      // intentions will be a proxy object
+      let intentions = await this.intentions;
+      let intention = intentions.find(item => {
+        return (
+          item.Datacenter === source.Datacenter &&
+          item.SourceName === source.Name &&
+          item.SourceNS === source.Namespace &&
+          item.DestinationName === destination.Name &&
+          item.DestinationNS === destination.Namespace
+        );
+      });
+      if (typeof intention === 'undefined') {
+        intention = this.repo.create({
+          Datacenter: source.Datacenter,
+          SourceName: source.Name,
+          SourceNS: source.Namespace || 'default',
+          DestinationName: destination.Name,
+          DestinationNS: destination.Namespace || 'default',
+        });
+      } else {
+        // we found an intention in the find higher up, so we are updating
+        notification = this.feedback.notification('update');
+      }
+      set(intention, 'Action', 'allow');
+      await this.repo.persist(intention);
+      notification.success(intention);
+    } catch (e) {
+      notification.error(e);
+    }
     this.refresh();
   }
 
+  afterModel(model, transition) {
+    this.intentions = this.data.source(
+      uri => uri`/${model.nspace}/${model.dc.Name}/intentions/for-service/${model.slug}`
+    );
+  }
+
+  async deactivate(transition) {
+    const intentions = await this.intentions;
+    intentions.destroy();
+  }
+
   async model() {
     const parent = this.routeName
       .split('.')

ui/packages/consul-ui/app/services/feedback.js

@@ -13,54 +13,64 @@ const notificationDefaults = function() {
   };
 };
 export default class FeedbackService extends Service {
-  @service('flashMessages')
-  notify;
+  @service('flashMessages') notify;
+  @service('logger') logger;
 
-  @service('logger')
-  logger;
+  notification(action) {
+    return {
+      success: item => this.success(item, action),
+      error: e => this.error(e, action),
+    };
+  }
 
-  execute(handle, action, status = defaultStatus, controller) {
+  success(item, action, status = defaultStatus) {
     const getAction = callableType(action);
     const getStatus = callableType(status);
-    const notify = this.notify;
-    return (
-      handle()
-        //TODO: pass this through to getAction..
-        .then(item => {
-          // returning exactly `false` for a feedback action means even though
-          // its successful, please skip this notification and don't display it
-          if (item !== false) {
-            notify.clearMessages();
-            // TODO right now the majority of `item` is a Transition
-            // but you can resolve an object
-            notify.add({
-              ...notificationDefaults(),
-              type: getStatus(TYPE_SUCCESS),
-              // here..
-              action: getAction(),
-              item: item,
-            });
-          }
-        })
-        .catch(e => {
-          notify.clearMessages();
-          this.logger.execute(e);
-          if (e.name === 'TransitionAborted') {
-            notify.add({
-              ...notificationDefaults(),
-              type: getStatus(TYPE_SUCCESS),
-              // and here
-              action: getAction(),
-            });
-          } else {
-            notify.add({
-              ...notificationDefaults(),
-              type: getStatus(TYPE_ERROR, e),
-              action: getAction(),
-              error: e,
-            });
-          }
-        })
-    );
+    // returning exactly `false` for a feedback action means even though
+    // its successful, please skip this notification and don't display it
+    if (item !== false) {
+      this.notify.clearMessages();
+      // TODO right now the majority of `item` is a Transition
+      // but you can resolve an object
+      this.notify.add({
+        ...notificationDefaults(),
+        type: getStatus(TYPE_SUCCESS),
+        // here..
+        action: getAction(),
+        item: item,
+      });
+    }
+  }
+
+  error(e, action, status = defaultStatus) {
+    const getAction = callableType(action);
+    const getStatus = callableType(status);
+    this.notify.clearMessages();
+    this.logger.execute(e);
+    if (e.name === 'TransitionAborted') {
+      this.notify.add({
+        ...notificationDefaults(),
+        type: getStatus(TYPE_SUCCESS),
+        // and here
+        action: getAction(),
+      });
+    } else {
+      this.notify.add({
+        ...notificationDefaults(),
+        type: getStatus(TYPE_ERROR, e),
+        action: getAction(),
+        error: e,
+      });
+    }
+  }
+
+  async execute(handle, action, status) {
+    let result;
+    try {
+      result = await handle();
+      this.success(result, action, status);
+    } catch (e) {
+      this.error(e, action, status);
+    }
   }
 }

ui/packages/consul-ui/app/templates/dc/services/show.hbs

@@ -34,6 +34,13 @@
 
     <BlockSlot @name="loaded">
       <AppView>
+        <BlockSlot @name="notification" as |status type item error|>
+          <TopologyMetrics::Notifications
+            @type={{type}}
+            @status={{status}}
+            @error={{error}}
+          />
+        </BlockSlot>
         <BlockSlot @name="breadcrumbs">
           <ol>
               <li><a data-test-back href={{href-to 'dc.services'}}>All Services</a></li>

ui/packages/consul-ui/tests/acceptance/dc/services/show/topology.feature

@@ -0,0 +1,43 @@
+@setupApplicationTest
+Feature: dc / services / show / topology: Intention Create
+  Background:
+    Given 1 datacenter model with the value "datacenter"
+    And 1 intention model from yaml
+    ---
+      SourceNS: default
+      SourceName: web
+      DestinationNS: default
+      DestinationName: db
+      ID: intention-id
+    ---
+    And 1 node model
+    And 1 service model from yaml
+    ---
+    - Service:
+        Name: web
+        Kind: ~
+    ---
+    And 1 topology model from yaml
+    ---
+      Downstreams: []
+      Upstreams:
+        - Name: db
+          Namespace: default
+          Datacenter: datacenter
+          Intention: {}
+    ---
+    When I visit the service page for yaml
+    ---
+      dc: datacenter
+      service: web
+    ---
+  Scenario: Allow a connection between service and upstream by saving an intention
+    When I click ".consul-topology-metrics [data-test-action]"
+    And I click ".consul-topology-metrics [data-test-confirm]"
+    And "[data-notification]" has the "success" class
+  Scenario: There was an error saving the intention
+    Given the url "/v1/connect/intentions/exact?source=default%2Fweb&destination=default%2Fdb&dc=datacenter" responds with a 500 status
+    When I click ".consul-topology-metrics [data-test-action]"
+    And I click ".consul-topology-metrics [data-test-confirm]"
+    And "[data-notification]" has the "error" class
+

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);
+  });
+}

ui/packages/consul-ui/tests/helpers/type-to-url.js

@@ -38,6 +38,9 @@ export default function(type) {
     case 'nspace':
       requests = ['/v1/namespaces', '/v1/namespace/'];
       break;
+    case 'topology':
+      requests = ['/v1/internal/ui/service-topology'];
+      break;
   }
   // TODO: An instance of URL should come in here (instead of 2 args)
   return function(url, method) {