From 56919c5d321d9df74f51fee9b3d2f8f88af976d0 Mon Sep 17 00:00:00 2001
From: Alex Corey <alex.swansboro@gmail.com>
Date: Wed, 18 Mar 2020 14:12:51 -0400
Subject: [PATCH 1/2] Moves template.jsx over to a functional component.

---
 awx/ui_next/src/screens/Template/Template.jsx | 400 ++++++++----------
 .../src/screens/Template/Template.test.jsx    | 128 +++---
 2 files changed, 237 insertions(+), 291 deletions(-)

diff --git a/awx/ui_next/src/screens/Template/Template.jsx b/awx/ui_next/src/screens/Template/Template.jsx
index fe997c5176..f8d932bb47 100644
--- a/awx/ui_next/src/screens/Template/Template.jsx
+++ b/awx/ui_next/src/screens/Template/Template.jsx
@@ -1,8 +1,18 @@
-import React, { Component } from 'react';
+import React, { useState, useEffect, useCallback } from 'react';
 import { t } from '@lingui/macro';
 import { withI18n } from '@lingui/react';
 import { Card, CardActions, PageSection } from '@patternfly/react-core';
-import { Switch, Route, Redirect, withRouter, Link } from 'react-router-dom';
+import {
+  Switch,
+  Route,
+  Redirect,
+  withRouter,
+  Link,
+  useLocation,
+  useParams,
+  useRouteMatch,
+} from 'react-router-dom';
+import useRequest from '@util/useRequest';
 
 import { TabbedCardHeader } from '@components/Card';
 import CardCloseButton from '@components/CardCloseButton';
@@ -17,261 +27,195 @@ import JobTemplateEdit from './JobTemplateEdit';
 import { JobTemplatesAPI, OrganizationsAPI } from '@api';
 import TemplateSurvey from './TemplateSurvey';
 
