diff --git a/awx/ui_next/src/api/models/Settings.js b/awx/ui_next/src/api/models/Settings.js index 8610fc029f..3c85f68da6 100644 --- a/awx/ui_next/src/api/models/Settings.js +++ b/awx/ui_next/src/api/models/Settings.js @@ -22,7 +22,7 @@ class Settings extends Base { return this.http.options(`${this.baseUrl}${category}/`); } - test(category, data) { + createTest(category, data) { return this.http.post(`${this.baseUrl}${category}/test/`, data); } } diff --git a/awx/ui_next/src/screens/Setting/ActivityStream/ActivityStream.test.jsx b/awx/ui_next/src/screens/Setting/ActivityStream/ActivityStream.test.jsx index 90500406b0..14b698cefb 100644 --- a/awx/ui_next/src/screens/Setting/ActivityStream/ActivityStream.test.jsx +++ b/awx/ui_next/src/screens/Setting/ActivityStream/ActivityStream.test.jsx @@ -1,7 +1,10 @@ import React from 'react'; import { act } from 'react-dom/test-utils'; import { createMemoryHistory } from 'history'; -import { mountWithContexts } from '../../../../testUtils/enzymeHelpers'; +import { + mountWithContexts, + waitForElement, +} from '../../../../testUtils/enzymeHelpers'; import ActivityStream from './ActivityStream'; import { SettingsAPI } from '../../../api'; @@ -56,4 +59,27 @@ describe('', () => { }); expect(wrapper.find('ContentError').length).toBe(1); }); + + test('should redirect to details for users without system admin permissions', async () => { + const history = createMemoryHistory({ + initialEntries: ['/settings/activity_stream/edit'], + }); + await act(async () => { + wrapper = mountWithContexts(, { + context: { + router: { + history, + }, + config: { + me: { + is_superuser: false, + }, + }, + }, + }); + }); + await waitForElement(wrapper, 'ContentLoading', el => el.length === 0); + expect(wrapper.find('ActivityStreamDetail').length).toBe(1); + expect(wrapper.find('ActivityStreamEdit').length).toBe(0); + }); }); diff --git a/awx/ui_next/src/screens/Setting/ActivityStream/ActivityStreamEdit/ActivityStreamEdit.jsx b/awx/ui_next/src/screens/Setting/ActivityStream/ActivityStreamEdit/ActivityStreamEdit.jsx index 4397c9cf48..489979c748 100644 --- a/awx/ui_next/src/screens/Setting/ActivityStream/ActivityStreamEdit/ActivityStreamEdit.jsx +++ b/awx/ui_next/src/screens/Setting/ActivityStream/ActivityStreamEdit/ActivityStreamEdit.jsx @@ -51,24 +51,17 @@ function ActivityStreamEdit() { request(); }, [request]); - const { - error: submitError, - request: submitForm, - result: submitResult, - } = useRequest( - useCallback(async values => { - const result = await SettingsAPI.updateAll(values); - return result; - }, []), + const { error: submitError, request: submitForm } = useRequest( + useCallback( + async values => { + await SettingsAPI.updateAll(values); + history.push('/settings/activity_stream/details'); + }, + [history] + ), null ); - useEffect(() => { - if (submitResult) { - history.push('/settings/activity_stream/details'); - } - }, [submitResult, history]); - const handleSubmit = async form => { await submitForm(form); }; diff --git a/awx/ui_next/src/screens/Setting/AzureAD/AzureADEdit/AzureADEdit.jsx b/awx/ui_next/src/screens/Setting/AzureAD/AzureADEdit/AzureADEdit.jsx index 115a854e8d..2bee3ae277 100644 --- a/awx/ui_next/src/screens/Setting/AzureAD/AzureADEdit/AzureADEdit.jsx +++ b/awx/ui_next/src/screens/Setting/AzureAD/AzureADEdit/AzureADEdit.jsx @@ -43,24 +43,17 @@ function AzureADEdit() { fetchAzureAD(); }, [fetchAzureAD]); - const { - error: submitError, - request: submitForm, - result: submitResult, - } = useRequest( - useCallback(async values => { - const result = await SettingsAPI.updateAll(values); - return result; - }, []), + const { error: submitError, request: submitForm } = useRequest( + useCallback( + async values => { + await SettingsAPI.updateAll(values); + history.push('/settings/azure/details'); + }, + [history] + ), null ); - useEffect(() => { - if (submitResult) { - history.push('/settings/azure/details'); - } - }, [submitResult, history]); - const handleSubmit = async form => { await submitForm({ ...form, @@ -90,7 +83,10 @@ function AzureADEdit() { const initialValues = fields => Object.keys(fields).reduce((acc, key) => { if (fields[key].type === 'list' || fields[key].type === 'nested object') { - acc[key] = JSON.stringify(fields[key].value, null, 2); + const emptyDefault = fields[key].type === 'list' ? '[]' : '{}'; + acc[key] = fields[key].value + ? JSON.stringify(fields[key].value, null, 2) + : emptyDefault; } else { acc[key] = fields[key].value ?? ''; } diff --git a/awx/ui_next/src/screens/Setting/Logging/Logging.test.jsx b/awx/ui_next/src/screens/Setting/Logging/Logging.test.jsx index 1fd76f8b3c..14f7bc5b15 100644 --- a/awx/ui_next/src/screens/Setting/Logging/Logging.test.jsx +++ b/awx/ui_next/src/screens/Setting/Logging/Logging.test.jsx @@ -80,4 +80,27 @@ describe('', () => { }); expect(wrapper.find('ContentError').length).toBe(1); }); + + test('should redirect to details for users without system admin permissions', async () => { + const history = createMemoryHistory({ + initialEntries: ['/settings/logging/edit'], + }); + await act(async () => { + wrapper = mountWithContexts(, { + context: { + router: { + history, + }, + config: { + me: { + is_superuser: false, + }, + }, + }, + }); + }); + await waitForElement(wrapper, 'ContentLoading', el => el.length === 0); + expect(wrapper.find('LoggingDetail').length).toBe(1); + expect(wrapper.find('LoggingEdit').length).toBe(0); + }); }); diff --git a/awx/ui_next/src/screens/Setting/Logging/LoggingEdit/LoggingEdit.jsx b/awx/ui_next/src/screens/Setting/Logging/LoggingEdit/LoggingEdit.jsx index 9411332b7c..d930d4ea27 100644 --- a/awx/ui_next/src/screens/Setting/Logging/LoggingEdit/LoggingEdit.jsx +++ b/awx/ui_next/src/screens/Setting/Logging/LoggingEdit/LoggingEdit.jsx @@ -51,24 +51,17 @@ function LoggingEdit({ i18n }) { fetchLogging(); }, [fetchLogging]); - const { - error: submitError, - request: submitForm, - result: submitResult, - } = useRequest( - useCallback(async values => { - const result = await SettingsAPI.updateAll(values); - return result; - }, []), + const { error: submitError, request: submitForm } = useRequest( + useCallback( + async values => { + await SettingsAPI.updateAll(values); + history.push('/settings/logging/details'); + }, + [history] + ), null ); - useEffect(() => { - if (submitResult) { - history.push('/settings/logging/details'); - } - }, [submitResult, history]); - const handleSubmit = async form => { await submitForm({ ...form, @@ -95,7 +88,7 @@ function LoggingEdit({ i18n }) { setValue: setTestLogging, } = useRequest( useCallback(async () => { - const result = await SettingsAPI.test('logging', {}); + const result = await SettingsAPI.createTest('logging', {}); return result; }, []), null diff --git a/awx/ui_next/src/screens/Setting/Logging/LoggingEdit/LoggingEdit.test.jsx b/awx/ui_next/src/screens/Setting/Logging/LoggingEdit/LoggingEdit.test.jsx index 4b97720cf0..5caf83714b 100644 --- a/awx/ui_next/src/screens/Setting/Logging/LoggingEdit/LoggingEdit.test.jsx +++ b/awx/ui_next/src/screens/Setting/Logging/LoggingEdit/LoggingEdit.test.jsx @@ -212,15 +212,15 @@ describe('', () => { }); test('should display successful toast when test button is clicked', async () => { - SettingsAPI.test.mockResolvedValue({}); - expect(SettingsAPI.test).toHaveBeenCalledTimes(0); + SettingsAPI.createTest.mockResolvedValue({}); + expect(SettingsAPI.createTest).toHaveBeenCalledTimes(0); expect(wrapper.find('LoggingTestAlert')).toHaveLength(0); await act(async () => { wrapper.find('button[aria-label="Test logging"]').invoke('onClick')(); }); wrapper.update(); await waitForElement(wrapper, 'LoggingTestAlert'); - expect(SettingsAPI.test).toHaveBeenCalledTimes(1); + expect(SettingsAPI.createTest).toHaveBeenCalledTimes(1); await act(async () => { wrapper.find('AlertActionCloseButton button').invoke('onClick')(); }); diff --git a/awx/ui_next/src/screens/Setting/MiscSystem/MiscSystem.test.jsx b/awx/ui_next/src/screens/Setting/MiscSystem/MiscSystem.test.jsx index 5ccc70487b..59bb991a82 100644 --- a/awx/ui_next/src/screens/Setting/MiscSystem/MiscSystem.test.jsx +++ b/awx/ui_next/src/screens/Setting/MiscSystem/MiscSystem.test.jsx @@ -58,4 +58,27 @@ describe('', () => { }); expect(wrapper.find('ContentError').length).toBe(1); }); + + test('should redirect to details for users without system admin permissions', async () => { + const history = createMemoryHistory({ + initialEntries: ['/settings/miscellaneous_system/edit'], + }); + await act(async () => { + wrapper = mountWithContexts(, { + context: { + router: { + history, + }, + config: { + me: { + is_superuser: false, + }, + }, + }, + }); + }); + await waitForElement(wrapper, 'ContentLoading', el => el.length === 0); + expect(wrapper.find('MiscSystemDetail').length).toBe(1); + expect(wrapper.find('MiscSystemEdit').length).toBe(0); + }); }); diff --git a/awx/ui_next/src/screens/Setting/MiscSystem/MiscSystemEdit/MiscSystemEdit.jsx b/awx/ui_next/src/screens/Setting/MiscSystem/MiscSystemEdit/MiscSystemEdit.jsx index f4800d6a81..94c64f91bb 100644 --- a/awx/ui_next/src/screens/Setting/MiscSystem/MiscSystemEdit/MiscSystemEdit.jsx +++ b/awx/ui_next/src/screens/Setting/MiscSystem/MiscSystemEdit/MiscSystemEdit.jsx @@ -107,15 +107,14 @@ function MiscSystemEdit({ i18n }) { fetchSystem(); }, [fetchSystem]); - const { - error: submitError, - request: submitForm, - result: submitResult, - } = useRequest( - useCallback(async values => { - const result = await SettingsAPI.updateAll(values); - return result; - }, []), + const { error: submitError, request: submitForm } = useRequest( + useCallback( + async values => { + await SettingsAPI.updateAll(values); + history.push('/settings/miscellaneous_system/details'); + }, + [history] + ), null ); @@ -138,12 +137,6 @@ function MiscSystemEdit({ i18n }) { }); }; - useEffect(() => { - if (submitResult) { - history.push('/settings/miscellaneous_system/details'); - } - }, [submitResult, history]); - const handleRevertAll = async () => { const { ACCESS_TOKEN_EXPIRE_SECONDS, diff --git a/awx/ui_next/src/screens/Setting/Settings.test.jsx b/awx/ui_next/src/screens/Setting/Settings.test.jsx index faead37231..24a82986f8 100644 --- a/awx/ui_next/src/screens/Setting/Settings.test.jsx +++ b/awx/ui_next/src/screens/Setting/Settings.test.jsx @@ -46,7 +46,7 @@ describe('', () => { expect(wrapper.find('SettingList').length).toBe(0); }); - test('should render Settings for users with permissions system admin or auditor permissions', async () => { + test('should render Settings for users with system admin or auditor permissions', async () => { const history = createMemoryHistory({ initialEntries: ['/settings'], }); diff --git a/awx/ui_next/src/screens/Setting/shared/RevertAllAlert.jsx b/awx/ui_next/src/screens/Setting/shared/RevertAllAlert.jsx index 42855de751..55b23437b9 100644 --- a/awx/ui_next/src/screens/Setting/shared/RevertAllAlert.jsx +++ b/awx/ui_next/src/screens/Setting/shared/RevertAllAlert.jsx @@ -30,8 +30,8 @@ function RevertAllAlert({ i18n, onClose, onRevertAll }) { , ]} > - {i18n._(t`This will revert all configuration values to their - factory defaults. Are you sure you want to proceed?`)} + {i18n._(t`This will revert all configuration values on this page to + their factory defaults. Are you sure you want to proceed?`)} ); } diff --git a/awx/ui_next/src/screens/Setting/shared/RevertButton.test.jsx b/awx/ui_next/src/screens/Setting/shared/RevertButton.test.jsx index 0592082bdf..1af4e7f223 100644 --- a/awx/ui_next/src/screens/Setting/shared/RevertButton.test.jsx +++ b/awx/ui_next/src/screens/Setting/shared/RevertButton.test.jsx @@ -18,7 +18,7 @@ describe('RevertButton', () => { test_input: 'foo', }} > - {() => } + ); expect(wrapper.find('button').text()).toEqual('Revert'); @@ -34,7 +34,7 @@ describe('RevertButton', () => { test_input: 'bar', }} > - {() => } + ); expect(wrapper.find('button').text()).toEqual('Revert'); @@ -47,7 +47,7 @@ describe('RevertButton', () => { test_input: 'foo', }} > - {() => } + ); expect(wrapper.find('button').text()).toEqual('Revert'); @@ -68,7 +68,7 @@ describe('RevertButton', () => { test_input: 'bar', }} > - {() => } + ); expect(wrapper.find('button').text()).toEqual('Revert'); diff --git a/awx/ui_next/src/screens/Setting/shared/SharedFields.jsx b/awx/ui_next/src/screens/Setting/shared/SharedFields.jsx index 29c084550b..6922dfa624 100644 --- a/awx/ui_next/src/screens/Setting/shared/SharedFields.jsx +++ b/awx/ui_next/src/screens/Setting/shared/SharedFields.jsx @@ -15,7 +15,13 @@ import CodeMirrorInput from '../../../components/CodeMirrorInput'; import { PasswordInput } from '../../../components/FormField'; import { FormFullWidthLayout } from '../../../components/FormLayout'; import Popover from '../../../components/Popover'; -import { combine, required, url, minMaxValue } from '../../../util/validators'; +import { + combine, + required, + url, + integer, + minMaxValue, +} from '../../../util/validators'; import RevertButton from './RevertButton'; const FormGroup = styled(PFFormGroup)` @@ -176,9 +182,11 @@ const InputField = withI18n()( max_value = Number.MAX_SAFE_INTEGER, } = config; const validators = [ - isRequired ? required(null, i18n) : null, - type === 'url' ? url(i18n) : null, - type === 'number' ? minMaxValue(min_value, max_value, i18n) : null, + ...(isRequired ? [required(null, i18n)] : []), + ...(type === 'url' ? [url(i18n)] : []), + ...(type === 'number' + ? [integer(i18n), minMaxValue(min_value, max_value, i18n)] + : []), ]; const [field, meta] = useField({ name, validate: combine(validators) }); const isValid = !(meta.touched && meta.error); @@ -197,7 +205,6 @@ const InputField = withI18n()( id={name} isRequired={isRequired} placeholder={config.placeholder} - type={type} validated={isValid ? 'default' : 'error'} value={field.value} onBlur={field.onBlur} diff --git a/awx/ui_next/src/screens/Setting/shared/ShareFields.test.jsx b/awx/ui_next/src/screens/Setting/shared/SharedFields.test.jsx similarity index 100% rename from awx/ui_next/src/screens/Setting/shared/ShareFields.test.jsx rename to awx/ui_next/src/screens/Setting/shared/SharedFields.test.jsx diff --git a/awx/ui_next/src/util/useModal.js b/awx/ui_next/src/util/useModal.js index acc853c703..ffddd59ada 100644 --- a/awx/ui_next/src/util/useModal.js +++ b/awx/ui_next/src/util/useModal.js @@ -2,6 +2,7 @@ import { useState } from 'react'; /** * useModal hook provides a way to read and update modal visibility + * Param: boolean that sets initial modal state * Returns: { * isModalOpen: boolean that indicates if modal is open * toggleModal: function that toggles the modal open and close @@ -9,8 +10,8 @@ import { useState } from 'react'; * } */ -export default function useModal() { - const [isModalOpen, setIsModalOpen] = useState(false); +export default function useModal(isOpen = false) { + const [isModalOpen, setIsModalOpen] = useState(isOpen); function toggleModal() { setIsModalOpen(!isModalOpen); diff --git a/awx/ui_next/src/util/useModal.test.jsx b/awx/ui_next/src/util/useModal.test.jsx index 6d1c11faf6..414c5b8b2c 100644 --- a/awx/ui_next/src/util/useModal.test.jsx +++ b/awx/ui_next/src/util/useModal.test.jsx @@ -17,7 +17,7 @@ describe('useModal hook', () => { let isModalOpen; let toggleModal; - test('should return expected initial values', () => { + test('isModalOpen should return expected default value', () => { testHook(() => { ({ isModalOpen, toggleModal, closeModal } = useModal()); }); @@ -26,6 +26,15 @@ describe('useModal hook', () => { expect(closeModal).toBeInstanceOf(Function); }); + test('isModalOpen should return expected initialized value', () => { + testHook(() => { + ({ isModalOpen, toggleModal, closeModal } = useModal(true)); + }); + expect(isModalOpen).toEqual(true); + expect(toggleModal).toBeInstanceOf(Function); + expect(closeModal).toBeInstanceOf(Function); + }); + test('should return expected isModalOpen value after modal toggle', () => { testHook(() => { ({ isModalOpen, toggleModal, closeModal } = useModal()); diff --git a/awx/ui_next/src/util/validators.jsx b/awx/ui_next/src/util/validators.jsx index 499957f6e5..561f3051ca 100644 --- a/awx/ui_next/src/util/validators.jsx +++ b/awx/ui_next/src/util/validators.jsx @@ -69,7 +69,7 @@ export function noWhiteSpace(i18n) { export function integer(i18n) { return value => { const str = String(value); - if (/[^0-9]/.test(str)) { + if (!Number.isInteger(value) && /[^0-9]/.test(str)) { return i18n._(t`This field must be an integer`); } return undefined;