Merge pull request #6268 from keithjgrant/survey-list-sort

Add survey list sorting

Reviewed-by: https://github.com/apps/softwarefactory-project-zuul

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 (
       <Fragment>
         <CardBody>
-          {response.config.method.toUpperCase()} {response.config.url}{' '}
+          {response?.config?.method.toUpperCase()} {response?.config?.url}{' '}
-          <strong>{response.status}</strong>
+          <strong>{response?.status}</strong>
         </CardBody>
         <CardBody>{message}</CardBody>
       </Fragment>

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 {
               </Route>
             )}
             {template && (
-              <Route
+              <Route path="/templates/:templateType/:id/survey">
-                path="/templates/:templateType/:id/survey"
+                <TemplateSurvey template={template} />
-                render={() => <SurveyList template={template} />}
+              </Route>
-              />
             )}
             <Route
               key="not-found"

awx/ui_next/src/screens/Template/TemplateSurvey.jsx

@@ -0,0 +1,93 @@
+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';
+
+function TemplateSurvey({ template, i18n }) {
+  const [surveyEnabled, setSurveyEnabled] = useState(template.survey_enabled);
+
+  const {
+    result: survey,
+    request: fetchSurvey,
+    isLoading,
+    error: loadingError,
+    setValue: setSurvey,
+  } = useRequest(
+    useCallback(async () => {
+      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 <ContentError error={loadingError} />;
+  }
+  return (
+    <>
+      <Switch>
+        <Route path="/templates/:templateType/:id/survey">
+          <SurveyList
+            isLoading={isLoading}
+            survey={survey}
+            surveyEnabled={surveyEnabled}
+            toggleSurvey={toggleSurvey}
+            updateSurvey={spec => updateSurvey({ ...survey, spec })}
+            deleteSurvey={deleteSurvey}
+          />
+        </Route>
+      </Switch>
+      {error && (
+        <AlertModal
+          isOpen={error}
+          variant="error"
+          title={i18n._(t`Error!`)}
+          onClose={dismissError}
+        >
+          {i18n._(t`Failed to update survey.`)}
+          <ErrorDetail error={error} />
+        </AlertModal>
+      )}
+    </>
+  );
+}
+
+export default withI18n()(TemplateSurvey);

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('<TemplateSurvey />', () => {
+  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(
+        <TemplateSurvey template={mockJobTemplateData} />,
+        {
+          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(
+        <TemplateSurvey template={{ ...mockJobTemplateData, id: 'a' }} />
+      );
+    });
+
+    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(
+        <TemplateSurvey template={mockJobTemplateData} />,
+        {
+          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' },
+      ],
+    });
+  });
+});

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 { DataList, Button } from '@patternfly/react-core';
-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 { getQSConfig } from '@util/qs';
 import AlertModal from '@components/AlertModal';
 import SurveyListItem from './SurveyListItem';
 import SurveyToolbar from './SurveyToolbar';
 
