From 0495214f475b6d695c5ba3a616349ee7058fe5b2 Mon Sep 17 00:00:00 2001
From: Kia Lam <digitalanime@gmail.com>
Date: Wed, 13 Mar 2019 15:59:42 -0400
Subject: [PATCH] Address PR feedback.

---
 __tests__/components/AccessList.test.jsx  | 19 ++++++-------------
 src/app.scss                              |  7 +++++++
 src/components/AccessList/Access.list.jsx | 16 ++++++++++------
 src/components/BasicChip/basicChip.scss   |  7 -------
 4 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/__tests__/components/AccessList.test.jsx b/__tests__/components/AccessList.test.jsx
index ce9a49c132..77aff16344 100644
--- a/__tests__/components/AccessList.test.jsx
+++ b/__tests__/components/AccessList.test.jsx
@@ -114,7 +114,6 @@ describe('<AccessList />', () => {
   });
 
   test('getTeamRoles returns empty array if dataset is missing team_id attribute', (done) => {
-    
     const wrapper = mount(
       <I18nProvider>
         <MemoryRouter>
@@ -140,7 +139,7 @@ describe('<AccessList />', () => {
   test('test handleWarning, confirmDelete, and removeRole methods for Alert component', (done) => {
     const handleWarning = jest.spyOn(AccessList.prototype, 'handleWarning');
     const confirmDelete = jest.spyOn(AccessList.prototype, 'confirmDelete');
-    const removeRole = jest.spyOn(AccessList.prototype, 'removeRole');
+    const removeRole = jest.spyOn(AccessList.prototype, 'removeAccessRole');
     const wrapper = mount(
       <I18nProvider>
         <MemoryRouter>
@@ -182,19 +181,13 @@ describe('<AccessList />', () => {
         </MemoryRouter>
       </I18nProvider>
     ).find('AccessList');
-    
-    expect(wrapper.state().warningMsg).not.toBeDefined;
-    expect(wrapper.state().warningTitle).not.toBeDefined;
-    expect(wrapper.state().deleteType).not.toBeDefined;
-    expect(wrapper.state().deleteRoleId).not.toBeDefined;
-    expect(wrapper.state().deleteResourceId).not.toBeDefined;
 
     setImmediate(() => {
       const expected = [
         {
           deleteType: 'users'
         },
-        { 
+        {
           deleteRoleId: mockData[0].summary_fields.foo[0].role.id
         },
         {
@@ -208,10 +201,10 @@ describe('<AccessList />', () => {
       expect(wrapper.state().warningTitle).not.toBe(null);
       expect(wrapper.state().warningMsg).not.toBe(null);
       expected.forEach(criteria => {
-        for (const prop in criteria) {
-          expect(wrapper.state()[prop]).toEqual(criteria[prop]);
-        }
-      })
+        Object.keys(criteria).forEach(key => {
+          expect(wrapper.state()[key]).toEqual(criteria[key]);
+        });
+      });
       done();
     });
   });
diff --git a/src/app.scss b/src/app.scss
index 9655d532b8..637bcdb636 100644
--- a/src/app.scss
+++ b/src/app.scss
@@ -275,3 +275,10 @@
 .at-u-textRight {
   text-align: right;
 }
+
+.awx-c-chip {
+  padding: 3px 0;
+  height: 24px;
+  margin-right: 10px;
+  margin-bottom: 10px;
+}
\ No newline at end of file
diff --git a/src/components/AccessList/Access.list.jsx b/src/components/AccessList/Access.list.jsx
index 01b0f085b7..7a55688d8a 100644
--- a/src/components/AccessList/Access.list.jsx
+++ b/src/components/AccessList/Access.list.jsx
@@ -116,7 +116,7 @@ class AccessList extends React.Component {
     this.onCompact = this.onCompact.bind(this);
     this.onSort = this.onSort.bind(this);
     this.getQueryParams = this.getQueryParams.bind(this);
-    this.removeRole = this.removeRole.bind(this);
+    this.removeAccessRole = this.removeAccessRole.bind(this);
     this.handleWarning = this.handleWarning.bind(this);
     this.hideWarning = this.hideWarning.bind(this);
     this.confirmDelete = this.confirmDelete.bind(this);
@@ -225,8 +225,8 @@ class AccessList extends React.Component {
           }
         });
 
-        result.teamRoles = teamRoles || [];
-        result.userRoles = userRoles || [];
+        result.teamRoles = teamRoles;
+        result.userRoles = userRoles;
       });
       this.setState(stateToUpdate);
     } catch (error) {
@@ -234,10 +234,14 @@ class AccessList extends React.Component {
     }
   }
 
-  async removeRole (roleId, resourceId, type) {
+  async removeAccessRole (roleId, resourceId, type) {
     const { removeRole } = this.props;
     const url = `/api/v2/${type}/${resourceId}/roles/`;
-    await removeRole(url, roleId);
+    try {
+      await removeRole(url, roleId);
+    } catch (error) {
+      this.setState({ error });
+    }
     const queryParams = this.getQueryParams();
     try {
       this.fetchOrgAccessList(queryParams);
@@ -280,7 +284,7 @@ class AccessList extends React.Component {
 
   confirmDelete () {
     const { deleteType, deleteResourceId, deleteRoleId } = this.state;
-    this.removeRole(deleteRoleId, deleteResourceId, deleteType);
+    this.removeAccessRole(deleteRoleId, deleteResourceId, deleteType);
   }
 
   render () {
diff --git a/src/components/BasicChip/basicChip.scss b/src/components/BasicChip/basicChip.scss
index 41e7e8f5df..8cea76d049 100644
--- a/src/components/BasicChip/basicChip.scss
+++ b/src/components/BasicChip/basicChip.scss
@@ -8,10 +8,3 @@
     display: none;
   }
 }
-
-.awx-c-chip {
-  padding: 3px 0;
-  height: 24px;
-  margin-right: 10px;
-  margin-bottom: 10px;
-}
\ No newline at end of file