From e6f0c01aa666d9a09ddf8b91bd18c4e27c555ba9 Mon Sep 17 00:00:00 2001
From: mabashian <mabashia@redhat.com>
Date: Fri, 21 Feb 2020 16:13:21 -0500
Subject: [PATCH] Schedule list now uses useRequest hooks for fetching and
 deleting.  Also rolled a component for schedule toggles that can be used
 throughout the tree.

---
 .../Schedule/ScheduleList/ScheduleList.jsx    | 132 ++++++------------
 .../ScheduleList/ScheduleListItem.jsx         |  35 +----
 .../ScheduleList/ScheduleListItem.test.jsx    |  11 --
 .../Schedule/shared/ScheduleToggle.jsx        |  78 +++++++++++
 .../Schedule/shared/ScheduleToggle.test.jsx   |  97 +++++++++++++
 5 files changed, 223 insertions(+), 130 deletions(-)
 create mode 100644 awx/ui_next/src/screens/Schedule/shared/ScheduleToggle.jsx
 create mode 100644 awx/ui_next/src/screens/Schedule/shared/ScheduleToggle.test.jsx

diff --git a/awx/ui_next/src/screens/Schedule/ScheduleList/ScheduleList.jsx b/awx/ui_next/src/screens/Schedule/ScheduleList/ScheduleList.jsx
index 36b0b9ccdd..2c1197e7c7 100644
--- a/awx/ui_next/src/screens/Schedule/ScheduleList/ScheduleList.jsx
+++ b/awx/ui_next/src/screens/Schedule/ScheduleList/ScheduleList.jsx
@@ -1,4 +1,4 @@
-import React, { useState, useEffect } from 'react';
+import React, { useState, useEffect, useCallback } from 'react';
 import { useLocation } from 'react-router-dom';
 import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
@@ -10,6 +10,7 @@ import DataListToolbar from '@components/DataListToolbar';
 import PaginatedDataList, {
   ToolbarDeleteButton,
 } from '@components/PaginatedDataList';
+import useRequest, { useDeleteItems } from '@util/useRequest';
 import { getQSConfig, parseQueryString } from '@util/qs';
 import { ScheduleListItem } from '.';
 
