Merge pull request #6687 from nixocio/ui_convert_user_to_be_function

Update User component to be function based

Reviewed-by: https://github.com/apps/softwarefactory-project-zuul

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

@@ -1,121 +1,96 @@
-import React, { Component } from 'react';
+import React, { useEffect, useCallback } from 'react';
 import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
-import { Switch, Route, withRouter, Redirect, Link } from 'react-router-dom';
+import {
+  Switch,
+  Route,
+  Redirect,
+  Link,
+  useRouteMatch,
+  useLocation,
+} from 'react-router-dom';
+import useRequest from '@util/useRequest';
+import { UsersAPI } from '@api';
 import { Card, CardActions, PageSection } from '@patternfly/react-core';
 import { TabbedCardHeader } from '@components/Card';
 import CardCloseButton from '@components/CardCloseButton';
-import RoutedTabs from '@components/RoutedTabs';
 import ContentError from '@components/ContentError';
+import ContentLoading from '@components/ContentLoading';
+import RoutedTabs from '@components/RoutedTabs';
 import UserDetail from './UserDetail';
 import UserEdit from './UserEdit';
 import UserOrganizations from './UserOrganizations';
 import UserTeams from './UserTeams';
 import UserTokens from './UserTokens';
-import { UsersAPI } from '@api';
 
