Fixes testing issues and removes list item action buttons

awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceList.jsx

@@ -4,7 +4,10 @@ import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
 import { Button, Tooltip } from '@patternfly/react-core';
 
-import useRequest, { useDeleteItems } from '../../../util/useRequest';
+import useRequest, {
+  useDeleteItems,
+  useDismissableError,
+} from '../../../util/useRequest';
 import { getQSConfig, parseQueryString } from '../../../util/qs';
 import { InventoriesAPI, InventorySourcesAPI } from '../../../api';
 import PaginatedDataList, {
@@ -30,7 +33,7 @@ function InventorySourceList({ i18n }) {
 
   const {
     isLoading,
-    error,
+    error: fetchError,
     result: { sources, sourceCount, sourceChoices, sourceChoicesOptions },
     request: fetchSources,
   } = useRequest(
@@ -64,9 +67,8 @@ function InventorySourceList({ i18n }) {
     useCallback(async () => {
       if (canSyncSources) {
         await InventoriesAPI.syncAllSources(id);
-        fetchSources();
       }
-    }, [id, fetchSources, canSyncSources])
+    }, [id, canSyncSources])
   );
 
   useEffect(() => {
@@ -97,6 +99,7 @@ function InventorySourceList({ i18n }) {
       qsConfig: QS_CONFIG,
     }
   );
+  const { error: syncError, dismissError } = useDismissableError(syncAllError);
 
   const handleDelete = async () => {
     await handleDeleteSources();
@@ -109,7 +112,7 @@ function InventorySourceList({ i18n }) {
   return (
     <>
       <PaginatedDataList
-        contentError={error || deletionError || syncAllError}
+        contentError={fetchError}
         hasContentLoading={isLoading || isDeleteLoading || isSyncAllLoading}
         items={sources}
         itemCount={sourceCount}
@@ -138,15 +141,15 @@ function InventorySourceList({ i18n }) {
                 ? [
                     <Tooltip
                       key="update"
-                      content={i18n._(t`Sync All Sources`)}
+                      content={i18n._(t`Sync all sources`)}
                       position="top"
                     >
                       <Button
                         onClick={syncAll}
-                        aria-label={i18n._(t`Sync All`)}
+                        aria-label={i18n._(t`Sync all`)}
                         variant="secondary"
                       >
-                        {i18n._(t`Sync All`)}
+                        {i18n._(t`Sync all`)}
                       </Button>
                     </Tooltip>,
                   ]
@@ -173,15 +176,28 @@ function InventorySourceList({ i18n }) {
           );
         }}
       />
+      {syncError && (
+        <AlertModal
+          aria-label={i18n._(t`Sync error`)}
+          isOpen={syncError}
+          variant="error"
+          title={i18n._(t`Error!`)}
+          onClose={dismissError}
+        >
+          {i18n._(t`Failed to sync some or all inventory sources.`)}
+          <ErrorDetail error={syncError} />
+        </AlertModal>
+      )}
+
       {deletionError && (
         <AlertModal
-          aria-label={i18n._(t`Delete Error`)}
+          aria-label={i18n._(t`Delete error`)}
           isOpen={deletionError}
           variant="error"
           title={i18n._(t`Error!`)}
           onClose={clearDeletionError}
         >
-          {i18n._(t`Failed to delete one or more Inventory Sources.`)}
+          {i18n._(t`Failed to delete one or more inventory sources.`)}
           <ErrorDetail error={deletionError} />
         </AlertModal>
       )}

awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceList.test.jsx

@@ -7,39 +7,57 @@ import {
   mountWithContexts,
   waitForElement,
 } from '../../../../testUtils/enzymeHelpers';
+
 import InventorySourceList from './InventorySourceList';
 
 jest.mock('../../../api/models/InventorySources');
 jest.mock('../../../api/models/Inventories');
 jest.mock('../../../api/models/InventoryUpdates');
 
+const sources = {
+  data: {
+    results: [
+      {
+        id: 1,
+        name: 'Source Foo',
+        status: '',
+        source: 'ec2',
+        url: '/api/v2/inventory_sources/56/',
+        summary_fields: {
+          user_capabilities: {
+            edit: true,
+            delete: true,
+            start: true,
+            schedule: true,
+          },
+        },
+      },
+      {
+        id: 2,
+        name: 'Source Bar',
+        status: '',
+        source: 'scm',
+        url: '/api/v2/inventory_sources/57/',
+        summary_fields: {
+          user_capabilities: {
+            edit: true,
+            delete: true,
+            start: true,
+            schedule: true,
+          },
+        },
+      },
+    ],
+    count: 1,
+  },
+};
+
 describe('<InventorySourceList />', () => {
   let wrapper;
   let history;
 
   beforeEach(async () => {
-    InventoriesAPI.readSources.mockResolvedValue({
+    InventoriesAPI.readSources.mockResolvedValue(sources);
-      data: {
-        results: [
-          {
-            id: 1,
-            name: 'Source Foo',
-            status: '',
-            source: 'ec2',
-            url: '/api/v2/inventory_sources/56/',
-            summary_fields: {
-              user_capabilities: {
-                edit: true,
-                delete: true,
-                start: true,
-                schedule: true,
-              },
-            },
-          },
-        ],
-        count: 1,
-      },
-    });
     InventorySourcesAPI.readOptions.mockResolvedValue({
       data: {
         actions: {
@@ -81,9 +99,11 @@ describe('<InventorySourceList />', () => {
     wrapper.unmount();
     jest.clearAllMocks();
   });
+
   test('should mount properly', async () => {
     await waitForElement(wrapper, 'InventorySourceList', el => el.length > 0);
   });
+
   test('api calls should be made on mount', async () => {
     await waitForElement(wrapper, 'InventorySourceList', el => el.length > 0);
     expect(InventoriesAPI.readSources).toHaveBeenCalledWith('1', {
@@ -94,15 +114,23 @@ describe('<InventorySourceList />', () => {
     });
     expect(InventorySourcesAPI.readOptions).toHaveBeenCalled();
   });
+
   test('source data should render properly', async () => {
     await waitForElement(wrapper, 'InventorySourceList', el => el.length > 0);
-    expect(wrapper.find('PFDataListCell[aria-label="name"]').text()).toBe(
+    expect(
-      'Source Foo'
+      wrapper
-    );
+        .find("DataListItem[aria-labelledby='check-action-1']")
-    expect(wrapper.find('PFDataListCell[aria-label="type"]').text()).toBe(
+        .find('PFDataListCell[aria-label="name"]')
-      'EC2'
+        .text()
-    );
+    ).toBe('Source Foo');
+    expect(
+      wrapper
+        .find("DataListItem[aria-labelledby='check-action-1']")
+        .find('PFDataListCell[aria-label="type"]')
+        .text()
+    ).toBe('EC2');
   });
+
   test('add button is not disabled and delete button is disabled', async () => {
     await waitForElement(wrapper, 'InventorySourceList', el => el.length > 0);
     const addButton = wrapper.find('ToolbarAddButton').find('Link');
@@ -118,7 +146,7 @@ describe('<InventorySourceList />', () => {
     expect(deleteButton.prop('isDisabled')).toBe(true);
 
     await act(async () =>
-      wrapper.find('DataListCheck').prop('onChange')({ id: 1 })
+      wrapper.find('DataListCheck#select-source-1').prop('onChange')({ id: 1 })
     );
     wrapper.update();
     expect(wrapper.find('input#select-source-1').prop('checked')).toBe(true);
@@ -134,6 +162,7 @@ describe('<InventorySourceList />', () => {
     );
     expect(InventorySourcesAPI.destroy).toHaveBeenCalledWith(1);
   });
+
   test('should throw error after deletion failure', async () => {
     InventorySourcesAPI.destroy.mockRejectedValue(
       new Error({
@@ -151,7 +180,7 @@ describe('<InventorySourceList />', () => {
     await waitForElement(wrapper, 'InventorySourceList', el => el.length > 0);
 
     await act(async () =>
-      wrapper.find('DataListCheck').prop('onChange')({ id: 1 })
+      wrapper.find('DataListCheck#select-source-1').prop('onChange')({ id: 1 })
     );
     wrapper.update();
 
@@ -164,10 +193,11 @@ describe('<InventorySourceList />', () => {
       wrapper.find('Button[aria-label="confirm delete"]').prop('onClick')()
     );
     wrapper.update();
-    expect(wrapper.find("AlertModal[aria-label='Delete Error']").length).toBe(
+    expect(wrapper.find("AlertModal[aria-label='Delete error']").length).toBe(
       1
     );
   });
+
   test('displays error after unsuccessful read sources fetch', async () => {
     InventorySourcesAPI.readOptions.mockRejectedValue(
       new Error({
@@ -225,42 +255,36 @@ describe('<InventorySourceList />', () => {
 
     expect(wrapper.find('ContentError').length).toBe(1);
   });
-  test('should render sync all button and make api call to start sync for all', async () => {
+
-    const readSourcesResponse = {
+  test('displays error after unsuccessful sync all button', async () => {
-      data: {
+    InventoriesAPI.syncAllSources.mockRejectedValue(
-        results: [
+      new Error({
-          {
+        response: {
-            id: 1,
+          config: {
-            name: 'Source Foo',
+            method: 'post',
-            status: '',
+            url: '/api/v2/inventories/',
-            source: 'ec2',
-            url: '/api/v2/inventory_sources/56/',
-            summary_fields: {
-              user_capabilities: {
-                edit: true,
-                delete: true,
-                start: true,
-                schedule: true,
-              },
-            },
           },
-        ],
+          data: 'An error occurred',
-        count: 1,
+          status: 403,
-      },
-    };
-    InventoriesAPI.readSources
-      .mockResolvedValue({ ...readSourcesResponse, status: 'pending' })
-      .mockResolvedValueOnce(readSourcesResponse);
-    InventorySourcesAPI.readOptions.mockResolvedValue({
-      data: {
-        actions: {
-          GET: { source: { choices: [['scm', 'SCM'], ['ec2', 'EC2']] } },
-          POST: {},
         },
-      },
+      })
-    });
+    );
     await waitForElement(wrapper, 'InventorySourceList', el => el.length > 0);
-    const syncAllButton = wrapper.find('Button[aria-label="Sync All"]');
+    await act(async () =>
+      wrapper.find('Button[aria-label="Sync all"]').prop('onClick')()
+    );
+    expect(InventoriesAPI.syncAllSources).toBeCalled();
+    wrapper.update();
+    expect(wrapper.find("AlertModal[aria-label='Sync error']").length).toBe(1);
+  });
+
+  test('should render sync all button and make api call to start sync for all', async () => {
+    await waitForElement(
+      wrapper,
+      'InventorySourceListItem',
+      el => el.length > 0
+    );
+    const syncAllButton = wrapper.find('Button[aria-label="Sync all"]');
     expect(syncAllButton.length).toBe(1);
     await act(async () => syncAllButton.prop('onClick')());
     expect(InventoriesAPI.syncAllSources).toBeCalled();
@@ -270,28 +294,13 @@ describe('<InventorySourceList />', () => {
 
 describe('<InventorySourceList /> RBAC testing', () => {
   test('should not render add button', async () => {
-    InventoriesAPI.readSources.mockResolvedValue({
+    sources.data.results[0].summary_fields.user_capabilities = {
-      data: {
+      edit: true,
-        results: [
+      delete: true,
-          {
+      start: true,
-            id: 1,
+      schedule: true,
-            name: 'Source Foo',
+    };
-            status: '',
+    InventoriesAPI.readSources.mockResolvedValue(sources);
-            source: 'ec2',
-            url: '/api/v2/inventory_sources/56/',
-            summary_fields: {
-              user_capabilities: {
-                edit: true,
-                delete: true,
-                start: true,
-                schedule: true,
-              },
-            },
-          },
-        ],
-        count: 1,
-      },
-    });
     InventorySourcesAPI.readOptions.mockResolvedValue({
       data: {
         actions: {
@@ -337,51 +346,15 @@ describe('<InventorySourceList /> RBAC testing', () => {
     newWrapper.unmount();
     jest.clearAllMocks();
   });
+
   test('should not render Sync All button', async () => {
-    InventoriesAPI.readSources.mockResolvedValue({
+    sources.data.results[0].summary_fields.user_capabilities = {
-      data: {
+      edit: true,
-        results: [
+      delete: true,
-          {
+      start: false,
-            id: 1,
+      schedule: true,
-            name: 'Source Foo',
+    };
-            status: '',
+    InventoriesAPI.readSources.mockResolvedValue(sources);
-            source: 'ec2',
-            url: '/api/v2/inventory_sources/56/',
-            summary_fields: {
-              user_capabilities: {
-                edit: true,
-                delete: true,
-                start: false,
-                schedule: true,
-              },
-            },
-          },
-          {
-            id: 2,
-            name: 'Source Bar',
-            status: '',
-            source: 'scm',
-            url: '/api/v2/inventory_sources/57/',
-            summary_fields: {
-              user_capabilities: {
-                edit: true,
-                delete: true,
-                start: true,
-                schedule: true,
-              },
-            },
-          },
-        ],
-        count: 1,
-      },
-    });
-    InventorySourcesAPI.readOptions.mockResolvedValue({
-      data: {
-        actions: {
-          GET: { source: { choices: [['scm', 'SCM'], ['ec2', 'EC2']] } },
-        },
-      },
-    });
     let newWrapper;
     const history = createMemoryHistory({
       initialEntries: ['/inventories/inventory/2/sources'],

awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceListItem.jsx

@@ -1,4 +1,4 @@
-import React, { useState } from 'react';
+import React from 'react';
 import { withI18n } from '@lingui/react';
 import { Link } from 'react-router-dom';
 import { t } from '@lingui/macro';
@@ -24,10 +24,7 @@ function InventorySourceListItem({
   i18n,
   detailUrl,
   label,
-  onFetchSources,
 }) {
-  const [isSyncLoading, setIsSyncLoading] = useState(false);
-
   const generateLastJobTooltip = job => {
     return (
       <>
@@ -51,7 +48,6 @@ function InventorySourceListItem({
       <DataListItem aria-labelledby={`check-action-${source.id}`}>
         <DataListItemRow>
           <DataListCheck
-            isDisabled={isSyncLoading}
             id={`select-source-${source.id}`}
             checked={isSelected}
             onChange={onSelect}
@@ -96,20 +92,13 @@ function InventorySourceListItem({
             aria-label="actions"
           >
             {source.summary_fields.user_capabilities.start && (
-              <InventorySourceSyncButton
+              <InventorySourceSyncButton source={source} />
-                onSyncLoading={isLoading => {
-                  setIsSyncLoading(isLoading);
-                }}
-                onFetchSources={onFetchSources}
-                source={source}
-              />
             )}
             {source.summary_fields.user_capabilities.edit && (
               <Button
                 aria-label={i18n._(t`Edit Source`)}
                 variant="plain"
                 component={Link}
-                isDisabled={isSyncLoading}
                 to={`${detailUrl}/edit`}
               >
                 <PencilAltIcon />

awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceSyncButton.jsx

@@ -1,4 +1,4 @@
-import React, { useCallback, useEffect } from 'react';
+import React, { useCallback } from 'react';
 import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
 import PropTypes from 'prop-types';
@@ -9,12 +9,7 @@ import AlertModal from '../../../components/AlertModal/AlertModal';
 import ErrorDetail from '../../../components/ErrorDetail/ErrorDetail';
 import { InventoryUpdatesAPI, InventorySourcesAPI } from '../../../api';
 
-function InventorySourceSyncButton({
+function InventorySourceSyncButton({ source, i18n }) {
-  onSyncLoading,
-  source,
-  i18n,
-  onFetchSources,
-}) {
   const {
     isLoading: startSyncLoading,
     error: startSyncError,
@@ -24,10 +19,9 @@ function InventorySourceSyncButton({
       const {
         data: { status },
       } = await InventorySourcesAPI.createSyncStart(source.id);
-      onFetchSources();
 
       return status;
-    }, [source.id, onFetchSources]),
+    }, [source.id]),
     {}
   );
 
@@ -46,16 +40,9 @@ function InventorySourceSyncButton({
       } = await InventorySourcesAPI.readDetail(source.id);
 
       await InventoryUpdatesAPI.createSyncCancel(id);
-      onFetchSources();
+    }, [source.id])
-    }, [source.id, onFetchSources])
   );
 
-  useEffect(() => onSyncLoading(startSyncLoading || cancelSyncLoading), [
-    onSyncLoading,
-    startSyncLoading,
-    cancelSyncLoading,
-  ]);
-
   const { error, dismissError } = useDismissableError(
     cancelSyncError || startSyncError
   );
@@ -107,9 +94,7 @@ InventorySourceSyncButton.defaultProps = {
 };
 
 InventorySourceSyncButton.propTypes = {
-  onSyncLoading: PropTypes.func.isRequired,
   source: PropTypes.shape({}),
-  onFetchSources: PropTypes.func.isRequired,
 };
 
 export default withI18n()(InventorySourceSyncButton);

awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceSyncButton.test.jsx

@@ -83,4 +83,24 @@ describe('<InventorySourceSyncButton />', () => {
     expect(InventorySourcesAPI.readDetail).toBeCalledWith(1);
     expect(InventoryUpdatesAPI.createSyncCancel).toBeCalledWith(120);
   });
+  test('should throw error on sync start properly', async () => {
+    InventorySourcesAPI.createSyncStart.mockRejectedValueOnce(
+      new Error({
+        response: {
+          config: {
+            method: 'post',
+            url: '/api/v2/inventory_sources/update',
+          },
+          data: 'An error occurred',
+          status: 403,
+        },
+      })
+    );
+
+    await act(async () =>
+      wrapper.find('Button[aria-label="Start sync source"]').simulate('click')
+    );
+    wrapper.update();
+    expect(wrapper.find('AlertModal').length).toBe(1);
+  });
 });