use HostToggle in HostDetail; update tests

awx/ui_next/src/components/AlertModal/AlertModal.jsx

@@ -16,7 +16,13 @@ const Header = styled.div`
   }
 `;
 
-export default ({ isOpen = null, title, variant, children, ...props }) => {
+export default function AlertModal({
+  isOpen = null,
+  title,
+  variant,
+  children,
+  ...props
+}) {
   const variantIcons = {
     danger: <ExclamationCircleIcon size="lg" css="color: #c9190b" />,
     error: <TimesCircleIcon size="lg" css="color: #c9190b" />,
@@ -44,4 +50,4 @@ export default ({ isOpen = null, title, variant, children, ...props }) => {
       {children}
     </Modal>
   );
-};
+}

awx/ui_next/src/screens/Host/HostDetail/HostDetail.jsx

@@ -3,7 +3,7 @@ import { Link, useHistory, useParams, useLocation } from 'react-router-dom';
 import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
 import { Host } from '@types';
-import { Button, Switch } from '@patternfly/react-core';
+import { Button } from '@patternfly/react-core';
 import { CardBody, CardActionsRow } from '@components/Card';
 import AlertModal from '@components/AlertModal';
 import ErrorDetail from '@components/ErrorDetail';
@@ -12,6 +12,7 @@ import { VariablesDetail } from '@components/CodeMirrorInput';
 import Sparkline from '@components/Sparkline';
 import DeleteButton from '@components/DeleteButton';
 import { HostsAPI } from '@api';
+import HostToggle from '../shared/HostToggle';
 
 function HostDetail({ host, i18n, onUpdateHost }) {
   const {
@@ -20,7 +21,6 @@ function HostDetail({ host, i18n, onUpdateHost }) {
     id,
     modified,
     name,
-    enabled,
     summary_fields: {
       inventory,
       recent_jobs,
@@ -36,25 +36,9 @@ function HostDetail({ host, i18n, onUpdateHost }) {
   const { id: inventoryId, hostId: inventoryHostId } = useParams();
   const [isLoading, setIsloading] = useState(false);
   const [deletionError, setDeletionError] = useState(false);
-  const [toggleLoading, setToggleLoading] = useState(false);
-  const [toggleError, setToggleError] = useState(false);
 
   const recentPlaybookJobs = recent_jobs.map(job => ({ ...job, type: 'job' }));
 
-  const handleHostToggle = async () => {
-    setToggleLoading(true);
-    try {
-      const { data } = await HostsAPI.update(id, {
-        enabled: !enabled,
-      });
-      onUpdateHost(data);
-    } catch (err) {
-      setToggleError(err);
-    } finally {
-      setToggleLoading(false);
-    }
-  };
-
   const handleHostDelete = async () => {
     setIsloading(true);
     try {
@@ -66,19 +50,6 @@ function HostDetail({ host, i18n, onUpdateHost }) {
     }
   };
 
-  if (toggleError && !toggleLoading) {
-    return (
-      <AlertModal
-        variant="error"
-        title={i18n._(t`Error!`)}
-        isOpen={toggleError && !toggleLoading}
-        onClose={() => setToggleError(false)}
-      >
-        {i18n._(t`Failed to toggle host.`)}
-        <ErrorDetail error={toggleError} />
-      </AlertModal>
-    );
-  }
   if (!isLoading && deletionError) {
     return (
       <AlertModal
@@ -94,15 +65,15 @@ function HostDetail({ host, i18n, onUpdateHost }) {
   }
   return (
     <CardBody>
-      <Switch
+      <HostToggle
+        host={host}
+        onToggle={enabled =>
+          onUpdateHost({
+            ...host,
+            enabled,
+          })
+        }
         css="padding-bottom: 40px"
-        id={`host-${id}-toggle`}
-        label={i18n._(t`On`)}
-        labelOff={i18n._(t`Off`)}
-        isChecked={enabled}
-        isDisabled={!user_capabilities.edit}
-        onChange={() => handleHostToggle()}
-        aria-label={i18n._(t`Toggle Host`)}
       />
       <DetailList gutter="sm">
         <Detail label={i18n._(t`Name`)} value={name} />

awx/ui_next/src/screens/Host/HostList/HostList.jsx

@@ -1,5 +1,5 @@
 import React, { Fragment, useState, useEffect, useCallback } from 'react';
-import { useLocation, useParams } from 'react-router-dom';
+import { useLocation, useRouteMatch } from 'react-router-dom';
 import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
 import { Card } from '@patternfly/react-core';
@@ -25,7 +25,7 @@ const QS_CONFIG = getQSConfig('host', {
 
 function HostList({ i18n }) {
   const location = useLocation();
-  const match = useParams();
+  const match = useRouteMatch();
   const [selected, setSelected] = useState([]);
 
   const {
@@ -146,13 +146,13 @@ function HostList({ i18n }) {
               ]}
             />
           )}
-          renderItem={o => (
+          renderItem={host => (
             <HostListItem
-              key={o.id}
-              host={o}
-              detailUrl={`${match.url}/${o.id}/details`}
-              isSelected={selected.some(row => row.id === o.id)}
-              onSelect={() => handleSelect(o)}
+              key={host.id}
+              host={host}
+              detailUrl={`${match.url}/${host.id}/details`}
+              isSelected={selected.some(row => row.id === host.id)}
+              onSelect={() => handleSelect(host)}
             />
           )}
           emptyStateControls={
@@ -177,4 +177,5 @@ function HostList({ i18n }) {
   );
 }
 
+export { HostList as _HostList };
 export default withI18n()(HostList);

awx/ui_next/src/screens/Host/HostList/HostList.test.jsx

@@ -1,9 +1,10 @@
 import React from 'react';
+import { act } from 'react-dom/test-utils';
 import { HostsAPI } from '@api';
 import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers';
 import { sleep } from '@testUtils/testUtils';
 
-import HostsList, { _HostsList } from './HostList';
+import HostList, { _HostList } from './HostList';
 
 jest.mock('@api');
 
@@ -68,7 +69,15 @@ const mockHosts = [
   },
 ];
 
-describe('<HostsList />', () => {
+function waitForLoaded(wrapper) {
+  return waitForElement(
+    wrapper,
+    'HostList',
+    el => el.find('ContentLoading').length === 0
+  );
+}
+
+describe('<HostList />', () => {
   beforeEach(() => {
     HostsAPI.read.mockResolvedValue({
       data: {
@@ -91,38 +100,33 @@ describe('<HostsList />', () => {
     jest.clearAllMocks();
   });
 
-  test('initially renders successfully', () => {
-    mountWithContexts(
-      <HostsList
-        match={{ path: '/hosts', url: '/hosts' }}
-        location={{ search: '', pathname: '/hosts' }}
-      />
-    );
+  test('initially renders successfully', async () => {
+    await act(async () => {
+      mountWithContexts(
+        <HostList
+          match={{ path: '/hosts', url: '/hosts' }}
+          location={{ search: '', pathname: '/hosts' }}
+        />
+      );
+    });
   });
 
-  test('Hosts are retrieved from the api and the components finishes loading', async done => {
-    const loadHosts = jest.spyOn(_HostsList.prototype, 'loadHosts');
-    const wrapper = mountWithContexts(<HostsList />);
-    await waitForElement(
-      wrapper,
-      'HostsList',
-      el => el.state('hasContentLoading') === true
-    );
-    expect(loadHosts).toHaveBeenCalled();
-    await waitForElement(
-      wrapper,
-      'HostsList',
-      el => el.state('hasContentLoading') === false
-    );
-    done();
+  test.only('Hosts are retrieved from the api and the components finishes loading', async () => {
+    let wrapper;
+    await act(async () => {
+      wrapper = mountWithContexts(<HostList />);
+    });
+    await waitForLoaded(wrapper);
+    expect(HostsAPI.read).toHaveBeenCalled();
+    expect(wrapper.find('HostListItem')).toHaveLength(3);
   });
 
-  test('handleSelect is called when a host list item is selected', async done => {
-    const handleSelect = jest.spyOn(_HostsList.prototype, 'handleSelect');
-    const wrapper = mountWithContexts(<HostsList />);
+  test('handleSelect is called when a host list item is selected', async () => {
+    const handleSelect = jest.spyOn(_HostList.prototype, 'handleSelect');
+    const wrapper = mountWithContexts(<HostList />);
     await waitForElement(
       wrapper,
-      'HostsList',
+      'HostList',
       el => el.state('hasContentLoading') === false
     );
     await wrapper
@@ -133,18 +137,17 @@ describe('<HostsList />', () => {
     expect(handleSelect).toBeCalled();
     await waitForElement(
       wrapper,
-      'HostsList',
+      'HostList',
       el => el.state('selected').length === 1
     );
-    done();
   });
 
-  test('handleSelectAll is called when select all checkbox is clicked', async done => {
-    const handleSelectAll = jest.spyOn(_HostsList.prototype, 'handleSelectAll');
-    const wrapper = mountWithContexts(<HostsList />);
+  test('handleSelectAll is called when select all checkbox is clicked', async () => {
+    const handleSelectAll = jest.spyOn(_HostList.prototype, 'handleSelectAll');
+    const wrapper = mountWithContexts(<HostList />);
     await waitForElement(
       wrapper,
-      'HostsList',
+      'HostList',
       el => el.state('hasContentLoading') === false
     );
     wrapper
@@ -154,15 +157,14 @@ describe('<HostsList />', () => {
     expect(handleSelectAll).toBeCalled();
     await waitForElement(
       wrapper,
-      'HostsList',
+      'HostList',
       el => el.state('selected').length === 3
     );
-    done();
   });
 
-  test('delete button is disabled if user does not have delete capabilities on a selected host', async done => {
-    const wrapper = mountWithContexts(<HostsList />);
-    wrapper.find('HostsList').setState({
+  test('delete button is disabled if user does not have delete capabilities on a selected host', async () => {
+    const wrapper = mountWithContexts(<HostList />);
+    wrapper.find('HostList').setState({
       hosts: mockHosts,
       itemCount: 3,
       isInitialized: true,
@@ -173,7 +175,7 @@ describe('<HostsList />', () => {
       'ToolbarDeleteButton * button',
       el => el.getDOMNode().disabled === false
     );
-    wrapper.find('HostsList').setState({
+    wrapper.find('HostList').setState({
       selected: mockHosts,
     });
     await waitForElement(
@@ -181,13 +183,12 @@ describe('<HostsList />', () => {
       'ToolbarDeleteButton * button',
       el => el.getDOMNode().disabled === true
     );
-    done();
   });
 
   test('api is called to delete hosts for each selected host.', () => {
     HostsAPI.destroy = jest.fn();
-    const wrapper = mountWithContexts(<HostsList />);
-    wrapper.find('HostsList').setState({
+    const wrapper = mountWithContexts(<HostList />);
+    wrapper.find('HostList').setState({
       hosts: mockHosts,
       itemCount: 2,
       isInitialized: true,
@@ -198,7 +199,7 @@ describe('<HostsList />', () => {
     expect(HostsAPI.destroy).toHaveBeenCalledTimes(2);
   });
 
-  test('error is shown when host not successfully deleted from api', async done => {
+  test('error is shown when host not successfully deleted from api', async () => {
     HostsAPI.destroy.mockRejectedValue(
       new Error({
         response: {
@@ -210,8 +211,8 @@ describe('<HostsList />', () => {
         },
       })
     );
-    const wrapper = mountWithContexts(<HostsList />);
-    wrapper.find('HostsList').setState({
+    const wrapper = mountWithContexts(<HostList />);
+    wrapper.find('HostList').setState({
       hosts: mockHosts,
       itemCount: 1,
       isInitialized: true,
@@ -226,27 +227,24 @@ describe('<HostsList />', () => {
       'Modal',
       el => el.props().isOpen === true && el.props().title === 'Error!'
     );
-
-    done();
   });
 
-  test('Add button shown for users without ability to POST', async done => {
-    const wrapper = mountWithContexts(<HostsList />);
+  test('Add button shown for users without ability to POST', async () => {
+    const wrapper = mountWithContexts(<HostList />);
     await waitForElement(
       wrapper,
-      'HostsList',
+      'HostList',
       el => el.state('hasContentLoading') === true
     );
     await waitForElement(
       wrapper,
-      'HostsList',
+      'HostList',
       el => el.state('hasContentLoading') === false
     );
     expect(wrapper.find('ToolbarAddButton').length).toBe(1);
-    done();
   });
 
-  test('Add button hidden for users without ability to POST', async done => {
+  test('Add button hidden for users without ability to POST', async () => {
     HostsAPI.readOptions.mockResolvedValue({
       data: {
         actions: {
@@ -254,18 +252,17 @@ describe('<HostsList />', () => {
         },
       },
     });
-    const wrapper = mountWithContexts(<HostsList />);
+    const wrapper = mountWithContexts(<HostList />);
     await waitForElement(
       wrapper,
-      'HostsList',
+      'HostList',
       el => el.state('hasContentLoading') === true
     );
     await waitForElement(
       wrapper,
-      'HostsList',
+      'HostList',
       el => el.state('hasContentLoading') === false
     );
     expect(wrapper.find('ToolbarAddButton').length).toBe(0);
-    done();
   });
 });

awx/ui_next/src/screens/Host/HostList/HostListItem.jsx

@@ -17,8 +17,8 @@ import { PencilAltIcon } from '@patternfly/react-icons';
 
 import Sparkline from '@components/Sparkline';
 import { Host } from '@types';
-import HostToggle from './HostToggle';
 import styled from 'styled-components';
+import HostToggle from '../shared/HostToggle';
 
 const DataListAction = styled(_DataListAction)`
   align-items: center;

awx/ui_next/src/screens/Host/HostList/HostListItem.test.jsx

@@ -1,5 +1,4 @@
 import React from 'react';
-
 import { mountWithContexts } from '@testUtils/enzymeHelpers';
 
 import HostsListItem from './HostListItem';
@@ -44,6 +43,7 @@ describe('<HostsListItem />', () => {
     );
     expect(wrapper.find('PencilAltIcon').exists()).toBeTruthy();
   });
+
   test('edit button hidden from users without edit capabilities', () => {
     const copyMockHost = Object.assign({}, mockHost);
     copyMockHost.summary_fields.user_capabilities.edit = false;
@@ -58,39 +58,4 @@ describe('<HostsListItem />', () => {
     );
     expect(wrapper.find('PencilAltIcon').exists()).toBeFalsy();
   });
-  test('handles toggle click when host is enabled', () => {
-    const wrapper = mountWithContexts(
-      <HostsListItem
-        isSelected={false}
-        detailUrl="/host/1"
-        onSelect={() => {}}
-        host={mockHost}
-        onToggleHost={onToggleHost}
-      />
-    );
-    wrapper
-      .find('Switch')
-      .first()
-      .find('input')
-      .simulate('change');
-    expect(onToggleHost).toHaveBeenCalledWith(mockHost);
-  });
-
-  test('handles toggle click when host is disabled', () => {
-    const wrapper = mountWithContexts(
-      <HostsListItem
-        isSelected={false}
-        detailUrl="/host/1"
-        onSelect={() => {}}
-        host={mockHost}
-        onToggleHost={onToggleHost}
-      />
-    );
-    wrapper
-      .find('Switch')
-      .first()
-      .find('input')
-      .simulate('change');
-    expect(onToggleHost).toHaveBeenCalledWith(mockHost);
-  });
 });

awx/ui_next/src/screens/Host/shared/HostToggle.jsx

@@ -7,7 +7,7 @@ import ErrorDetail from '@components/ErrorDetail';
 import useRequest from '@util/useRequest';
 import { HostsAPI } from '@api';
 
-function HostToggle({ host, i18n }) {
+function HostToggle({ host, onToggle, className, i18n }) {
   const [isEnabled, setIsEnabled] = useState(host.enabled);
   const [showError, setShowError] = useState(false);
 
@@ -24,8 +24,11 @@ function HostToggle({ host, i18n }) {
   useEffect(() => {
     if (result !== isEnabled) {
       setIsEnabled(result);
+      if (onToggle) {
+        onToggle(result);
+      }
     }
-  }, [result, isEnabled]);
+  }, [result, isEnabled, onToggle]);
 
   useEffect(() => {
     if (error) {
@@ -44,6 +47,7 @@ function HostToggle({ host, i18n }) {
         position="top"
       >
         <Switch
+          className={className}
           css="display: inline-flex;"
           id={`host-${host.id}-toggle`}
           label={i18n._(t`On`)}
@@ -56,7 +60,7 @@ function HostToggle({ host, i18n }) {
       </Tooltip>
       {showError && error && !isLoading && (
         <AlertModal
-          variant="danger"
+          variant="error"
           title={i18n._(t`Error!`)}
           isOpen={error && !isLoading}
           onClose={() => setShowError(false)}

awx/ui_next/src/screens/Host/shared/HostToggle.test.jsx

@@ -0,0 +1,92 @@
+import React from 'react';
+import { act } from 'react-dom/test-utils';
+import { HostsAPI } from '@api';
+import { mountWithContexts } from '@testUtils/enzymeHelpers';
+import HostToggle from './HostToggle';
+
+jest.mock('@api');
+
+const mockHost = {
+  id: 1,
+  name: 'Host 1',
+  url: '/api/v2/hosts/1',
+  inventory: 1,
+  enabled: true,
+  summary_fields: {
+    inventory: {
+      id: 1,
+      name: 'inv 1',
+    },
+    user_capabilities: {
+      delete: true,
+      update: true,
+    },
+    recent_jobs: [],
+  },
+};
+
+describe('<HostToggle>', () => {
+  test('should should toggle off', async () => {
+    const onToggle = jest.fn();
+    const wrapper = mountWithContexts(
+      <HostToggle host={mockHost} onToggle={onToggle} />
+    );
+    expect(wrapper.find('Switch').prop('isChecked')).toEqual(true);
+
+    await act(async () => {
+      wrapper.find('Switch').invoke('onChange')();
+    });
+    expect(HostsAPI.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(
+      <HostToggle
+        host={{
+          ...mockHost,
+          enabled: false,
+        }}
+        onToggle={onToggle}
+      />
+    );
+    expect(wrapper.find('Switch').prop('isChecked')).toEqual(false);
+
+    await act(async () => {
+      wrapper.find('Switch').invoke('onChange')();
+    });
+    expect(HostsAPI.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 () => {
+    HostsAPI.update.mockImplementation(() => {
+      throw new Error('nope');
+    });
+    const wrapper = mountWithContexts(<HostToggle host={mockHost} />);
+    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);
+  });
+});