From d55d6419e4a560651d4c6122beaeb1e6de689d97 Mon Sep 17 00:00:00 2001
From: Susmitha Girumala <susmitha@hashicorp.com>
Date: Tue, 1 Sep 2020 15:29:19 -0400
Subject: [PATCH] feat: Add checkbox table for user/group assignment to role

---
 .../form/role/add-principals/index.hbs        | 101 ++++++++----------
 .../form/role/add-principals/index.js         |  18 +++-
 .../project/targets/target/add-host-sets.js   |   2 +-
 .../scopes/scope/roles/role/add-principals.js |  26 +++--
 .../project/targets/target/add-host-sets.hbs  |   4 +
 .../scope/roles/role/add-principals.hbs       |   4 +-
 ui/core/tests/acceptance/roles-test.js        |   8 ++
 .../form/role/add-principals/index-test.js    |  26 -----
 ui/core/translations/en-us.yaml               |   3 +
 9 files changed, 97 insertions(+), 95 deletions(-)
 delete mode 100644 ui/core/tests/integration/components/form/role/add-principals/index-test.js

diff --git a/ui/core/app/components/form/role/add-principals/index.hbs b/ui/core/app/components/form/role/add-principals/index.hbs
index d15f49bc3..c47e8e51d 100644
--- a/ui/core/app/components/form/role/add-principals/index.hbs
+++ b/ui/core/app/components/form/role/add-principals/index.hbs
@@ -1,66 +1,53 @@
-{{#if this.hasAvailablePrincipals}}
-  <Rose::Form
-    class="full-width"
-    @onSubmit={{fn this.submit @submit}}
-    @cancel={{@cancel}}
-    @disabled={{@model.isSaving}}
-    @error={{@model.errors.base}}
-    as |form|
-  >
+<Rose::Form
+  class="full-width"
+  @onSubmit={{fn this.submit @submit}}
+  @cancel={{@cancel}}
+  @disabled={{@model.isSaving}}
+  @error={{@model.errors.base}}
+  as |form|
+>
 
-    <form.actions
-      @submitText={{t "actions.add-principals"}}
-      @cancelText={{t "actions.cancel"}} />
+  <form.actions
+    @submitText={{t "actions.add-principals"}}
+    @cancelText={{t "actions.cancel"}} />
 
-    <Rose::Table as |table|>
-      <table.header as |header|>
-        <header.row as |row|>
-          <row.headerCell>{{t 'form.label.id'}}</row.headerCell>
-          <row.headerCell>{{t 'form.label.name'}}</row.headerCell>
-          <row.headerCell>{{t 'form.label.type'}}</row.headerCell>
-        </header.row>
-      </table.header>
-      <table.body as |body|>
-        {{#each @groups as |group|}}
-          <body.row as |row|>
-            <row.cell>
-              <form.checkbox
-                @label={{group.id}}
-                @description={{group.description}}
-                onChange={{fn this.togglePrincipal group}} />
-            </row.cell>
-            <row.cell>{{group.name}}</row.cell>
-            <row.cell>
-              <Rose::Badge>{{t "resources.group"}}</Rose::Badge>
-            </row.cell>
-          </body.row>
-        {{/each}}
-        {{#each @users as |user|}}
-          <body.row as |row|>
-            <row.cell>
-              <form.checkbox
-                @label={{user.id}}
-                @description={{user.description}}
-                onChange={{fn this.togglePrincipal user}} />
-            </row.cell>
-            <row.cell>{{user.name}}</row.cell>
-            <row.cell>
-              <Rose::Badge>{{t "resources.user"}}</Rose::Badge>
-            </row.cell>
-          </body.row>
-        {{/each}}
-      </table.body>
-    </Rose::Table>
+  <Rose::Table as |table|>
+    <table.header as |header|>
+      <header.row as |row|>
+        <row.headerCell>{{t 'form.label.id'}}</row.headerCell>
+        <row.headerCell>{{t 'form.label.name'}}</row.headerCell>
+        <row.headerCell>{{t 'form.label.type'}}</row.headerCell>
+      </header.row>
+    </table.header>
+    <table.body as |body|>
+      {{#each this.filteredPrincipals as |principal|}}
+        <body.row as |row|>
+          <row.cell>
+            <form.checkbox
+              @label={{principal.id}}
+              @description={{principal.description}}
+              onChange={{fn this.togglePrincipal principal}} />
+          </row.cell>
+          <row.cell>{{principal.name}}</row.cell>
+          <row.cell>
+            <Rose::Badge>{{t (concat 'resources.' principal.constructor.modelName)}}</Rose::Badge>
+          </row.cell>
+        </body.row>
+      {{/each}}
+    </table.body>
+  </Rose::Table>
+
+</Rose::Form>
 
-  </Rose::Form>
-{{/if}}
 
-{{#unless this.hasAvailablePrincipals}}
   <Rose::Layout::Centered>
-    <Rose::Message @title={{t "titles.no-principals"}} as |message|>
+    <Rose::Message @title={{t "titles.no-principals-available"}} as |message|>
       <message.description>
-        {{t "descriptions.no-principals"}}
+        {{t "descriptions.no-principals-available"}}
       </message.description>
+      <message.link @route="scopes.scope.roles.role.principals">
+        <Rose::Icon @name="chevron-left" />
+        {{t "actions.back"}}
+      </message.link>
     </Rose::Message>
   </Rose::Layout::Centered>
-{{/unless}}
diff --git a/ui/core/app/components/form/role/add-principals/index.js b/ui/core/app/components/form/role/add-principals/index.js
index 906d4d9b8..913a53e03 100644
--- a/ui/core/app/components/form/role/add-principals/index.js
+++ b/ui/core/app/components/form/role/add-principals/index.js
@@ -1,8 +1,8 @@
 import Component from '@glimmer/component';
-import { action } from '@ember/object';
+import { computed, action } from '@ember/object';
 import { A } from '@ember/array';
 
-export default class FormRoleAddPrincipalsComponent extends Component {
+export default class FormRoleAddPrincipalsIndexComponent extends Component {
 
   // =properties
 
@@ -19,6 +19,20 @@ export default class FormRoleAddPrincipalsComponent extends Component {
    */
   hasAvailablePrincipals = true;
 
+  /**
+   * Principals not currently assigned to a role.
+   * @type {[UserModel, GroupModel]}
+   */
+  @computed('args.{users,groups}')
+  get filteredPrincipals() {
+    // Retrieve principal IDs assigned to current role
+    const currentPrincipalIDs = this.args.model.principals.map((principal) => principal.principal_id);
+    // Retrieve principal IDs not assigned to current role
+    const unassignedUsers = this.args.users.filter(({ id }) => !currentPrincipalIDs.includes(id));
+    const unassignedGroups = this.args.groups.filter(({ id }) => !currentPrincipalIDs.includes(id));
+    return unassignedUsers.concat(unassignedGroups);
+  }
+
   // =actions
 
   @action
diff --git a/ui/core/app/routes/scopes/scope/projects/project/targets/target/add-host-sets.js b/ui/core/app/routes/scopes/scope/projects/project/targets/target/add-host-sets.js
index f58b7ebb7..f1fdc55ec 100644
--- a/ui/core/app/routes/scopes/scope/projects/project/targets/target/add-host-sets.js
+++ b/ui/core/app/routes/scopes/scope/projects/project/targets/target/add-host-sets.js
@@ -13,7 +13,7 @@ export default class ScopesScopeProjectsProjectTargetsTargetAddHostSetsRoute ext
   // =methods
 
   /**
-   * Emtpy out any previously loaded host catalogs and host sets.
+   * Empty out any previously loaded host catalogs and host sets.
    */
   beforeModel() {
     this.store.unloadAll('host-catalog');
diff --git a/ui/core/app/routes/scopes/scope/roles/role/add-principals.js b/ui/core/app/routes/scopes/scope/roles/role/add-principals.js
index e8d175dcb..73d8cd7e1 100644
--- a/ui/core/app/routes/scopes/scope/roles/role/add-principals.js
+++ b/ui/core/app/routes/scopes/scope/roles/role/add-principals.js
@@ -13,26 +13,38 @@ export default class ScopesScopeRolesRoleAddPrincipalsRoute extends Route {
   // =methods
 
   /**
-   * Empty out any previously loaded users and groups
-   * (collectively, principals).
+   * Empty out any previously loaded users and groups.
    */
   beforeModel() {
     this.store.unloadAll('user');
     this.store.unloadAll('group');
   }
 
+  /**
+   * Returns the current role, all users, and all groups
+   * @return {{role: RoleModel, users: [UserModel], groups: [GroupModel]}}
+   */
   model() {
     const role = this.modelFor('scopes.scope.roles.role');
     const { scopeID } = role;
     const options = { adapterOptions: { scopeID } };
-    return hash({
+
+    return {
       role,
       users: this.store.findAll('user', options),
       groups: this.store.findAll('group', options)
-    });
+    };
   }
 
-  // =methods
+  /**
+   * Checks for unassigned principals.
+   * @param {RoleModel} role
+   * @param {[UserModel]} users
+   * @param {[GroupModel]} groups
+   */
+  canAssignPrincipals(role, users, groups) {
+    return role.principals.length != (users.length + groups.length);
+  }
 
   /**
    * Renders the add-principals-specific header template.
@@ -57,8 +69,10 @@ export default class ScopesScopeRolesRoleAddPrincipalsRoute extends Route {
     });
   }
 
+  // =actions
+
   /**
-   * Saves principal IDs into the role via the API.
+   * Save principal IDs to current role.
    * @param {RoleModel} role
    * @param {[string]} principalIDs
    */
diff --git a/ui/core/app/templates/scopes/scope/projects/project/targets/target/add-host-sets.hbs b/ui/core/app/templates/scopes/scope/projects/project/targets/target/add-host-sets.hbs
index 206de2d68..e487f20f6 100644
--- a/ui/core/app/templates/scopes/scope/projects/project/targets/target/add-host-sets.hbs
+++ b/ui/core/app/templates/scopes/scope/projects/project/targets/target/add-host-sets.hbs
@@ -16,6 +16,10 @@
       <message.description>
         {{t "descriptions.no-host-sets-for-target"}}
       </message.description>
+      <message.link @route="scopes.scope.projects.project.targets.target.host-sets">
+        <Rose::Icon @name="chevron-left" />
+        {{t "actions.back"}}
+      </message.link>
     </Rose::Message>
   </Rose::Layout::Centered>
 {{/unless}}
diff --git a/ui/core/app/templates/scopes/scope/roles/role/add-principals.hbs b/ui/core/app/templates/scopes/scope/roles/role/add-principals.hbs
index 5110dbec0..81fc1baa6 100644
--- a/ui/core/app/templates/scopes/scope/roles/role/add-principals.hbs
+++ b/ui/core/app/templates/scopes/scope/roles/role/add-principals.hbs
@@ -1,9 +1,7 @@
-{{title (t "actions.add-principals")}}
-
 <Form::Role::AddPrincipals
   @model={{@model.role}}
   @users={{@model.users}}
   @groups={{@model.groups}}
   @submit={{route-action "addPrincipals" @model.role}}
   @cancel={{route-action "cancel"}}
-/>
+/>
\ No newline at end of file
diff --git a/ui/core/tests/acceptance/roles-test.js b/ui/core/tests/acceptance/roles-test.js
index 07f55d080..f88517210 100644
--- a/ui/core/tests/acceptance/roles-test.js
+++ b/ui/core/tests/acceptance/roles-test.js
@@ -226,6 +226,14 @@ module('Acceptance | roles', function (hooks) {
     assert.expect(0);
   });
 
+  test('select and save principals to add', async function (assert) {
+    assert.expect(0);
+  });
+
+  test('select and cancel principals to add', async function (assert) {
+    assert.expect(0);
+  });
+
   test('grants can be updated and saved or canceled', async function (assert) {
     assert.expect(0);
   });
diff --git a/ui/core/tests/integration/components/form/role/add-principals/index-test.js b/ui/core/tests/integration/components/form/role/add-principals/index-test.js
deleted file mode 100644
index 12716f951..000000000
--- a/ui/core/tests/integration/components/form/role/add-principals/index-test.js
+++ /dev/null
@@ -1,26 +0,0 @@
-import { module, test } from 'qunit';
-import { setupRenderingTest } from 'ember-qunit';
-import { render } from '@ember/test-helpers';
-import { hbs } from 'ember-cli-htmlbars';
-
-module('Integration | Component | form/role/add-principals/index', function(hooks) {
-  setupRenderingTest(hooks);
-
-  test('it renders', async function(assert) {
-    // Set any properties with this.set('myProperty', 'value');
-    // Handle any actions with this.set('myAction', function(val) { ... });
-
-    await render(hbs`<Form::Role::AddPrincipals::Index />`);
-
-    assert.equal(this.element.textContent.trim(), '');
-
-    // Template block usage:
-    await render(hbs`
-      <Form::Role::AddPrincipals::Index>
-        template block text
-      </Form::Role::AddPrincipals::Index>
-    `);
-
-    assert.equal(this.element.textContent.trim(), 'template block text');
-  });
-});
diff --git a/ui/core/translations/en-us.yaml b/ui/core/translations/en-us.yaml
index 427c1b956..e79d240e0 100644
--- a/ui/core/translations/en-us.yaml
+++ b/ui/core/translations/en-us.yaml
@@ -7,6 +7,7 @@ actions:
   save: Save
   manage: Manage
   help: Need help?
+  back: Back
   authenticate: Authenticate
   deauthenticate: Deauthenticate
   choose-org: Choose Scope
@@ -94,6 +95,7 @@ titles:
   no-host-set-hosts: No Hosts Available
   no-host-sets-for-target: No Host Sets Available
   role-principals-welcome: No Principals Associated
+  no-principals-available: No Principals Available
 descriptions:
   host-catalogs: Overview of collections of hosts and host sets.
   empty-set: There are no items to display yet.  You may be able to add items or try back later.
@@ -110,6 +112,7 @@ descriptions:
   no-host-set-hosts: Before associating hosts with this host set, create hosts within the host catalog.
   no-host-sets-for-target: No host sets are available to add to this target.
   role-principals-welcome: No principals are available in this role.
+  no-principals-available: Principals are not available for assignment.
 states:
   info: Info
   success: Success