-class Template extends Component {
-  constructor(props) {
-    super(props);
+function Template({ i18n, me, setBreadcrumb }) {
+  const location = useLocation();
+  const { id: templateId } = useParams();
+  const match = useRouteMatch();
 
-    this.state = {
-      contentError: null,
-      hasContentLoading: true,
-      template: null,
-      isNotifAdmin: false,
-    };
-    this.loadTemplate = this.loadTemplate.bind(this);
-    this.loadTemplateAndRoles = this.loadTemplateAndRoles.bind(this);
-    this.loadSchedules = this.loadSchedules.bind(this);
-    this.loadScheduleOptions = this.loadScheduleOptions.bind(this);
-  }
-
-  async componentDidMount() {
-    await this.loadTemplateAndRoles();
-  }
-
-  async componentDidUpdate(prevProps) {
-    const { location } = this.props;
-    if (location !== prevProps.location) {
-      await this.loadTemplate();
-    }
-  }
-
-  async loadTemplateAndRoles() {
-    const { match, setBreadcrumb } = this.props;
-    const id = parseInt(match.params.id, 10);
-
-    this.setState({ contentError: null, hasContentLoading: true });
-    try {
+  const {
+    result: { isNotifAdmin, template },
+    isLoading: hasRolesandTemplateLoading,
+    error: rolesAndTemplateError,
+    request: loadTempplateAndRoles,
+  } = useRequest(
+    useCallback(async () => {
       const [{ data }, notifAdminRes] = await Promise.all([
-        JobTemplatesAPI.readDetail(id),
+        JobTemplatesAPI.readDetail(templateId),
         OrganizationsAPI.read({
           page_size: 1,
           role_level: 'notification_admin_role',
         }),
       ]);
       setBreadcrumb(data);
-      this.setState({
+
+      return {
         template: data,
         isNotifAdmin: notifAdminRes.data.results.length > 0,
-      });
-    } catch (err) {
-      this.setState({ contentError: err });
-    } finally {
-      this.setState({ hasContentLoading: false });
-    }
-  }
+      };
+    }, [setBreadcrumb, templateId]),
+    { isNotifAdmin: false, template: null }
+  );
+  useEffect(() => {
+    loadTempplateAndRoles();
+  }, [loadTempplateAndRoles]);
 
-  async loadTemplate() {
-    const { setBreadcrumb, match } = this.props;
-    const { id } = match.params;
+  const loadScheduleOptions = () => {
+    return JobTemplatesAPI.readScheduleOptions(templateId);
+  };
 
-    this.setState({ contentError: null, hasContentLoading: true });
-    try {
-      const { data } = await JobTemplatesAPI.readDetail(id);
-      setBreadcrumb(data);
-      this.setState({ template: data });
-    } catch (err) {
-      this.setState({ contentError: err });
-    } finally {
-      this.setState({ hasContentLoading: false });
-    }
-  }
+  const loadSchedules = params => {
+    return JobTemplatesAPI.readSchedules(templateId, params);
+  };
 
-  loadScheduleOptions() {
-    const { template } = this.state;
-    return JobTemplatesAPI.readScheduleOptions(template.id);
-  }
+  const canSeeNotificationsTab = me.is_system_auditor || isNotifAdmin;
 
-  loadSchedules(params) {
-    const { template } = this.state;
-    return JobTemplatesAPI.readSchedules(template.id, params);
-  }
+  const tabsArray = [
+    { name: i18n._(t`Details`), link: `${match.url}/details` },
+    { name: i18n._(t`Access`), link: `${match.url}/access` },
+  ];
 
-  render() {
-    const { i18n, location, match, me, setBreadcrumb } = this.props;
-    const {
-      contentError,
-      hasContentLoading,
-      isNotifAdmin,
-      template,
-    } = this.state;
-
-    const canSeeNotificationsTab = me.is_system_auditor || isNotifAdmin;
-
-    const tabsArray = [
-      { 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`,
-      });
-    }
-
-    if (template) {
-      tabsArray.push({
-        name: i18n._(t`Schedules`),
-        link: `${match.url}/schedules`,
-      });
-    }
-
-    tabsArray.push(
-      {
-        name: i18n._(t`Completed Jobs`),
-        link: `${match.url}/completed_jobs`,
-      },
-      {
-        name: i18n._(t`Survey`),
-        link: `${match.url}/survey`,
-      }
-    );
-
-    tabsArray.forEach((tab, n) => {
-      tab.id = n;
+  if (canSeeNotificationsTab) {
+    tabsArray.push({
+      name: i18n._(t`Notifications`),
+      link: `${match.url}/notifications`,
     });
+  }
 
-    let cardHeader = (
-      <TabbedCardHeader>
-        <RoutedTabs tabsArray={tabsArray} />
-        <CardActions>
-          <CardCloseButton linkTo="/templates" />
-        </CardActions>
-      </TabbedCardHeader>
-    );
+  if (template) {
+    tabsArray.push({
+      name: i18n._(t`Schedules`),
+      link: `${match.url}/schedules`,
+    });
+  }
 
-    if (
-      location.pathname.endsWith('edit') ||
-      location.pathname.includes('schedules/')
-    ) {
-      cardHeader = null;
+  tabsArray.push(
+    {
+      name: i18n._(t`Completed Jobs`),
+      link: `${match.url}/completed_jobs`,
+    },
+    {
+      name: i18n._(t`Survey`),
+      link: `${match.url}/survey`,
     }
+  );
 
-    if (!hasContentLoading && contentError) {
-      return (
-        <PageSection>
-          <Card>
-            <ContentError error={contentError}>
-              {contentError.response.status === 404 && (
-                <span>
-                  {i18n._(`Template not found.`)}{' '}
-                  <Link to="/templates">{i18n._(`View all Templates.`)}</Link>
-                </span>
-              )}
-            </ContentError>
-          </Card>
-        </PageSection>
-      );
-    }
+  tabsArray.forEach((tab, n) => {
+    tab.id = n;
+  });
 
+  let cardHeader = (
+    <TabbedCardHeader>
+      <RoutedTabs tabsArray={tabsArray} />
+      <CardActions>
+        <CardCloseButton linkTo="/templates" />
+      </CardActions>
+    </TabbedCardHeader>
+  );
+  if (
+    location.pathname.endsWith('edit') ||
+    location.pathname.includes('schedules/')
+  ) {
+    cardHeader = null;
+  }
+
+  const contentError = rolesAndTemplateError;
+  if (!hasRolesandTemplateLoading && contentError) {
     return (
       <PageSection>
         <Card>
-          {cardHeader}
-          <Switch>
-            <Redirect
-              from="/templates/:templateType/:id"
-              to="/templates/:templateType/:id/details"
-              exact
-            />
-            {template && (
-              <Route
-                key="details"
-                path="/templates/:templateType/:id/details"
-                render={() => (
-                  <JobTemplateDetail
-                    hasTemplateLoading={hasContentLoading}
-                    template={template}
-                  />
-                )}
-              />
+          <ContentError error={contentError}>
+            {contentError.response.status === 404 && (
+              <span>
+                {i18n._(`Template not found.`)}{' '}
+                <Link to="/templates">{i18n._(`View all Templates.`)}</Link>
+              </span>
             )}
-            {template && (
-              <Route
-                key="edit"
-                path="/templates/:templateType/:id/edit"
-                render={() => <JobTemplateEdit template={template} />}
-              />
-            )}
-            {template && (
-              <Route
-                key="access"
-                path="/templates/:templateType/:id/access"
-                render={() => (
-                  <ResourceAccessList
-                    resource={template}
-                    apiModel={JobTemplatesAPI}
-                  />
-                )}
-              />
-            )}
-            {template && (
-              <Route
-                key="schedules"
-                path="/templates/:templateType/:id/schedules"
-                render={() => (
-                  <Schedules
-                    setBreadcrumb={setBreadcrumb}
-                    unifiedJobTemplate={template}
-                    loadSchedules={this.loadSchedules}
-                    loadScheduleOptions={this.loadScheduleOptions}
-                  />
-                )}
-              />
-            )}
-            {canSeeNotificationsTab && (
-              <Route
-                path="/templates/:templateType/:id/notifications"
-                render={() => (
-                  <NotificationList
-                    id={Number(match.params.id)}
-                    canToggleNotifications={isNotifAdmin}
-                    apiModel={JobTemplatesAPI}
-                  />
-                )}
-              />
-            )}
-            {template?.id && (
-              <Route path="/templates/:templateType/:id/completed_jobs">
-                <JobList defaultParams={{ job__job_template: template.id }} />
-              </Route>
-            )}
-            {template && (
-              <Route path="/templates/:templateType/:id/survey">
-                <TemplateSurvey template={template} />
-              </Route>
-            )}
-            <Route
-              key="not-found"
-              path="*"
-              render={() =>
-                !hasContentLoading && (
-                  <ContentError isNotFound>
-                    {match.params.id && (
-                      <Link
-                        to={`/templates/${match.params.templateType}/${match.params.id}/details`}
-                      >
-                        {i18n._(`View Template Details`)}
-                      </Link>
-                    )}
-                  </ContentError>
-                )
-              }
-            />
-          </Switch>
+          </ContentError>
         </Card>
       </PageSection>
     );
   }
+
+  return (
+    <PageSection>
+      <Card>
+        {cardHeader}
+        <Switch>
+          <Redirect
+            from="/templates/:templateType/:id"
+            to="/templates/:templateType/:id/details"
+            exact
+          />
+          {template && (
+            <Route key="details" path="/templates/:templateType/:id/details">
+              <JobTemplateDetail
+                hasTemplateLoading={hasRolesandTemplateLoading}
+                template={template}
+              />
+            </Route>
+          )}
+          {template && (
+            <Route key="edit" path="/templates/:templateType/:id/edit">
+              <JobTemplateEdit template={template} />
+            </Route>
+          )}
+          {template && (
+            <Route key="access" path="/templates/:templateType/:id/access">
+              <ResourceAccessList
+                resource={template}
+                apiModel={JobTemplatesAPI}
+              />
+            </Route>
+          )}
+          {template && (
+            <Route
+              key="schedules"
+              path="/templates/:templateType/:id/schedules"
+            >
+              <Schedules
+                setBreadcrumb={setBreadcrumb}
+                unifiedJobTemplate={template}
+                loadSchedules={loadSchedules}
+                loadScheduleOptions={loadScheduleOptions}
+              />
+            </Route>
+          )}
+          {canSeeNotificationsTab && (
+            <Route path="/templates/:templateType/:id/notifications">
+              <NotificationList
+                id={Number(templateId)}
+                canToggleNotifications={isNotifAdmin}
+                apiModel={JobTemplatesAPI}
+              />
+            </Route>
+          )}
+          {template?.id && (
+            <Route path="/templates/:templateType/:id/completed_jobs">
+              <JobList defaultParams={{ job__job_template: template.id }} />
+            </Route>
+          )}
+          {template && (
+            <Route path="/templates/:templateType/:id/survey">
+              <TemplateSurvey template={template} />
+            </Route>
+          )}
+          {!hasRolesandTemplateLoading && (
+            <Route key="not-found" path="*">
+              <ContentError isNotFound>
+                {match.params.id && (
+                  <Link
+                    to={`/templates/${match.params.templateType}/${match.params.id}/details`}
+                  >
+                    {i18n._(`View Template Details`)}
+                  </Link>
+                )}
+              </ContentError>
+            </Route>
+          )}
+        </Switch>
+      </Card>
+    </PageSection>
+  );
 }
 
 export { Template as _Template };
diff --git a/awx/ui_next/src/screens/Template/Template.test.jsx b/awx/ui_next/src/screens/Template/Template.test.jsx
index 26583f5f0b..1d9b3651f4 100644
--- a/awx/ui_next/src/screens/Template/Template.test.jsx
+++ b/awx/ui_next/src/screens/Template/Template.test.jsx
@@ -1,63 +1,58 @@
 import React from 'react';
 import { createMemoryHistory } from 'history';
 import { JobTemplatesAPI, OrganizationsAPI } from '@api';
+import { act } from 'react-dom/test-utils';
+import { sleep } from '@testUtils/testUtils';
+
 import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers';
-import Template, { _Template } from './Template';
+import Template from './Template';
 import mockJobTemplateData from './shared/data.job_template.json';
 
-jest.mock('@api');
-
-JobTemplatesAPI.readDetail.mockResolvedValue({
-  data: mockJobTemplateData,
-});
-
-OrganizationsAPI.read.mockResolvedValue({
-  data: {
-    count: 1,
-    next: null,
-    previous: null,
-    results: [
-      {
-        id: 1,
-      },
-    ],
-  },
-});
+jest.mock('@api/models/JobTemplates');
+jest.mock('@api/models/Organizations');
 
 const mockMe = {
   is_super_user: true,
   is_system_auditor: false,
 };
-
 describe('<Template />', () => {
-  test('initially renders succesfully', () => {
-    mountWithContexts(<Template setBreadcrumb={() => {}} me={mockMe} />);
+  beforeEach(() => {
+    JobTemplatesAPI.readDetail.mockResolvedValue({
+      data: mockJobTemplateData,
+    });
+
+    OrganizationsAPI.read.mockResolvedValue({
+      data: {
+        count: 1,
+        next: null,
+        previous: null,
+        results: [
+          {
+            id: 1,
+          },
+        ],
+      },
+    });
   });
-  test('When component mounts API is called and the response is put in state', async done => {
-    const loadTemplateAndRoles = jest.spyOn(
-      _Template.prototype,
-      'loadTemplateAndRoles'
-    );
-    const wrapper = mountWithContexts(
-      <Template setBreadcrumb={() => {}} me={mockMe} />
-    );
-    await waitForElement(
-      wrapper,
-      'Template',
-      el => el.state('hasContentLoading') === true
-    );
-    expect(loadTemplateAndRoles).toHaveBeenCalled();
-    await waitForElement(
-      wrapper,
-      'Template',
-      el => el.state('hasContentLoading') === true
-    );
-    done();
+  test('initially renders succesfully', async () => {
+    await act(async () => {
+      mountWithContexts(<Template setBreadcrumb={() => {}} me={mockMe} />);
+    });
+  });
+  test('When component mounts API is called and the response is put in state', async () => {
+    await act(async () => {
+      mountWithContexts(<Template setBreadcrumb={() => {}} me={mockMe} />);
+    });
+    expect(JobTemplatesAPI.readDetail).toBeCalled();
+    expect(OrganizationsAPI.read).toBeCalled();
   });
   test('notifications tab shown for admins', async done => {
-    const wrapper = mountWithContexts(
-      <Template setBreadcrumb={() => {}} me={mockMe} />
-    );
+    let wrapper;
+    await act(async () => {
+      wrapper = mountWithContexts(
+        <Template setBreadcrumb={() => {}} me={mockMe} />
+      );
+    });
 
     const tabs = await waitForElement(
       wrapper,
@@ -77,9 +72,12 @@ describe('<Template />', () => {
       },
     });
 
-    const wrapper = mountWithContexts(
-      <Template setBreadcrumb={() => {}} me={mockMe} />
-    );
+    let wrapper;
+    await act(async () => {
+      wrapper = mountWithContexts(
+        <Template setBreadcrumb={() => {}} me={mockMe} />
+      );
+    });
     const tabs = await waitForElement(
       wrapper,
       '.pf-c-tabs__item',
@@ -93,24 +91,28 @@ describe('<Template />', () => {
     const history = createMemoryHistory({
       initialEntries: ['/templates/job_template/1/foobar'],
     });
-    const wrapper = mountWithContexts(
-      <Template setBreadcrumb={() => {}} me={mockMe} />,
-      {
-        context: {
-          router: {
-            history,
-            route: {
-              location: history.location,
-              match: {
-                params: { id: 1 },
-                url: '/templates/job_template/1/foobar',
-                path: '/templates/job_template/1/foobar',
+    let wrapper;
+    await act(async () => {
+      wrapper = mountWithContexts(
+        <Template setBreadcrumb={() => {}} me={mockMe} />,
+        {
+          context: {
+            router: {
+              history,
+              route: {
+                location: history.location,
+                match: {
+                  params: { id: 1 },
+                  url: '/templates/job_template/1/foobar',
+                  path: '/templates/job_template/1/foobar',
+                },
               },
             },
           },
-        },
-      }
-    );
+        }
+      );
+    });
+
     await waitForElement(wrapper, 'ContentError', el => el.length === 1);
   });
 });

From a02b332b10930888d005f3868c792fd5105db81b Mon Sep 17 00:00:00 2001
From: Alex Corey <alex.swansboro@gmail.com>
Date: Thu, 19 Mar 2020 16:46:21 -0400
Subject: [PATCH 2/2] fixes linting and spelling errors

---
 awx/ui_next/src/screens/Template/Template.jsx      | 8 ++++----
 awx/ui_next/src/screens/Template/Template.test.jsx | 1 -
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/awx/ui_next/src/screens/Template/Template.jsx b/awx/ui_next/src/screens/Template/Template.jsx
index f8d932bb47..e1da6c64f5 100644
--- a/awx/ui_next/src/screens/Template/Template.jsx
+++ b/awx/ui_next/src/screens/Template/Template.jsx
@@ -1,4 +1,4 @@
-import React, { useState, useEffect, useCallback } from 'react';
+import React, { useEffect, useCallback } from 'react';
 import { t } from '@lingui/macro';
 import { withI18n } from '@lingui/react';
 import { Card, CardActions, PageSection } from '@patternfly/react-core';
@@ -36,7 +36,7 @@ function Template({ i18n, me, setBreadcrumb }) {
     result: { isNotifAdmin, template },
     isLoading: hasRolesandTemplateLoading,
     error: rolesAndTemplateError,
-    request: loadTempplateAndRoles,
+    request: loadTemplateAndRoles,
   } = useRequest(
     useCallback(async () => {
       const [{ data }, notifAdminRes] = await Promise.all([
@@ -56,8 +56,8 @@ function Template({ i18n, me, setBreadcrumb }) {
     { isNotifAdmin: false, template: null }
   );
   useEffect(() => {
-    loadTempplateAndRoles();
-  }, [loadTempplateAndRoles]);
+    loadTemplateAndRoles();
+  }, [loadTemplateAndRoles]);
 
   const loadScheduleOptions = () => {
     return JobTemplatesAPI.readScheduleOptions(templateId);
diff --git a/awx/ui_next/src/screens/Template/Template.test.jsx b/awx/ui_next/src/screens/Template/Template.test.jsx
index 1d9b3651f4..cebe757064 100644
--- a/awx/ui_next/src/screens/Template/Template.test.jsx
+++ b/awx/ui_next/src/screens/Template/Template.test.jsx
@@ -2,7 +2,6 @@ import React from 'react';
 import { createMemoryHistory } from 'history';
 import { JobTemplatesAPI, OrganizationsAPI } from '@api';
 import { act } from 'react-dom/test-utils';
-import { sleep } from '@testUtils/testUtils';
 
 import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers';
 import Template from './Template';