From 4d7b5adf123e0bfbefcbdc94966f19bf42915d6d Mon Sep 17 00:00:00 2001 From: nixocio Date: Mon, 13 Apr 2020 11:20:54 -0400 Subject: [PATCH] Update User component to be function based Update User component to be function based. Also update related unit-tests. --- awx/ui_next/src/screens/User/User.jsx | 198 ++++++++---------- awx/ui_next/src/screens/User/User.test.jsx | 79 +++++-- .../src/screens/User/UserList/UserList.jsx | 2 +- awx/ui_next/src/screens/User/Users.jsx | 45 ++-- awx/ui_next/src/screens/User/Users.test.jsx | 2 +- 5 files changed, 170 insertions(+), 156 deletions(-) diff --git a/awx/ui_next/src/screens/User/User.jsx b/awx/ui_next/src/screens/User/User.jsx index 76fb7ec202..f7a15c48f9 100644 --- a/awx/ui_next/src/screens/User/User.jsx +++ b/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 { - constructor(props) { - super(props); +function User({ i18n, setBreadcrumb }) { + const location = useLocation(); + 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 = { - user: null, - hasContentLoading: true, - contentError: null, - isInitialized: false, - }; - this.loadUser = this.loadUser.bind(this); - } + useEffect(() => { + fetchUser(); + }, [fetchUser, location.pathname]); - async componentDidMount() { - await this.loadUser(); - this.setState({ isInitialized: true }); - } - - 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(); + useEffect(() => { + if (user) { + setBreadcrumb(user); } - } + }, [user, setBreadcrumb]); - async loadUser() { - const { match, setBreadcrumb } = this.props; - const id = parseInt(match.params.id, 10); - - this.setState({ contentError: null, hasContentLoading: true }); - try { - const { data } = await UsersAPI.readDetail(id); - setBreadcrumb(data); - this.setState({ user: data }); - } catch (err) { - 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 = ( - - - - - - - ); - - if (!isInitialized) { - cardHeader = null; - } - - if (location.pathname.endsWith('edit')) { - cardHeader = null; - } - - if (!hasContentLoading && contentError) { - return ( - - - - {contentError.response.status === 404 && ( - - {i18n._(`User not found.`)}{' '} - {i18n._(`View all Users.`)} - - )} - - - - ); - } + 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 }, + ]; + if (contentError) { return ( - {cardHeader} + + {contentError.response && contentError.response.status === 404 && ( + + {i18n._(`User not found.`)}{' '} + {i18n._(`View all Users.`)} + + )} + + + + ); + } + return ( + + + {['edit'].some(name => location.pathname.includes(name)) ? null : ( + + + + + + + )} + {isLoading && } + {!isLoading && user && ( {user && ( @@ -146,22 +121,19 @@ class User extends Component { - {!hasContentLoading && ( - - {match.params.id && ( - - {i18n._(`View User Details`)} - - )} - - )} + + {match.params.id && ( + + {i18n._(`View User Details`)} + + )} + - - - ); - } + )} + + + ); } -export default withI18n()(withRouter(User)); -export { User as _User }; +export default withI18n()(User); diff --git a/awx/ui_next/src/screens/User/User.test.jsx b/awx/ui_next/src/screens/User/User.test.jsx index 3d2c366d19..d389a54c9d 100644 --- a/awx/ui_next/src/screens/User/User.test.jsx +++ b/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('', () => { - test('initially renders succesfully', () => { + test('initially renders successfully', async () => { UsersAPI.readDetail.mockResolvedValue({ data: mockDetails }); UsersAPI.read.mockImplementation(getUsers); - mountWithContexts( {}} me={mockMe} />); + const history = createMemoryHistory({ + initialEntries: ['/users/1'], + }); + await act(async () => { + mountWithContexts( {}} />, { + 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 wrapper = mountWithContexts( - {}} me={mockMe} /> - ); + const history = createMemoryHistory({ + initialEntries: ['/users/1'], + }); + let wrapper; + await act(async () => { + wrapper = mountWithContexts( {}} />, { + 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([ + , + , + , + , + , + ]) + ).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( - {}} me={mockMe} />, - { + let wrapper; + await act(async () => { + wrapper = mountWithContexts( {}} />, { context: { router: { history, @@ -60,8 +105,8 @@ describe('', () => { }, }, }, - } - ); + }); + }); await waitForElement(wrapper, 'ContentError', el => el.length === 1); }); }); diff --git a/awx/ui_next/src/screens/User/UserList/UserList.jsx b/awx/ui_next/src/screens/User/UserList/UserList.jsx index 3c9590040c..375f078eb1 100644 --- a/awx/ui_next/src/screens/User/UserList/UserList.jsx +++ b/awx/ui_next/src/screens/User/UserList/UserList.jsx @@ -212,7 +212,7 @@ class UsersList extends Component { row.id === o.id)} onSelect={() => this.handleSelect(o)} /> diff --git a/awx/ui_next/src/screens/User/Users.jsx b/awx/ui_next/src/screens/User/Users.jsx index bf74439ac5..9a49579c3b 100644 --- a/awx/ui_next/src/screens/User/Users.jsx +++ b/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 => { - 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`), - }); - }; + const addUserBreadcrumb = useCallback( + user => { + 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`), + }); + }, + [i18n] + ); return ( @@ -43,11 +44,7 @@ function Users({ i18n }) { - - {({ me }) => ( - - )} - + diff --git a/awx/ui_next/src/screens/User/Users.test.jsx b/awx/ui_next/src/screens/User/Users.test.jsx index a6368f51cd..531729d11e 100644 --- a/awx/ui_next/src/screens/User/Users.test.jsx +++ b/awx/ui_next/src/screens/User/Users.test.jsx @@ -6,7 +6,7 @@ import { mountWithContexts } from '@testUtils/enzymeHelpers'; import Users from './Users'; describe('', () => { - test('initially renders succesfully', () => { + test('initially renders successfully', () => { mountWithContexts(); });