UIIN-1560: Improve performance on instance view (folio-org#1434)

* UIIN-1560: Improve performance on instance view
VSnehalatha · Aug 12, 2021 · 430243c · 430243c
1 parent 2866286
commit 430243c
Show file tree

Hide file tree

Showing 12 changed files with 100 additions and 95 deletions.
diff --git a/src/Instance/HoldingsList/Holding/Holding.js b/src/Instance/HoldingsList/Holding/Holding.js
@@ -51,17 +51,11 @@ const Holding = ({
           onViewHolding={onViewHolding}
           onAddItem={onAddItem}
         >
-          {
-            ({ items, handleAccordionToggle }) => (
-              <ItemsListContainer
-                holding={holding}
-                draggable={draggable}
-                droppable={droppable}
-                items={items}
-                handleAccordionToggle={handleAccordionToggle}
-              />
-            )
-          }
+          <ItemsListContainer
+            holding={holding}
+            draggable={draggable}
+            droppable={droppable}
+          />
         </HoldingAccordion>
       </DropZone>
     </div>

diff --git a/src/Instance/HoldingsList/Holding/HoldingAccordion.js b/src/Instance/HoldingsList/Holding/HoldingAccordion.js
@@ -1,5 +1,4 @@
 import React, {
-  useEffect,
   useState,
   useContext,
 } from 'react';
@@ -25,21 +24,23 @@ const HoldingAccordion = ({
   onViewHolding,
   onAddItem,
   withMoveDropdown,
-  mutator: {
-    instanceHoldingItems: {
-      reset,
-      GET,
-    },
+  resources: {
+    instanceHoldingItems,
   },
 }) => {
-  const [items, setItems] = useState();
-  const [isLoading, setIsLoading] = useState(true);
   const { locationsById } = useContext(DataContext);
   const labelLocation = holding.permanentLocationId ? locationsById[holding.permanentLocationId].name : '';
   const [open, setOpen] = useState(false);
-  const handleAccordionToggle = () => setOpen(!open);
-  const itemCount = items?.length ?? 0;
+  const [openFirstTime, setOpenFirstTime] = useState(false);
+  const handleAccordionToggle = () => {
+    if (!open && !openFirstTime) {
+      setOpenFirstTime(true);
+    }
+
+    setOpen(!open);
+  };
 
+  const itemCount = instanceHoldingItems?.other?.totalRecords;
   const holdingButtonsGroup = <HoldingButtonsGroup
     holding={holding}
     holdings={holdings}
@@ -51,16 +52,6 @@ const HoldingAccordion = ({
     isOpen={open}
   />;
 
-  useEffect(() => {
-    setIsLoading(true);
-    reset();
-    GET()
-      .then(setItems)
-      .finally(() => setIsLoading(false));
-  }, []);
-
-  if (isLoading) return null;
-
   return (
     <Accordion
       id={holding.id}
@@ -90,11 +81,14 @@ const HoldingAccordion = ({
       displayWhenOpen={holdingButtonsGroup}
       displayWhenClosed={holdingButtonsGroup}
     >
-      <Row>
-        <Col sm={12}>
-          {children({ items })}
-        </Col>
-      </Row>
+      {
+        (open || openFirstTime) &&
+        <Row>
+          <Col sm={12}>
+            {children}
+          </Col>
+        </Row>
+      }
     </Accordion>
   );
 };
@@ -106,16 +100,15 @@ HoldingAccordion.manifest = Object.freeze({
     path: 'inventory/items',
     params: {
       query: 'holdingsRecordId==!{holding.id}',
-      limit: '5000',
+      limit: '1',
     },
-    accumulate: true,
     resourceShouldRefresh: true,
     abortOnUnmount: true,
   },
 });
 
 HoldingAccordion.propTypes = {
-  mutator: PropTypes.object.isRequired,
+  resources: PropTypes.object.isRequired,
   holding: PropTypes.object.isRequired,
   onViewHolding: PropTypes.func.isRequired,
   onAddItem: PropTypes.func.isRequired,

diff --git a/src/Instance/HoldingsList/Holding/HoldingAccordion.test.js b/src/Instance/HoldingsList/Holding/HoldingAccordion.test.js
@@ -36,11 +36,13 @@ const HoldingAccordionSetup = ({
           onViewHolding={noop}
           onAddItem={noop}
           withMoveDropdown={false}
-          mutator={{
+          resources={{
             instanceHoldingItems: {
-              GET: () => new Promise(resolve => resolve(items)),
-              reset: noop,
-            },
+              records: items,
+              other: {
+                totalRecords: 3,
+              }
+            }
           }}
         >
           {() => null}

diff --git a/src/Instance/HoldingsList/Holding/HoldingButtonsGroup.js b/src/Instance/HoldingsList/Holding/HoldingButtonsGroup.js
@@ -6,6 +6,7 @@ import { IfPermission } from '@folio/stripes/core';
 import {
   Button,
   Badge,
+  Icon,
 } from '@folio/stripes/components';
 
 import { MoveToDropdown } from './MoveToDropdown';
@@ -48,7 +49,7 @@ const HoldingButtonsGroup = ({
         <FormattedMessage id="ui-inventory.addItem" />
       </Button>
     </IfPermission>
-    {!isOpen && <Badge>{itemCount}</Badge>}
+    {!isOpen && <Badge>{itemCount !== undefined ? itemCount : <Icon icon="spinner-ellipsis" width="10px" />}</Badge>}
   </>
 );
 

diff --git a/src/Instance/ItemsList/ItemsListContainer.js b/src/Instance/ItemsList/ItemsListContainer.js
@@ -1,12 +1,17 @@
 import React, { useContext, memo } from 'react';
 import PropTypes from 'prop-types';
 
+import {
+  Loading,
+} from '@folio/stripes/components';
+
 import DnDContext from '../DnDContext';
 import ItemsList from './ItemsList';
 
+import useHoldingItemsQuery from '../../hooks/useHoldingItemsQuery';
+
 const ItemsListContainer = ({
   holding,
-  items,
   draggable,
   droppable,
 }) => {
@@ -17,6 +22,11 @@ const ItemsListContainer = ({
     activeDropZone,
     isItemsDroppable,
   } = useContext(DnDContext);
+  const { isLoading, items } = useHoldingItemsQuery(holding.id);
+
+  if (isLoading) {
+    return <Loading size="large" />;
+  }
 
   return (
     <ItemsList
@@ -34,7 +44,6 @@ const ItemsListContainer = ({
 };
 
 ItemsListContainer.propTypes = {
-  items: PropTypes.arrayOf(PropTypes.object).isRequired,
   holding: PropTypes.object.isRequired,
   draggable: PropTypes.bool,
   droppable: PropTypes.bool

diff --git a/src/ViewInstance.js b/src/ViewInstance.js
@@ -31,10 +31,6 @@ import ViewHoldingsRecord from './ViewHoldingsRecord';
 import makeConnectedInstance from './ConnectedInstance';
 import withLocation from './withLocation';
 import InstancePlugin from './components/InstancePlugin';
-import {
-  batchFetchItems,
-  batchFetchRequests,
-} from './Instance/ViewRequests/utils';
 import { getPublishingInfo } from './Instance/InstanceDetails/utils';
 import { getDate } from './utils';
 import { indentifierTypeNames, layers } from './constants';
@@ -148,7 +144,10 @@ class ViewInstance extends React.Component {
 
   componentDidUpdate(prevProps) {
     const { resources: prevResources } = prevProps;
-    const { mutator, resources } = this.props;
+    const {
+      // mutator,
+      resources,
+    } = this.props;
     const instanceRecords = resources?.selectedInstance?.records;
     const instanceRecordsId = instanceRecords[0]?.id;
     const prevInstanceRecordsId = prevResources?.selectedInstance?.records[0]?.id;
@@ -168,25 +167,6 @@ class ViewInstance extends React.Component {
       // eslint-disable-next-line
       this.setState({ afterCreate: true });
     }
-
-    const { allInstanceHoldings, allInstanceItems } = resources;
-    const instanceHoldings = resources.allInstanceHoldings.records;
-    const shouldFetchItems = instanceHoldings !== prevResources.allInstanceHoldings.records ||
-      (!this.state.items && !allInstanceItems.hasLoaded && !allInstanceHoldings.isPending && !allInstanceItems.isPending);
-    if (shouldFetchItems) {
-      batchFetchItems(mutator.allInstanceItems, instanceHoldings)
-        .then(
-          (items) => {
-            this.setState({ items });
-            return batchFetchRequests(mutator.allInstanceRequests, items);
-          },
-          () => this.setState({ items: [] }),
-        )
-        .then(
-          (requests) => this.setState({ requests }),
-          () => this.setState({ requests: [] }),
-        );
-    }
   }
 
   componentWillUnmount() {
@@ -357,7 +337,7 @@ class ViewInstance extends React.Component {
       onCopy,
       stripes
     } = this.props;
-    const { items, marcRecord, requests } = this.state;
+    const { marcRecord, requests } = this.state;
     const isSourceMARC = get(instance, ['source'], '') === 'MARC';
     const canEditInstance = stripes.hasPerm('ui-inventory.instance.edit');
     const canCreateInstance = stripes.hasPerm('ui-inventory.instance.create');
@@ -488,23 +468,22 @@ class ViewInstance extends React.Component {
             </Button>
           )
         }
-        {!items?.length ? null : (
-          <Button
-            id="view-requests"
-            onClick={() => {
-              onToggle();
-              this.onClickViewRequests();
-            }}
-            buttonStyle="dropdownItem"
-          >
-            <Icon icon="eye-open">
-              <FormattedMessage
-                id="ui-inventory.viewRequests"
-                values={{ count: requests?.length ?? '?' }}
-              />
-            </Icon>
-          </Button>
-        )}
+
+        <Button
+          id="view-requests"
+          onClick={() => {
+            onToggle();
+            this.onClickViewRequests();
+          }}
+          buttonStyle="dropdownItem"
+        >
+          <Icon icon="eye-open">
+            <FormattedMessage
+              id="ui-inventory.viewRequests"
+              values={{ count: requests?.length ?? '' }}
+            />
+          </Icon>
+        </Button>
 
         <IfInterface name="copycat-imports">
           <IfPermission perm="copycat.profiles.collection.get">

diff --git a/src/constants.js b/src/constants.js
@@ -151,3 +151,5 @@ export const layers = {
 export const INSTANCES_ID_REPORT_TIMEOUT = 2000;
 
 export const QUICK_EXPORT_LIMIT = process.env.NODE_ENV !== 'test' ? 100 : 2;
+
+export const LIMIT_MAX = 5000;
diff --git a/src/hooks/useHoldingItemsQuery.js b/src/hooks/useHoldingItemsQuery.js
@@ -0,0 +1,25 @@
+import { useQuery } from 'react-query';
+
+import { useOkapiKy, useNamespace } from '@folio/stripes/core';
+
+import { LIMIT_MAX } from '../constants';
+
+const useHoldingItemsQuery = (holdingsRecordId, options = {}) => {
+  const ky = useOkapiKy();
+  const [namespace] = useNamespace();
+  const queryKey = [namespace, 'items', holdingsRecordId];
+  const searchParams = {
+    limit: LIMIT_MAX,
+    query: `holdingsRecordId==${holdingsRecordId}`,
+  };
+
+  const queryFn = () => ky.get('inventory/items', { searchParams }).json();
+  const { data, isLoading } = useQuery({ queryKey, queryFn, ...options });
+
+  return {
+    isLoading,
+    items: data?.items,
+  };
+};
+
+export default useHoldingItemsQuery;
diff --git a/translations/ui-inventory/en.json b/translations/ui-inventory/en.json
@@ -47,7 +47,7 @@
   "newInstance": "New instance",
   "editInstance": "Edit",
   "moveItems": "Move holdings/items to another instance",
-  "viewRequests": "View requests ({count})",
+  "viewRequests": "View requests",
   "instanceDetails": "Instance details",
   "addHoldings": "Add holdings",
   "itemDotStatus": "Item • {barcode} • {status}",

diff --git a/translations/ui-inventory/en_GB.json b/translations/ui-inventory/en_GB.json
@@ -570,7 +570,7 @@
     "selectHoldingsSource": "Select holdings source",
     "holdingsSourceLabel": "Source",
     "uniqueName": "Name must be unique",
-    "viewRequests": "View requests ({count})",
+    "viewRequests": "View requests",
     "instanceRecordRequestsTitle": "Requests on items - {instanceResourceTitle}, {instancePublicationDate}",
     "instanceRecordRequestsSubtitle": "{count} items found",
     "item.requestQueue": "Request queue",
@@ -679,4 +679,4 @@
     "acq.orderType": "Order type",
     "acq.orderType.One-Time": "One-time",
     "acq.orderType.Ongoing": "Ongoing"
-}
+}
diff --git a/translations/ui-inventory/en_SE.json b/translations/ui-inventory/en_SE.json
@@ -570,7 +570,7 @@
     "selectHoldingsSource": "Select holdings source",
     "holdingsSourceLabel": "Source",
     "uniqueName": "Name must be unique",
-    "viewRequests": "View requests ({count})",
+    "viewRequests": "View requests",
     "instanceRecordRequestsTitle": "Requests on items - {instanceResourceTitle}, {instancePublicationDate}",
     "instanceRecordRequestsSubtitle": "{count} items found",
     "item.requestQueue": "Request queue",
@@ -679,4 +679,4 @@
     "acq.orderType": "Order type",
     "acq.orderType.One-Time": "One-time",
     "acq.orderType.Ongoing": "Ongoing"
-}
+}
diff --git a/translations/ui-inventory/en_US.json b/translations/ui-inventory/en_US.json
@@ -570,7 +570,7 @@
     "selectHoldingsSource": "Select holdings source",
     "holdingsSourceLabel": "Source",
     "uniqueName": "Name must be unique",
-    "viewRequests": "View requests ({count})",
+    "viewRequests": "View requests",
     "instanceRecordRequestsTitle": "Requests on items - {instanceResourceTitle}, {instancePublicationDate}",
     "instanceRecordRequestsSubtitle": "{count} items found",
     "item.requestQueue": "Request queue",
@@ -679,4 +679,4 @@
     "acq.orderType": "Order type",
     "acq.orderType.One-Time": "One-time",
     "acq.orderType.Ongoing": "Ongoing"
-}
+}
Original file line number	Diff line number	Diff line change
Expand Up		@@ -151,3 +151,5 @@ export const layers = {
		export const INSTANCES_ID_REPORT_TIMEOUT = 2000;

		export const QUICK_EXPORT_LIMIT = process.env.NODE_ENV !== 'test' ? 100 : 2;

		export const LIMIT_MAX = 5000;