Merge branch 'master' into UIIN-3120

folio-org · Nov 14, 2024 · bc3d4f3 · bc3d4f3
2 parents a2c9b21 + afaefb5
commit bc3d4f3
Show file tree

Hide file tree

Showing 11 changed files with 193 additions and 106 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,10 @@
 ## [12.0.1] (IN PROGRESS)
 
 * Run `history.replace` once during component mount and update to avoid URL rewriting. Refs UIIN-3099.
+* ECS | "FOLIO/MARC-shared" source is displayed in manually shared instance record. Fixes UIIN-3115.
+* Allow user to move item to another holdings associated with another instance. Fixes UIIN-3102.
+* ECS | Instance details pane does not contain all tenants when user does not have affiliations / permissions in all tenants. Fixes UIIN-3113.
+* Cautiously evaluate tenant permissions to avoid NPEs. Refs UIIN-3124.
 
 ## [12.0.0](https://github.com/folio-org/ui-inventory/tree/v12.0.0) (2024-10-31)
 [Full Changelog](https://github.com/folio-org/ui-inventory/compare/v11.0.5...v12.0.0)

diff --git a/src/Instance/MoveHoldingContext/MoveHoldingContext.js b/src/Instance/MoveHoldingContext/MoveHoldingContext.js
@@ -8,28 +8,38 @@ import {
 } from 'react';
 import { DragDropContext } from 'react-beautiful-dnd';
 import { useIntl } from 'react-intl';
+import isEmpty from 'lodash/isEmpty';
 
 import {
   Loading,
   MessageBanner,
 } from '@folio/stripes/components';
-import { useOkapiKy } from '@folio/stripes/core';
+import {
+  useOkapiKy,
+  useStripes
+} from '@folio/stripes/core';
 import { LINES_API, LIMIT_MAX } from '@folio/stripes-acq-components';
 
 import {
   callNumberLabel
 } from '../../utils';
 import { DataContext } from '../../contexts';
-import { useHoldings, useInstanceHoldingsQuery } from '../../providers';
+import {
+  useHoldings,
+  useInstanceHoldingsQuery,
+} from '../../providers';
 import * as RemoteStorage from '../../RemoteStorageService';
 
 import {
   isItemsSelected,
   selectItems,
 } from '../utils';
 import DnDContext from '../DnDContext';
-import { HoldingContainer } from '../HoldingsList';
-import * as Move from '../Move';
+import { HoldingsList } from '../HoldingsList';
+import {
+  useItems,
+  ConfirmationModal
+} from '../Move';
 import { getPOLineHoldingIds } from './utils';
 
 const MoveHoldingContext = ({
@@ -40,12 +50,13 @@ const MoveHoldingContext = ({
 }) => {
   const intl = useIntl();
   const ky = useOkapiKy();
+  const stripes = useStripes();
 
   const { locationsById } = useContext(DataContext);
   const { holdingsById } = useHoldings();
 
   const checkFromRemoteToNonRemote = RemoteStorage.Check.useByHoldings();
-  const { moveItems, isMoving: isItemsMoving } = Move.useItems();
+  const { moveItems, isMoving: isItemsMoving } = useItems();
 
   const [isMoving, setIsMoving] = useState(false);
   const [selectedItemsMap, setSelectedItemsMap] = useState({});
@@ -172,6 +183,13 @@ const MoveHoldingContext = ({
   }, [setIsModalOpen]);
 
   const checkHasMultiplePOLsOrHoldings = async (holdingIds = []) => {
+    if (isEmpty(holdingIds)) {
+      return {
+        hasLinkedPOLs: false,
+        poLineHoldingIds: [],
+      };
+    }
+
     try {
       const { poLines = [] } = await ky.get(LINES_API, {
         searchParams: {
@@ -181,7 +199,7 @@ const MoveHoldingContext = ({
       }).json();
 
       return {
-        hasLinkedPOLs:  poLines.length > 1 || poLines[0]?.locations?.length > 1,
+        hasLinkedPOLs: poLines.length > 1 || poLines[0]?.locations?.length > 1,
         poLineHoldingIds: getPOLineHoldingIds(poLines, holdingIds),
       };
     } catch (error) {
@@ -219,88 +237,80 @@ const MoveHoldingContext = ({
 
     const holdingIdsToMove = uniq([...poLineHoldingIds, ...holdingIdsFromSelection]);
 
-    setMovingItems(holdingIdsToMove);
+    setMovingItems(isHolding || hasLinkedPOLs ? holdingIdsToMove : items);
     setHasLinkedPOLsOrHoldings(hasLinkedPOLs);
     setSelectedHoldingIds(holdingIdsToMove);
     setIsModalOpen(true);
   }, [selectedItemsMap, leftInstance, rightHoldings, leftHoldings, selectedHoldingsMap, checkHasMultiplePOLsOrHoldings]);
 
-  const movingMessage = useMemo(
-    () => {
-      const targetHolding = holdingsById[dragToId];
-      const callNumber = callNumberLabel(targetHolding);
-      const labelLocation = targetHolding?.permanentLocationId ? locationsById[targetHolding.permanentLocationId]?.name : '';
-      const holdings = allHoldings.filter(holding => selectedHoldingIds.includes(holding.id));
-
-      const count = movingItems.length;
-
-      if (hasLinkedPOLsOrHoldings) {
-        return (
-          <>
-            { intl.formatMessage(
-              { id: 'ui-inventory.moveItems.modal.message.hasLinkedPOLsOrHoldings' },
-            )}
-            {
-              selectedHoldingIds.map(holdingId => (
-                <HoldingContainer
-                  key={holdingId}
-                  holding={allHoldings.find(holding => holding.id === holdingId)}
-                  holdings={holdings}
-                  isDraggable={false}
-                  instance={rightInstance.id === dragToId ? rightInstance : leftInstance}
-                />
-              ))
-            }
-          </>
-        );
-      }
+  const movingMessage = useMemo(() => {
+    const targetHolding = holdingsById[dragToId];
+    const callNumber = callNumberLabel(targetHolding);
+    const labelLocation = targetHolding?.permanentLocationId ? locationsById[targetHolding.permanentLocationId]?.name : '';
+    const holdings = allHoldings.filter(holding => selectedHoldingIds.includes(holding.id));
+    const currentInstance = rightInstance.id === dragToId ? rightInstance : leftInstance;
 
-      if (isHoldingMoved) {
-        return intl.formatMessage(
-          { id: 'ui-inventory.moveItems.modal.message.holdings' },
-          {
-            count,
-            targetName: <b>{rightInstance.id === dragToId ? rightInstance.title : leftInstance.title}</b>
-          }
-        );
-      }
+    const count = movingItems.length;
 
-      const moveMsg = intl.formatMessage(
-        { id: 'ui-inventory.moveItems.modal.message.items' },
+    if (hasLinkedPOLsOrHoldings) {
+      return (
+        <>
+          { intl.formatMessage(
+            { id: 'ui-inventory.moveItems.modal.message.hasLinkedPOLsOrHoldings' },
+          )}
+          <HoldingsList
+            holdings={holdings}
+            instance={currentInstance}
+            tenantId={stripes.okapi?.tenant}
+          />
+        </>
+      );
+    }
+
+    if (isHoldingMoved) {
+      return intl.formatMessage(
+        { id: 'ui-inventory.moveItems.modal.message.holdings' },
         {
           count,
-          targetName: <b>{`${labelLocation} ${callNumber}`}</b>
+          targetName: <b>{rightInstance.id === dragToId ? rightInstance.title : leftInstance.title}</b>
         }
       );
+    }
 
-      return (
-        <>
-          {moveMsg}
-          <MessageBanner
-            show={checkFromRemoteToNonRemote({ fromHoldingsId: dragFromId, toHoldingsId: dragToId })}
-            type="warning"
-          >
-            <RemoteStorage.Confirmation.Message count={count} />
-          </MessageBanner>
-        </>
-      );
-    },
-    [
-      holdingsById,
-      dragToId,
-      locationsById,
-      movingItems.length,
-      isHoldingMoved,
-      intl,
-      checkFromRemoteToNonRemote,
-      dragFromId,
-      rightInstance.id,
-      rightInstance.title,
-      leftInstance.title,
-      allHoldings,
-      selectedHoldingIds,
-    ],
-  );
+    const moveMsg = intl.formatMessage(
+      { id: 'ui-inventory.moveItems.modal.message.items' },
+      {
+        count,
+        targetName: <b>{`${labelLocation} ${callNumber}`}</b>
+      }
+    );
+
+    return (
+      <>
+        {moveMsg}
+        <MessageBanner
+          show={checkFromRemoteToNonRemote({ fromHoldingsId: dragFromId, toHoldingsId: dragToId })}
+          type="warning"
+        >
+          <RemoteStorage.Confirmation.Message count={count} />
+        </MessageBanner>
+      </>
+    );
+  }, [
+    holdingsById,
+    dragToId,
+    locationsById,
+    movingItems.length,
+    isHoldingMoved,
+    intl,
+    checkFromRemoteToNonRemote,
+    dragFromId,
+    rightInstance.id,
+    rightInstance.title,
+    leftInstance.title,
+    allHoldings,
+    selectedHoldingIds,
+  ]);
 
   if (isMoving || isItemsMoving) {
     return <Loading size="large" />;
@@ -333,7 +343,7 @@ const MoveHoldingContext = ({
         }}
       >
         {children}
-        <Move.ConfirmationModal
+        <ConfirmationModal
           id="move-holding-confirmation"
           message={movingMessage}
           onCancel={closeModal}

diff --git a/src/common/hooks/useInstance.js b/src/common/hooks/useInstance.js
@@ -3,18 +3,23 @@ import {
   useMemo,
 } from 'react';
 
+import { useStripes } from '@folio/stripes/core';
+
 import useSearchInstanceByIdQuery from './useSearchInstanceByIdQuery';
 import useInstanceQuery from './useInstanceQuery';
 
 const useInstance = (id) => {
+  const stripes = useStripes();
+  const centralTenantId = stripes.user.user?.consortium?.centralTenantId;
+
   const {
     refetch: refetchSearch,
     isLoading: isSearchInstanceByIdLoading,
     instance: _instance,
   } = useSearchInstanceByIdQuery(id);
 
-  const instanceTenantId = _instance?.tenantId;
   const isShared = _instance?.shared;
+  const instanceTenantId = isShared ? centralTenantId : _instance?.tenantId;
 
   const {
     refetch: refetchInstance,
@@ -26,7 +31,7 @@ const useInstance = (id) => {
   } = useInstanceQuery(
     id,
     { tenantId: instanceTenantId },
-    { enabled: Boolean(id && !isSearchInstanceByIdLoading) }
+    { enabled: Boolean(id && !isSearchInstanceByIdLoading && instanceTenantId) }
   );
 
   const instance = useMemo(

diff --git a/src/hooks/useMemberTenantHoldings/useMemberTenantHoldings.js b/src/hooks/useMemberTenantHoldings/useMemberTenantHoldings.js
@@ -11,10 +11,14 @@ const useMemberTenantHoldings = (instance, tenantId, userTenantPermissions) => {
   const { holdingsRecords: expandedHoldings, isLoading: isExpandedHoldingsLoading } = useInstanceHoldingsQuery(instance?.id, { tenantId, enabled: canViewHoldings });
   const { holdings: limitedHoldings, isLoading: isLimitedHoldingsLoading } = useConsortiumHoldings(instance?.id, tenantId, { enabled: !canViewHoldings });
 
-  const holdings = useMemo(() => expandedHoldings || limitedHoldings || [],
-    [expandedHoldings, limitedHoldings, isExpandedHoldingsLoading, isLimitedHoldingsLoading]);
-  const isLoading = useMemo(() => isExpandedHoldingsLoading || isLimitedHoldingsLoading,
-    [isExpandedHoldingsLoading, isLimitedHoldingsLoading]);
+  const holdings = useMemo(
+    () => (expandedHoldings?.length ? expandedHoldings : limitedHoldings?.length ? limitedHoldings : []),
+    [expandedHoldings, limitedHoldings, isExpandedHoldingsLoading, isLimitedHoldingsLoading],
+  );
+  const isLoading = useMemo(
+    () => isExpandedHoldingsLoading || isLimitedHoldingsLoading,
+    [isExpandedHoldingsLoading, isLimitedHoldingsLoading],
+  );
 
   return {
     holdings,

diff --git a/src/utils.js b/src/utils.js
@@ -749,13 +749,32 @@ export const isUserInConsortiumMode = stripes => stripes.hasInterface('consortia
 
 export const isInstanceShadowCopy = (source) => [`${CONSORTIUM_PREFIX}FOLIO`, `${CONSORTIUM_PREFIX}MARC`].includes(source);
 
+/**
+ * hasMemberTenantPermission
+ * return true if permissionName is present in the array of permissions for
+ * the given tenant. permissionName may match a top-level permissionName or a
+ * permission's subPermissions.
+ *
+ * @param {string} permissionName
+ * @param {string} tenantId
+ * @param {array} permissions array of objects shaped like
+ * {
+ *   permissionNames: [{
+ *     permissionName: 'string',
+ *     subPermissions: ['string'],
+ *   }],
+ *   tenantId: 'string'
+ * }
+ *
+ * @returns true if permissions contains a value matching the given permissionName and tenant
+ */
 export const hasMemberTenantPermission = (permissionName, tenantId, permissions = []) => {
   const tenantPermissions = permissions?.find(permission => permission?.tenantId === tenantId)?.permissionNames || [];
 
   const hasPermission = tenantPermissions?.some(tenantPermission => tenantPermission?.permissionName === permissionName);
 
   if (!hasPermission) {
-    return tenantPermissions.some(tenantPermission => tenantPermission.subPermissions.includes(permissionName));
+    return tenantPermissions.some(tenantPermission => tenantPermission.subPermissions?.includes(permissionName));
   }
 
   return hasPermission;

diff --git a/src/utils.test.js b/src/utils.test.js
@@ -8,6 +8,7 @@ import { OKAPI_TENANT_HEADER } from '@folio/stripes-inventory-components';
 import buildStripes from '../test/jest/__mock__/stripesCore.mock';
 
 import {
+  hasMemberTenantPermission,
   validateRequiredField,
   validateFieldLength,
   validateNumericField,
@@ -445,3 +446,47 @@ describe('marshalInstance', () => {
     });
   });
 });
+
+describe('hasMemberTenantPermission', () => {
+  it('returns false without permissions', () => {
+    expect(hasMemberTenantPermission('foo', 'tenant')).toBe(false);
+  });
+
+  it('returns false with empty permissions', () => {
+    expect(hasMemberTenantPermission('foo', 'tenant', [])).toBe(false);
+  });
+
+  it('returns false if tenant does not match', () => {
+    expect(hasMemberTenantPermission(
+      'foo',
+      'tenant',
+      [{ permissionNames: [{ permissionName: 'foo' }], tenantId: 'asdf' }]
+    )).toBe(false);
+  });
+
+  describe('when tenant matches', () => {
+    it('returns true if matching permissionName is present', () => {
+      expect(hasMemberTenantPermission(
+        'foo',
+        'tenant',
+        [{ permissionNames: [{ permissionName: 'foo' }], tenantId: 'tenant' }]
+      )).toBe(true);
+    });
+
+    it('returns true if matching subPermissions entry is present', () => {
+      expect(hasMemberTenantPermission(
+        'foo',
+        'tenant',
+        [{ permissionNames: [{ permissionName: 'bar', subPermissions: ['foo'] }], tenantId: 'tenant' }]
+      )).toBe(true);
+    });
+
+    it('returns false if no matching permissions entry is present', () => {
+      expect(hasMemberTenantPermission(
+        'foo',
+        'tenant',
+        [{ permissionNames: [{ permissionName: 'bar' }], tenantId: 'tenant' }]
+      )).toBe(false);
+    });
+  });
+});