From 2f47bacb4f99498d07d5ae01f41aac7406141139 Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Tue, 17 Nov 2020 16:59:49 -0500 Subject: [PATCH 1/4] Convert project/* components into functional components --- awx/ui_next/src/screens/Project/Project.jsx | 298 ++++++++---------- .../src/screens/Project/Project.test.jsx | 88 +++--- awx/ui_next/src/screens/Project/Projects.jsx | 118 +++---- .../Project/shared/ProjectSyncButton.jsx | 90 +++--- .../Project/shared/ProjectSyncButton.test.jsx | 46 ++- 5 files changed, 284 insertions(+), 356 deletions(-) diff --git a/awx/ui_next/src/screens/Project/Project.jsx b/awx/ui_next/src/screens/Project/Project.jsx index ea4c3e2761..ce460d29aa 100644 --- a/awx/ui_next/src/screens/Project/Project.jsx +++ b/awx/ui_next/src/screens/Project/Project.jsx @@ -1,11 +1,22 @@ -import React, { Component } from 'react'; +import React, { useCallback, useEffect } 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, + withRouter, + Redirect, + Link, + useParams, + useLocation, +} from 'react-router-dom'; import { CaretLeftIcon } from '@patternfly/react-icons'; import { Card, PageSection } from '@patternfly/react-core'; +import { useConfig } from '../../contexts/Config'; +import useRequest from '../../util/useRequest'; import RoutedTabs from '../../components/RoutedTabs'; import ContentError from '../../components/ContentError'; +import ContentLoading from '../../components/ContentLoading'; import NotificationList from '../../components/NotificationList'; import { ResourceAccessList } from '../../components/ResourceAccessList'; import { Schedules } from '../../components/Schedule'; @@ -14,48 +25,18 @@ import ProjectEdit from './ProjectEdit'; import ProjectJobTemplatesList from './ProjectJobTemplatesList'; import { OrganizationsAPI, ProjectsAPI } from '../../api'; -class Project extends Component { - constructor(props) { - super(props); +function Project({ i18n, setBreadcrumb }) { + const { me = {} } = useConfig(); + const { id } = useParams(); + const location = useLocation(); - this.state = { - project: null, - hasContentLoading: true, - contentError: null, - isInitialized: false, - isNotifAdmin: false, - }; - this.createSchedule = this.createSchedule.bind(this); - this.loadProject = this.loadProject.bind(this); - this.loadProjectAndRoles = this.loadProjectAndRoles.bind(this); - this.loadSchedules = this.loadSchedules.bind(this); - this.loadScheduleOptions = this.loadScheduleOptions.bind(this); - } - - async componentDidMount() { - await this.loadProjectAndRoles(); - this.setState({ isInitialized: true }); - } - - async componentDidUpdate(prevProps) { - const { location, match } = this.props; - const url = `/projects/${match.params.id}/`; - - if ( - prevProps.location.pathname.startsWith(url) && - prevProps.location !== location && - location.pathname === `${url}details` - ) { - await this.loadProject(); - } - } - - async loadProjectAndRoles() { - const { match, setBreadcrumb } = this.props; - const id = parseInt(match.params.id, 10); - - this.setState({ contentError: null, hasContentLoading: true }); - try { + const { + request: fetchProjectAndRoles, + result: { project, isNotifAdmin }, + isLoading: hasContentLoading, + error: contentError, + } = useRequest( + useCallback(async () => { const [{ data }, notifAdminRes] = await Promise.all([ ProjectsAPI.readDetail(id), OrganizationsAPI.read({ @@ -63,188 +44,155 @@ class Project extends Component { role_level: 'notification_admin_role', }), ]); - setBreadcrumb(data); - this.setState({ + return { project: data, isNotifAdmin: notifAdminRes.data.results.length > 0, - }); - } catch (err) { - this.setState({ contentError: err }); - } finally { - this.setState({ hasContentLoading: false }); + }; + }, [id]), + { + project: null, + notifAdminRes: null, } - } + ); - async loadProject() { - const { match, setBreadcrumb } = this.props; - const id = parseInt(match.params.id, 10); + useEffect(() => { + fetchProjectAndRoles(); + }, [fetchProjectAndRoles]); - this.setState({ contentError: null, hasContentLoading: true }); - try { - const { data } = await ProjectsAPI.readDetail(id); - setBreadcrumb(data); - this.setState({ project: data }); - } catch (err) { - this.setState({ contentError: err }); - } finally { - this.setState({ hasContentLoading: false }); + useEffect(() => { + if (project) { + setBreadcrumb(project); } - } + }, [project, setBreadcrumb]); - createSchedule(data) { - const { project } = this.state; + function createSchedule(data) { return ProjectsAPI.createSchedule(project.id, data); } - loadScheduleOptions() { - const { project } = this.state; + function loadScheduleOptions() { return ProjectsAPI.readScheduleOptions(project.id); } - loadSchedules(params) { - const { project } = this.state; + function loadSchedules(params) { return ProjectsAPI.readSchedules(project.id, params); } - render() { - const { location, match, me, i18n, setBreadcrumb } = this.props; - - const { - project, - contentError, - hasContentLoading, - isInitialized, - isNotifAdmin, - } = this.state; - const canSeeNotificationsTab = me.is_system_auditor || isNotifAdmin; - const canToggleNotifications = isNotifAdmin; - - const tabsArray = [ - { - name: ( - <> - - {i18n._(t`Back to Projects`)} - - ), - link: `/projects`, - id: 99, - }, - { name: i18n._(t`Details`), link: `${match.url}/details` }, - { name: i18n._(t`Access`), link: `${match.url}/access` }, - ]; - - if (canSeeNotificationsTab) { - tabsArray.push({ - name: i18n._(t`Notifications`), - link: `${match.url}/notifications`, - }); - } + const canSeeNotificationsTab = me.is_system_auditor || isNotifAdmin; + const canToggleNotifications = isNotifAdmin; + const tabsArray = [ + { + name: ( + <> + + {i18n._(t`Back to Projects`)} + + ), + link: `/projects`, + id: 99, + }, + { name: i18n._(t`Details`), link: `/projects/${id}/details` }, + { name: i18n._(t`Access`), link: `/projects/${id}/access` }, + ]; + if (canSeeNotificationsTab) { tabsArray.push({ - name: i18n._(t`Job Templates`), - link: `${match.url}/job_templates`, + name: i18n._(t`Notifications`), + link: `/projects/${id}/notifications`, }); + } - if (project?.scm_type) { - tabsArray.push({ - name: i18n._(t`Schedules`), - link: `${match.url}/schedules`, - }); - } + tabsArray.push({ + name: i18n._(t`Job Templates`), + link: `/projects/${id}/job_templates`, + }); - tabsArray.forEach((tab, n) => { - tab.id = n; + if (project?.scm_type) { + tabsArray.push({ + name: i18n._(t`Schedules`), + link: `/projects/${id}/schedules`, }); + } - let showCardHeader = true; - - if ( - !isInitialized || - location.pathname.endsWith('edit') || - location.pathname.includes('schedules/') - ) { - showCardHeader = false; - } - - if (!hasContentLoading && contentError) { - return ( - - - - {contentError.response.status === 404 && ( - - {i18n._(t`Project not found.`)}{' '} - {i18n._(t`View all Projects.`)} - - )} - - - - ); - } + tabsArray.forEach((tab, n) => { + tab.id = n; + }); + if (contentError) { return ( - {showCardHeader && } + + {contentError.response.status === 404 && ( + + {i18n._(t`Project not found.`)}{' '} + {i18n._(t`View all Projects.`)} + + )} + + + + ); + } + + let showCardHeader = true; + if (['edit', 'schedules/'].some(name => location.pathname.includes(name))) { + showCardHeader = false; + } + + return ( + + + {showCardHeader && } + {hasContentLoading && } + {!hasContentLoading && project && ( - {project && ( - - - - )} - {project && ( - - - - )} - {project && ( - - - - )} + + + + + + + + + {canSeeNotificationsTab && ( )} - + {project?.scm_type && project.scm_type !== '' && ( )} - {!hasContentLoading && ( - - {match.params.id && ( - - {i18n._(t`View Project Details`)} - - )} - - )} + + {id && ( + + {i18n._(t`View Project Details`)} + + )} + - , - - - ); - } + )} + + + ); } export default withI18n()(withRouter(Project)); diff --git a/awx/ui_next/src/screens/Project/Project.test.jsx b/awx/ui_next/src/screens/Project/Project.test.jsx index 7d8080e8a9..03777176e0 100644 --- a/awx/ui_next/src/screens/Project/Project.test.jsx +++ b/awx/ui_next/src/screens/Project/Project.test.jsx @@ -1,4 +1,5 @@ import React from 'react'; +import { act } from 'react-dom/test-utils'; import { createMemoryHistory } from 'history'; import { OrganizationsAPI, ProjectsAPI } from '../../api'; import { @@ -28,29 +29,34 @@ async function getOrganizations() { } describe('', () => { - test('initially renders succesfully', () => { + let wrapper; + + test('initially renders successfully', async () => { ProjectsAPI.readDetail.mockResolvedValue({ data: mockDetails }); OrganizationsAPI.read.mockImplementation(getOrganizations); - mountWithContexts( {}} me={mockMe} />); + await act(async () => { + mountWithContexts( {}} me={mockMe} />); + }); }); - test('notifications tab shown for admins', async done => { + test('notifications tab shown for admins', async () => { ProjectsAPI.readDetail.mockResolvedValue({ data: mockDetails }); OrganizationsAPI.read.mockImplementation(getOrganizations); - const wrapper = mountWithContexts( - {}} me={mockMe} /> - ); + await act(async () => { + wrapper = mountWithContexts( + {}} me={mockMe} /> + ); + }); const tabs = await waitForElement( wrapper, '.pf-c-tabs__item', el => el.length === 6 ); expect(tabs.at(3).text()).toEqual('Notifications'); - done(); }); - test('notifications tab hidden with reduced permissions', async done => { + test('notifications tab hidden with reduced permissions', async () => { ProjectsAPI.readDetail.mockResolvedValue({ data: mockDetails }); OrganizationsAPI.read.mockResolvedValue({ count: 0, @@ -58,20 +64,20 @@ describe('', () => { previous: null, data: { results: [] }, }); - - const wrapper = mountWithContexts( - {}} me={mockMe} /> - ); + await act(async () => { + wrapper = mountWithContexts( + {}} me={mockMe} /> + ); + }); const tabs = await waitForElement( wrapper, '.pf-c-tabs__item', el => el.length === 5 ); tabs.forEach(tab => expect(tab.text()).not.toEqual('Notifications')); - done(); }); - test('schedules tab shown for scm based projects.', async done => { + test('schedules tab shown for scm based projects.', async () => { ProjectsAPI.readDetail.mockResolvedValue({ data: mockDetails }); OrganizationsAPI.read.mockResolvedValue({ count: 0, @@ -80,19 +86,21 @@ describe('', () => { data: { results: [] }, }); - const wrapper = mountWithContexts( - {}} me={mockMe} /> - ); + await act(async () => { + wrapper = mountWithContexts( + {}} me={mockMe} /> + ); + }); + const tabs = await waitForElement( wrapper, '.pf-c-tabs__item', el => el.length === 5 ); expect(tabs.at(4).text()).toEqual('Schedules'); - done(); }); - test('schedules tab hidden for manual projects.', async done => { + test('schedules tab hidden for manual projects.', async () => { const manualDetails = Object.assign(mockDetails, { scm_type: '' }); ProjectsAPI.readDetail.mockResolvedValue({ data: manualDetails }); OrganizationsAPI.read.mockResolvedValue({ @@ -102,40 +110,44 @@ describe('', () => { data: { results: [] }, }); - const wrapper = mountWithContexts( - {}} me={mockMe} /> - ); + await act(async () => { + wrapper = mountWithContexts( + {}} me={mockMe} /> + ); + }); + const tabs = await waitForElement( wrapper, '.pf-c-tabs__item', el => el.length === 4 ); tabs.forEach(tab => expect(tab.text()).not.toEqual('Schedules')); - done(); }); test('should show content error when user attempts to navigate to erroneous route', async () => { const history = createMemoryHistory({ initialEntries: ['/projects/1/foobar'], }); - const wrapper = mountWithContexts( - {}} me={mockMe} />, - { - context: { - router: { - history, - route: { - location: history.location, - match: { - params: { id: 1 }, - url: '/projects/1/foobar', - path: '/project/1/foobar', + await act(async () => { + wrapper = mountWithContexts( + {}} me={mockMe} />, + { + context: { + router: { + history, + route: { + location: history.location, + match: { + params: { id: 1 }, + url: '/projects/1/foobar', + path: '/project/1/foobar', + }, }, }, }, - }, - } - ); + } + ); + }); await waitForElement(wrapper, 'ContentError', el => el.length === 1); }); }); diff --git a/awx/ui_next/src/screens/Project/Projects.jsx b/awx/ui_next/src/screens/Project/Projects.jsx index 0bb53de606..a45dd1890b 100644 --- a/awx/ui_next/src/screens/Project/Projects.jsx +++ b/awx/ui_next/src/screens/Project/Projects.jsx @@ -1,90 +1,64 @@ -import React, { Component, Fragment } from 'react'; +import React, { useState, useCallback } from 'react'; import { Route, withRouter, 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 ProjectsList from './ProjectList/ProjectList'; import ProjectAdd from './ProjectAdd/ProjectAdd'; import Project from './Project'; -class Projects extends Component { - constructor(props) { - super(props); +function Projects({ i18n }) { + const [breadcrumbConfig, setBreadcrumbConfig] = useState({ + '/projects': i18n._(t`Projects`), + '/projects/add': i18n._(t`Create New Project`), + }); - const { i18n } = props; - - this.state = { - breadcrumbConfig: { + const buildBreadcrumbConfig = useCallback( + (project, nested) => { + if (!project) { + return; + } + const projectSchedulesPath = `/projects/${project.id}/schedules`; + setBreadcrumbConfig({ '/projects': i18n._(t`Projects`), '/projects/add': i18n._(t`Create New Project`), - }, - }; - } + [`/projects/${project.id}`]: `${project.name}`, + [`/projects/${project.id}/edit`]: i18n._(t`Edit Details`), + [`/projects/${project.id}/details`]: i18n._(t`Details`), + [`/projects/${project.id}/access`]: i18n._(t`Access`), + [`/projects/${project.id}/notifications`]: i18n._(t`Notifications`), + [`/projects/${project.id}/job_templates`]: i18n._(t`Job Templates`), - setBreadcrumbConfig = (project, nested) => { - const { i18n } = this.props; + [`${projectSchedulesPath}`]: i18n._(t`Schedules`), + [`${projectSchedulesPath}/add`]: i18n._(t`Create New Schedule`), + [`${projectSchedulesPath}/${nested?.id}`]: `${nested?.name}`, + [`${projectSchedulesPath}/${nested?.id}/details`]: i18n._( + t`Schedule Details` + ), + [`${projectSchedulesPath}/${nested?.id}/edit`]: i18n._(t`Edit Details`), + }); + }, + [i18n] + ); - if (!project) { - return; - } - - const projectSchedulesPath = `/projects/${project.id}/schedules`; - - const breadcrumbConfig = { - '/projects': i18n._(t`Projects`), - '/projects/add': i18n._(t`Create New Project`), - [`/projects/${project.id}`]: `${project.name}`, - [`/projects/${project.id}/edit`]: i18n._(t`Edit Details`), - [`/projects/${project.id}/details`]: i18n._(t`Details`), - [`/projects/${project.id}/access`]: i18n._(t`Access`), - [`/projects/${project.id}/notifications`]: i18n._(t`Notifications`), - [`/projects/${project.id}/job_templates`]: i18n._(t`Job Templates`), - - [`${projectSchedulesPath}`]: i18n._(t`Schedules`), - [`${projectSchedulesPath}/add`]: i18n._(t`Create New Schedule`), - [`${projectSchedulesPath}/${nested?.id}`]: `${nested?.name}`, - [`${projectSchedulesPath}/${nested?.id}/details`]: i18n._( - t`Schedule Details` - ), - [`${projectSchedulesPath}/${nested?.id}/edit`]: i18n._(t`Edit Details`), - }; - - this.setState({ breadcrumbConfig }); - }; - - render() { - const { match, history, location } = this.props; - const { breadcrumbConfig } = this.state; - - return ( - - - - - - - - - {({ me }) => ( - - )} - - - - - - - - ); - } + return ( + <> + + + + + + + + + + + + + + ); } export { Projects as _Projects }; diff --git a/awx/ui_next/src/screens/Project/shared/ProjectSyncButton.jsx b/awx/ui_next/src/screens/Project/shared/ProjectSyncButton.jsx index 3ef97e9657..2940a22925 100644 --- a/awx/ui_next/src/screens/Project/shared/ProjectSyncButton.jsx +++ b/awx/ui_next/src/screens/Project/shared/ProjectSyncButton.jsx @@ -1,70 +1,52 @@ -import React, { Fragment } from 'react'; +import React, { useCallback } from 'react'; import { number } from 'prop-types'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; +import useRequest, { useDismissableError } from '../../../util/useRequest'; import AlertModal from '../../../components/AlertModal'; import ErrorDetail from '../../../components/ErrorDetail'; import { ProjectsAPI } from '../../../api'; -class ProjectSyncButton extends React.Component { - static propTypes = { - projectId: number.isRequired, - }; - - constructor(props) { - super(props); - - this.state = { - syncError: null, - }; - - this.handleSync = this.handleSync.bind(this); - this.handleSyncErrorClose = this.handleSyncErrorClose.bind(this); - } - - handleSyncErrorClose() { - this.setState({ syncError: null }); - } - - async handleSync() { - const { i18n, projectId } = this.props; - try { - const { data: syncConfig } = await ProjectsAPI.readSync(projectId); - if (syncConfig.can_update) { +function ProjectSyncButton({ i18n, children, projectId }) { + const { request: handleSync, error: syncError } = useRequest( + useCallback(async () => { + const { data } = await ProjectsAPI.readSync(projectId); + if (data.can_update) { await ProjectsAPI.sync(projectId); } else { - this.setState({ - syncError: i18n._( + throw new Error( + i18n._( t`You don't have the necessary permissions to sync this project.` - ), - }); + ) + ); } - } catch (err) { - this.setState({ syncError: err }); - } - } + }, [i18n, projectId]), + null + ); - render() { - const { syncError } = this.state; - const { i18n, children } = this.props; - return ( - - {children(this.handleSync)} - {syncError && ( - - {i18n._(t`Failed to sync job.`)} - - - )} - - ); - } + const { error, dismissError } = useDismissableError(syncError); + + return ( + <> + {children(handleSync)} + {error && ( + + {i18n._(t`Failed to sync job.`)} + + + )} + + ); } +ProjectSyncButton.propTypes = { + projectId: number.isRequired, +}; + export default withI18n()(ProjectSyncButton); diff --git a/awx/ui_next/src/screens/Project/shared/ProjectSyncButton.test.jsx b/awx/ui_next/src/screens/Project/shared/ProjectSyncButton.test.jsx index 87b9b7bfb1..4bc302f09b 100644 --- a/awx/ui_next/src/screens/Project/shared/ProjectSyncButton.test.jsx +++ b/awx/ui_next/src/screens/Project/shared/ProjectSyncButton.test.jsx @@ -1,4 +1,5 @@ import React from 'react'; +import { act } from 'react-dom/test-utils'; import { mountWithContexts } from '../../../../testUtils/enzymeHelpers'; import { sleep } from '../../../../testUtils/testUtils'; @@ -8,6 +9,7 @@ import { ProjectsAPI } from '../../../api'; jest.mock('../../../api'); describe('ProjectSyncButton', () => { + let wrapper; ProjectsAPI.readSync.mockResolvedValue({ data: { can_update: true, @@ -18,29 +20,34 @@ describe('ProjectSyncButton', () => {