From 75e221d256a2026a587eb21f21ee3623ad8136d4 Mon Sep 17 00:00:00 2001
From: John Cowen <johncowen@users.noreply.github.com>
Date: Tue, 4 Jun 2019 15:53:10 +0100
Subject: [PATCH] ui: ACL Policies. Catch all server errors (#5836)

Always show any server errors under Rules, not just invalid HCL
---
 ui-v2/app/components/policy-selector.js       |  4 ++-
 .../app/templates/components/policy-form.hbs  |  2 +-
 .../dc/acls/policies/as-many/add-new.feature  | 27 +++++++++++++++
 ui-v2/tests/pages.js                          |  2 +-
 ui-v2/tests/pages/components/policy-form.js   |  9 +++--
 .../tests/pages/components/policy-selector.js |  2 +-
 ui-v2/tests/steps/assertions/page.js          | 34 ++++++++++++-------
 7 files changed, 62 insertions(+), 18 deletions(-)

diff --git a/ui-v2/app/components/policy-selector.js b/ui-v2/app/components/policy-selector.js
index 778c7296eb..295afce423 100644
--- a/ui-v2/app/components/policy-selector.js
+++ b/ui-v2/app/components/policy-selector.js
@@ -4,6 +4,7 @@ import { inject as service } from '@ember/service';
 import updateArrayObject from 'consul-ui/utils/update-array-object';
 
 const ERROR_PARSE_RULES = 'Failed to parse ACL rules';
+const ERROR_INVALID_POLICY = 'Invalid service policy';
 const ERROR_NAME_EXISTS = 'Invalid Policy: A Policy with Name';
 
 export default ChildSelectorComponent.extend({
@@ -38,10 +39,11 @@ export default ChildSelectorComponent.extend({
     const err = e.error;
     if (typeof err.errors !== 'undefined') {
       const error = err.errors[0];
-      let prop;
+      let prop = 'Rules';
       let message = error.detail;
       switch (true) {
         case message.indexOf(ERROR_PARSE_RULES) === 0:
+        case message.indexOf(ERROR_INVALID_POLICY) === 0:
           prop = 'Rules';
           message = error.detail;
           break;
diff --git a/ui-v2/app/templates/components/policy-form.hbs b/ui-v2/app/templates/components/policy-form.hbs
index eeee86e85f..c573ee5a1a 100644
--- a/ui-v2/app/templates/components/policy-form.hbs
+++ b/ui-v2/app/templates/components/policy-form.hbs
@@ -28,7 +28,7 @@
           <strong>{{item.error.Name.validation}}</strong>
         {{/if}}
     </label>
-    <label class="type-text">
+    <label class="type-text" data-test-rules>
         <span>Rules <a href="{{env 'CONSUL_DOCUMENTATION_URL'}}/guides/acl.html#rule-specification" rel="help noopener noreferrer" target="_blank">(HCL Format)</a></span>
 {{#if (eq item.template '') }}
         {{code-editor syntax='hcl' class=(if item.error.Rules 'error') name=(concat name '[Rules]') value=item.Rules onkeyup=(action 'change' (concat name '[Rules]'))}}
diff --git a/ui-v2/tests/acceptance/dc/acls/policies/as-many/add-new.feature b/ui-v2/tests/acceptance/dc/acls/policies/as-many/add-new.feature
index 2e26510568..73f60a7718 100644
--- a/ui-v2/tests/acceptance/dc/acls/policies/as-many/add-new.feature
+++ b/ui-v2/tests/acceptance/dc/acls/policies/as-many/add-new.feature
@@ -67,6 +67,33 @@ Feature: dc / acls / policies / as many / add new: Add new policy
     | token     |
     | role      |
     -------------
+  Scenario: Adding a new policy as a child of [Model] and getting an error
+    Given the url "/v1/acl/policy" responds with from yaml
+    ---
+    status: 500
+    body: |
+      Invalid service policy: acl.ServicePolicy{Name:"service", Policy:"", Sentinel:acl.Sentinel{Code:"", EnforcementLevel:""}, Intentions:""}
+    ---
+    Then I fill in the policies.form with yaml
+    ---
+      Name: New-Policy
+      Description: New Policy Description
+      Rules: key {}
+    ---
+    And I click submit on the policies.form
+    Then the last PUT request was made to "/v1/acl/policy?dc=datacenter" with the body from yaml
+    ---
+      Name: New-Policy
+      Description: New Policy Description
+      Rules: key {}
+    ---
+    And I see error on the policies.form.rules like 'Invalid service policy: acl.ServicePolicy{Name:"service", Policy:"", Sentinel:acl.Sentinel{Code:"", EnforcementLevel:""}, Intentions:""}'
+  Where:
+    -------------
+    | Model     |
+    | token     |
+    | role      |
+    -------------
 @ignore:
   Scenario: Click the cancel form
     Then ok
diff --git a/ui-v2/tests/pages.js b/ui-v2/tests/pages.js
index 5047067251..cb301ce3d2 100644
--- a/ui-v2/tests/pages.js
+++ b/ui-v2/tests/pages.js
@@ -48,7 +48,7 @@ const cancelable = createCancelable(clickable, is);
 
 const tokenList = tokenListFactory(clickable, attribute, collection, deletable);
 
-const policyForm = policyFormFactory(submitable, cancelable, radiogroup);
+const policyForm = policyFormFactory(submitable, cancelable, radiogroup, text);
 const policySelector = policySelectorFactory(clickable, deletable, collection, alias, policyForm);
 
 const roleForm = roleFormFactory(submitable, cancelable, policySelector);
diff --git a/ui-v2/tests/pages/components/policy-form.js b/ui-v2/tests/pages/components/policy-form.js
index 71c5e0a37a..bb00787765 100644
--- a/ui-v2/tests/pages/components/policy-form.js
+++ b/ui-v2/tests/pages/components/policy-form.js
@@ -1,11 +1,16 @@
-export default (submitable, cancelable, radiogroup) => () => {
+export default (submitable, cancelable, radiogroup, text) => (
+  scope = '[data-test-policy-form]'
+) => {
   return {
     // this should probably be settable
     resetScope: true,
-    scope: '[data-test-policy-form]',
+    scope: scope,
     prefix: 'policy',
     ...submitable(),
     ...cancelable(),
     ...radiogroup('template', ['', 'service-identity'], 'policy'),
+    rules: {
+      error: text('[data-test-rules] strong'),
+    },
   };
 };
diff --git a/ui-v2/tests/pages/components/policy-selector.js b/ui-v2/tests/pages/components/policy-selector.js
index 25c7a4a420..a2c684e7d7 100644
--- a/ui-v2/tests/pages/components/policy-selector.js
+++ b/ui-v2/tests/pages/components/policy-selector.js
@@ -5,7 +5,7 @@ export default (clickable, deletable, collection, alias, policyForm) => (
   return {
     scope: scope,
     create: clickable(createSelector),
-    form: policyForm(),
+    form: policyForm('#new-policy-toggle + div'),
     policies: alias('selectedOptions'),
     selectedOptions: collection(
       '[data-test-policies] [data-test-tabular-row]',
diff --git a/ui-v2/tests/steps/assertions/page.js b/ui-v2/tests/steps/assertions/page.js
index 8e93cf85f7..f7f4ae6bed 100644
--- a/ui-v2/tests/steps/assertions/page.js
+++ b/ui-v2/tests/steps/assertions/page.js
@@ -96,18 +96,28 @@ export default function(scenario, assert, find, currentPage) {
     .then(['I see $property'], function(property) {
       assert.ok(currentPage()[property], `Expected to see ${property}`);
     })
-    .then(['I see $property on the $component like "$value"'], function(
-      property,
-      component,
-      value
-    ) {
-      const target = currentPage()[component][property];
-      assert.equal(
-        target,
-        value,
-        `Expected to see ${property} on ${component} as ${value}, was ${target}`
-      );
-    })
+    .then(
+      [
+        'I see $property on the $component like "$value"',
+        "I see $property on the $component like '$value'",
+      ],
+      function(property, component, value) {
+        let target;
+        try {
+          if (typeof component === 'string') {
+            property = `${component}.${property}`;
+          }
+          target = find(property);
+        } catch (e) {
+          throw e;
+        }
+        assert.equal(
+          target,
+          value,
+          `Expected to see ${property} on ${component} as ${value}, was ${target}`
+        );
+      }
+    )
     .then(['I see $property like "$value"'], function(property, value) {
       const target = currentPage()[property];
       assert.equal(target, value, `Expected to see ${property} as ${value}, was ${target}`);