-class User extends Component {
+function User({ i18n, setBreadcrumb }) {
-  constructor(props) {
+  const location = useLocation();
-    super(props);
+  const match = useRouteMatch('/users/:id');
+  const userListUrl = `/users`;
+  const {
+    result: user,
+    error: contentError,
+    isLoading,
+    request: fetchUser,
+  } = useRequest(
+    useCallback(async () => {
+      const { data } = await UsersAPI.readDetail(match.params.id);
+      return data;
+    }, [match.params.id]),
+    null
+  );
 
-    this.state = {
+  useEffect(() => {
-      user: null,
+    fetchUser();
-      hasContentLoading: true,
+  }, [fetchUser, location.pathname]);
-      contentError: null,
-      isInitialized: false,
-    };
-    this.loadUser = this.loadUser.bind(this);
-  }
 
-  async componentDidMount() {
+  useEffect(() => {
-    await this.loadUser();
+    if (user) {
-    this.setState({ isInitialized: true });
+      setBreadcrumb(user);
-  }
-
-  async componentDidUpdate(prevProps) {
-    const { location, match } = this.props;
-    const url = `/users/${match.params.id}/`;
-
-    if (
-      prevProps.location.pathname.startsWith(url) &&
-      prevProps.location !== location &&
-      location.pathname === `${url}details`
-    ) {
-      await this.loadUser();
     }
-  }
+  }, [user, setBreadcrumb]);
 
-  async loadUser() {
+  const tabsArray = [
-    const { match, setBreadcrumb } = this.props;
+    { name: i18n._(t`Details`), link: `${match.url}/details`, id: 0 },
-    const id = parseInt(match.params.id, 10);
+    {
-
+      name: i18n._(t`Organizations`),
-    this.setState({ contentError: null, hasContentLoading: true });
+      link: `${match.url}/organizations`,
-    try {
+      id: 1,
-      const { data } = await UsersAPI.readDetail(id);
+    },
-      setBreadcrumb(data);
+    { name: i18n._(t`Teams`), link: `${match.url}/teams`, id: 2 },
-      this.setState({ user: data });
+    { name: i18n._(t`Access`), link: `${match.url}/access`, id: 3 },
-    } catch (err) {
+    { name: i18n._(t`Tokens`), link: `${match.url}/tokens`, id: 4 },
-      this.setState({ contentError: err });
+  ];
-    } finally {
-      this.setState({ hasContentLoading: false });
-    }
-  }
-
-  render() {
-    const { location, match, i18n } = this.props;
-
-    const { user, contentError, hasContentLoading, isInitialized } = this.state;
-
-    const tabsArray = [
-      { name: i18n._(t`Details`), link: `${match.url}/details`, id: 0 },
-      {
-        name: i18n._(t`Organizations`),
-        link: `${match.url}/organizations`,
-        id: 1,
-      },
-      { name: i18n._(t`Teams`), link: `${match.url}/teams`, id: 2 },
-      { name: i18n._(t`Access`), link: `${match.url}/access`, id: 3 },
-      { name: i18n._(t`Tokens`), link: `${match.url}/tokens`, id: 4 },
-    ];
-
-    let cardHeader = (
-      <TabbedCardHeader>
-        <RoutedTabs tabsArray={tabsArray} />
-        <CardActions>
-          <CardCloseButton linkTo="/users" />
-        </CardActions>
-      </TabbedCardHeader>
-    );
-
-    if (!isInitialized) {
-      cardHeader = null;
-    }
-
-    if (location.pathname.endsWith('edit')) {
-      cardHeader = null;
-    }
-
-    if (!hasContentLoading && contentError) {
-      return (
-        <PageSection>
-          <Card>
-            <ContentError error={contentError}>
-              {contentError.response.status === 404 && (
-                <span>
-                  {i18n._(`User not found.`)}{' '}
-                  <Link to="/users">{i18n._(`View all Users.`)}</Link>
-                </span>
-              )}
-            </ContentError>
-          </Card>
-        </PageSection>
-      );
-    }
 
+  if (contentError) {
     return (
       <PageSection>
         <Card>
-          {cardHeader}
+          <ContentError error={contentError}>
+            {contentError.response && contentError.response.status === 404 && (
+              <span>
+                {i18n._(`User not found.`)}{' '}
+                <Link to={userListUrl}>{i18n._(`View all Users.`)}</Link>
+              </span>
+            )}
+          </ContentError>
+        </Card>
+      </PageSection>
+    );
+  }
+  return (
+    <PageSection>
+      <Card>
+        {['edit'].some(name => location.pathname.includes(name)) ? null : (
+          <TabbedCardHeader>
+            <RoutedTabs tabsArray={tabsArray} />
+            <CardActions>
+              <CardCloseButton linkTo={userListUrl} />
+            </CardActions>
+          </TabbedCardHeader>
+        )}
+        {isLoading && <ContentLoading />}
+        {!isLoading && user && (
           <Switch>
             <Redirect from="/users/:id" to="/users/:id/details" exact />
             {user && (
@@ -146,22 +121,19 @@ class User extends Component {
               <UserTokens id={Number(match.params.id)} />
             </Route>
             <Route key="not-found" path="*">
-              {!hasContentLoading && (
+              <ContentError isNotFound>
-                <ContentError isNotFound>
+                {match.params.id && (
-                  {match.params.id && (
+                  <Link to={`/users/${match.params.id}/details`}>
-                    <Link to={`/users/${match.params.id}/details`}>
+                    {i18n._(`View User Details`)}
-                      {i18n._(`View User Details`)}
+                  </Link>
-                    </Link>
+                )}
-                  )}
+              </ContentError>
-                </ContentError>
-              )}
             </Route>
           </Switch>
-        </Card>
+        )}
-      </PageSection>
+      </Card>
-    );
+    </PageSection>
-  }
+  );
 }
 
-export default withI18n()(withRouter(User));
+export default withI18n()(User);
-export { User as _User };

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

@@ -1,4 +1,5 @@
 import React from 'react';
+import { act } from 'react-dom/test-utils';
 import { createMemoryHistory } from 'history';
 import { UsersAPI } from '@api';
 import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers';
@@ -7,11 +8,6 @@ import User from './User';
 
 jest.mock('@api');
 
-const mockMe = {
-  is_super_user: true,
-  is_system_auditor: false,
-};
-
 async function getUsers() {
   return {
     count: 1,
@@ -24,29 +20,78 @@ async function getUsers() {
 }
 
 describe('<User />', () => {
-  test('initially renders succesfully', () => {
+  test('initially renders successfully', async () => {
     UsersAPI.readDetail.mockResolvedValue({ data: mockDetails });
     UsersAPI.read.mockImplementation(getUsers);
-    mountWithContexts(<User setBreadcrumb={() => {}} me={mockMe} />);
+    const history = createMemoryHistory({
+      initialEntries: ['/users/1'],
+    });
+    await act(async () => {
+      mountWithContexts(<User setBreadcrumb={() => {}} />, {
+        context: {
+          router: {
+            history,
+            route: {
+              location: history.location,
+              match: {
+                params: { id: 1 },
+                url: '/users/1',
+                path: '/users/1',
+              },
+            },
+          },
+        },
+      });
+    });
   });
 
-  test('notifications tab shown for admins', async () => {
+  test('tabs shown for users', async () => {
     UsersAPI.readDetail.mockResolvedValue({ data: mockDetails });
     UsersAPI.read.mockImplementation(getUsers);
-
+    const history = createMemoryHistory({
-    const wrapper = mountWithContexts(
+      initialEntries: ['/users/1'],
-      <User setBreadcrumb={() => {}} me={mockMe} />
+    });
-    );
+    let wrapper;
+    await act(async () => {
+      wrapper = mountWithContexts(<User setBreadcrumb={() => {}} />, {
+        context: {
+          router: {
+            history,
+            route: {
+              location: history.location,
+              match: {
+                params: { id: 1 },
+                url: '/users/1',
+                path: '/users/1',
+              },
+            },
+          },
+        },
+      });
+    });
     await waitForElement(wrapper, '.pf-c-tabs__item', el => el.length === 5);
+
+    /* eslint-disable react/button-has-type */
+    expect(
+      wrapper
+        .find('Tabs')
+        .containsAllMatchingElements([
+          <button aria-label="Details">Details</button>,
+          <button aria-label="Organizations">Organizations</button>,
+          <button aria-label="Teams">Teams</button>,
+          <button aria-label="Access">Access</button>,
+          <button aria-label="Tokens">Tokens</button>,
+        ])
+    ).toEqual(true);
   });
 
   test('should show content error when user attempts to navigate to erroneous route', async () => {
     const history = createMemoryHistory({
       initialEntries: ['/users/1/foobar'],
     });
-    const wrapper = mountWithContexts(
+    let wrapper;
-      <User setBreadcrumb={() => {}} me={mockMe} />,
+    await act(async () => {
-      {
+      wrapper = mountWithContexts(<User setBreadcrumb={() => {}} />, {
         context: {
           router: {
             history,
@@ -60,8 +105,8 @@ describe('<User />', () => {
             },
           },
         },
-      }
+      });
-    );
+    });
     await waitForElement(wrapper, 'ContentError', el => el.length === 1);
   });
 });

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

@@ -212,7 +212,7 @@ class UsersList extends Component {
                 <UserListItem
                   key={o.id}
                   user={o}
-                  detailUrl={`${match.url}/${o.id}`}
+                  detailUrl={`${match.url}/${o.id}/details`}
                   isSelected={selected.some(row => row.id === o.id)}
                   onSelect={() => this.handleSelect(o)}
                 />

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

@@ -1,9 +1,8 @@
-import React, { Fragment, useState } from 'react';
+import React, { Fragment, useState, useCallback } from 'react';
 import { Route, useRouteMatch, Switch } from 'react-router-dom';
 import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
 
-import { Config } from '@contexts/Config';
 import Breadcrumbs from '@components/Breadcrumbs/Breadcrumbs';
 
 import UsersList from './UserList/UserList';
@@ -17,24 +16,26 @@ function Users({ i18n }) {
   });
   const match = useRouteMatch();
 
-  const addUserBreadcrumb = user => {
+  const addUserBreadcrumb = useCallback(
-    if (!user) {
+    user => {
-      return;
+      if (!user) {
-    }
+        return;
-
+      }
-    setBreadcrumbConfig({
-      '/users': i18n._(t`Users`),
-      '/users/add': i18n._(t`Create New User`),
-      [`/users/${user.id}`]: `${user.username}`,
-      [`/users/${user.id}/edit`]: i18n._(t`Edit Details`),
-      [`/users/${user.id}/details`]: i18n._(t`Details`),
-      [`/users/${user.id}/access`]: i18n._(t`Access`),
-      [`/users/${user.id}/teams`]: i18n._(t`Teams`),
-      [`/users/${user.id}/organizations`]: i18n._(t`Organizations`),
-      [`/users/${user.id}/tokens`]: i18n._(t`Tokens`),
-    });
-  };
 
+      setBreadcrumbConfig({
+        '/users': i18n._(t`Users`),
+        '/users/add': i18n._(t`Create New User`),
+        [`/users/${user.id}`]: `${user.username}`,
+        [`/users/${user.id}/edit`]: i18n._(t`Edit Details`),
+        [`/users/${user.id}/details`]: i18n._(t`Details`),
+        [`/users/${user.id}/access`]: i18n._(t`Access`),
+        [`/users/${user.id}/teams`]: i18n._(t`Teams`),
+        [`/users/${user.id}/organizations`]: i18n._(t`Organizations`),
+        [`/users/${user.id}/tokens`]: i18n._(t`Tokens`),
+      });
+    },
+    [i18n]
+  );
   return (
     <Fragment>
       <Breadcrumbs breadcrumbConfig={breadcrumbConfig} />
@@ -43,11 +44,7 @@ function Users({ i18n }) {
           <UserAdd />
         </Route>
         <Route path={`${match.path}/:id`}>
-          <Config>
+          <User setBreadcrumb={addUserBreadcrumb} />
-            {({ me }) => (
-              <User setBreadcrumb={addUserBreadcrumb} me={me || {}} />
-            )}
-          </Config>
         </Route>
         <Route path={`${match.path}`}>
           <UsersList />

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

@@ -6,7 +6,7 @@ import { mountWithContexts } from '@testUtils/enzymeHelpers';
 import Users from './Users';
 
 describe('<Users />', () => {
-  test('initially renders succesfully', () => {
+  test('initially renders successfully', () => {
     mountWithContexts(<Users />);
   });