Flush out useDeleteItems hook

refactor TemplateList to use useRequest, useDeleteItems hooks refactor CredentialList and OrganizationList to use useDeleteItems hook
2026-01-14 11:20:39 -03:30 · 2020-02-14 16:01:18 -08:00 · 2020-02-14 16:01:18 -08:00 · 54ddeaf046
commit 54ddeaf046
parent 69a1a02c70
5 changed files with 104 additions and 157 deletions
--- a/awx/ui_next/src/screens/Credential/CredentialList/CredentialList.jsx
+++ b/awx/ui_next/src/screens/Credential/CredentialList/CredentialList.jsx
@ -54,6 +54,8 @@ function CredentialList({ i18n }) {
    fetchCredentials();
  }, [fetchCredentials]);

+  const isAllSelected =
+    selected.length > 0 && selected.length === credentials.length;
  const {
    isLoading: isDeleteLoading,
    deleteItems: deleteCredentials,
@ -65,8 +67,7 @@ function CredentialList({ i18n }) {
    }, [selected]),
    {
      qsConfig: QS_CONFIG,
-      items: credentials,
-      selected,
+      allItemsSelected: isAllSelected,
      fetchItems: fetchCredentials,
    }
  );
@ -90,8 +91,6 @@ function CredentialList({ i18n }) {

  const canAdd =
    actions && Object.prototype.hasOwnProperty.call(actions, 'POST');
-  const isAllSelected =
-    selected.length > 0 && selected.length === credentials.length;

  return (
    <PageSection>
--- a/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx
+++ b/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx
@ -5,7 +5,7 @@ import { t } from '@lingui/macro';
 import { Card, PageSection } from '@patternfly/react-core';

 import { OrganizationsAPI } from '@api';
-import useRequest from '@util/useRequest';
+import useRequest, { useDeleteItems } from '@util/useRequest';
 import AlertModal from '@components/AlertModal';
 import DataListToolbar from '@components/DataListToolbar';
 import ErrorDetail from '@components/ErrorDetail';
@ -14,7 +14,6 @@ import PaginatedDataList, {
  ToolbarDeleteButton,
 } from '@components/PaginatedDataList';
 import { getQSConfig, parseQueryString } from '@util/qs';
-import updateUrlAfterDelete from '@util/updateUrlAfterDelete';
 import OrganizationListItem from './OrganizationListItem';

 const QS_CONFIG = getQSConfig('organization', {
@ -29,7 +28,6 @@ function OrganizationsList({ i18n }) {
  const match = useRouteMatch();

  const [selected, setSelected] = useState([]);
-  const [deletionError, setDeletionError] = useState(null);

  const addUrl = `${match.url}/add`;

@ -58,48 +56,37 @@ function OrganizationsList({ i18n }) {
    }
  );

-  const {
-    isLoading: isDeleteLoading,
-    error: dError,
-    request: deleteOrganizations,
-  } = useRequest(
-    useCallback(async () => {
-      return Promise.all(
-        selected.map(({ id }) => OrganizationsAPI.destroy(id))
-      );
-    }, [selected])
-  );
-
-  useEffect(() => {
-    if (dError) {
-      setDeletionError(dError);
-    }
-  }, [dError]);
-
  useEffect(() => {
    fetchOrganizations();
  }, [fetchOrganizations]);

+  const isAllSelected =
+    selected.length === organizations.length && selected.length > 0;
+  const {
+    isLoading: isDeleteLoading,
+    deleteItems: deleteOrganizations,
+    deletionError,
+    clearDeletionError,
+  } = useDeleteItems(
+    useCallback(async () => {
+      return Promise.all(
+        selected.map(({ id }) => OrganizationsAPI.destroy(id))
+      );
+    }, [selected]),
+    {
+      qsConfig: QS_CONFIG,
+      allItemsSelected: isAllSelected,
+      fetchItems: fetchOrganizations,
+    }
+  );
+
  const handleOrgDelete = async () => {
    await deleteOrganizations();
-    const url = updateUrlAfterDelete(
-      QS_CONFIG,
-      location,
-      organizations,
-      selected
-    );
-    if (url) {
-      history.push(url);
-    } else {
-      fetchOrganizations();
-    }
    setSelected([]);
  };

  const hasContentLoading = isDeleteLoading || isOrgsLoading;
  const canAdd = actions && actions.POST;
-  const isAllSelected =
-    selected.length === organizations.length && selected.length > 0;

  const handleSelectAll = isSelected => {
    if (isSelected) {
@ -189,7 +176,7 @@ function OrganizationsList({ i18n }) {
        isOpen={deletionError}
        variant="danger"
        title={i18n._(t`Error!`)}
-        onClose={() => setDeletionError(null)}
+        onClose={clearDeletionError}
      >
        {i18n._(t`Failed to delete one or more organizations.`)}
        <ErrorDetail error={deletionError} />
--- a/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx
+++ b/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx
@ -1,4 +1,4 @@
-import React, { useEffect, useState } from 'react';
+import React, { useEffect, useState, useCallback } from 'react';
 import { useParams, useLocation } from 'react-router-dom';
 import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
@ -15,6 +15,7 @@ import ErrorDetail from '@components/ErrorDetail';
 import PaginatedDataList, {
  ToolbarDeleteButton,
 } from '@components/PaginatedDataList';
+import useRequest, { useDeleteItems } from '@util/useRequest';
 import { getQSConfig, parseQueryString } from '@util/qs';

 import AddDropDownButton from '@components/AddDropDownButton';
@ -31,79 +32,78 @@ const QS_CONFIG = getQSConfig('template', {

 function TemplateList({ i18n }) {
  const { id: projectId } = useParams();
-  const { pathname, search } = useLocation();
+  const location = useLocation();

-  const [deletionError, setDeletionError] = useState(null);
-  const [contentError, setContentError] = useState(null);
-  const [hasContentLoading, setHasContentLoading] = useState(true);
-  const [jtActions, setJTActions] = useState(null);
-  const [wfjtActions, setWFJTActions] = useState(null);
-  const [count, setCount] = useState(0);
-  const [templates, setTemplates] = useState([]);
  const [selected, setSelected] = useState([]);

-  useEffect(
-    () => {
-      const loadTemplates = async () => {
-        const params = {
-          ...parseQueryString(QS_CONFIG, search),
-        };
-
-        let jtOptionsPromise;
-        if (jtActions) {
-          jtOptionsPromise = Promise.resolve({
-            data: { actions: jtActions },
-          });
-        } else {
-          jtOptionsPromise = JobTemplatesAPI.readOptions();
-        }
-
-        let wfjtOptionsPromise;
-        if (wfjtActions) {
-          wfjtOptionsPromise = Promise.resolve({
-            data: { actions: wfjtActions },
-          });
-        } else {
-          wfjtOptionsPromise = WorkflowJobTemplatesAPI.readOptions();
-        }
-        if (pathname.startsWith('/projects') && projectId) {
-          params.jobtemplate__project = projectId;
-        }
-
-        const promises = Promise.all([
-          UnifiedJobTemplatesAPI.read(params),
-          jtOptionsPromise,
-          wfjtOptionsPromise,
-        ]);
-        setDeletionError(null);
-
-        try {
-          const [
-            {
-              data: { count: itemCount, results },
-            },
-            {
-              data: { actions: jobTemplateActions },
-            },
-            {
-              data: { actions: workFlowJobTemplateActions },
-            },
-          ] = await promises;
-          setJTActions(jobTemplateActions);
-          setWFJTActions(workFlowJobTemplateActions);
-          setCount(itemCount);
-          setTemplates(results);
-          setHasContentLoading(false);
-        } catch (err) {
-          setContentError(err);
-        }
+  const {
+    result: { templates, count, jtActions, wfjtActions },
+    error: contentError,
+    isLoading,
+    request: fetchTemplates,
+  } = useRequest(
+    useCallback(async () => {
+      const params = parseQueryString(QS_CONFIG, location.search);
+      if (location.pathname.startsWith('/projects') && projectId) {
+        params.jobtemplate__project = projectId;
+      }
+      const results = await Promise.all([
+        UnifiedJobTemplatesAPI.read(params),
+        JobTemplatesAPI.readOptions(),
+        WorkflowJobTemplatesAPI.readOptions(),
+      ]);
+      return {
+        templates: results[0].data.results,
+        count: results[0].data.count,
+        jtActions: results[1].data.actions,
+        wfjtActions: results[2].data.actions,
      };
-      loadTemplates();
-    },
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-    [pathname, search, count, projectId]
+    }, [location, projectId]),
+    {
+      templates: [],
+      count: 0,
+      jtActions: {},
+      wfjtActions: {},
+    }
  );

+  useEffect(() => {
+    fetchTemplates();
+  }, [fetchTemplates]);
+
+  const isAllSelected =
+    selected.length === templates.length && selected.length > 0;
+  const {
+    isLoading: isDeleteLoading,
+    deleteItems: deleteTemplates,
+    deletionError,
+    clearDeletionError,
+  } = useDeleteItems(
+    useCallback(async () => {
+      return Promise.all(
+        selected.map(({ type, id }) => {
+          if (type === 'job_template') {
+            return JobTemplatesAPI.destroy(id);
+          }
+          if (type === 'workflow_job_template') {
+            return WorkflowJobTemplatesAPI.destroy(id);
+          }
+          return false;
+        })
+      );
+    }, [selected]),
+    {
+      qsConfig: QS_CONFIG,
+      allItemsSelected: isAllSelected,
+      fetchItems: fetchTemplates,
+    }
+  );
+
+  const handleTemplateDelete = async () => {
+    await deleteTemplates();
+    setSelected([]);
+  };
+
  const handleSelectAll = isSelected => {
    const selectedItems = isSelected ? [...templates] : [];
    setSelected(selectedItems);
@ -117,26 +117,6 @@ function TemplateList({ i18n }) {
    }
  };

-  const handleTemplateDelete = async () => {
-    setHasContentLoading(true);
-    try {
-      await Promise.all(
-        selected.map(({ type, id }) => {
-          let deletePromise;
-          if (type === 'job_template') {
-            deletePromise = JobTemplatesAPI.destroy(id);
-          } else if (type === 'workflow_job_template') {
-            deletePromise = WorkflowJobTemplatesAPI.destroy(id);
-          }
-          return deletePromise;
-        })
-      );
-      setCount(count - selected.length);
-    } catch (err) {
-      setDeletionError(err);
-    }
-  };
-
  const canAddJT =
    jtActions && Object.prototype.hasOwnProperty.call(jtActions, 'POST');
  const canAddWFJT =
@ -154,8 +134,6 @@ function TemplateList({ i18n }) {
      url: `/templates/workflow_job_template/add/`,
    });
  }
-  const isAllSelected =
-    selected.length === templates.length && selected.length > 0;
  const addButton = (
    <AddDropDownButton key="add" dropdownItems={addButtonOptions} />
  );
@ -164,7 +142,7 @@ function TemplateList({ i18n }) {
      <Card>
        <PaginatedDataList
          contentError={contentError}
-          hasContentLoading={hasContentLoading}
+          hasContentLoading={isLoading || isDeleteLoading}
          items={templates}
          itemCount={count}
          pluralizedItemName={i18n._(t`Templates`)}
@ -247,7 +225,7 @@ function TemplateList({ i18n }) {
              key={template.id}
              value={template.name}
              template={template}
-              detailUrl={`${pathname}/${template.type}/${template.id}`}
+              detailUrl={`${location.pathname}/${template.type}/${template.id}`}
              onSelect={() => handleSelect(template)}
              isSelected={selected.some(row => row.id === template.id)}
            />
@ -259,7 +237,7 @@ function TemplateList({ i18n }) {
        isOpen={deletionError}
        variant="danger"
        title={i18n._(t`Error!`)}
-        onClose={() => setDeletionError(null)}
+        onClose={clearDeletionError}
      >
        {i18n._(t`Failed to delete one or more templates.`)}
        <ErrorDetail error={deletionError} />
--- a/awx/ui_next/src/util/updateUrlAfterDelete.js
+++ b/awx/ui_next/src/util/updateUrlAfterDelete.js
@ -1,22 +0,0 @@
-import {
-  parseQueryString,
-  replaceParams,
-  encodeNonDefaultQueryString,
-} from './qs';
-
-export default function updateUrlAfterDelete(
-  qsConfig,
-  location,
-  items,
-  selectedItems
-) {
-  const params = parseQueryString(qsConfig, location.search);
-  if (params.page > 1 && selectedItems.length === items.length) {
-    const newParams = encodeNonDefaultQueryString(
-      qsConfig,
-      replaceParams(params, { page: params.page - 1 })
-    );
-    return `${location.pathname}?${newParams}`;
-  }
-  return false;
-}
--- a/awx/ui_next/src/util/useRequest.js
+++ b/awx/ui_next/src/util/useRequest.js
@ -56,18 +56,23 @@ export default function useRequest(makeRequest, initialValue) {

 export function useDeleteItems(
  makeRequest,
-  { qsConfig, items, selected, fetchItems }
+  { qsConfig, allItemsSelected, fetchItems }
 ) {
  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 deleteItems = async () => {
    await request();
    const params = parseQueryString(qsConfig, location.search);
-    if (params.page > 1 && selected.length === items.length) {
+    if (params.page > 1 && allItemsSelected) {
      const newParams = encodeNonDefaultQueryString(
        qsConfig,
        replaceParams(params, { page: params.page - 1 })