-const QS_CONFIG = getQSConfig('survey', {
+function SurveyList({
-  page: 1,
+  isLoading,
-});
+  survey,
-
+  surveyEnabled,
-function SurveyList({ template, i18n }) {
+  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)) {
+    if (selected.some(q => q.variable === item.variable)) {
-      setSelected(selected.filter(s => s.id !== item.id));
+      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 = <ContentLoading />;
-  } else if (contentError) {
-    content = <ContentError error={contentError} />;
   } else if (!questions || questions?.length <= 0) {
     content = (
       <ContentEmpty
@@ -132,17 +78,24 @@ function SurveyList({ template, i18n }) {
       />
     );
   } else {
-    content = questions?.map((question, index) => (
+    content = (
-      <SurveyListItem
+      <DataList aria-label={i18n._(t`Survey List`)}>
-        key={question.id}
+        {questions?.map((question, index) => (
-        isLast={index === questions.length - 1}
+          <SurveyListItem
-        isFirst={index === 0}
+            key={question.variable}
-        question={question}
+            isLast={index === questions.length - 1}
-        isChecked={selected.some(s => s.id === question.id)}
+            isFirst={index === 0}
-        onSelect={() => handleSelect(question)}
+            question={question}
-      />
+            isChecked={selected.some(q => q.variable === question.variable)}
-    ));
+            onSelect={() => handleSelect(question)}
+            onMoveUp={moveUp}
+            onMoveDown={moveDown}
+          />
+        ))}
+      </DataList>
+    );
   }
+
   return (
     <>
       <SurveyToolbar
@@ -150,7 +103,7 @@ function SurveyList({ template, i18n }) {
         onSelectAll={handleSelectAll}
         surveyEnabled={surveyEnabled}
         onToggleSurvey={toggleSurvey}
-        isDeleteDisabled={selected?.length === 0 || !canEdit || !canDelete}
+        isDeleteDisabled={selected?.length === 0}
         onToggleDeleteModal={() => setIsDeleteModalOpen(true)}
       />
       {content}
@@ -165,7 +118,6 @@ function SurveyList({ template, i18n }) {
           isOpen={isDeleteModalOpen}
           onClose={() => {
             setIsDeleteModalOpen(false);
-            setSelected([]);
           }}
           actions={[
             <Button
@@ -182,7 +134,6 @@ function SurveyList({ template, i18n }) {
               aria-label={i18n._(t`cancel delete`)}
               onClick={() => {
                 setIsDeleteModalOpen(false);
-                setSelected([]);
               }}
             >
               {i18n._(t`Cancel`)}
@@ -190,34 +141,13 @@ function SurveyList({ template, i18n }) {
           ]}
         >
           <div>{i18n._(t`This action will delete the following:`)}</div>
-          {selected.map(question => (
+          <ul>
-            <span key={question.id}>
+            {selected.map(question => (
-              <strong>{question.question_name}</strong>
+              <li key={question.variable}>
-              <br />
+                <strong>{question.question_name}</strong>
-            </span>
+              </li>
-          ))}
+            ))}
-        </AlertModal>
+          </ul>
-      )}
-      {deletionError && (
-        <AlertModal
-          isOpen={deletionError}
-          variant="error"
-          title={i18n._(t`Error!`)}
-          onClose={clearDeletionError}
-        >
-          {i18n._(t`Failed to delete one or more jobs.`)}
-          <ErrorDetail error={deletionError} />
-        </AlertModal>
-      )}
-      {toggleError && (
-        <AlertModal
-          variant="error"
-          title={i18n._(t`Error!`)}
-          isOpen={showToggleError && !isLoading}
-          onClose={() => setShowToggleError(false)}
-        >
-          {i18n._(t`Failed to toggle host.`)}
-          <ErrorDetail error={toggleError} />
         </AlertModal>
       )}
     </>

awx/ui_next/src/screens/Template/shared/SurveyList.test.jsx

@@ -1,91 +1,62 @@
 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('<SurveyList />', () => {
-  beforeEach(() => {
-    JobTemplatesAPI.readSurvey.mockResolvedValue({
-      data: {
-        name: 'Survey',
-        description: 'description for survey',
-        spec: [{ question_name: 'Foo', type: 'text', default: 'Bar' }],
-      },
-    });
-  });
   test('expect component to mount successfully', async () => {
     let wrapper;
     await act(async () => {
-      wrapper = mountWithContexts(
+      wrapper = mountWithContexts(<SurveyList survey={surveyData} />);
-        <SurveyList template={mockJobTemplateData} />
-      );
     });
     expect(wrapper.length).toBe(1);
   });
-  test('expect api to be called to get survey', async () => {
-    let wrapper;
-    await act(async () => {
-      wrapper = mountWithContexts(
-        <SurveyList template={mockJobTemplateData} />
-      );
-    });
-    expect(JobTemplatesAPI.readSurvey).toBeCalledWith(7);
 
-    wrapper.update();
+  test('should toggle survey', async () => {
-    expect(wrapper.find('SurveyListItem').length).toBe(1);
+    const toggleSurvey = jest.fn();
-  });
-  test('error in retrieving the survey throws an error', async () => {
-    JobTemplatesAPI.readSurvey.mockRejectedValue(new Error());
-    let wrapper;
-    await act(async () => {
-      wrapper = mountWithContexts(
-        <SurveyList template={{ ...mockJobTemplateData, id: 'a' }} />
-      );
-    });
-
-    wrapper.update();
-
-    expect(wrapper.find('ContentError').length).toBe(1);
-  });
-  test('can toggle survey on and off', async () => {
     JobTemplatesAPI.update.mockResolvedValue();
     let wrapper;
     await act(async () => {
       wrapper = mountWithContexts(
         <SurveyList
-          template={{ ...mockJobTemplateData, survey_enabled: false }}
+          survey={surveyData}
+          surveyEnabled
+          toggleSurvey={toggleSurvey}
         />
       );
     });
 
     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 () => {
       wrapper.find('Switch').invoke('onChange')(true);
     });
-
     wrapper.update();
 
-    expect(wrapper.find('Switch').prop('isChecked')).toBe(true);
+    expect(toggleSurvey).toHaveBeenCalled();
-    expect(JobTemplatesAPI.update).toBeCalledWith(7, {
-      survey_enabled: true,
-    });
   });
 
-  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 () => {
       wrapper = mountWithContexts(
-        <SurveyList
+        <SurveyList survey={surveyData} deleteSurvey={deleteSurvey} />
-          template={{ ...mockJobTemplateData, survey_enabled: false }}
-        />
       );
     });
-    await waitForElement(wrapper, 'SurveyListItem');
+    wrapper.update();
     expect(wrapper.find('Button[variant="danger"]').prop('isDisabled')).toBe(
       true
     );
@@ -99,28 +70,26 @@ describe('<SurveyList />', () => {
         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({});

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,
@@ -35,56 +34,58 @@ function SurveyListItem({
   isFirst,
   isChecked,
   onSelect,
+  onMoveUp,
+  onMoveDown,
 }) {
   return (
-    <DataList aria-label={i18n._(t`Survey List`)}>
+    <DataListItem aria-labelledby={i18n._(t`Survey questions`)}>
-      <DataListItem aria-labelledby={i18n._(t`Survey questions`)}>
+      <DataListItemRow css="padding-left:16px">
-        <DataListItemRow css="padding-left:16px">
+        <DataListAction
-          <DataListAction
+          id="sortQuestions"
-            id="sortQuestions"
+          aria-labelledby={i18n._(t`Sort question order`)}
-            aria-labelledby={i18n._(t`Sort question order`)}
+          aria-label={i18n._(t`Sort question order`)}
-            aria-label={i18n._(t`Sort question order`)}
+        >
-          >
+          <Stack>
-            <Stack>
+            <StackItem>
-              <StackItem>
+              <Button
-                <Button
+                variant="plain"
-                  variant="plain"
+                aria-label={i18n._(t`move up`)}
-                  aria-label={i18n._(t`move up`)}
+                isDisabled={isFirst}
-                  isDisabled={isFirst}
+                onClick={() => onMoveUp(question)}
-                >
+              >
-                  <CaretUpIcon />
+                <CaretUpIcon />
-                </Button>
+              </Button>
-              </StackItem>
+            </StackItem>
-              <StackItem>
+            <StackItem>
-                <Button
+              <Button
-                  variant="plain"
+                variant="plain"
-                  aria-label={i18n._(t`move down`)}
+                aria-label={i18n._(t`move down`)}
-                  isDisabled={isLast}
+                isDisabled={isLast}
-                >
+                onClick={() => onMoveDown(question)}
-                  <CaretDownIcon />
+              >
-                </Button>
+                <CaretDownIcon />
-              </StackItem>
+              </Button>
-            </Stack>
+            </StackItem>
-          </DataListAction>
+          </Stack>
-          <DataListCheck
+        </DataListAction>
-            checked={isChecked}
+        <DataListCheck
-            onChange={onSelect}
+          checked={isChecked}
-            aria-labelledby="survey check"
+          onChange={onSelect}
-          />
+          aria-labelledby="survey check"
-          <DataListItemCells
+        />
-            dataListCells={[
+        <DataListItemCells
-              <DataListCell key={question.question_name}>
+          dataListCells={[
-                {question.question_name}
+            <DataListCell key={question.question_name}>
-              </DataListCell>,
+              {question.question_name}
-              <DataListCell key={question.type}>{question.type}</DataListCell>,
+            </DataListCell>,
-              <DataListCell key={question.default}>
+            <DataListCell key={question.type}>{question.type}</DataListCell>,
-                {question.default}
+            <DataListCell key={question.default}>
-              </DataListCell>,
+              {question.default}
-            ]}
+            </DataListCell>,
-          />
+          ]}
-        </DataListItemRow>
+        />
-      </DataListItem>
+      </DataListItemRow>
-    </DataList>
+    </DataListItem>
   );
 }
 

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);

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 () => {
+    request: useCallback(
-      setIsLoading(true);
+      async (...args) => {
-      try {
+        setIsLoading(true);
-        const response = await makeRequest();
+        try {
-        if (isMounted.current) {
+          const response = await makeRequest(...args);
-          setResult(response);
+          if (isMounted.current) {
+            setResult(response);
+          }
+        } catch (err) {
+          if (isMounted.current) {
+            setError(err);
+          }
+        } finally {
+          if (isMounted.current) {
+            setIsLoading(false);
+          }
         }
-      } catch (err) {
+      },
-        if (isMounted.current) {
+      [makeRequest]
-          setError(err);
+    ),
-        }
+    setValue: setResult,
-      } finally {
-        if (isMounted.current) {
-          setIsLoading(false);
-        }
-      }
-    }, [makeRequest]),
   };
 }
 
-export function useDeleteItems(
+/*
-  makeRequest,
+ * Provides controls for "dismissing" an error message
-  { qsConfig, allItemsSelected, fetchItems }
+ *
-) {
+ * Params: an error object
-  const location = useLocation();
+ * Returns: { error, dismissError }
-  const history = useHistory();
+ *   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,
+    deletionError: error,
-    clearDeletionError: () => setShowError(false),
+    clearDeletionError: dismissError,
   };
 }

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 <div />;
@@ -10,100 +11,179 @@ function Test({ makeRequest, initialValue = {} }) {
   const request = useRequest(makeRequest, initialValue);
   return <TestInner {...request} />;
 }
+function DeleteTest({ makeRequest, args = {} }) {
+  const request = useDeleteItems(makeRequest, args);
+  return <TestInner {...request} />;
+}
 
-describe('useRequest', () => {
+describe('useRequest hooks', () => {
-  test('should return initial value as result', async () => {
+  describe('useRequest', () => {
-    const makeRequest = jest.fn();
+    test('should return initial value as result', async () => {
-    makeRequest.mockResolvedValue({ data: 'foo' });
+      const makeRequest = jest.fn();
-    const wrapper = mount(
+      makeRequest.mockResolvedValue({ data: 'foo' });
-      <Test
+      const wrapper = mount(
-        makeRequest={makeRequest}
+        <Test
-        initialValue={{
+          makeRequest={makeRequest}
-          initial: true,
+          initialValue={{
-        }}
+            initial: true,
-      />
+          }}
-    );
+        />
+      );
 
-    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(<Test makeRequest={makeRequest} />);
+
+      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(<Test makeRequest={makeRequest} />);
+
+      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(<Test makeRequest={makeRequest} />);
+
+      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(<Test makeRequest={makeRequest} />);
+
+      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(<Test makeRequest={makeRequest} />);
+
+      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 () => {
+  describe('useDeleteItems', () => {
-    const makeRequest = jest.fn();
+    test('should invoke delete function', async () => {
-    makeRequest.mockResolvedValue({ data: 'foo' });
+      const makeRequest = jest.fn();
-    const wrapper = mount(<Test makeRequest={makeRequest} />);
+      makeRequest.mockResolvedValue({ data: 'foo' });
+      const wrapper = mountWithContexts(
+        <DeleteTest
+          makeRequest={makeRequest}
+          args={{
+            qsConfig: {},
+            fetchItems: () => {},
+          }}
+        />
+      );
 
-    await act(async () => {
+      expect(makeRequest).not.toHaveBeenCalled();
-      wrapper.find('TestInner').invoke('request')();
+      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 () => {
+    test('should return error object thrown by function', async () => {
-    const makeRequest = jest.fn();
+      const error = new Error('error');
-    let resolve;
+      const makeRequest = () => {
-    const promise = new Promise(r => {
+        throw error;
-      resolve = r;
+      };
+      const wrapper = mountWithContexts(
+        <DeleteTest
+          makeRequest={makeRequest}
+          args={{
+            qsConfig: {},
+            fetchItems: () => {},
+          }}
+        />
+      );
+
+      await act(async () => {
+        wrapper.find('TestInner').invoke('deleteItems')();
+      });
+      wrapper.update();
+      expect(wrapper.find('TestInner').prop('deletionError')).toEqual(error);
     });
-    makeRequest.mockReturnValue(promise);
-    const wrapper = mount(<Test makeRequest={makeRequest} />);
 
-    await act(async () => {
+    test('should dismiss error', async () => {
-      wrapper.find('TestInner').invoke('request')();
+      const error = new Error('error');
-    });
+      const makeRequest = () => {
-    wrapper.update();
+        throw error;
-    expect(wrapper.find('TestInner').prop('isLoading')).toEqual(true);
+      };
-    await act(async () => {
+      const wrapper = mountWithContexts(
-      resolve({ data: 'foo' });
+        <DeleteTest
-    });
+          makeRequest={makeRequest}
-    wrapper.update();
+          args={{
-    expect(wrapper.find('TestInner').prop('isLoading')).toEqual(false);
+            qsConfig: {},
-    expect(wrapper.find('TestInner').prop('result')).toEqual({ data: 'foo' });
+            fetchItems: () => {},
-  });
+          }}
+        />
+      );
 
-  test('should invoke request function', async () => {
+      await act(async () => {
-    const makeRequest = jest.fn();
+        wrapper.find('TestInner').invoke('deleteItems')();
-    makeRequest.mockResolvedValue({ data: 'foo' });
+      });
-    const wrapper = mount(<Test makeRequest={makeRequest} />);
+      wrapper.update();
-
+      await act(async () => {
-    expect(makeRequest).not.toHaveBeenCalled();
+        wrapper.find('TestInner').invoke('clearDeletionError')();
-    await act(async () => {
+      });
-      wrapper.find('TestInner').invoke('request')();
+      wrapper.update();
-    });
+      expect(wrapper.find('TestInner').prop('deletionError')).toEqual(null);
-    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(<Test makeRequest={makeRequest} />);
-
-    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(<Test makeRequest={makeRequest} />);
-
-    expect(makeRequest).not.toHaveBeenCalled();
-    await act(async () => {
-      wrapper.find('TestInner').invoke('request')();
-    });
-    wrapper.unmount();
-    await act(async () => {
-      resolve({ data: 'foo' });
     });
   });
 });