fix(RHINENG-9836) duplicate fetch on systems table

Inventory makes multiple, duplicate, request on page load. Load up inventory to then see that these request are sent up multiple times. - Issue is reproducible when you apply some filters. For example, apply any OS filter and in network tab search for `/api/inventory/v1/hosts` - you will see 2 duplicated requests for hosts and for tags - Issue is reproducible when you apply global filters - Issue is also accompanied by the problem of missing debounce - Issue is visible on Advisor -> Recommendation Details page even without applying any additional filters This PR fixes issues where too many refreshes were being called from too many locations. We add a debounced call to the refresh data function and pass that debounce down where needed. We also cleaned up react hook dependencies and consolidated them.
RedHatInsights · Aug 28, 2024 · 21c8232 · 21c8232
1 parent 4aa3120
commit 21c8232
Show file tree

Hide file tree

Showing 20 changed files with 361 additions and 488 deletions.
diff --git a/doc/props_table.md b/doc/props_table.md
@@ -100,24 +100,6 @@ Example in [Patchman UI](https://github.com/RedHatInsights/patchman-ui/blob/mast
 
 Props passed to paginations components.
 
-## autoRefresh
-
-*boolean*
-
-When `true`, the table is refreshed when `customFilters` are changed.
-
-## initialLoading
-
-*boolean*
-
-When `true`, the table is in loading state on mount until `entities.loaded` is set to `false` (and from that point, `loaded` is the only determinator.). Use when users can go back to already loaded table, this prop ensures that there will be no change from `loaded` > `loading` > `loaded`.
-
-## ignoreRefresh
-
-*boolean = true*
-
-On the initial mount and when items/sortBy are changed, the inventoryTable ignores `onRefresh` prop. By setting the prop to false, you can control this behavior.
-
 ## showTagModal
 
 *boolean*

diff --git a/src/Routes.js b/src/Routes.js
@@ -123,9 +123,7 @@ export const Routes = () => {
     },
     {
       path: '/manage-edge-inventory',
-      element: (
-        <RenderWrapper cmp={InventoryTable} isRbacEnabled isImmutableTabOpen />
-      ),
+      element: <RenderWrapper cmp={InventoryTable} isImmutableTabOpen />,
     },
     {
       path: '*',

diff --git a/src/Utilities/Wrapper.js b/src/Utilities/Wrapper.js
@@ -3,13 +3,7 @@ import PropTypes from 'prop-types';
 import { usePermissionsWithContext } from '@redhat-cloud-services/frontend-components-utilities/RBACHook';
 import { GENERAL_HOSTS_READ_PERMISSIONS } from '../constants';
 
-const RenderWrapper = ({
-  cmp: Component,
-  isRbacEnabled,
-  inventoryRef,
-  store,
-  ...props
-}) => {
+const RenderWrapper = ({ cmp: Component, inventoryRef, store, ...props }) => {
   const { hasAccess } = usePermissionsWithContext(
     [GENERAL_HOSTS_READ_PERMISSIONS],
     true,
@@ -22,7 +16,6 @@ const RenderWrapper = ({
       {...(inventoryRef && {
         ref: inventoryRef,
       })}
-      isRbacEnabled={isRbacEnabled}
       hasAccess={hasAccess}
       store={store}
     />
@@ -34,7 +27,6 @@ RenderWrapper.propTypes = {
   inventoryRef: PropTypes.any,
   store: PropTypes.object,
   customRender: PropTypes.bool,
-  isRbacEnabled: PropTypes.bool,
 };
 
 export default RenderWrapper;
diff --git a/src/components/GroupSystems/GroupSystems.js b/src/components/GroupSystems/GroupSystems.js
@@ -137,7 +137,6 @@ const GroupSystems = ({ groupName, groupId }) => {
         <InventoryTable
           columns={(columns) => prepareColumns(columns, true)}
           hideFilters={{ hostGroupFilter: true }}
-          initialLoading
           getEntities={async (items, config, showTags, defaultGetEntities) =>
             await defaultGetEntities(
               items,
@@ -213,7 +212,6 @@ const GroupSystems = ({ groupName, groupId }) => {
           ref={inventory}
           showCentosVersions
           customFilters={{ filters: initialFilters, globalFilter }}
-          autoRefresh
           onRefresh={onRefresh}
           ignoreRefresh
         />

diff --git a/src/components/ImmutableDevices/ImmutableDevices.js b/src/components/ImmutableDevices/ImmutableDevices.js
@@ -38,7 +38,6 @@ const ImmutableDevices = ({
 
   return (
     <InventoryTable
-      initialLoading
       disableDefaultColumns
       onLoad={onLoad}
       hideFilters={hideFilters}
@@ -54,7 +53,6 @@ const ImmutableDevices = ({
       hasCheckbox={false}
       isFullView
       ref={inventoryRef}
-      autoRefresh
       key="inventory"
       customFilters={customFilters}
       columns={(defaultColumns) => mergeColumns(defaultColumns)}

diff --git a/src/components/InventoryGroups/Modals/AddSystemsToGroupModal.js b/src/components/InventoryGroups/Modals/AddSystemsToGroupModal.js
@@ -171,7 +171,6 @@ const AddSystemsToGroupModal = ({
         canSelectAll: false,
       }}
       bulkSelect={bulkSelectConfig}
-      initialLoading={true}
       showTags
       showCentosVersions
       showNoGroupOption

diff --git a/src/components/InventoryTable/EntityTable.js b/src/components/InventoryTable/EntityTable.js
@@ -158,7 +158,6 @@ EntityTable.propTypes = {
   hasCheckbox: PropTypes.bool,
   showActions: PropTypes.bool,
   hasItems: PropTypes.bool,
-  showHealth: PropTypes.bool,
   sortBy: PropTypes.shape({
     key: PropTypes.string,
     direction: PropTypes.oneOf(['asc', 'desc']),
@@ -186,7 +185,6 @@ EntityTable.propTypes = {
 
 EntityTable.defaultProps = {
   loaded: false,
-  showHealth: false,
   expandable: false,
   hasCheckbox: true,
   showActions: false,

diff --git a/src/components/InventoryTable/EntityTableToolbar.js b/src/components/InventoryTable/EntityTableToolbar.js
@@ -1,6 +1,6 @@
 /* eslint-disable camelcase */
 import './EntityTableToolbar.scss';
-import React, { Fragment, useEffect, useReducer } from 'react';
+import React, { Fragment, useCallback, useEffect, useReducer } from 'react';
 import { useDispatch, useSelector } from 'react-redux';
 import PropTypes from 'prop-types';
 import {
@@ -267,32 +267,17 @@ const EntityTableToolbar = ({
   /**
    * Function to dispatch load systems and fetch all tags.
    */
-  const onRefreshDataInner = (options) => {
-    if (hasAccess) {
-      onRefreshData(options);
-      if (showTags && !hasItems) {
-        dispatch(fetchAllTags(filterTagsBy, {}, getTags));
+  const onRefreshDataInner = useCallback(
+    (options) => {
+      if (hasAccess) {
+        onRefreshData(options);
+        if (showTags && !hasItems) {
+          dispatch(fetchAllTags(filterTagsBy, {}, getTags));
+        }
       }
-    }
-  };
-
-  /**
-   * Function used to update data, it either calls `onRefresh` from props or dispatches `onRefreshData`.
-   * `onRefresh` function takes two parameters
-   *   * entire config with new changes.
-   *   * callback to update data.
-   * @param {*} config new config to fetch data.
-   */
-  const updateData = (config) => {
-    if (hasAccess) {
-      onRefreshDataInner(config);
-    }
-  };
-
-  /**
-   * Debounced `updateData` function.
-   */
-  const debouncedRefresh = debounce((config) => updateData(config), 800);
+    },
+    [hasAccess]
+  );
 
   /**
    * Component did mount effect to calculate actual filters from redux.
@@ -314,7 +299,6 @@ const EntityTableToolbar = ({
       ...(customFilters?.filters || []),
     ]);
 
-    debouncedRefresh();
     enabledFilters.name && setTextFilter(textFilter);
     enabledFilters.stale && setStaleFilter(staleFilter);
     enabledFilters.registeredWith &&
@@ -334,7 +318,7 @@ const EntityTableToolbar = ({
    * @param {*} value new value used for filtering.
    * @param {*} debounced if debounce function should be used.
    */
-  const onSetTextFilter = (value, debounced = true) => {
+  const onSetTextFilter = (value) => {
     const trimmedValue = value?.trim();
 
     const textualFilter = activeFilters?.find(
@@ -347,8 +331,7 @@ const EntityTableToolbar = ({
       activeFilters?.push({ value: TEXT_FILTER, filter: trimmedValue });
     }
 
-    const refresh = debounced ? debouncedRefresh : updateData;
-    refresh({ page: 1, perPage, filters: activeFilters });
+    onRefreshDataInner({ page: 1, perPage, filters: activeFilters });
   };
 
   /**
@@ -384,7 +367,7 @@ const EntityTableToolbar = ({
 
   useEffect(() => {
     if (shouldReload && enabledFilters.stale) {
-      onSetFilter(staleFilter, 'staleFilter', debouncedRefresh);
+      onSetFilter(staleFilter, 'staleFilter', onRefreshDataInner);
     }
   }, [staleFilter]);
 
@@ -393,50 +376,50 @@ const EntityTableToolbar = ({
       onSetFilter(
         registeredWithFilter,
         'registeredWithFilter',
-        debouncedRefresh
+        onRefreshDataInner
       );
     }
   }, [registeredWithFilter]);
 
   useEffect(() => {
     if (shouldReload && showTags && enabledFilters.tags) {
-      onSetFilter(mapGroups(selectedTags), 'tagFilters', debouncedRefresh);
+      onSetFilter(mapGroups(selectedTags), 'tagFilters', onRefreshDataInner);
     }
   }, [selectedTags]);
 
   useEffect(() => {
     if (shouldReload && enabledFilters.operatingSystem) {
-      onSetFilter(osFilterValue, 'osFilter', debouncedRefresh);
+      onSetFilter(osFilterValue, 'osFilter', onRefreshDataInner);
     }
   }, [osFilterValue]);
 
   useEffect(() => {
     if (shouldReload && enabledFilters.rhcdFilter) {
-      onSetFilter(rhcdFilterValue, 'rhcdFilter', debouncedRefresh);
+      onSetFilter(rhcdFilterValue, 'rhcdFilter', onRefreshDataInner);
     }
   }, [rhcdFilterValue]);
 
   useEffect(() => {
     if (shouldReload && enabledFilters.lastSeenFilter) {
-      onSetFilter(lastSeenFilterValue, 'lastSeenFilter', debouncedRefresh);
+      onSetFilter(lastSeenFilterValue, 'lastSeenFilter', onRefreshDataInner);
     }
   }, [lastSeenFilterValue]);
 
   useEffect(() => {
     if (shouldReload && enabledFilters.updateMethodFilter) {
-      onSetFilter(updateMethodValue, 'updateMethodFilter', debouncedRefresh);
+      onSetFilter(updateMethodValue, 'updateMethodFilter', onRefreshDataInner);
     }
   }, [updateMethodValue]);
 
   useEffect(() => {
     if (shouldReload && enabledFilters.hostGroupFilter) {
-      onSetFilter(hostGroupValue, 'hostGroupFilter', debouncedRefresh);
+      onSetFilter(hostGroupValue, 'hostGroupFilter', onRefreshDataInner);
     }
   }, [hostGroupValue]);
 
   useEffect(() => {
     if (shouldReload && enabledFilters.systemTypeFilter) {
-      onSetFilter(systemTypeValue, 'systemTypeFilter', debouncedRefresh);
+      onSetFilter(systemTypeValue, 'systemTypeFilter', onRefreshDataInner);
     }
   }, [systemTypeValue]);
 
@@ -448,7 +431,7 @@ const EntityTableToolbar = ({
     [TAG_CHIP]: (deleted) =>
       setSelectedTags(
         onDeleteTag(deleted, selectedTags, (selectedTags) =>
-          onSetFilter(mapGroups(selectedTags), 'tagFilters', updateData)
+          onSetFilter(mapGroups(selectedTags), 'tagFilters', onRefreshDataInner)
         )
       ),
     [STALE_CHIP]: (deleted) =>
@@ -490,7 +473,7 @@ const EntityTableToolbar = ({
     setEndDate();
     setStartDate(oldestDate);
     dispatch(setFilter([]));
-    updateData({ page: 1, filters: [] });
+    onRefreshDataInner({ page: 1, filters: [] });
   };
 
   /**
@@ -686,7 +669,6 @@ EntityTableToolbar.propTypes = {
   paginationProps: PropTypes.object,
   loaded: PropTypes.bool,
   onRefresh: PropTypes.func,
-  hasCheckbox: PropTypes.bool,
   isLoaded: PropTypes.bool,
   items: PropTypes.array,
   sortBy: PropTypes.object,
@@ -702,7 +684,7 @@ EntityTableToolbar.propTypes = {
 
 EntityTableToolbar.defaultProps = {
   showTags: false,
-  hasAccess: true,
+  hasAccess: false,
   activeFiltersConfig: {},
   hideFilters: {},
   showNoGroupOption: false,