Merge pull request #7743 from mabashian/convert-UserList-functional

Converts UserList to functional component

Reviewed-by: Jake McDermott <yo@jakemcdermott.me>
             https://github.com/jakemcdermott

awx/ui_next/src/screens/User/UserList/UserList.jsx

@@ -1,9 +1,8 @@
-import React, { Component, Fragment } from 'react';
+import React, { useEffect, useCallback } from 'react';
-import { withRouter } from 'react-router-dom';
+import { useLocation, useRouteMatch } from 'react-router-dom';
 import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
 import { Card, PageSection } from '@patternfly/react-core';
-
 import { UsersAPI } from '../../../api';
 import AlertModal from '../../../components/AlertModal';
 import DataListToolbar from '../../../components/DataListToolbar';
@@ -12,8 +11,9 @@ import PaginatedDataList, {
   ToolbarAddButton,
   ToolbarDeleteButton,
 } from '../../../components/PaginatedDataList';
+import useRequest, { useDeleteItems } from '../../../util/useRequest';
+import useSelected from '../../../util/useSelected';
 import { getQSConfig, parseQueryString } from '../../../util/qs';
-
 import UserListItem from './UserListItem';
 
 const QS_CONFIG = getQSConfig('user', {
@@ -22,128 +22,69 @@ const QS_CONFIG = getQSConfig('user', {
   order_by: 'username',
 });
 
-class UsersList extends Component {
+function UserList({ i18n }) {
-  constructor(props) {
+  const location = useLocation();
-    super(props);
+  const match = useRouteMatch();
 
-    this.state = {
+  const {
-      hasContentLoading: true,
+    result: { users, itemCount, actions },
-      contentError: null,
+    error: contentError,
-      deletionError: null,
+    isLoading,
+    request: fetchUsers,
+  } = useRequest(
+    useCallback(async () => {
+      const params = parseQueryString(QS_CONFIG, location.search);
+      const [response, actionsResponse] = await Promise.all([
+        UsersAPI.read(params),
+        UsersAPI.readOptions(),
+      ]);
+      return {
+        users: response.data.results,
+        itemCount: response.data.count,
+        actions: actionsResponse.data.actions,
+      };
+    }, [location]),
+    {
       users: [],
-      selected: [],
       itemCount: 0,
-      actions: null,
+      actions: {},
+    }
+  );
+
+  useEffect(() => {
+    fetchUsers();
+  }, [fetchUsers]);
+
+  const { selected, isAllSelected, handleSelect, setSelected } = useSelected(
+    users
+  );
+
+  const {
+    isLoading: isDeleteLoading,
+    deleteItems: deleteUsers,
+    deletionError,
+    clearDeletionError,
+  } = useDeleteItems(
+    useCallback(async () => {
+      return Promise.all(selected.map(user => UsersAPI.destroy(user.id)));
+    }, [selected]),
+    {
+      qsConfig: QS_CONFIG,
+      allItemsSelected: isAllSelected,
+      fetchItems: fetchUsers,
+    }
+  );
+
+  const handleUserDelete = async () => {
+    await deleteUsers();
+    setSelected([]);
   };
 
-    this.handleSelectAll = this.handleSelectAll.bind(this);
+  const hasContentLoading = isDeleteLoading || isLoading;
-    this.handleSelect = this.handleSelect.bind(this);
+  const canAdd = actions && actions.POST;
-    this.handleUserDelete = this.handleUserDelete.bind(this);
-    this.handleDeleteErrorClose = this.handleDeleteErrorClose.bind(this);
-    this.loadUsers = this.loadUsers.bind(this);
-  }
-
-  componentDidMount() {
-    this.loadUsers();
-  }
-
-  componentDidUpdate(prevProps) {
-    const { location } = this.props;
-    if (location !== prevProps.location) {
-      this.loadUsers();
-    }
-  }
-
-  handleSelectAll(isSelected) {
-    const { users } = this.state;
-
-    const selected = isSelected ? [...users] : [];
-    this.setState({ selected });
-  }
-
-  handleSelect(row) {
-    const { selected } = this.state;
-
-    if (selected.some(s => s.id === row.id)) {
-      this.setState({ selected: selected.filter(s => s.id !== row.id) });
-    } else {
-      this.setState({ selected: selected.concat(row) });
-    }
-  }
-
-  handleDeleteErrorClose() {
-    this.setState({ deletionError: null });
-  }
-
-  async handleUserDelete() {
-    const { selected } = this.state;
-
-    this.setState({ hasContentLoading: true });
-    try {
-      await Promise.all(selected.map(org => UsersAPI.destroy(org.id)));
-    } catch (err) {
-      this.setState({ deletionError: err });
-    } finally {
-      await this.loadUsers();
-    }
-  }
-
-  async loadUsers() {
-    const { location } = this.props;
-    const { actions: cachedActions } = this.state;
-    const params = parseQueryString(QS_CONFIG, location.search);
-
-    let optionsPromise;
-    if (cachedActions) {
-      optionsPromise = Promise.resolve({ data: { actions: cachedActions } });
-    } else {
-      optionsPromise = UsersAPI.readOptions();
-    }
-
-    const promises = Promise.all([UsersAPI.read(params), optionsPromise]);
-
-    this.setState({ contentError: null, hasContentLoading: true });
-    try {
-      const [
-        {
-          data: { count, results },
-        },
-        {
-          data: { actions },
-        },
-      ] = await promises;
-      this.setState({
-        actions,
-        itemCount: count,
-        users: results,
-        selected: [],
-      });
-    } catch (err) {
-      this.setState({ contentError: err });
-    } finally {
-      this.setState({ hasContentLoading: false });
-    }
-  }
-
-  render() {
-    const {
-      actions,
-      itemCount,
-      contentError,
-      hasContentLoading,
-      deletionError,
-      selected,
-      users,
-    } = this.state;
-    const { match, i18n } = this.props;
-
-    const canAdd =
-      actions && Object.prototype.hasOwnProperty.call(actions, 'POST');
-    const isAllSelected =
-      selected.length === users.length && selected.length > 0;
 
   return (
-      <Fragment>
+    <>
       <PageSection>
         <Card>
           <PaginatedDataList
@@ -153,7 +94,7 @@ class UsersList extends Component {
             itemCount={itemCount}
             pluralizedItemName={i18n._(t`Users`)}
             qsConfig={QS_CONFIG}
-              onRowClick={this.handleSelect}
+            onRowClick={handleSelect}
             toolbarSearchColumns={[
               {
                 name: i18n._(t`Username`),
@@ -161,11 +102,11 @@ class UsersList extends Component {
                 isDefault: true,
               },
               {
-                  name: i18n._(t`First Name`),
+                name: i18n._(t`First name`),
                 key: 'first_name',
               },
               {
-                  name: i18n._(t`Last Name`),
+                name: i18n._(t`Last name`),
                 key: 'last_name',
               },
             ]}
@@ -175,11 +116,11 @@ class UsersList extends Component {
                 key: 'username',
               },
               {
-                  name: i18n._(t`First Name`),
+                name: i18n._(t`First name`),
                 key: 'first_name',
               },
               {
-                  name: i18n._(t`Last Name`),
+                name: i18n._(t`Last name`),
                 key: 'last_name',
               },
             ]}
@@ -188,7 +129,9 @@ class UsersList extends Component {
                 {...props}
                 showSelectAll
                 isAllSelected={isAllSelected}
-                  onSelectAll={this.handleSelectAll}
+                onSelectAll={isSelected =>
+                  setSelected(isSelected ? [...users] : [])
+                }
                 qsConfig={QS_CONFIG}
                 additionalControls={[
                   ...(canAdd
@@ -201,7 +144,7 @@ class UsersList extends Component {
                     : []),
                   <ToolbarDeleteButton
                     key="delete"
-                      onDelete={this.handleUserDelete}
+                    onDelete={handleUserDelete}
                     itemsToDelete={selected}
                     pluralizedItemName="Users"
                   />,
@@ -214,7 +157,7 @@ class UsersList extends Component {
                 user={o}
                 detailUrl={`${match.url}/${o.id}/details`}
                 isSelected={selected.some(row => row.id === o.id)}
-                  onSelect={() => this.handleSelect(o)}
+                onSelect={() => handleSelect(o)}
               />
             )}
             emptyStateControls={
@@ -225,19 +168,19 @@ class UsersList extends Component {
           />
         </Card>
       </PageSection>
+      {deletionError && (
         <AlertModal
           isOpen={deletionError}
           variant="error"
           title={i18n._(t`Error!`)}
-          onClose={this.handleDeleteErrorClose}
+          onClose={clearDeletionError}
         >
           {i18n._(t`Failed to delete one or more users.`)}
           <ErrorDetail error={deletionError} />
         </AlertModal>
-      </Fragment>
+      )}
+    </>
   );
-  }
 }
 
-export { UsersList as _UsersList };
+export default withI18n()(UserList);
-export default withI18n()(withRouter(UsersList));

awx/ui_next/src/screens/User/UserList/UserList.test.jsx

@@ -1,16 +1,16 @@
 import React from 'react';
+import { act } from 'react-dom/test-utils';
 import { UsersAPI } from '../../../api';
 import {
   mountWithContexts,
   waitForElement,
 } from '../../../../testUtils/enzymeHelpers';
 
-import UsersList, { _UsersList } from './UserList';
+import UsersList from './UserList';
 
 jest.mock('../../../api');
 
 let wrapper;
-const loadUsers = jest.spyOn(_UsersList.prototype, 'loadUsers');
 const mockUsers = [
   {
     id: 1,
@@ -84,7 +84,8 @@ const mockUsers = [
   },
 ];
 
-beforeAll(() => {
+beforeEach(() => {
+  UsersAPI.destroy = jest.fn();
   UsersAPI.read.mockResolvedValue({
     data: {
       count: mockUsers.length,
@@ -110,146 +111,96 @@ describe('UsersList with full permissions', () => {
     });
   });
 
-  beforeEach(() => {
+  beforeEach(async () => {
+    await act(async () => {
       wrapper = mountWithContexts(<UsersList />);
     });
-
+    wrapper.update();
-  test('initially renders successfully', () => {
-    mountWithContexts(
-      <UsersList
-        match={{ path: '/users', url: '/users' }}
-        location={{ search: '', pathname: '/users' }}
-      />
-    );
   });
 
   test('Users are retrieved from the api and the components finishes loading', async () => {
     await waitForElement(wrapper, 'ContentLoading', el => el.length === 0);
-    expect(loadUsers).toHaveBeenCalled();
+    expect(UsersAPI.read).toHaveBeenCalled();
   });
 
-  test('Selects one team when row is checked', async () => {
+  test('should show add button', () => {
-    await waitForElement(
+    expect(wrapper.find('ToolbarAddButton').length).toBe(1);
-      wrapper,
+  });
-      'UsersList',
+
-      el => el.state('hasContentLoading') === false
+  test('should check and uncheck the row item', async () => {
-    );
     expect(
-      wrapper
+      wrapper.find('DataListCheck[id="select-user-1"]').props().checked
-        .find('input[type="checkbox"]')
+    ).toBe(false);
-        .findWhere(n => n.prop('checked') === true).length
+    await act(async () => {
-    ).toBe(0);
+      wrapper.find('DataListCheck[id="select-user-1"]').invoke('onChange')(
-    wrapper
+        true
-      .find('UserListItem')
+      );
-      .at(0)
+    });
-      .find('DataListCheck')
-      .props()
-      .onChange(true);
     wrapper.update();
     expect(
-      wrapper
+      wrapper.find('DataListCheck[id="select-user-1"]').props().checked
-        .find('input[type="checkbox"]')
+    ).toBe(true);
-        .findWhere(n => n.prop('checked') === true).length
+    await act(async () => {
-    ).toBe(1);
+      wrapper.find('DataListCheck[id="select-user-1"]').invoke('onChange')(
-  });
+        false
-
-  test('Select all checkbox selects and unselects all rows', async () => {
-    await waitForElement(
-      wrapper,
-      'UsersList',
-      el => el.state('hasContentLoading') === false
       );
-    expect(
+    });
-      wrapper
-        .find('input[type="checkbox"]')
-        .findWhere(n => n.prop('checked') === true).length
-    ).toBe(0);
-    wrapper
-      .find('Checkbox#select-all')
-      .props()
-      .onChange(true);
     wrapper.update();
     expect(
-      wrapper
+      wrapper.find('DataListCheck[id="select-user-1"]').props().checked
-        .find('input[type="checkbox"]')
+    ).toBe(false);
-        .findWhere(n => n.prop('checked') === true).length
+  });
-    ).toBe(3);
+
-    wrapper
+  test('should check all row items when select all is checked', async () => {
-      .find('Checkbox#select-all')
+    wrapper.find('DataListCheck').forEach(el => {
-      .props()
+      expect(el.props().checked).toBe(false);
-      .onChange(false);
+    });
+    await act(async () => {
+      wrapper.find('Checkbox#select-all').invoke('onChange')(true);
+    });
     wrapper.update();
-    expect(
+    wrapper.find('DataListCheck').forEach(el => {
-      wrapper
+      expect(el.props().checked).toBe(true);
-        .find('input[type="checkbox"]')
+    });
-        .findWhere(n => n.prop('checked') === true).length
+    await act(async () => {
-    ).toBe(0);
+      wrapper.find('Checkbox#select-all').invoke('onChange')(false);
+    });
+    wrapper.update();
+    wrapper.find('DataListCheck').forEach(el => {
+      expect(el.props().checked).toBe(false);
+    });
   });
 
-  test('delete button is disabled if user does not have delete capabilities on a selected user', async () => {
+  test('should call api delete users for each selected user', async () => {
-    wrapper.find('UsersList').setState({
+    await act(async () => {
-      users: mockUsers,
+      wrapper.find('DataListCheck[id="select-user-1"]').invoke('onChange')();
-      itemCount: 2,
-      isInitialized: true,
-      selected: mockUsers.slice(0, 1),
     });
-    await waitForElement(
+    wrapper.update();
-      wrapper,
+    await act(async () => {
-      'ToolbarDeleteButton * button',
+      wrapper.find('ToolbarDeleteButton').invoke('onDelete')();
-      el => el.getDOMNode().disabled === false
-    );
-    wrapper.find('UsersList').setState({
-      selected: mockUsers,
     });
-    await waitForElement(
+    wrapper.update();
-      wrapper,
+    expect(UsersAPI.destroy).toHaveBeenCalledTimes(1);
-      'ToolbarDeleteButton * button',
-      el => el.getDOMNode().disabled === true
-    );
   });
 
-  test('api is called to delete users for each selected user.', async () => {
+  test('should show error modal when user is not successfully deleted from api', async () => {
-    UsersAPI.destroy = jest.fn();
+    UsersAPI.destroy.mockImplementationOnce(() => Promise.reject(new Error()));
-    wrapper.find('UsersList').setState({
+    // expect(wrapper.debug()).toBe(false);
-      users: mockUsers,
+    expect(wrapper.find('Modal').length).toBe(0);
-      itemCount: 2,
+    await act(async () => {
-      isInitialized: true,
+      wrapper.find('DataListCheck[id="select-user-1"]').invoke('onChange')();
-      isModalOpen: true,
-      selected: mockUsers,
     });
-    await wrapper.find('ToolbarDeleteButton').prop('onDelete')();
+    wrapper.update();
-    expect(UsersAPI.destroy).toHaveBeenCalledTimes(2);
+    await act(async () => {
+      wrapper.find('ToolbarDeleteButton').invoke('onDelete')();
     });
-
+    wrapper.update();
-  test('error is shown when user not successfully deleted from api', async () => {
+    expect(wrapper.find('Modal').length).toBe(1);
-    UsersAPI.destroy.mockRejectedValue(
+    await act(async () => {
-      new Error({
+      wrapper.find('ModalBoxCloseButton').invoke('onClose')();
-        response: {
-          config: {
-            method: 'delete',
-            url: '/api/v2/users/1',
-          },
-          data: 'An error occurred',
-        },
-      })
-    );
-    wrapper.find('UsersList').setState({
-      users: mockUsers,
-      itemCount: 1,
-      isInitialized: true,
-      isModalOpen: true,
-      selected: mockUsers.slice(0, 1),
     });
-    wrapper.find('ToolbarDeleteButton').prop('onDelete')();
+    wrapper.update();
-    await waitForElement(
+    expect(wrapper.find('Modal').length).toBe(0);
-      wrapper,
-      'Modal',
-      el => el.props().isOpen === true && el.props().title === 'Error!'
-    );
-  });
-
-  test('Add button shown for users with ability to POST', async () => {
-    await waitForElement(wrapper, 'ToolbarAddButton', el => el.length === 1);
   });
 });
 
@@ -263,9 +214,21 @@ describe('UsersList without full permissions', () => {
       },
     });
 
+    await act(async () => {
       wrapper = mountWithContexts(<UsersList />);
-    await waitForElement(wrapper, 'ContentLoading', el => el.length === 1);
+    });
-    await waitForElement(wrapper, 'ContentLoading', el => el.length === 0);
+    wrapper.update();
     expect(wrapper.find('ToolbarAddButton').length).toBe(0);
   });
 });
+
+describe('read call unsuccessful', () => {
+  test('should show content error when read call unsuccessful', async () => {
+    UsersAPI.read.mockRejectedValue(new Error());
+    await act(async () => {
+      wrapper = mountWithContexts(<UsersList />);
+    });
+    wrapper.update();
+    expect(wrapper.find('ContentError').length).toBe(1);
+  });
+});