Merge pull request #41 from ansible/devel

Rebase

awx/ui_next/src/components/CopyButton/CopyButton.jsx

@@ -3,7 +3,7 @@ import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
 import PropTypes from 'prop-types';
 
-import { Button, Tooltip } from '@patternfly/react-core';
+import { Button } from '@patternfly/react-core';
 import { CopyIcon } from '@patternfly/react-icons';
 import useRequest, { useDismissableError } from '../../util/useRequest';
 import AlertModal from '../AlertModal';
@@ -15,7 +15,7 @@ function CopyButton({
   isDisabled,
   onCopyStart,
   onCopyFinish,
-  helperText,
+  errorMessage,
   i18n,
 }) {
   const { isLoading, error: copyError, request: copyItemToAPI } = useRequest(
@@ -33,17 +33,15 @@ function CopyButton({
 
   return (
     <>
-      <Tooltip content={helperText.tooltip} position="top">
+      <Button
-        <Button
+        id={id}
-          id={id}
+        isDisabled={isLoading || isDisabled}
-          isDisabled={isLoading || isDisabled}
+        aria-label={i18n._(t`Copy`)}
-          aria-label={i18n._(t`Copy`)}
+        variant="plain"
-          variant="plain"
+        onClick={copyItemToAPI}
-          onClick={copyItemToAPI}
+      >
-        >
+        <CopyIcon />
-          <CopyIcon />
+      </Button>
-        </Button>
-      </Tooltip>
       <AlertModal
         aria-label={i18n._(t`Copy Error`)}
         isOpen={error}
@@ -51,7 +49,7 @@ function CopyButton({
         title={i18n._(t`Error!`)}
         onClose={dismissError}
       >
-        {helperText.errorMessage}
+        {errorMessage}
         <ErrorDetail error={error} />
       </AlertModal>
     </>
@@ -62,10 +60,7 @@ CopyButton.propTypes = {
   copyItem: PropTypes.func.isRequired,
   onCopyStart: PropTypes.func.isRequired,
   onCopyFinish: PropTypes.func.isRequired,
-  helperText: PropTypes.shape({
+  errorMessage: PropTypes.string.isRequired,
-    tooltip: PropTypes.string.isRequired,
-    errorMessage: PropTypes.string.isRequired,
-  }).isRequired,
   isDisabled: PropTypes.bool,
 };
 

awx/ui_next/src/components/CopyButton/CopyButton.test.jsx

@@ -1,36 +1,44 @@
 import React from 'react';
+import { act } from 'react-dom/test-utils';
 import { mountWithContexts } from '../../../testUtils/enzymeHelpers';
 import CopyButton from './CopyButton';
 
 jest.mock('../../api');
 
+let wrapper;
+
 describe('<CopyButton/>', () => {
-  test('shold mount properly', () => {
+  afterEach(() => {
-    const wrapper = mountWithContexts(
+    wrapper.unmount();
-      <CopyButton
+  });
-        onCopyStart={() => {}}
+  test('should mount properly', async () => {
-        onCopyFinish={() => {}}
+    await act(async () => {
-        copyItem={() => {}}
+      wrapper = mountWithContexts(
-        helperText={{
+        <CopyButton
-          tooltip: `Copy Template`,
+          onCopyStart={() => {}}
-          errorMessage: `Failed to copy template.`,
+          onCopyFinish={() => {}}
-        }}
+          copyItem={() => {}}
-      />
+          errorMessage="Failed to copy template."
-    );
+        />
+      );
+    });
     expect(wrapper.find('CopyButton').length).toBe(1);
   });
-  test('should render proper tooltip', () => {
+  test('should call the correct function on button click', async () => {
-    const wrapper = mountWithContexts(
+    const copyItem = jest.fn();
-      <CopyButton
+    await act(async () => {
-        onCopyStart={() => {}}
+      wrapper = mountWithContexts(
-        onCopyFinish={() => {}}
+        <CopyButton
-        copyItem={() => {}}
+          onCopyStart={() => {}}
-        helperText={{
+          onCopyFinish={() => {}}
-          tooltip: `Copy Template`,
+          copyItem={copyItem}
-          errorMessage: `Failed to copy template.`,
+          errorMessage="Failed to copy template."
-        }}
+        />
-      />
+      );
-    );
+    });
-    expect(wrapper.find('Tooltip').prop('content')).toBe('Copy Template');
+    await act(async () => {
+      wrapper.find('button').simulate('click');
+    });
+    expect(copyItem).toHaveBeenCalledTimes(1);
   });
 });

awx/ui_next/src/components/PaginatedTable/ActionItem.jsx

@@ -14,7 +14,7 @@ export default function ActionItem({ column, tooltip, visible, children }) {
       `}
     >
       <Tooltip content={tooltip} position="top">
-        {children}
+        <div>{children}</div>
       </Tooltip>
     </div>
   );

awx/ui_next/src/components/PaginatedTable/ActionItem.test.jsx

@@ -12,7 +12,7 @@ describe('<ActionItem />', () => {
 
     const tooltip = wrapper.find('Tooltip');
     expect(tooltip.prop('content')).toEqual('a tooltip');
-    expect(tooltip.prop('children')).toEqual('foo');
+    expect(tooltip.prop('children')).toEqual(<div>foo</div>);
   });
 
   test('should render null if not visible', async () => {

awx/ui_next/src/components/TemplateList/TemplateListItem.jsx

@@ -177,13 +177,13 @@ function TemplateListItem({
               <PencilAltIcon />
             </Button>
           </ActionItem>
-          <ActionItem visible={template.summary_fields.user_capabilities.copy}>
+          <ActionItem
+            tooltip={i18n._(t`Copy Template`)}
+            visible={template.summary_fields.user_capabilities.copy}
+          >
             <CopyButton
               id={`template-action-copy-${template.id}`}
-              helperText={{
+              errorMessage={i18n._(t`Failed to copy template.`)}
-                errorMessage: i18n._(t`Failed to copy template.`),
-                tooltip: i18n._(t`Copy Template`),
-              }}
               isDisabled={isDisabled}
               onCopyStart={handleCopyStart}
               onCopyFinish={handleCopyFinish}

awx/ui_next/src/screens/Credential/CredentialList/CredentialListItem.jsx

@@ -72,16 +72,16 @@ function CredentialListItem({
             <PencilAltIcon />
           </Button>
         </ActionItem>
-        <ActionItem visible={credential.summary_fields.user_capabilities.copy}>
+        <ActionItem
+          tooltip={i18n._(t`Copy Credential`)}
+          visible={credential.summary_fields.user_capabilities.copy}
+        >
           <CopyButton
             isDisabled={isDisabled}
             onCopyStart={handleCopyStart}
             onCopyFinish={handleCopyFinish}
             copyItem={copyCredential}
-            helperText={{
+            errorMessage={i18n._(t`Failed to copy credential.`)}
-              tooltip: i18n._(t`Copy Credential`),
-              errorMessage: i18n._(t`Failed to copy credential.`),
-            }}
           />
         </ActionItem>
       </ActionsTd>

awx/ui_next/src/screens/Inventory/InventoryList/InventoryListItem.jsx

@@ -28,7 +28,7 @@ function InventoryListItem({
     isSelected: bool.isRequired,
     onSelect: func.isRequired,
   };
-  const [isDisabled, setIsDisabled] = useState(false);
+  const [isCopying, setIsCopying] = useState(false);
 
   const copyInventory = useCallback(async () => {
     await InventoriesAPI.copy(inventory.id, {
@@ -38,11 +38,11 @@ function InventoryListItem({
   }, [inventory.id, inventory.name, fetchInventories]);
 
   const handleCopyStart = useCallback(() => {
-    setIsDisabled(true);
+    setIsCopying(true);
   }, []);
 
   const handleCopyFinish = useCallback(() => {
-    setIsDisabled(false);
+    setIsCopying(false);
   }, []);
 
   const labelId = `check-action-${inventory.id}`;
@@ -115,7 +115,7 @@ function InventoryListItem({
             tooltip={i18n._(t`Edit Inventory`)}
           >
             <Button
-              isDisabled={isDisabled}
+              isDisabled={isCopying}
               aria-label={i18n._(t`Edit Inventory`)}
               variant="plain"
               component={Link}
@@ -128,17 +128,18 @@ function InventoryListItem({
           </ActionItem>
           <ActionItem
             visible={inventory.summary_fields.user_capabilities.copy}
-            tooltip={i18n._(t`Copy Inventory`)}
+            tooltip={
+              inventory.has_inventory_sources
+                ? i18n._(t`Inventories with sources cannot be copied`)
+                : i18n._(t`Copy Inventory`)
+            }
           >
             <CopyButton
               copyItem={copyInventory}
-              isDisabled={isDisabled}
+              isDisabled={isCopying || inventory.has_inventory_sources}
               onCopyStart={handleCopyStart}
               onCopyFinish={handleCopyFinish}
-              helperText={{
+              errorMessage={i18n._(t`Failed to copy inventory.`)}
-                tooltip: i18n._(t`Copy Inventory`),
-                errorMessage: i18n._(t`Failed to copy inventory.`),
-              }}
             />
           </ActionItem>
         </ActionsTd>

awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateListItem.jsx

@@ -159,16 +159,15 @@ function NotificationTemplateListItem({
             <div />
           )}
           {template.summary_fields.user_capabilities.copy && (
-            <CopyButton
+            <Tooltip content={i18n._(t`Copy Notification Template`)}>
-              copyItem={copyTemplate}
+              <CopyButton
-              isCopyDisabled={isCopyDisabled}
+                copyItem={copyTemplate}
-              onCopyStart={handleCopyStart}
+                isCopyDisabled={isCopyDisabled}
-              onCopyFinish={handleCopyFinish}
+                onCopyStart={handleCopyStart}
-              helperText={{
+                onCopyFinish={handleCopyFinish}
-                tooltip: i18n._(t`Copy Notification Template`),
+                errorMessage={i18n._(t`Failed to copy template.`)}
-                errorMessage: i18n._(t`Failed to copy template.`),
+              />
-              }}
+            </Tooltip>
-            />
           )}
         </DataListAction>
       </DataListItemRow>

awx/ui_next/src/screens/Project/ProjectList/ProjectListItem.jsx

@@ -143,16 +143,16 @@ function ProjectListItem({
             <PencilAltIcon />
           </Button>
         </ActionItem>
-        <ActionItem visible={project.summary_fields.user_capabilities.copy}>
+        <ActionItem
+          tooltip={i18n._(t`Copy Project`)}
+          visible={project.summary_fields.user_capabilities.copy}
+        >
           <CopyButton
             copyItem={copyProject}
             isDisabled={isDisabled}
             onCopyStart={handleCopyStart}
             onCopyFinish={handleCopyFinish}
-            helperText={{
+            errorMessage={i18n._(t`Failed to copy project.`)}
-              tooltip: i18n._(t`Copy Project`),
-              errorMessage: i18n._(t`Failed to copy project.`),
-            }}
           />
         </ActionItem>
       </ActionsTd>

awx_collection/plugins/modules/tower_settings.py

@@ -133,7 +133,7 @@ def main():
     existing_settings = module.get_endpoint('settings/all')['json']
 
     # Begin a json response
-    json_response = {'changed': False, 'old_values': {}}
+    json_output = {'changed': False, 'old_values': {}, 'new_values': {}}
 
     # Check any of the settings to see if anything needs to be updated
     needs_update = False
@@ -141,18 +141,29 @@ def main():
         if a_setting not in existing_settings or existing_settings[a_setting] != new_settings[a_setting]:
             # At least one thing is different so we need to patch
             needs_update = True
-            json_response['old_values'][a_setting] = existing_settings[a_setting]
+            json_output['old_values'][a_setting] = existing_settings[a_setting]
+            json_output['new_values'][a_setting] = new_settings[a_setting]
+
+    if module._diff:
+        json_output['diff'] = {
+            'before': json_output['old_values'],
+            'after': json_output['new_values']
+        }
 
     # If nothing needs an update we can simply exit with the response (as not changed)
     if not needs_update:
-        module.exit_json(**json_response)
+        module.exit_json(**json_output)
+
+    if module.check_mode and module._diff:
+        json_output['changed'] = True
+        module.exit_json(**json_output)
 
     # Make the call to update the settings
     response = module.patch_endpoint('settings/all', **{'data': new_settings})
 
     if response['status_code'] == 200:
         # Set the changed response to True
-        json_response['changed'] = True
+        json_output['changed'] = True
 
         # To deal with the old style values we need to return 'value' in the response
         new_values = {}
@@ -161,11 +172,11 @@ def main():
 
         # If we were using a name we will just add a value of a string, otherwise we will return an array in values
         if name is not None:
-            json_response['value'] = new_values[name]
+            json_output['value'] = new_values[name]
         else:
-            json_response['values'] = new_values
+            json_output['values'] = new_values
 
-        module.exit_json(**json_response)
+        module.exit_json(**json_output)
     elif 'json' in response and '__all__' in response['json']:
         module.fail_json(msg=response['json']['__all__'])
     else: