From 64b1aa43b1586bc3d5d646f68860334747a77139 Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Mon, 9 Mar 2020 12:13:00 -0400 Subject: [PATCH 1/9] Adds SurveyList tool bar --- .../src/screens/Template/shared/SurveyList.jsx | 2 +- .../screens/Template/shared/SurveyList.test.jsx | 17 +++++++++++++++++ awx/ui_next/src/util/useRequest.js | 4 +++- 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/awx/ui_next/src/screens/Template/shared/SurveyList.jsx b/awx/ui_next/src/screens/Template/shared/SurveyList.jsx index 3758dbdba9..f205c2b0cd 100644 --- a/awx/ui_next/src/screens/Template/shared/SurveyList.jsx +++ b/awx/ui_next/src/screens/Template/shared/SurveyList.jsx @@ -36,7 +36,7 @@ function SurveyList({ template, i18n }) { data: { spec = [], description: surveyDescription, name: surveyName }, } = await JobTemplatesAPI.readSurvey(template.id); return { - questions: spec.map((s, index) => ({ ...s, id: index })), + questions: spec?.map((s, index) => ({ ...s, id: index })), description: surveyDescription, name: surveyName, }; diff --git a/awx/ui_next/src/screens/Template/shared/SurveyList.test.jsx b/awx/ui_next/src/screens/Template/shared/SurveyList.test.jsx index c044eb9da4..bfd74023b3 100644 --- a/awx/ui_next/src/screens/Template/shared/SurveyList.test.jsx +++ b/awx/ui_next/src/screens/Template/shared/SurveyList.test.jsx @@ -36,6 +36,7 @@ describe('', () => { expect(JobTemplatesAPI.readSurvey).toBeCalledWith(7); wrapper.update(); + expect(wrapper.find('SurveyListItem').length).toBe(1); }); test('error in retrieving the survey throws an error', async () => { @@ -55,7 +56,11 @@ describe('', () => { JobTemplatesAPI.update.mockResolvedValue(); let wrapper; await act(async () => { +<<<<<<< HEAD wrapper = mountWithContexts( +======= + wrapper = await mountWithContexts( +>>>>>>> Adds SurveyList tool bar @@ -65,7 +70,11 @@ describe('', () => { expect(wrapper.find('Switch').length).toBe(1); expect(wrapper.find('Switch').prop('isChecked')).toBe(false); await act(async () => { +<<<<<<< HEAD wrapper.find('Switch').invoke('onChange')(true); +======= + await wrapper.find('Switch').invoke('onChange')(true); +>>>>>>> Adds SurveyList tool bar }); wrapper.update(); @@ -79,7 +88,11 @@ describe('', () => { test('selectAll enables delete button and calls the api to delete properly', async () => { let wrapper; await act(async () => { +<<<<<<< HEAD wrapper = mountWithContexts( +======= + wrapper = await mountWithContexts( +>>>>>>> Adds SurveyList tool bar @@ -126,7 +139,11 @@ describe('Survey with no questions', () => { JobTemplatesAPI.readSurvey.mockResolvedValue({}); let wrapper; await act(async () => { +<<<<<<< HEAD wrapper = mountWithContexts( +======= + wrapper = await mountWithContexts( +>>>>>>> Adds SurveyList tool bar ); }); diff --git a/awx/ui_next/src/util/useRequest.js b/awx/ui_next/src/util/useRequest.js index 64a62ebf45..ad8ed747ba 100644 --- a/awx/ui_next/src/util/useRequest.js +++ b/awx/ui_next/src/util/useRequest.js @@ -75,7 +75,9 @@ export function useDeleteItems( 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 { From 286cec8bc34ba682752c7c610e7d6ef719c4c13c Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Mon, 9 Mar 2020 16:04:01 -0400 Subject: [PATCH 2/9] WIP --- .../screens/Template/shared/SurveyList.jsx | 146 +++++++++++------- .../Template/shared/SurveyListItem.jsx | 4 + awx/ui_next/src/util/useRequest.js | 5 +- 3 files changed, 95 insertions(+), 60 deletions(-) diff --git a/awx/ui_next/src/screens/Template/shared/SurveyList.jsx b/awx/ui_next/src/screens/Template/shared/SurveyList.jsx index f205c2b0cd..abc5a2e59e 100644 --- a/awx/ui_next/src/screens/Template/shared/SurveyList.jsx +++ b/awx/ui_next/src/screens/Template/shared/SurveyList.jsx @@ -10,23 +10,20 @@ 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 }) { const [selected, setSelected] = useState([]); + // const [updated, setUpdated] = useState(null); const [surveyEnabled, setSurveyEnabled] = useState(template.survey_enabled); - const [showToggleError, setShowToggleError] = useState(false); + const [showError, setShowError] = useState(false); const [isDeleteModalOpen, setIsDeleteModalOpen] = useState(false); + const [questions, setQuestions] = useState(null); const { - result: { questions, name, description }, + result: { fetchedQuestions, name, description }, error: contentError, isLoading, request: fetchSurvey, @@ -35,8 +32,9 @@ function SurveyList({ template, i18n }) { const { data: { spec = [], description: surveyDescription, name: surveyName }, } = await JobTemplatesAPI.readSurvey(template.id); + return { - questions: spec?.map((s, index) => ({ ...s, id: index })), + fetchedQuestions: spec?.map((s, index) => ({ ...s, id: index })), description: surveyDescription, name: surveyName, }; @@ -44,41 +42,27 @@ function SurveyList({ template, i18n }) { { questions: [], name: '', description: '' } ); + useEffect(() => { + setQuestions(fetchedQuestions); + }, [fetchedQuestions]); + useEffect(() => { fetchSurvey(); }, [fetchSurvey]); const isAllSelected = selected.length === questions?.length && selected.length > 0; - const { isLoading: isDeleteLoading, - deleteItems: deleteQuestions, + deleteItems: deleteSurvey, 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, - } + await JobTemplatesAPI.destroySurvey(template.id); + fetchSurvey(); + }, [template.id, fetchSurvey]) ); + const { isToggleLoading, error: toggleError, @@ -93,12 +77,6 @@ function SurveyList({ template, i18n }) { template.survey_enabled ); - useEffect(() => { - if (toggleError) { - setShowToggleError(true); - } - }, [toggleError]); - const handleSelectAll = isSelected => { setSelected(isSelected ? [...questions] : []); }; @@ -112,15 +90,56 @@ function SurveyList({ template, i18n }) { }; const handleDelete = async () => { - await deleteQuestions(); - setIsDeleteModalOpen(false); + if (isAllSelected) { + await deleteSurvey(); + setIsDeleteModalOpen(false); + } else { + setQuestions(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 { + isLoading: isUpdateLoading, + error: updateError, + // request: updateSurvey, + } = useRequest( + useCallback(async () => { + return JobTemplatesAPI.updateSurvey(template.id, { + name, + description, + spec: questions, + }); + }, [template.id, name, description, questions]) + ); + + 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); + setQuestions([...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); + setQuestions([...beginning, swapWith, question, ...end]); + }; let content; - if (isLoading || isToggleLoading || isDeleteLoading) { + if ( + (isLoading || isToggleLoading || isDeleteLoading || isUpdateLoading) && + questions?.length <= 0 + ) { content = ; } else if (contentError) { content = ; @@ -140,9 +159,29 @@ function SurveyList({ template, i18n }) { question={question} isChecked={selected.some(s => s.id === question.id)} onSelect={() => handleSelect(question)} + onMoveUp={moveUp} + onMoveDown={moveDown} /> )); } + + const error = deletionError || toggleError || updateError; + let errorMessage = ''; + if (updateError) { + errorMessage = i18n._(t`Failed to update survey`); + } + if (toggleError) { + errorMessage = i18n._(t`Failed to toggle survey`); + } + if (deletionError) { + errorMessage = i18n._(t`Failed to delete survey`); + } + useEffect(() => { + if (error) { + setShowError(true); + } + }, [error]); + return ( <> setIsDeleteModalOpen(true)} /> {content} @@ -198,26 +237,15 @@ function SurveyList({ template, i18n }) { ))} )} - {deletionError && ( + {showError && ( setShowError(false)} > - {i18n._(t`Failed to delete one or more jobs.`)} - - - )} - {toggleError && ( - setShowToggleError(false)} - > - {i18n._(t`Failed to toggle host.`)} - + {errorMessage} + )} diff --git a/awx/ui_next/src/screens/Template/shared/SurveyListItem.jsx b/awx/ui_next/src/screens/Template/shared/SurveyListItem.jsx index ca99cf17f7..9181230419 100644 --- a/awx/ui_next/src/screens/Template/shared/SurveyListItem.jsx +++ b/awx/ui_next/src/screens/Template/shared/SurveyListItem.jsx @@ -35,6 +35,8 @@ function SurveyListItem({ isFirst, isChecked, onSelect, + onMoveUp, + onMoveDown, }) { return ( @@ -51,6 +53,7 @@ function SurveyListItem({ variant="plain" aria-label={i18n._(t`move up`)} isDisabled={isFirst} + onClick={() => onMoveUp(question)} > @@ -60,6 +63,7 @@ function SurveyListItem({ variant="plain" aria-label={i18n._(t`move down`)} isDisabled={isLast} + onClick={() => onMoveDown(question)} > diff --git a/awx/ui_next/src/util/useRequest.js b/awx/ui_next/src/util/useRequest.js index ad8ed747ba..906f3160be 100644 --- a/awx/ui_next/src/util/useRequest.js +++ b/awx/ui_next/src/util/useRequest.js @@ -56,7 +56,7 @@ export default function useRequest(makeRequest, initialValue) { export function useDeleteItems( makeRequest, - { qsConfig, allItemsSelected, fetchItems } + { qsConfig = null, allItemsSelected = false, fetchItems = null } = {} ) { const location = useLocation(); const history = useHistory(); @@ -71,6 +71,9 @@ export function useDeleteItems( const deleteItems = async () => { await request(); + if (!qsConfig) { + return; + } const params = parseQueryString(qsConfig, location.search); if (params.page > 1 && allItemsSelected) { const newParams = encodeNonDefaultQueryString( From 2584f7359ebdf201628f65ffd727ec0e3e57ce66 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Wed, 11 Mar 2020 09:32:36 -0700 Subject: [PATCH 3/9] restructure TemplateSurvey & list components --- awx/ui_next/src/screens/Template/Template.jsx | 10 +- .../src/screens/Template/TemplateSurvey.jsx | 56 ++++ .../screens/Template/shared/SurveyList.jsx | 141 +++------- .../Template/shared/SurveyList.orig.jsx | 255 ++++++++++++++++++ .../screens/Template/shared/surveyReducer.js | 15 ++ 5 files changed, 370 insertions(+), 107 deletions(-) create mode 100644 awx/ui_next/src/screens/Template/TemplateSurvey.jsx create mode 100644 awx/ui_next/src/screens/Template/shared/SurveyList.orig.jsx create mode 100644 awx/ui_next/src/screens/Template/shared/surveyReducer.js diff --git a/awx/ui_next/src/screens/Template/Template.jsx b/awx/ui_next/src/screens/Template/Template.jsx index 6baa9eb041..71cab03f77 100644 --- a/awx/ui_next/src/screens/Template/Template.jsx +++ b/awx/ui_next/src/screens/Template/Template.jsx @@ -15,7 +15,8 @@ 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'; +// import SurveyList from './shared/SurveyList'; class Template extends Component { constructor(props) { @@ -246,10 +247,9 @@ class Template extends Component { )} {template && ( - } - /> + + + )} { + (async () => { + const { data } = await JobTemplatesAPI.readSurvey(template.id); + setSurvey(data); + })(); + }, [template.id]); + + const updateSurvey = async newQuestions => { + await JobTemplatesAPI.updateSurvey(template.id, { + ...survey, + spec: newQuestions, + }); + setSurvey({ + ...survey, + spec: newQuestions, + }); + }; + + const deleteSurvey = async () => { + await JobTemplatesAPI.destroySurvey(template.id); + setSurvey(null); + }; + + const toggleSurvey = async () => { + await JobTemplatesAPI.update(template.id, { + survey_enabled: !surveyEnabled, + }); + setSurveyEnabled(!surveyEnabled); + }; + + // TODO + // if (contentError) { + // return ; + return ( + + + + + + ); +} diff --git a/awx/ui_next/src/screens/Template/shared/SurveyList.jsx b/awx/ui_next/src/screens/Template/shared/SurveyList.jsx index abc5a2e59e..d13ad3a2b7 100644 --- a/awx/ui_next/src/screens/Template/shared/SurveyList.jsx +++ b/awx/ui_next/src/screens/Template/shared/SurveyList.jsx @@ -2,7 +2,7 @@ import React, { useEffect, useCallback, useState } from 'react'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; -import useRequest, { useDeleteItems } from '@util/useRequest'; +import useRequest from '@util/useRequest'; import { Button } from '@patternfly/react-core'; import ContentError from '@components/ContentError'; @@ -14,68 +14,24 @@ import AlertModal from '@components/AlertModal'; import SurveyListItem from './SurveyListItem'; import SurveyToolbar from './SurveyToolbar'; -function SurveyList({ template, i18n }) { +// survey.name +// survey.description +// survey.spec +function SurveyList({ + survey, + surveyEnabled, + toggleSurvey, + updateSurvey, + deleteSurvey, + i18n, +}) { + const questions = survey?.spec || []; const [selected, setSelected] = useState([]); - // const [updated, setUpdated] = useState(null); - const [surveyEnabled, setSurveyEnabled] = useState(template.survey_enabled); - const [showError, setShowError] = useState(false); + // const [showError, setShowError] = useState(false); const [isDeleteModalOpen, setIsDeleteModalOpen] = useState(false); - const [questions, setQuestions] = useState(null); - - const { - result: { fetchedQuestions, name, description }, - error: contentError, - isLoading, - request: fetchSurvey, - } = useRequest( - useCallback(async () => { - const { - data: { spec = [], description: surveyDescription, name: surveyName }, - } = await JobTemplatesAPI.readSurvey(template.id); - - return { - fetchedQuestions: spec?.map((s, index) => ({ ...s, id: index })), - description: surveyDescription, - name: surveyName, - }; - }, [template.id]), - { questions: [], name: '', description: '' } - ); - - useEffect(() => { - setQuestions(fetchedQuestions); - }, [fetchedQuestions]); - - useEffect(() => { - fetchSurvey(); - }, [fetchSurvey]); const isAllSelected = selected.length === questions?.length && selected.length > 0; - const { - isLoading: isDeleteLoading, - deleteItems: deleteSurvey, - deletionError, - } = useDeleteItems( - useCallback(async () => { - await JobTemplatesAPI.destroySurvey(template.id); - fetchSurvey(); - }, [template.id, 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 - ); const handleSelectAll = isSelected => { setSelected(isSelected ? [...questions] : []); @@ -92,28 +48,13 @@ function SurveyList({ template, i18n }) { const handleDelete = async () => { if (isAllSelected) { await deleteSurvey(); - setIsDeleteModalOpen(false); } else { - setQuestions(questions.filter(q => !selected.includes(q))); - setIsDeleteModalOpen(false); + await updateSurvey(questions.filter(q => !selected.includes(q))); } + setIsDeleteModalOpen(false); setSelected([]); }; - const { - isLoading: isUpdateLoading, - error: updateError, - // request: updateSurvey, - } = useRequest( - useCallback(async () => { - return JobTemplatesAPI.updateSurvey(template.id, { - name, - description, - spec: questions, - }); - }, [template.id, name, description, questions]) - ); - const moveUp = question => { const index = questions.indexOf(question); if (index < 1) { @@ -122,7 +63,7 @@ function SurveyList({ template, i18n }) { const beginning = questions.slice(0, index - 1); const swapWith = questions[index - 1]; const end = questions.slice(index + 1); - setQuestions([...beginning, question, swapWith, ...end]); + updateSurvey([...beginning, question, swapWith, ...end]); }; const moveDown = question => { const index = questions.indexOf(question); @@ -132,17 +73,13 @@ function SurveyList({ template, i18n }) { const beginning = questions.slice(0, index); const swapWith = questions[index + 1]; const end = questions.slice(index + 2); - setQuestions([...beginning, swapWith, question, ...end]); + updateSurvey([...beginning, swapWith, question, ...end]); }; let content; - if ( - (isLoading || isToggleLoading || isDeleteLoading || isUpdateLoading) && - questions?.length <= 0 - ) { + // TODO + if (false) { content = ; - } else if (contentError) { - content = ; } else if (!questions || questions?.length <= 0) { content = ( ( { - if (error) { - setShowError(true); - } - }, [error]); + // const error = deletionError || toggleError || updateError; + // let errorMessage = ''; + // if (updateError) { + // errorMessage = i18n._(t`Failed to update survey`); + // } + // if (toggleError) { + // errorMessage = i18n._(t`Failed to toggle survey`); + // } + // if (deletionError) { + // errorMessage = i18n._(t`Failed to delete survey`); + // } + // useEffect(() => { + // if (error) { + // setShowError(true); + // } + // }, [error]); return ( <> @@ -237,7 +174,7 @@ function SurveyList({ template, i18n }) { ))} )} - {showError && ( + {/* {showError && ( - )} + )} */} ); } diff --git a/awx/ui_next/src/screens/Template/shared/SurveyList.orig.jsx b/awx/ui_next/src/screens/Template/shared/SurveyList.orig.jsx new file mode 100644 index 0000000000..abc5a2e59e --- /dev/null +++ b/awx/ui_next/src/screens/Template/shared/SurveyList.orig.jsx @@ -0,0 +1,255 @@ +import React, { useEffect, useCallback, 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 ContentLoading from '@components/ContentLoading'; +import ErrorDetail from '@components/ErrorDetail'; +import { JobTemplatesAPI } from '@api'; +import ContentEmpty from '@components/ContentEmpty'; +import AlertModal from '@components/AlertModal'; +import SurveyListItem from './SurveyListItem'; +import SurveyToolbar from './SurveyToolbar'; + +function SurveyList({ template, i18n }) { + const [selected, setSelected] = useState([]); + // const [updated, setUpdated] = useState(null); + const [surveyEnabled, setSurveyEnabled] = useState(template.survey_enabled); + const [showError, setShowError] = useState(false); + const [isDeleteModalOpen, setIsDeleteModalOpen] = useState(false); + const [questions, setQuestions] = useState(null); + + const { + result: { fetchedQuestions, name, description }, + error: contentError, + isLoading, + request: fetchSurvey, + } = useRequest( + useCallback(async () => { + const { + data: { spec = [], description: surveyDescription, name: surveyName }, + } = await JobTemplatesAPI.readSurvey(template.id); + + return { + fetchedQuestions: spec?.map((s, index) => ({ ...s, id: index })), + description: surveyDescription, + name: surveyName, + }; + }, [template.id]), + { questions: [], name: '', description: '' } + ); + + useEffect(() => { + setQuestions(fetchedQuestions); + }, [fetchedQuestions]); + + useEffect(() => { + fetchSurvey(); + }, [fetchSurvey]); + + const isAllSelected = + selected.length === questions?.length && selected.length > 0; + const { + isLoading: isDeleteLoading, + deleteItems: deleteSurvey, + deletionError, + } = useDeleteItems( + useCallback(async () => { + await JobTemplatesAPI.destroySurvey(template.id); + fetchSurvey(); + }, [template.id, 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 + ); + + 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)); + } else { + setSelected(selected.concat(item)); + } + }; + + const handleDelete = async () => { + if (isAllSelected) { + await deleteSurvey(); + setIsDeleteModalOpen(false); + } else { + setQuestions(questions.filter(q => !selected.includes(q))); + setIsDeleteModalOpen(false); + } + setSelected([]); + }; + + const { + isLoading: isUpdateLoading, + error: updateError, + // request: updateSurvey, + } = useRequest( + useCallback(async () => { + return JobTemplatesAPI.updateSurvey(template.id, { + name, + description, + spec: questions, + }); + }, [template.id, name, description, questions]) + ); + + 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); + setQuestions([...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); + setQuestions([...beginning, swapWith, question, ...end]); + }; + + let content; + if ( + (isLoading || isToggleLoading || isDeleteLoading || isUpdateLoading) && + questions?.length <= 0 + ) { + 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)} + onMoveUp={moveUp} + onMoveDown={moveDown} + /> + )); + } + + const error = deletionError || toggleError || updateError; + let errorMessage = ''; + if (updateError) { + errorMessage = i18n._(t`Failed to update survey`); + } + if (toggleError) { + errorMessage = i18n._(t`Failed to toggle survey`); + } + if (deletionError) { + errorMessage = i18n._(t`Failed to delete survey`); + } + useEffect(() => { + if (error) { + setShowError(true); + } + }, [error]); + + return ( + <> + setIsDeleteModalOpen(true)} + /> + {content} + {isDeleteModalOpen && ( + { + setIsDeleteModalOpen(false); + setSelected([]); + }} + actions={[ + , + , + ]} + > +
{i18n._(t`This action will delete the following:`)}
+ {selected.map(question => ( + + {question.question_name} +
+ + ))} + + )} + {showError && ( + setShowError(false)} + > + {errorMessage} + + + )} + + ); +} + +export default withI18n()(SurveyList); diff --git a/awx/ui_next/src/screens/Template/shared/surveyReducer.js b/awx/ui_next/src/screens/Template/shared/surveyReducer.js new file mode 100644 index 0000000000..418792a1e4 --- /dev/null +++ b/awx/ui_next/src/screens/Template/shared/surveyReducer.js @@ -0,0 +1,15 @@ +export default function surveyReducer(state, action) { + switch (action.type) { + case 'THING': + return state; + default: + throw new Error(`Unrecognized action type: ${action.type}`); + } +} + +// move up/down -> Update +// delete -> Update +// delete all -> destroySurvey +// toggle -> Update survey_enabled +// select +// select all From 07565b5efcc4d5d51bbb562b3c4b268a62e9c91b Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Wed, 11 Mar 2020 12:16:00 -0700 Subject: [PATCH 4/9] add error handling to TemplateSurvey --- .../components/ErrorDetail/ErrorDetail.jsx | 6 +- .../src/screens/Template/TemplateSurvey.jsx | 132 ++++++++++++------ .../screens/Template/shared/SurveyList.jsx | 8 +- awx/ui_next/src/util/useRequest.js | 69 +++++---- 4 files changed, 136 insertions(+), 79 deletions(-) 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/TemplateSurvey.jsx b/awx/ui_next/src/screens/Template/TemplateSurvey.jsx index 9c24b29439..bb19f04df4 100644 --- a/awx/ui_next/src/screens/Template/TemplateSurvey.jsx +++ b/awx/ui_next/src/screens/Template/TemplateSurvey.jsx @@ -1,56 +1,104 @@ -import React, { useState, useEffect } from 'react'; +import React, { useState, useEffect, useCallback } from 'react'; import { Switch, Route } from 'react-router-dom'; +import { withI18n } from '@lingui/react'; +import { t } from '@lingui/macro'; import { JobTemplatesAPI } from '@api'; +import ContentError from '@components/ContentError'; +import AlertModal from '@components/AlertModal'; +import ErrorDetail from '@components/ErrorDetail'; +import useRequest, { useDismissableError } from '@util/useRequest'; import SurveyList from './shared/SurveyList'; -export default function TemplateSurvey({ template }) { - const [survey, setSurvey] = useState(null); +function TemplateSurvey({ template, i18n }) { const [surveyEnabled, setSurveyEnabled] = useState(template.survey_enabled); - useEffect(() => { - (async () => { + const { + result: survey, + request: fetchSurvey, + isLoading, + error: loadingError, + setValue: setSurvey, + } = useRequest( + useCallback(async () => { const { data } = await JobTemplatesAPI.readSurvey(template.id); - setSurvey(data); - })(); - }, [template.id]); + return data; + }, [template.id]) + ); + useEffect(() => { + fetchSurvey(); + }, [fetchSurvey]); - const updateSurvey = async newQuestions => { - await JobTemplatesAPI.updateSurvey(template.id, { - ...survey, - spec: newQuestions, - }); - setSurvey({ - ...survey, - spec: newQuestions, - }); - }; + const { + request: updateSurvey, + isLoading: isUpdateLoading, + error: updateError, + } = useRequest( + useCallback( + async updatedSurvey => { + await JobTemplatesAPI.updateSurvey(template.id, updatedSurvey); + setSurvey(updatedSurvey); + }, + [template.id, setSurvey] + ) + ); - const deleteSurvey = async () => { - await JobTemplatesAPI.destroySurvey(template.id); - setSurvey(null); - }; + const { + request: deleteSurvey, + isLoading: isDeleteLoading, + error: deleteError, + } = useRequest( + useCallback(async () => { + await JobTemplatesAPI.destroySurvey(template.id); + setSurvey(null); + }, [template.id, setSurvey]) + ); - const toggleSurvey = async () => { - await JobTemplatesAPI.update(template.id, { - survey_enabled: !surveyEnabled, - }); - setSurveyEnabled(!surveyEnabled); - }; + const { + request: toggleSurvey, + isLoading: isToggleLoading, + error: toggleError, + } = useRequest( + useCallback(async () => { + await JobTemplatesAPI.update(template.id, { + survey_enabled: !surveyEnabled, + }); + setSurveyEnabled(!surveyEnabled); + }, [template.id, surveyEnabled]) + ); - // TODO - // if (contentError) { - // return ; + 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/shared/SurveyList.jsx b/awx/ui_next/src/screens/Template/shared/SurveyList.jsx index d13ad3a2b7..f608d4caae 100644 --- a/awx/ui_next/src/screens/Template/shared/SurveyList.jsx +++ b/awx/ui_next/src/screens/Template/shared/SurveyList.jsx @@ -1,22 +1,17 @@ -import React, { useEffect, useCallback, useState } from 'react'; +import React, { useState } from 'react'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; -import useRequest from '@util/useRequest'; import { Button } from '@patternfly/react-core'; import ContentError from '@components/ContentError'; import ContentLoading from '@components/ContentLoading'; import ErrorDetail from '@components/ErrorDetail'; -import { JobTemplatesAPI } from '@api'; import ContentEmpty from '@components/ContentEmpty'; import AlertModal from '@components/AlertModal'; import SurveyListItem from './SurveyListItem'; import SurveyToolbar from './SurveyToolbar'; -// survey.name -// survey.description -// survey.spec function SurveyList({ survey, surveyEnabled, @@ -27,7 +22,6 @@ function SurveyList({ }) { const questions = survey?.spec || []; const [selected, setSelected] = useState([]); - // const [showError, setShowError] = useState(false); const [isDeleteModalOpen, setIsDeleteModalOpen] = useState(false); const isAllSelected = diff --git a/awx/ui_next/src/util/useRequest.js b/awx/ui_next/src/util/useRequest.js index 906f3160be..9f0ea0c91f 100644 --- a/awx/ui_next/src/util/useRequest.js +++ b/awx/ui_next/src/util/useRequest.js @@ -8,11 +8,13 @@ import { /* * The useRequest hook accepts a request function and returns an object with - * four values: + * six 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 + * isInitialized: set to true once the result is initially fetched * * 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,23 +36,42 @@ 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 useDismissableError(error) { + const [showError, setShowError] = useState(false); + + useEffect(() => { + if (error) { + setShowError(true); + } + }, [error]); + + return { + error: showError && error, + dismissError: () => setShowError(false), }; } @@ -60,14 +81,8 @@ export function useDeleteItems( ) { const location = useLocation(); const history = useHistory(); - const [showError, setShowError] = useState(false); - const { error, isLoading, request } = useRequest(makeRequest, null); - - useEffect(() => { - if (error) { - setShowError(true); - } - }, [error]); + const { requestError, isLoading, request } = useRequest(makeRequest, null); + const { error, dismissError } = useDismissableError(requestError); const deleteItems = async () => { await request(); @@ -91,7 +106,7 @@ export function useDeleteItems( return { isLoading, deleteItems, - deletionError: showError && error, - clearDeletionError: () => setShowError(false), + deletionError: error, + clearDeletionError: dismissError, }; } From eea80c45d126deb471c9ba2899bb083f9b9cadaa Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Wed, 11 Mar 2020 15:44:02 -0700 Subject: [PATCH 5/9] fix keys, clean up surveylist --- awx/ui_next/src/screens/Template/Template.jsx | 1 - .../src/screens/Template/TemplateSurvey.jsx | 19 +- .../screens/Template/shared/SurveyList.jsx | 85 ++---- .../Template/shared/SurveyList.orig.jsx | 255 ------------------ .../Template/shared/SurveyListItem.jsx | 101 ++++--- 5 files changed, 81 insertions(+), 380 deletions(-) delete mode 100644 awx/ui_next/src/screens/Template/shared/SurveyList.orig.jsx diff --git a/awx/ui_next/src/screens/Template/Template.jsx b/awx/ui_next/src/screens/Template/Template.jsx index 71cab03f77..fe997c5176 100644 --- a/awx/ui_next/src/screens/Template/Template.jsx +++ b/awx/ui_next/src/screens/Template/Template.jsx @@ -16,7 +16,6 @@ import JobTemplateDetail from './JobTemplateDetail'; import JobTemplateEdit from './JobTemplateEdit'; import { JobTemplatesAPI, OrganizationsAPI } from '@api'; import TemplateSurvey from './TemplateSurvey'; -// import SurveyList from './shared/SurveyList'; class Template extends Component { constructor(props) { diff --git a/awx/ui_next/src/screens/Template/TemplateSurvey.jsx b/awx/ui_next/src/screens/Template/TemplateSurvey.jsx index bb19f04df4..5c3a3657a6 100644 --- a/awx/ui_next/src/screens/Template/TemplateSurvey.jsx +++ b/awx/ui_next/src/screens/Template/TemplateSurvey.jsx @@ -28,11 +28,7 @@ function TemplateSurvey({ template, i18n }) { fetchSurvey(); }, [fetchSurvey]); - const { - request: updateSurvey, - isLoading: isUpdateLoading, - error: updateError, - } = useRequest( + const { request: updateSurvey, error: updateError } = useRequest( useCallback( async updatedSurvey => { await JobTemplatesAPI.updateSurvey(template.id, updatedSurvey); @@ -42,22 +38,14 @@ function TemplateSurvey({ template, i18n }) { ) ); - const { - request: deleteSurvey, - isLoading: isDeleteLoading, - error: deleteError, - } = useRequest( + const { request: deleteSurvey, error: deleteError } = useRequest( useCallback(async () => { await JobTemplatesAPI.destroySurvey(template.id); setSurvey(null); }, [template.id, setSurvey]) ); - const { - request: toggleSurvey, - isLoading: isToggleLoading, - error: toggleError, - } = useRequest( + const { request: toggleSurvey, error: toggleError } = useRequest( useCallback(async () => { await JobTemplatesAPI.update(template.id, { survey_enabled: !surveyEnabled, @@ -78,6 +66,7 @@ function TemplateSurvey({ template, i18n }) { { - 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)); } @@ -71,8 +68,7 @@ function SurveyList({ }; let content; - // TODO - if (false) { + if (isLoading) { content = ; } else if (!questions || questions?.length <= 0) { content = ( @@ -82,37 +78,24 @@ function SurveyList({ /> ); } else { - content = questions?.map((question, index) => ( - s.id === question.id)} - onSelect={() => handleSelect(question)} - onMoveUp={moveUp} - onMoveDown={moveDown} - /> - )); + content = ( + + {questions?.map((question, index) => ( + q.variable === question.variable)} + onSelect={() => handleSelect(question)} + onMoveUp={moveUp} + onMoveDown={moveDown} + /> + ))} + + ); } - // const error = deletionError || toggleError || updateError; - // let errorMessage = ''; - // if (updateError) { - // errorMessage = i18n._(t`Failed to update survey`); - // } - // if (toggleError) { - // errorMessage = i18n._(t`Failed to toggle survey`); - // } - // if (deletionError) { - // errorMessage = i18n._(t`Failed to delete survey`); - // } - // useEffect(() => { - // if (error) { - // setShowError(true); - // } - // }, [error]); - return ( <> { setIsDeleteModalOpen(false); - setSelected([]); }} actions={[ , - , - ]} - > -
{i18n._(t`This action will delete the following:`)}
- {selected.map(question => ( - - {question.question_name} -
- - ))} - - )} - {showError && ( - setShowError(false)} - > - {errorMessage} - - - )} - - ); -} - -export default withI18n()(SurveyList); diff --git a/awx/ui_next/src/screens/Template/shared/SurveyListItem.jsx b/awx/ui_next/src/screens/Template/shared/SurveyListItem.jsx index 9181230419..b91e92ae9b 100644 --- a/awx/ui_next/src/screens/Template/shared/SurveyListItem.jsx +++ b/awx/ui_next/src/screens/Template/shared/SurveyListItem.jsx @@ -1,10 +1,8 @@ import React from 'react'; import { t } from '@lingui/macro'; import { withI18n } from '@lingui/react'; - import { Button as _Button, - DataList, DataListAction as _DataListAction, DataListCheck, DataListItemCells, @@ -28,6 +26,7 @@ const Button = styled(_Button)` padding-bottom: 0; padding-left: 0; `; + function SurveyListItem({ question, i18n, @@ -39,56 +38,54 @@ function SurveyListItem({ onMoveDown, }) { return ( - - - - - - - - - - - - - - - - {question.question_name} - , - {question.type}, - - {question.default} - , - ]} - /> - - - + + + + + + + + + + + + + + + {question.question_name} + , + {question.type}, + + {question.default} + , + ]} + /> + + ); } From ac9f526cf0089f79d704fac5d39dc4821197bfb4 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Thu, 12 Mar 2020 17:08:21 -0700 Subject: [PATCH 6/9] fix useRequest error bug --- awx/ui_next/src/util/qs.js | 2 +- awx/ui_next/src/util/useRequest.js | 11 +- awx/ui_next/src/util/useRequest.test.jsx | 250 +++++++++++++++-------- 3 files changed, 174 insertions(+), 89 deletions(-) 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 9f0ea0c91f..6c06cc0a34 100644 --- a/awx/ui_next/src/util/useRequest.js +++ b/awx/ui_next/src/util/useRequest.js @@ -70,8 +70,10 @@ export function useDismissableError(error) { }, [error]); return { - error: showError && error, - dismissError: () => setShowError(false), + error: showError ? error : null, + dismissError: () => { + setShowError(false); + }, }; } @@ -81,7 +83,10 @@ export function useDeleteItems( ) { const location = useLocation(); const history = useHistory(); - const { requestError, isLoading, request } = useRequest(makeRequest, null); + const { error: requestError, isLoading, request } = useRequest( + makeRequest, + null + ); const { error, dismissError } = useDismissableError(requestError); const deleteItems = async () => { 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); }); }); }); From 3e616f2770f67a5fd197f2cefdd911363c406347 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Fri, 13 Mar 2020 10:17:39 -0700 Subject: [PATCH 7/9] update SurveyList tests, add TemplateSurvey tests --- .../screens/Template/TemplateSurvey.test.jsx | 89 +++++++++++++++ .../Template/shared/SurveyList.test.jsx | 101 ++++++------------ 2 files changed, 119 insertions(+), 71 deletions(-) create mode 100644 awx/ui_next/src/screens/Template/TemplateSurvey.test.jsx 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.test.jsx b/awx/ui_next/src/screens/Template/shared/SurveyList.test.jsx index bfd74023b3..9998e6569f 100644 --- a/awx/ui_next/src/screens/Template/shared/SurveyList.test.jsx +++ b/awx/ui_next/src/screens/Template/shared/SurveyList.test.jsx @@ -1,104 +1,69 @@ import React from 'react'; import { act } from 'react-dom/test-utils'; -import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers'; +import { mountWithContexts } from '@testUtils/enzymeHelpers'; import SurveyList from './SurveyList'; import { JobTemplatesAPI } from '@api'; import mockJobTemplateData from './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: { - name: 'Survey', - description: 'description for survey', - spec: [{ question_name: 'Foo', type: 'text', default: 'Bar' }], - }, - }); - }); + // beforeEach(() => { + // JobTemplatesAPI.readSurvey.mockResolvedValue({ + // data: { + // }, + // }); + // }); + test('expect component to mount successfully', async () => { let wrapper; await act(async () => { - wrapper = mountWithContexts( - - ); + wrapper = mountWithContexts(); }); expect(wrapper.length).toBe(1); }); - test('expect api to be called to get survey', async () => { - let wrapper; - await act(async () => { - wrapper = mountWithContexts( - - ); - }); - expect(JobTemplatesAPI.readSurvey).toBeCalledWith(7); - wrapper.update(); - - expect(wrapper.find('SurveyListItem').length).toBe(1); - }); - test('error in retrieving the survey throws an error', async () => { - JobTemplatesAPI.readSurvey.mockRejectedValue(new Error()); - let wrapper; - await act(async () => { - wrapper = mountWithContexts( - - ); - }); - - wrapper.update(); - - expect(wrapper.find('ContentError').length).toBe(1); - }); - test('can toggle survey on and off', async () => { + test('should toggle survey', async () => { + const toggleSurvey = jest.fn(); JobTemplatesAPI.update.mockResolvedValue(); let wrapper; await act(async () => { -<<<<<<< HEAD wrapper = mountWithContexts( -======= - wrapper = await mountWithContexts( ->>>>>>> Adds SurveyList tool bar ); }); expect(wrapper.find('Switch').length).toBe(1); - expect(wrapper.find('Switch').prop('isChecked')).toBe(false); + expect(wrapper.find('Switch').prop('isChecked')).toBe(true); await act(async () => { -<<<<<<< HEAD wrapper.find('Switch').invoke('onChange')(true); -======= - await wrapper.find('Switch').invoke('onChange')(true); ->>>>>>> Adds SurveyList tool bar }); - wrapper.update(); - expect(wrapper.find('Switch').prop('isChecked')).toBe(true); - expect(JobTemplatesAPI.update).toBeCalledWith(7, { - survey_enabled: true, - }); + expect(toggleSurvey).toHaveBeenCalled(); }); - test('selectAll enables delete button and calls the api to delete properly', async () => { + test('should select all and delete', async () => { + const deleteSurvey = jest.fn(); let wrapper; await act(async () => { -<<<<<<< HEAD wrapper = mountWithContexts( -======= - wrapper = await mountWithContexts( ->>>>>>> Adds SurveyList tool bar - + ); }); - await waitForElement(wrapper, 'SurveyListItem'); + wrapper.update(); expect(wrapper.find('Button[variant="danger"]').prop('isDisabled')).toBe( true ); @@ -112,38 +77,32 @@ describe('', () => { true ); }); - wrapper.update(); expect( wrapper.find('Checkbox[aria-label="Select all"]').prop('isChecked') ).toBe(true); - expect(wrapper.find('Button[variant="danger"]').prop('isDisabled')).toBe( false ); act(() => { wrapper.find('Button[variant="danger"]').invoke('onClick')(); }); - wrapper.update(); await act(() => wrapper.find('Button[aria-label="confirm delete"]').invoke('onClick')() ); - expect(JobTemplatesAPI.destroySurvey).toBeCalledWith(7); + expect(deleteSurvey).toHaveBeenCalled(); }); }); + describe('Survey with no questions', () => { test('Survey with no questions renders empty state', async () => { JobTemplatesAPI.readSurvey.mockResolvedValue({}); let wrapper; await act(async () => { -<<<<<<< HEAD wrapper = mountWithContexts( -======= - wrapper = await mountWithContexts( ->>>>>>> Adds SurveyList tool bar ); }); From a2eeb6e7b5c52b18c2ec324b9841e48498abf768 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Fri, 13 Mar 2020 11:38:49 -0700 Subject: [PATCH 8/9] delete unused file --- .../src/screens/Template/shared/surveyReducer.js | 15 --------------- awx/ui_next/src/util/useRequest.js | 1 - 2 files changed, 16 deletions(-) delete mode 100644 awx/ui_next/src/screens/Template/shared/surveyReducer.js diff --git a/awx/ui_next/src/screens/Template/shared/surveyReducer.js b/awx/ui_next/src/screens/Template/shared/surveyReducer.js deleted file mode 100644 index 418792a1e4..0000000000 --- a/awx/ui_next/src/screens/Template/shared/surveyReducer.js +++ /dev/null @@ -1,15 +0,0 @@ -export default function surveyReducer(state, action) { - switch (action.type) { - case 'THING': - return state; - default: - throw new Error(`Unrecognized action type: ${action.type}`); - } -} - -// move up/down -> Update -// delete -> Update -// delete all -> destroySurvey -// toggle -> Update survey_enabled -// select -// select all diff --git a/awx/ui_next/src/util/useRequest.js b/awx/ui_next/src/util/useRequest.js index 6c06cc0a34..a5a3e8556d 100644 --- a/awx/ui_next/src/util/useRequest.js +++ b/awx/ui_next/src/util/useRequest.js @@ -14,7 +14,6 @@ import { * 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 - * isInitialized: set to true once the result is initially fetched * * 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. From 86aabb297e366d7b4c6ad25b63c47117c3844baf Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Fri, 13 Mar 2020 13:14:03 -0700 Subject: [PATCH 9/9] add documentation for useDismissableError, useDeleteItems --- .../Template/shared/SurveyList.test.jsx | 7 ------- awx/ui_next/src/util/useRequest.js | 21 ++++++++++++++++++- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/awx/ui_next/src/screens/Template/shared/SurveyList.test.jsx b/awx/ui_next/src/screens/Template/shared/SurveyList.test.jsx index 9998e6569f..d0e7e2df15 100644 --- a/awx/ui_next/src/screens/Template/shared/SurveyList.test.jsx +++ b/awx/ui_next/src/screens/Template/shared/SurveyList.test.jsx @@ -16,13 +16,6 @@ const surveyData = { }; describe('', () => { - // beforeEach(() => { - // JobTemplatesAPI.readSurvey.mockResolvedValue({ - // data: { - // }, - // }); - // }); - test('expect component to mount successfully', async () => { let wrapper; await act(async () => { diff --git a/awx/ui_next/src/util/useRequest.js b/awx/ui_next/src/util/useRequest.js index a5a3e8556d..0e95be4a69 100644 --- a/awx/ui_next/src/util/useRequest.js +++ b/awx/ui_next/src/util/useRequest.js @@ -8,7 +8,7 @@ import { /* * The useRequest hook accepts a request function and returns an object with - * six 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 @@ -59,6 +59,15 @@ export default function useRequest(makeRequest, initialValue) { }; } +/* + * 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); @@ -76,6 +85,16 @@ export function useDismissableError(error) { }; } +/* + * 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 } = {}