diff --git a/awx/ui_next/src/screens/Inventory/Inventory.jsx b/awx/ui_next/src/screens/Inventory/Inventory.jsx index 3a445e0cf6..5a3d26179f 100644 --- a/awx/ui_next/src/screens/Inventory/Inventory.jsx +++ b/awx/ui_next/src/screens/Inventory/Inventory.jsx @@ -37,7 +37,8 @@ function Inventory({ i18n, setBreadcrumb }) { useEffect(() => { async function fetchData() { try { - setHasContentLoading(true); + // TODO: delete next line + // setHasContentLoading(true); const { data } = await InventoriesAPI.readDetail(match.params.id); setBreadcrumb(data); setInventory(data); diff --git a/awx/ui_next/src/screens/Inventory/InventoryDetail/InventoryDetail.jsx b/awx/ui_next/src/screens/Inventory/InventoryDetail/InventoryDetail.jsx index a9d558624c..d7cca7727a 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryDetail/InventoryDetail.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryDetail/InventoryDetail.jsx @@ -1,4 +1,4 @@ -import React, { useCallback } from 'react'; +import React, { useCallback, useEffect } from 'react'; import { Link, useHistory } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; @@ -11,20 +11,38 @@ import DeleteButton from '@components/DeleteButton'; import ContentError from '@components/ContentError'; import ContentLoading from '@components/ContentLoading'; import { InventoriesAPI } from '@api'; -import useEndpoint from './useEndpoint'; +import useEndpoint, { useFetch } from './useEndpoint'; import { Inventory } from '../../../types'; function InventoryDetail({ inventory, i18n }) { const history = useHistory(); - const { results: instanceGroups, isLoading, error } = useEndpoint( + // initial approach with useEndpoint() + // const { results: instanceGroups, isLoading, error } = useEndpoint( + // useCallback(async () => { + // const { data } = await InventoriesAPI.readInstanceGroups(inventory.id); + // return data.results; + // }, [inventory.id]) + // ); + + // more versatile approach with useFetch() + const { + result: instanceGroups, + isLoading, + error, + fetch: fetchInstanceGroups, + } = useFetch( useCallback(async () => { const { data } = await InventoriesAPI.readInstanceGroups(inventory.id); return data.results; }, [inventory.id]), - inventory.id + [] ); + useEffect(() => { + fetchInstanceGroups(inventory.id); + }, [fetchInstanceGroups, inventory.id]); + const deleteInventory = async () => { await InventoriesAPI.destroy(inventory.id); history.push(`/inventories`); diff --git a/awx/ui_next/src/screens/Inventory/InventoryDetail/useEndpoint.js b/awx/ui_next/src/screens/Inventory/InventoryDetail/useEndpoint.js index 9c45d7e32e..4ace382a1a 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryDetail/useEndpoint.js +++ b/awx/ui_next/src/screens/Inventory/InventoryDetail/useEndpoint.js @@ -1,5 +1,6 @@ -import { useEffect, useState, useRef } from 'react'; +import { useEffect, useState, useRef, useCallback } from 'react'; +// Initial approach (useEffect baked in) export default function useEndpoint(fetch) { const [results, setResults] = useState([]); const [error, setError] = useState(null); @@ -38,3 +39,41 @@ export default function useEndpoint(fetch) { error, }; } + +// more versatile approach (returns function to make API request) +export function useFetch(fetch, initialValue) { + const [result, setResult] = useState(initialValue); + const [error, setError] = useState(null); + const [isLoading, setIsLoading] = useState(false); + const isMounted = useRef(null); + + useEffect(() => { + isMounted.current = true; + return () => { + isMounted.current = false; + }; + }, []); + + return { + result, + error, + isLoading, + fetch: useCallback(async () => { + setIsLoading(true); + try { + const response = await fetch(); + if (isMounted.current) { + setResult(response); + } + } catch (err) { + if (isMounted.current) { + setError(err); + } + } finally { + if (isMounted.current) { + setIsLoading(false); + } + } + }, [fetch]), + }; +} diff --git a/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx b/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx index 827b7dd092..6abe157d03 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx @@ -1,4 +1,4 @@ -import React, { useEffect, useState } from 'react'; +import React, { useState, useEffect, useCallback } from 'react'; import { useLocation, useRouteMatch } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; @@ -13,6 +13,10 @@ import PaginatedDataList, { ToolbarDeleteButton, } from '@components/PaginatedDataList'; import { getQSConfig, parseQueryString } from '@util/qs'; +// TODO: move useEndpoint to better location +import useEndpoint, { + useFetch, +} from '../../Inventory/InventoryDetail/useEndpoint'; import OrganizationListItem from './OrganizationListItem'; @@ -25,64 +29,89 @@ const QS_CONFIG = getQSConfig('organization', { function OrganizationsList({ i18n }) { const location = useLocation(); const match = useRouteMatch(); - const [contentError, setContentError] = useState(null); - const [deletionError, setDeletionError] = useState(null); - const [hasContentLoading, setHasContentLoading] = useState(true); - const [itemCount, setItemCount] = useState(0); - const [organizations, setOrganizations] = useState([]); - const [orgActions, setOrgActions] = useState(null); + const [selected, setSelected] = useState([]); + const [deletionError, setDeletionError] = useState(null); const addUrl = `${match.url}/add`; - const canAdd = orgActions && orgActions.POST; - const isAllSelected = - selected.length === organizations.length && selected.length > 0; - const loadOrganizations = async ({ search }) => { - const params = parseQueryString(QS_CONFIG, search); - setContentError(null); - setHasContentLoading(true); - try { - const [ - { - data: { count, results }, - }, - { - data: { actions }, - }, - ] = await Promise.all([ + // const { + // results: organizations, + // error: contentError, + // isLoading: isOrgsLoading, + // } = useEndpoint( + // useCallback(async () => { + // const params = parseQueryString(QS_CONFIG, location.search); + // const [ + // { + // data: { count, results }, + // }, + // { + // data: { actions }, + // }, + // ] = await Promise.all([ + // OrganizationsAPI.read(params), + // loadOrganizationActions(), + // ]); + // return { + // organizations: results, + // count, + // actions, + // }; + // }, [location]) + // ); + + const { + result: { organizations = {}, organizationCount, actions }, + error: contentError, + isLoading: isOrgsLoading, + fetch: fetchOrganizations, + } = useFetch( + useCallback(async () => { + const params = parseQueryString(QS_CONFIG, location.search); + const [orgs, orgActions] = await Promise.all([ OrganizationsAPI.read(params), - loadOrganizationActions(), + OrganizationsAPI.readOptions(), ]); - setItemCount(count); - setOrganizations(results); - setOrgActions(actions); - setSelected([]); - } catch (error) { - setContentError(error); - } finally { - setHasContentLoading(false); + return { + organizations: orgs.data.results, + organizationCount: orgs.data.count, + actions: orgActions.data.actions, + }; + }, [location]), + { + organizations: [], + organizationCount: 0, + actions: {}, } - }; + ); - const loadOrganizationActions = () => { - if (orgActions) { - return Promise.resolve({ data: { actions: orgActions } }); - } - return OrganizationsAPI.readOptions(); - }; + const { + isLoading: isDeleteLoading, + // error: deletionError, + fetch: deleteOrganizations, + } = useFetch( + useCallback(async () => { + return Promise.all( + selected.map(({ id }) => OrganizationsAPI.destroy(id)) + ); + }, [selected]) + ); + + useEffect(() => { + fetchOrganizations(); + }, [fetchOrganizations]); const handleOrgDelete = async () => { - setHasContentLoading(true); - try { - await Promise.all(selected.map(({ id }) => OrganizationsAPI.destroy(id))); - } catch (error) { - setDeletionError(error); - } finally { - await loadOrganizations(location); - } + await deleteOrganizations(); + await fetchOrganizations(); }; + const hasContentLoading = isDeleteLoading || isOrgsLoading; + const canAdd = actions && actions.POST; + const isAllSelected = + selected.length === organizations.length && selected.length > 0; + const handleSelectAll = isSelected => { if (isSelected) { setSelected(organizations); @@ -99,14 +128,6 @@ function OrganizationsList({ i18n }) { } }; - const handleDeleteErrorClose = () => { - setDeletionError(null); - }; - - useEffect(() => { - loadOrganizations(location); - }, [location]); // eslint-disable-line react-hooks/exhaustive-deps - return ( <> @@ -115,7 +136,7 @@ function OrganizationsList({ i18n }) { contentError={contentError} hasContentLoading={hasContentLoading} items={organizations} - itemCount={itemCount} + itemCount={organizationCount} pluralizedItemName="Organizations" qsConfig={QS_CONFIG} onRowClick={handleSelect} @@ -179,7 +200,7 @@ function OrganizationsList({ i18n }) { isOpen={deletionError} variant="danger" title={i18n._(t`Error!`)} - onClose={handleDeleteErrorClose} + onClose={() => setDeletionError(null)} > {i18n._(t`Failed to delete one or more organizations.`)}