@@ -20,38 +21,54 @@ const QS_CONFIG = getQSConfig('schedule', {
 });
 
 function ScheduleList({ i18n }) {
-  const [contentError, setContentError] = useState(null);
-  const [scheduleCount, setScheduleCount] = useState(0);
-  const [schedules, setSchedules] = useState([]);
-  const [deletionError, setDeletionError] = useState(null);
-  const [hasContentLoading, setHasContentLoading] = useState(true);
   const [selected, setSelected] = useState([]);
-  const [toggleError, setToggleError] = useState(null);
-  const [toggleLoading, setToggleLoading] = useState(null);
 
   const location = useLocation();
 
-  const loadSchedules = async ({ search }) => {
-    const params = parseQueryString(QS_CONFIG, search);
-    setContentError(null);
-    setHasContentLoading(true);
-    try {
+  const {
+    result: { schedules, itemCount },
+    error: contentError,
+    isLoading,
+    request: fetchSchedules,
+  } = useRequest(
+    useCallback(async () => {
+      const params = parseQueryString(QS_CONFIG, location.search);
       const {
         data: { count, results },
       } = await SchedulesAPI.read(params);
-
-      setSchedules(results);
-      setScheduleCount(count);
-    } catch (error) {
-      setContentError(error);
-    } finally {
-      setHasContentLoading(false);
+      return {
+        itemCount: count,
+        schedules: results,
+      };
+    }, [location]),
+    {
+      schedules: [],
+      itemCount: 0,
     }
-  };
+  );
 
   useEffect(() => {
-    loadSchedules(location);
-  }, [location]); // eslint-disable-line react-hooks/exhaustive-deps
+    fetchSchedules();
+  }, [fetchSchedules]);
+
+  const isAllSelected =
+    selected.length === schedules.length && selected.length > 0;
+
+  const {
+    isLoading: isDeleteLoading,
+    deleteItems: deleteJobs,
+    deletionError,
+    clearDeletionError,
+  } = useDeleteItems(
+    useCallback(async () => {
+      return Promise.all(selected.map(({ id }) => SchedulesAPI.destroy(id)));
+    }, [selected]),
+    {
+      qsConfig: QS_CONFIG,
+      allItemsSelected: isAllSelected,
+      fetchItems: fetchSchedules,
+    }
+  );
 
   const handleSelectAll = isSelected => {
     setSelected(isSelected ? [...schedules] : []);
@@ -66,64 +83,18 @@ function ScheduleList({ i18n }) {
   };
 
   const handleDelete = async () => {
-    setHasContentLoading(true);
-
-    try {
-      await Promise.all(
-        selected.map(schedule => SchedulesAPI.destroy(schedule.id))
-      );
-    } catch (error) {
-      setDeletionError(error);
-    }
-
-    const params = parseQueryString(QS_CONFIG, location.search);
-    try {
-      const {
-        data: { count, results },
-      } = await SchedulesAPI.read(params);
-
-      setSchedules(results);
-      setScheduleCount(count);
-      setSelected([]);
-    } catch (error) {
-      setContentError(error);
-    }
-
-    setHasContentLoading(false);
+    await deleteJobs();
+    setSelected([]);
   };
 
-  const handleScheduleToggle = async scheduleToToggle => {
-    setToggleLoading(scheduleToToggle.id);
-    try {
-      const { data: updatedSchedule } = await SchedulesAPI.update(
-        scheduleToToggle.id,
-        {
-          enabled: !scheduleToToggle.enabled,
-        }
-      );
-      setSchedules(
-        schedules.map(schedule =>
-          schedule.id === updatedSchedule.id ? updatedSchedule : schedule
-        )
-      );
-    } catch (err) {
-      setToggleError(err);
-    } finally {
-      setToggleLoading(null);
-    }
-  };
-
-  const isAllSelected =
-    selected.length > 0 && selected.length === schedules.length;
-
   return (
     <PageSection>
       <Card>
         <PaginatedDataList
           contentError={contentError}
-          hasContentLoading={hasContentLoading}
+          hasContentLoading={isLoading || isDeleteLoading}
           items={schedules}
-          itemCount={scheduleCount}
+          itemCount={itemCount}
           qsConfig={QS_CONFIG}
           onRowClick={handleSelect}
           renderItem={item => (
@@ -131,9 +102,7 @@ function ScheduleList({ i18n }) {
               isSelected={selected.some(row => row.id === item.id)}
               key={item.id}
               onSelect={() => handleSelect(item)}
-              onToggleSchedule={handleScheduleToggle}
               schedule={item}
-              toggleLoading={toggleLoading === item.id}
             />
           )}
           toolbarSearchColumns={[
@@ -176,23 +145,12 @@ function ScheduleList({ i18n }) {
           )}
         />
       </Card>
-      {toggleError && !toggleLoading && (
-        <AlertModal
-          variant="danger"
-          title={i18n._(t`Error!`)}
-          isOpen={toggleError && !toggleLoading}
-          onClose={() => setToggleError(null)}
-        >
-          {i18n._(t`Failed to toggle schedule.`)}
-          <ErrorDetail error={toggleError} />
-        </AlertModal>
-      )}
       {deletionError && (
         <AlertModal
           isOpen={deletionError}
           variant="danger"
           title={i18n._(t`Error!`)}
-          onClose={() => setDeletionError(null)}
+          onClose={clearDeletionError}
         >
           {i18n._(t`Failed to delete one or more schedules.`)}
           <ErrorDetail error={deletionError} />
diff --git a/awx/ui_next/src/screens/Schedule/ScheduleList/ScheduleListItem.jsx b/awx/ui_next/src/screens/Schedule/ScheduleList/ScheduleListItem.jsx
index 0e6207ebe8..7d34f8b52d 100644
--- a/awx/ui_next/src/screens/Schedule/ScheduleList/ScheduleListItem.jsx
+++ b/awx/ui_next/src/screens/Schedule/ScheduleList/ScheduleListItem.jsx
@@ -11,15 +11,14 @@ import {
   DataListItem,
   DataListItemRow,
   DataListItemCells,
-  Switch,
   Tooltip,
 } from '@patternfly/react-core';
 import { PencilAltIcon } from '@patternfly/react-icons';
-
 import { DetailList, Detail } from '@components/DetailList';
 import styled from 'styled-components';
 import { Schedule } from '@types';
 import { formatDateString } from '@util/dates';
+import ScheduleToggle from '../shared/ScheduleToggle';
 
 const DataListAction = styled(_DataListAction)`
   align-items: center;
@@ -28,14 +27,7 @@ const DataListAction = styled(_DataListAction)`
   grid-template-columns: auto 40px;
 `;
 
-function ScheduleListItem({
-  i18n,
-  isSelected,
-  onSelect,
-  onToggleSchedule,
-  schedule,
-  toggleLoading,
-}) {
+function ScheduleListItem({ i18n, isSelected, onSelect, schedule }) {
   const labelId = `check-action-${schedule.id}`;
 
   const jobTypeLabels = {
@@ -111,27 +103,7 @@ function ScheduleListItem({
               id={labelId}
               key="actions"
             >
-              <Tooltip
-                content={
-                  schedule.enabled
-                    ? i18n._(t`Schedule is active`)
-                    : i18n._(t`Schedule is inactive`)
-                }
-                position="top"
-              >
-                <Switch
-                  id={`schedule-${schedule.id}-toggle`}
-                  label={i18n._(t`On`)}
-                  labelOff={i18n._(t`Off`)}
-                  isChecked={schedule.enabled}
-                  isDisabled={
-                    toggleLoading ||
-                    !schedule.summary_fields.user_capabilities.edit
-                  }
-                  onChange={() => onToggleSchedule(schedule)}
-                  aria-label={i18n._(t`Toggle schedule`)}
-                />
-              </Tooltip>
+              <ScheduleToggle schedule={schedule} />
               {schedule.summary_fields.user_capabilities.edit && (
                 <Tooltip content={i18n._(t`Edit Schedule`)} position="top">
                   <Button
@@ -154,7 +126,6 @@ function ScheduleListItem({
 
 ScheduleListItem.propTypes = {
   isSelected: bool.isRequired,
-  onToggleSchedule: func.isRequired,
   onSelect: func.isRequired,
   schedule: Schedule.isRequired,
 };
diff --git a/awx/ui_next/src/screens/Schedule/ScheduleList/ScheduleListItem.test.jsx b/awx/ui_next/src/screens/Schedule/ScheduleList/ScheduleListItem.test.jsx
index a835345edc..8e64b651a2 100644
--- a/awx/ui_next/src/screens/Schedule/ScheduleList/ScheduleListItem.test.jsx
+++ b/awx/ui_next/src/screens/Schedule/ScheduleList/ScheduleListItem.test.jsx
@@ -43,7 +43,6 @@ const mockSchedule = {
   until: '',
 };
 
-const onToggleSchedule = jest.fn();
 const onSelect = jest.fn();
 
 describe('ScheduleListItem', () => {
@@ -55,7 +54,6 @@ describe('ScheduleListItem', () => {
           isSelected={false}
           onSelect={onSelect}
           schedule={mockSchedule}
-          onToggleSchedule={onToggleSchedule}
         />
       );
     });
@@ -102,14 +100,6 @@ describe('ScheduleListItem', () => {
           .props().isDisabled
       ).toBe(false);
     });
-    test('Clicking toggle makes expected callback', () => {
-      wrapper
-        .find('Switch')
-        .first()
-        .find('input')
-        .simulate('change');
-      expect(onToggleSchedule).toHaveBeenCalledWith(mockSchedule);
-    });
     test('Clicking checkbox makes expected callback', () => {
       wrapper
         .find('DataListCheck')
@@ -135,7 +125,6 @@ describe('ScheduleListItem', () => {
               },
             },
           }}
-          onToggleSchedule={onToggleSchedule}
         />
       );
     });
diff --git a/awx/ui_next/src/screens/Schedule/shared/ScheduleToggle.jsx b/awx/ui_next/src/screens/Schedule/shared/ScheduleToggle.jsx
new file mode 100644
index 0000000000..699bb138ed
--- /dev/null
+++ b/awx/ui_next/src/screens/Schedule/shared/ScheduleToggle.jsx
@@ -0,0 +1,78 @@
+import React, { Fragment, useState, useEffect, useCallback } from 'react';
+import { withI18n } from '@lingui/react';
+import { t } from '@lingui/macro';
+import { Switch, Tooltip } from '@patternfly/react-core';
+import AlertModal from '@components/AlertModal';
+import ErrorDetail from '@components/ErrorDetail';
+import useRequest from '@util/useRequest';
+import { SchedulesAPI } from '@api';
+
+function ScheduleToggle({ schedule, onToggle, className, i18n }) {
+  const [isEnabled, setIsEnabled] = useState(schedule.enabled);
+  const [showError, setShowError] = useState(false);
+
+  const { result, isLoading, error, request: toggleSchedule } = useRequest(
+    useCallback(async () => {
+      await SchedulesAPI.update(schedule.id, {
+        enabled: !isEnabled,
+      });
+      return !isEnabled;
+    }, [schedule, isEnabled]),
+    schedule.enabled
+  );
+
+  useEffect(() => {
+    if (result !== isEnabled) {
+      setIsEnabled(result);
+      if (onToggle) {
+        onToggle(result);
+      }
+    }
+  }, [result, isEnabled, onToggle]);
+
+  useEffect(() => {
+    if (error) {
+      setShowError(true);
+    }
+  }, [error]);
+
+  return (
+    <Fragment>
+      <Tooltip
+        content={
+          schedule.enabled
+            ? i18n._(t`Schedule is active`)
+            : i18n._(t`Schedule is inactive`)
+        }
+        position="top"
+      >
+        <Switch
+          className={className}
+          css="display: inline-flex;"
+          id={`schedule-${schedule.id}-toggle`}
+          label={i18n._(t`On`)}
+          labelOff={i18n._(t`Off`)}
+          isChecked={isEnabled}
+          isDisabled={
+            isLoading || !schedule.summary_fields.user_capabilities.edit
+          }
+          onChange={toggleSchedule}
+          aria-label={i18n._(t`Toggle schedule`)}
+        />
+      </Tooltip>
+      {showError && error && !isLoading && (
+        <AlertModal
+          variant="error"
+          title={i18n._(t`Error!`)}
+          isOpen={error && !isLoading}
+          onClose={() => setShowError(false)}
+        >
+          {i18n._(t`Failed to toggle schedule.`)}
+          <ErrorDetail error={error} />
+        </AlertModal>
+      )}
+    </Fragment>
+  );
+}
+
+export default withI18n()(ScheduleToggle);
diff --git a/awx/ui_next/src/screens/Schedule/shared/ScheduleToggle.test.jsx b/awx/ui_next/src/screens/Schedule/shared/ScheduleToggle.test.jsx
new file mode 100644
index 0000000000..61828cfda9
--- /dev/null
+++ b/awx/ui_next/src/screens/Schedule/shared/ScheduleToggle.test.jsx
@@ -0,0 +1,97 @@
+import React from 'react';
+import { act } from 'react-dom/test-utils';
+import { SchedulesAPI } from '@api';
+import { mountWithContexts } from '@testUtils/enzymeHelpers';
+import ScheduleToggle from './ScheduleToggle';
+
+jest.mock('@api');
+
+const mockSchedule = {
+  url: '/api/v2/schedules/1',
+  rrule:
+    'DTSTART;TZID=America/New_York:20200220T000000 RRULE:FREQ=DAILY;INTERVAL=1;COUNT=1',
+  id: 1,
+  summary_fields: {
+    unified_job_template: {
+      id: 6,
+      name: 'Mock JT',
+      description: '',
+      unified_job_type: 'job',
+    },
+    user_capabilities: {
+      edit: true,
+      delete: true,
+    },
+  },
+  name: 'Mock JT Schedule',
+  next_run: '2020-02-20T05:00:00Z',
+  enabled: true,
+};
+
+describe('<ScheduleToggle>', () => {
+  test('should should toggle off', async () => {
+    const onToggle = jest.fn();
+    const wrapper = mountWithContexts(
+      <ScheduleToggle schedule={mockSchedule} onToggle={onToggle} />
+    );
+    expect(wrapper.find('Switch').prop('isChecked')).toEqual(true);
+
+    await act(async () => {
+      wrapper.find('Switch').invoke('onChange')();
+    });
+    expect(SchedulesAPI.update).toHaveBeenCalledWith(1, {
+      enabled: false,
+    });
+    wrapper.update();
+    expect(wrapper.find('Switch').prop('isChecked')).toEqual(false);
+    expect(onToggle).toHaveBeenCalledWith(false);
+  });
+
+  test('should should toggle on', async () => {
+    const onToggle = jest.fn();
+    const wrapper = mountWithContexts(
+      <ScheduleToggle
+        schedule={{
+          ...mockSchedule,
+          enabled: false,
+        }}
+        onToggle={onToggle}
+      />
+    );
+    expect(wrapper.find('Switch').prop('isChecked')).toEqual(false);
+
+    await act(async () => {
+      wrapper.find('Switch').invoke('onChange')();
+    });
+    expect(SchedulesAPI.update).toHaveBeenCalledWith(1, {
+      enabled: true,
+    });
+    wrapper.update();
+    expect(wrapper.find('Switch').prop('isChecked')).toEqual(true);
+    expect(onToggle).toHaveBeenCalledWith(true);
+  });
+
+  test('should show error modal', async () => {
+    SchedulesAPI.update.mockImplementation(() => {
+      throw new Error('nope');
+    });
+    const wrapper = mountWithContexts(
+      <ScheduleToggle schedule={mockSchedule} />
+    );
+    expect(wrapper.find('Switch').prop('isChecked')).toEqual(true);
+
+    await act(async () => {
+      wrapper.find('Switch').invoke('onChange')();
+    });
+    wrapper.update();
+    const modal = wrapper.find('AlertModal');
+    expect(modal).toHaveLength(1);
+    expect(modal.prop('isOpen')).toEqual(true);
+
+    act(() => {
+      modal.invoke('onClose')();
+    });
+    wrapper.update();
+    expect(wrapper.find('AlertModal')).toHaveLength(0);
+  });
+});