diff --git a/awx/ui_next/src/components/ErrorDetail/ErrorDetail.jsx b/awx/ui_next/src/components/ErrorDetail/ErrorDetail.jsx index 1ef3cabc99..f1c309e959 100644 --- a/awx/ui_next/src/components/ErrorDetail/ErrorDetail.jsx +++ b/awx/ui_next/src/components/ErrorDetail/ErrorDetail.jsx @@ -54,7 +54,7 @@ class ErrorDetail extends Component { const { response } = error; let message = ''; - if (response.data) { + if (response?.data) { message = typeof response.data === 'string' ? response.data @@ -64,8 +64,8 @@ class ErrorDetail extends Component { return ( - {response.config.method.toUpperCase()} {response.config.url}{' '} - {response.status} + {response?.config?.method.toUpperCase()} {response?.config?.url}{' '} + {response?.status} {message} diff --git a/awx/ui_next/src/screens/Template/Template.jsx b/awx/ui_next/src/screens/Template/Template.jsx index 6baa9eb041..fe997c5176 100644 --- a/awx/ui_next/src/screens/Template/Template.jsx +++ b/awx/ui_next/src/screens/Template/Template.jsx @@ -15,7 +15,7 @@ import { ResourceAccessList } from '@components/ResourceAccessList'; import JobTemplateDetail from './JobTemplateDetail'; import JobTemplateEdit from './JobTemplateEdit'; import { JobTemplatesAPI, OrganizationsAPI } from '@api'; -import SurveyList from './shared/SurveyList'; +import TemplateSurvey from './TemplateSurvey'; class Template extends Component { constructor(props) { @@ -246,10 +246,9 @@ class Template extends Component { )} {template && ( - } - /> + + + )} { + const { data } = await JobTemplatesAPI.readSurvey(template.id); + return data; + }, [template.id]) + ); + useEffect(() => { + fetchSurvey(); + }, [fetchSurvey]); + + const { request: updateSurvey, error: updateError } = useRequest( + useCallback( + async updatedSurvey => { + await JobTemplatesAPI.updateSurvey(template.id, updatedSurvey); + setSurvey(updatedSurvey); + }, + [template.id, setSurvey] + ) + ); + + const { request: deleteSurvey, error: deleteError } = useRequest( + useCallback(async () => { + await JobTemplatesAPI.destroySurvey(template.id); + setSurvey(null); + }, [template.id, setSurvey]) + ); + + const { request: toggleSurvey, error: toggleError } = useRequest( + useCallback(async () => { + await JobTemplatesAPI.update(template.id, { + survey_enabled: !surveyEnabled, + }); + setSurveyEnabled(!surveyEnabled); + }, [template.id, surveyEnabled]) + ); + + const { error, dismissError } = useDismissableError( + updateError || deleteError || toggleError + ); + + if (loadingError) { + return ; + } + return ( + <> + + + updateSurvey({ ...survey, spec })} + deleteSurvey={deleteSurvey} + /> + + + {error && ( + + {i18n._(t`Failed to update survey.`)} + + + )} + + ); +} + +export default withI18n()(TemplateSurvey); diff --git a/awx/ui_next/src/screens/Template/TemplateSurvey.test.jsx b/awx/ui_next/src/screens/Template/TemplateSurvey.test.jsx new file mode 100644 index 0000000000..e1846923ec --- /dev/null +++ b/awx/ui_next/src/screens/Template/TemplateSurvey.test.jsx @@ -0,0 +1,89 @@ +import React from 'react'; +import { act } from 'react-dom/test-utils'; +import { createMemoryHistory } from 'history'; +import { mountWithContexts } from '@testUtils/enzymeHelpers'; +import TemplateSurvey from './TemplateSurvey'; +import { JobTemplatesAPI } from '@api'; +import mockJobTemplateData from './shared/data.job_template.json'; + +jest.mock('@api/models/JobTemplates'); + +const surveyData = { + name: 'Survey', + description: 'description for survey', + spec: [ + { question_name: 'Foo', type: 'text', default: 'Bar', variable: 'foo' }, + ], +}; + +describe('', () => { + beforeEach(() => { + JobTemplatesAPI.readSurvey.mockResolvedValue({ + data: surveyData, + }); + }); + + test('should fetch survey from API', async () => { + const history = createMemoryHistory({ + initialEntries: ['/templates/job_template/1/survey'], + }); + let wrapper; + await act(async () => { + wrapper = mountWithContexts( + , + { + context: { router: { history } }, + } + ); + }); + wrapper.update(); + expect(JobTemplatesAPI.readSurvey).toBeCalledWith(7); + + expect(wrapper.find('SurveyList').prop('survey')).toEqual(surveyData); + }); + + test('should display error in retrieving survey', async () => { + JobTemplatesAPI.readSurvey.mockRejectedValue(new Error()); + let wrapper; + await act(async () => { + wrapper = mountWithContexts( + + ); + }); + + wrapper.update(); + + expect(wrapper.find('ContentError').length).toBe(1); + }); + + test('should update API with survey changes', async () => { + const history = createMemoryHistory({ + initialEntries: ['/templates/job_template/1/survey'], + }); + let wrapper; + await act(async () => { + wrapper = mountWithContexts( + , + { + context: { router: { history } }, + } + ); + }); + wrapper.update(); + + await act(async () => { + await wrapper.find('SurveyList').invoke('updateSurvey')([ + { question_name: 'Foo', type: 'text', default: 'One', variable: 'foo' }, + { question_name: 'Bar', type: 'text', default: 'Two', variable: 'bar' }, + ]); + }); + expect(JobTemplatesAPI.updateSurvey).toHaveBeenCalledWith(7, { + name: 'Survey', + description: 'description for survey', + spec: [ + { question_name: 'Foo', type: 'text', default: 'One', variable: 'foo' }, + { question_name: 'Bar', type: 'text', default: 'Two', variable: 'bar' }, + ], + }); + }); +}); diff --git a/awx/ui_next/src/screens/Template/shared/SurveyList.jsx b/awx/ui_next/src/screens/Template/shared/SurveyList.jsx index 3758dbdba9..e04f99d288 100644 --- a/awx/ui_next/src/screens/Template/shared/SurveyList.jsx +++ b/awx/ui_next/src/screens/Template/shared/SurveyList.jsx @@ -1,129 +1,75 @@ -import React, { useEffect, useCallback, useState } from 'react'; +import React, { useState } from 'react'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; - -import useRequest, { useDeleteItems } from '@util/useRequest'; -import { Button } from '@patternfly/react-core'; - -import ContentError from '@components/ContentError'; +import { DataList, Button } from '@patternfly/react-core'; import ContentLoading from '@components/ContentLoading'; -import ErrorDetail from '@components/ErrorDetail'; -import { JobTemplatesAPI } from '@api'; import ContentEmpty from '@components/ContentEmpty'; -import { getQSConfig } from '@util/qs'; import AlertModal from '@components/AlertModal'; import SurveyListItem from './SurveyListItem'; import SurveyToolbar from './SurveyToolbar'; -const QS_CONFIG = getQSConfig('survey', { - page: 1, -}); - -function SurveyList({ template, i18n }) { +function SurveyList({ + isLoading, + survey, + surveyEnabled, + toggleSurvey, + updateSurvey, + deleteSurvey, + i18n, +}) { + const questions = survey?.spec || []; const [selected, setSelected] = useState([]); - const [surveyEnabled, setSurveyEnabled] = useState(template.survey_enabled); - const [showToggleError, setShowToggleError] = useState(false); const [isDeleteModalOpen, setIsDeleteModalOpen] = useState(false); - const { - result: { questions, name, description }, - error: contentError, - isLoading, - request: fetchSurvey, - } = useRequest( - useCallback(async () => { - const { - data: { spec = [], description: surveyDescription, name: surveyName }, - } = await JobTemplatesAPI.readSurvey(template.id); - return { - questions: spec.map((s, index) => ({ ...s, id: index })), - description: surveyDescription, - name: surveyName, - }; - }, [template.id]), - { questions: [], name: '', description: '' } - ); - - useEffect(() => { - fetchSurvey(); - }, [fetchSurvey]); - const isAllSelected = selected.length === questions?.length && selected.length > 0; - const { - isLoading: isDeleteLoading, - deleteItems: deleteQuestions, - deletionError, - clearDeletionError, - } = useDeleteItems( - useCallback(async () => { - if (isAllSelected) { - return JobTemplatesAPI.destroySurvey(template.id); - } - const surveyQuestions = []; - questions.forEach(q => { - if (!selected.some(s => s.id === q.id)) { - surveyQuestions.push(q); - } - }); - return JobTemplatesAPI.updateSurvey(template.id, { - name, - description, - spec: surveyQuestions, - }); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [selected]), - { - qsConfig: QS_CONFIG, - fetchItems: fetchSurvey, - } - ); - const { - isToggleLoading, - error: toggleError, - request: toggleSurvey, - } = useRequest( - useCallback(async () => { - await JobTemplatesAPI.update(template.id, { - survey_enabled: !surveyEnabled, - }); - return setSurveyEnabled(!surveyEnabled); - }, [template, surveyEnabled]), - template.survey_enabled - ); - - useEffect(() => { - if (toggleError) { - setShowToggleError(true); - } - }, [toggleError]); - const handleSelectAll = isSelected => { setSelected(isSelected ? [...questions] : []); }; const handleSelect = item => { - if (selected.some(s => s.id === item.id)) { - setSelected(selected.filter(s => s.id !== item.id)); + if (selected.some(q => q.variable === item.variable)) { + setSelected(selected.filter(q => q.variable !== item.variable)); } else { setSelected(selected.concat(item)); } }; const handleDelete = async () => { - await deleteQuestions(); + if (isAllSelected) { + await deleteSurvey(); + } else { + await updateSurvey(questions.filter(q => !selected.includes(q))); + } setIsDeleteModalOpen(false); setSelected([]); }; - const canEdit = template.summary_fields.user_capabilities.edit; - const canDelete = template.summary_fields.user_capabilities.delete; + + const moveUp = question => { + const index = questions.indexOf(question); + if (index < 1) { + return; + } + const beginning = questions.slice(0, index - 1); + const swapWith = questions[index - 1]; + const end = questions.slice(index + 1); + updateSurvey([...beginning, question, swapWith, ...end]); + }; + const moveDown = question => { + const index = questions.indexOf(question); + if (index === -1 || index > questions.length - 1) { + return; + } + const beginning = questions.slice(0, index); + const swapWith = questions[index + 1]; + const end = questions.slice(index + 2); + updateSurvey([...beginning, swapWith, question, ...end]); + }; let content; - if (isLoading || isToggleLoading || isDeleteLoading) { + if (isLoading) { content = ; - } else if (contentError) { - content = ; } else if (!questions || questions?.length <= 0) { content = ( ); } else { - content = questions?.map((question, index) => ( - s.id === question.id)} - onSelect={() => handleSelect(question)} - /> - )); + content = ( + + {questions?.map((question, index) => ( + q.variable === question.variable)} + onSelect={() => handleSelect(question)} + onMoveUp={moveUp} + onMoveDown={moveDown} + /> + ))} + + ); } + return ( <> setIsDeleteModalOpen(true)} /> {content} @@ -165,7 +118,6 @@ function SurveyList({ template, i18n }) { isOpen={isDeleteModalOpen} onClose={() => { setIsDeleteModalOpen(false); - setSelected([]); }} actions={[ - - - - - - - - - {question.question_name} - , - {question.type}, - - {question.default} - , - ]} - /> - - - + + + + + + + + + + + + + + + {question.question_name} + , + {question.type}, + + {question.default} + , + ]} + /> + + ); } diff --git a/awx/ui_next/src/util/qs.js b/awx/ui_next/src/util/qs.js index c2d25024a9..b97f3ce9ac 100644 --- a/awx/ui_next/src/util/qs.js +++ b/awx/ui_next/src/util/qs.js @@ -34,7 +34,7 @@ export function getQSConfig( */ export function parseQueryString(config, queryString) { if (!queryString) { - return config.defaultParams; + return config.defaultParams || {}; } const params = stringToObject(config, queryString); return addDefaultsToObject(config, params); diff --git a/awx/ui_next/src/util/useRequest.js b/awx/ui_next/src/util/useRequest.js index 64a62ebf45..0e95be4a69 100644 --- a/awx/ui_next/src/util/useRequest.js +++ b/awx/ui_next/src/util/useRequest.js @@ -8,11 +8,12 @@ import { /* * The useRequest hook accepts a request function and returns an object with - * four values: + * five values: * request: a function to call to invoke the request * result: the value returned from the request function (once invoked) * isLoading: boolean state indicating whether the request is in active/in flight * error: any caught error resulting from the request + * setValue: setter to explicitly set the result value * * The hook also accepts an optional second parameter which is a default * value to set as result before the first time the request is made. @@ -34,34 +35,41 @@ export default function useRequest(makeRequest, initialValue) { result, error, isLoading, - request: useCallback(async () => { - setIsLoading(true); - try { - const response = await makeRequest(); - if (isMounted.current) { - setResult(response); + request: useCallback( + async (...args) => { + setIsLoading(true); + try { + const response = await makeRequest(...args); + if (isMounted.current) { + setResult(response); + } + } catch (err) { + if (isMounted.current) { + setError(err); + } + } finally { + if (isMounted.current) { + setIsLoading(false); + } } - } catch (err) { - if (isMounted.current) { - setError(err); - } - } finally { - if (isMounted.current) { - setIsLoading(false); - } - } - }, [makeRequest]), + }, + [makeRequest] + ), + setValue: setResult, }; } -export function useDeleteItems( - makeRequest, - { qsConfig, allItemsSelected, fetchItems } -) { - const location = useLocation(); - const history = useHistory(); +/* + * Provides controls for "dismissing" an error message + * + * Params: an error object + * Returns: { error, dismissError } + * The returned error object is the same object passed in via the paremeter, + * until the dismissError function is called, at which point the returned + * error will be set to null on the subsequent render. + */ +export function useDismissableError(error) { const [showError, setShowError] = useState(false); - const { error, isLoading, request } = useRequest(makeRequest, null); useEffect(() => { if (error) { @@ -69,13 +77,48 @@ export function useDeleteItems( } }, [error]); + return { + error: showError ? error : null, + dismissError: () => { + setShowError(false); + }, + }; +} + +/* + * Hook to assist with deletion of items from a paginated item list. The page + * url will be navigated back one page on a paginated list if needed to prevent + * the UI from re-loading an empty set and displaying a "No items found" + * message. + * + * Params: a callback function that will be invoked in order to delete items, + * and an object with structure { qsConfig, allItemsSelected, fetchItems } + * Returns: { isLoading, deleteItems, deletionError, clearDeletionError } + */ +export function useDeleteItems( + makeRequest, + { qsConfig = null, allItemsSelected = false, fetchItems = null } = {} +) { + const location = useLocation(); + const history = useHistory(); + const { error: requestError, isLoading, request } = useRequest( + makeRequest, + null + ); + const { error, dismissError } = useDismissableError(requestError); + const deleteItems = async () => { await request(); + if (!qsConfig) { + return; + } const params = parseQueryString(qsConfig, location.search); if (params.page > 1 && allItemsSelected) { const newParams = encodeNonDefaultQueryString( qsConfig, - replaceParams(params, { page: params.page - 1 }) + replaceParams(params, { + page: params.page - 1, + }) ); history.push(`${location.pathname}?${newParams}`); } else { @@ -86,7 +129,7 @@ export function useDeleteItems( return { isLoading, deleteItems, - deletionError: showError && error, - clearDeletionError: () => setShowError(false), + deletionError: error, + clearDeletionError: dismissError, }; } diff --git a/awx/ui_next/src/util/useRequest.test.jsx b/awx/ui_next/src/util/useRequest.test.jsx index 1cf7ed955b..1d8185229a 100644 --- a/awx/ui_next/src/util/useRequest.test.jsx +++ b/awx/ui_next/src/util/useRequest.test.jsx @@ -1,7 +1,8 @@ import React from 'react'; import { act } from 'react-dom/test-utils'; import { mount } from 'enzyme'; -import useRequest from './useRequest'; +import { mountWithContexts } from '@testUtils/enzymeHelpers'; +import useRequest, { useDeleteItems } from './useRequest'; function TestInner() { return
; @@ -10,100 +11,179 @@ function Test({ makeRequest, initialValue = {} }) { const request = useRequest(makeRequest, initialValue); return ; } +function DeleteTest({ makeRequest, args = {} }) { + const request = useDeleteItems(makeRequest, args); + return ; +} -describe('useRequest', () => { - test('should return initial value as result', async () => { - const makeRequest = jest.fn(); - makeRequest.mockResolvedValue({ data: 'foo' }); - const wrapper = mount( - - ); +describe('useRequest hooks', () => { + describe('useRequest', () => { + test('should return initial value as result', async () => { + const makeRequest = jest.fn(); + makeRequest.mockResolvedValue({ data: 'foo' }); + const wrapper = mount( + + ); - expect(wrapper.find('TestInner').prop('result')).toEqual({ initial: true }); + expect(wrapper.find('TestInner').prop('result')).toEqual({ + initial: true, + }); + }); + + test('should return result', async () => { + const makeRequest = jest.fn(); + makeRequest.mockResolvedValue({ data: 'foo' }); + const wrapper = mount(); + + await act(async () => { + wrapper.find('TestInner').invoke('request')(); + }); + wrapper.update(); + expect(wrapper.find('TestInner').prop('result')).toEqual({ data: 'foo' }); + }); + + test('should is isLoading flag', async () => { + const makeRequest = jest.fn(); + let resolve; + const promise = new Promise(r => { + resolve = r; + }); + makeRequest.mockReturnValue(promise); + const wrapper = mount(); + + await act(async () => { + wrapper.find('TestInner').invoke('request')(); + }); + wrapper.update(); + expect(wrapper.find('TestInner').prop('isLoading')).toEqual(true); + await act(async () => { + resolve({ data: 'foo' }); + }); + wrapper.update(); + expect(wrapper.find('TestInner').prop('isLoading')).toEqual(false); + expect(wrapper.find('TestInner').prop('result')).toEqual({ data: 'foo' }); + }); + + test('should invoke request function', async () => { + const makeRequest = jest.fn(); + makeRequest.mockResolvedValue({ data: 'foo' }); + const wrapper = mount(); + + expect(makeRequest).not.toHaveBeenCalled(); + await act(async () => { + wrapper.find('TestInner').invoke('request')(); + }); + wrapper.update(); + expect(makeRequest).toHaveBeenCalledTimes(1); + }); + + test('should return error thrown from request function', async () => { + const error = new Error('error'); + const makeRequest = () => { + throw error; + }; + const wrapper = mount(); + + await act(async () => { + wrapper.find('TestInner').invoke('request')(); + }); + wrapper.update(); + expect(wrapper.find('TestInner').prop('error')).toEqual(error); + }); + + test('should not update state after unmount', async () => { + const makeRequest = jest.fn(); + let resolve; + const promise = new Promise(r => { + resolve = r; + }); + makeRequest.mockReturnValue(promise); + const wrapper = mount(); + + expect(makeRequest).not.toHaveBeenCalled(); + await act(async () => { + wrapper.find('TestInner').invoke('request')(); + }); + wrapper.unmount(); + await act(async () => { + resolve({ data: 'foo' }); + }); + }); }); - test('should return result', async () => { - const makeRequest = jest.fn(); - makeRequest.mockResolvedValue({ data: 'foo' }); - const wrapper = mount(); + describe('useDeleteItems', () => { + test('should invoke delete function', async () => { + const makeRequest = jest.fn(); + makeRequest.mockResolvedValue({ data: 'foo' }); + const wrapper = mountWithContexts( + {}, + }} + /> + ); - await act(async () => { - wrapper.find('TestInner').invoke('request')(); + expect(makeRequest).not.toHaveBeenCalled(); + await act(async () => { + wrapper.find('TestInner').invoke('deleteItems')(); + }); + wrapper.update(); + expect(makeRequest).toHaveBeenCalledTimes(1); }); - wrapper.update(); - expect(wrapper.find('TestInner').prop('result')).toEqual({ data: 'foo' }); - }); - test('should is isLoading flag', async () => { - const makeRequest = jest.fn(); - let resolve; - const promise = new Promise(r => { - resolve = r; + test('should return error object thrown by function', async () => { + const error = new Error('error'); + const makeRequest = () => { + throw error; + }; + const wrapper = mountWithContexts( + {}, + }} + /> + ); + + await act(async () => { + wrapper.find('TestInner').invoke('deleteItems')(); + }); + wrapper.update(); + expect(wrapper.find('TestInner').prop('deletionError')).toEqual(error); }); - makeRequest.mockReturnValue(promise); - const wrapper = mount(); - await act(async () => { - wrapper.find('TestInner').invoke('request')(); - }); - wrapper.update(); - expect(wrapper.find('TestInner').prop('isLoading')).toEqual(true); - await act(async () => { - resolve({ data: 'foo' }); - }); - wrapper.update(); - expect(wrapper.find('TestInner').prop('isLoading')).toEqual(false); - expect(wrapper.find('TestInner').prop('result')).toEqual({ data: 'foo' }); - }); + test('should dismiss error', async () => { + const error = new Error('error'); + const makeRequest = () => { + throw error; + }; + const wrapper = mountWithContexts( + {}, + }} + /> + ); - test('should invoke request function', async () => { - const makeRequest = jest.fn(); - makeRequest.mockResolvedValue({ data: 'foo' }); - const wrapper = mount(); - - expect(makeRequest).not.toHaveBeenCalled(); - await act(async () => { - wrapper.find('TestInner').invoke('request')(); - }); - wrapper.update(); - expect(makeRequest).toHaveBeenCalledTimes(1); - }); - - test('should return error thrown from request function', async () => { - const error = new Error('error'); - const makeRequest = () => { - throw error; - }; - const wrapper = mount(); - - await act(async () => { - wrapper.find('TestInner').invoke('request')(); - }); - wrapper.update(); - expect(wrapper.find('TestInner').prop('error')).toEqual(error); - }); - - test('should not update state after unmount', async () => { - const makeRequest = jest.fn(); - let resolve; - const promise = new Promise(r => { - resolve = r; - }); - makeRequest.mockReturnValue(promise); - const wrapper = mount(); - - expect(makeRequest).not.toHaveBeenCalled(); - await act(async () => { - wrapper.find('TestInner').invoke('request')(); - }); - wrapper.unmount(); - await act(async () => { - resolve({ data: 'foo' }); + await act(async () => { + wrapper.find('TestInner').invoke('deleteItems')(); + }); + wrapper.update(); + await act(async () => { + wrapper.find('TestInner').invoke('clearDeletionError')(); + }); + wrapper.update(); + expect(wrapper.find('TestInner').prop('deletionError')).toEqual(null); }); }); });