Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UISACQCOMP-223: ECS - Add reusable custom hooks to fix invalid reference issues related to holding names and locations #824

Merged
merged 6 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
* ECS - expand `ConsortiumFieldInventory` component with `additionalAffiliationIds` prop to display affiliation name for User without affiliation in specific tenant. Refs UISACQCOMP-220.
* Change `FundFilter` component to support multi-selection for Fund codes. Refs UISACQCOMP-221.
* Use `actionDate` value for version history card title instead of `eventDate`. Refs UISACQCOMP-222.
* ECS - Add reusable custom hooks to fix invalid reference issues related to holding names and locations. Refs UISACQCOMP-223.

## [5.1.2](https://github.com/folio-org/stripes-acq-components/tree/v5.1.2) (2024-09-13)
[Full Changelog](https://github.com/folio-org/stripes-acq-components/compare/v5.1.1...v5.1.2)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export const useConsortiumLocations = (options = {}) => {
const stripes = useStripes();
const centralTenantId = getConsortiumCentralTenantId(stripes);
const ky = useOkapiKy({ tenant: centralTenantId });
const [namespace] = useNamespace({ key: 'locations' });
const [namespace] = useNamespace({ key: 'consortium-locations' });

const {
enabled = true,
Expand Down
2 changes: 2 additions & 0 deletions lib/hooks/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export * from './useDefaultReceivingSearchSettings';
export * from './useEventEmitter';
export * from './useExchangeRateValue';
export * from './useFunds';
export * from './useHoldingsAndLocations';
export * from './useIdentifierTypes';
export * from './useInstanceHoldings';
export * from './useInstanceHoldingsQuery';
Expand All @@ -29,6 +30,7 @@ export * from './useModalToggle';
export * from './useOrderLine';
export * from './useOrganization';
export * from './usePaneFocus';
export * from './useReceivingTenantIdsAndLocations';
export * from './useShowCallout';
export * from './useTags';
export * from './useToggle';
Expand Down
1 change: 1 addition & 0 deletions lib/hooks/useHoldingsAndLocations/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { useHoldingsAndLocations } from './useHoldingsAndLocations';
80 changes: 80 additions & 0 deletions lib/hooks/useHoldingsAndLocations/useHoldingsAndLocations.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import { useQuery } from 'react-query';

import {
useNamespace,
useOkapiKy,
} from '@folio/stripes/core';

import { LIMIT_MAX } from '../../constants';
import {
getHoldingLocations,
getHoldingLocationsByTenants,
} from '../../utils';

const DEFAULT_DATA = [];

export const useHoldingsAndLocations = ({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hook's name is too general given that it requires many specific arguments.

instanceId,
options = {},
tenantId,
/*
`receivingTenantIds` is a unique list of tenantIds from the pieces list.
The purpose is that we need to be able to fetch locations from other
tenants so that we can display all the locations on the full-screen page
*/
receivingTenantIds = DEFAULT_DATA,
/*
ECS mode:
`additionalTenantLocationIdsMap` is a map of tenantId to locationIds for ECS mode.
The format can be: { tenantId: [locationId1, locationId2] }
The purpose is that we need to fetch newly added locations when we select a location
from "Create new holdings for location" modal so that the value is displayed in the selection
*/
additionalTenantLocationIdsMap = {},
/*
Non-ECS mode:
`additionalLocationIds` is a list of locationIds for the case when we need to fetch additional
locations for the selected location in the form so that the value is displayed in the selection.
*/
additionalLocationIds = [],
}) => {
const { enabled = true, ...queryOptions } = options;

const ky = useOkapiKy({ tenant: tenantId });
const [namespace] = useNamespace({ key: 'holdings-and-location' });
const queryKey = [
namespace,
tenantId,
instanceId,
...receivingTenantIds,
...additionalLocationIds,
...Object.values(additionalTenantLocationIdsMap),
];
const searchParams = {
query: `instanceId==${instanceId}`,
limit: LIMIT_MAX,
};

const {
data,
isFetching,
isLoading,
} = useQuery({
queryKey,
queryFn: ({ signal }) => {
return receivingTenantIds.length
? getHoldingLocationsByTenants({ ky, instanceId, receivingTenantIds, additionalTenantLocationIdsMap })
: getHoldingLocations({ ky, searchParams, signal, additionalLocationIds });
},
enabled: enabled && Boolean(instanceId),
...queryOptions,
});

return ({
holdings: data?.holdings || DEFAULT_DATA,
locations: data?.locations || DEFAULT_DATA,
locationIds: data?.locationIds || DEFAULT_DATA,
isFetching,
isLoading,
});
};
91 changes: 91 additions & 0 deletions lib/hooks/useHoldingsAndLocations/useHoldingsAndLocations.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import {
QueryClient,
QueryClientProvider,
} from 'react-query';

import {
renderHook,
} from '@testing-library/react-hooks';
import { useOkapiKy } from '@folio/stripes/core';

import { HOLDINGS_API } from '../../constants';
import { extendKyWithTenant } from '../../utils';
import { useHoldingsAndLocations } from './useHoldingsAndLocations';

jest.mock('../../utils', () => ({
...jest.requireActual('../../utils'),
extendKyWithTenant: jest.fn().mockReturnValue({ extend: jest.fn() }),
}));

const queryClient = new QueryClient();
const wrapper = ({ children }) => (
<QueryClientProvider client={queryClient}>
{children}
</QueryClientProvider>
);

const holdingsRecords = [
{
'id': 'd7e99892-6d7d-46eb-8a4c-3aa471785819',
'instanceId': '8804aeeb-db18-4c42-b0e9-28d63c7855e6',
'permanentLocationId': '9d1b77e8-f02e-4b7f-b296-3f2042ddac54',
},
{
'id': '5636949f-8f97-4b25-a0fc-90fdb050ffb0',
'instanceId': '8804aeeb-db18-4c42-b0e9-28d63c7855e6',
'permanentLocationId': '9d1b77e8-f02e-4b7f-b296-3f2042ddac54',
},
];

const locations = [
{
'id': '9d1b77e8-f02e-4b7f-b296-3f2042ddac54',
'name': 'DCB',
'code': '000',
},
];

const getMock = jest.fn()
.mockReturnValueOnce({ json: () => Promise.resolve({ holdingsRecords }) })
.mockReturnValue({ json: () => Promise.resolve({ locations }) });

describe('useHoldingsAndLocations', () => {
beforeEach(() => {
useOkapiKy
.mockClear()
.mockReturnValue({
get: getMock,
extend: jest.fn(() => ({
get: jest.fn((path) => {
if (path === HOLDINGS_API) {
return {
json: jest.fn().mockResolvedValue({ holdingsRecords }),
};
}

return ({
json: jest.fn().mockResolvedValue({ locations }),
});
}),
})),
});
extendKyWithTenant.mockClear().mockReturnValue({ extend: jest.fn() });
});

it('should fetch holding locations', async () => {
const { result, waitFor } = renderHook(() => useHoldingsAndLocations({ instanceId: '1', tenantId: '2' }), { wrapper });

await waitFor(() => expect(result.current.isLoading).toBeFalsy());

expect(result.current.locations).toEqual(locations);
});

it('should fetch holding locations with different tenants', async () => {
const receivingTenantIds = ['1', '2'];
const { result, waitFor } = renderHook(() => useHoldingsAndLocations({ instanceId: '1', receivingTenantIds, tenantId: '2' }), { wrapper });

await waitFor(() => expect(result.current.isLoading).toBeFalsy());

expect(result.current.locations).toHaveLength(2);
});
});
1 change: 1 addition & 0 deletions lib/hooks/useReceivingTenantIdsAndLocations/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './useReceivingTenantIdsAndLocations';
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import uniq from 'lodash/uniq';
import { useMemo } from 'react';

import { useCurrentUserTenants } from '../consortia';

export const useReceivingTenantIdsAndLocations = ({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please leave the purpose of this hook in the comments?

currentReceivingTenantId,
currentLocationId: locationId,
receivingTenantIds = [],
}) => {
const currentUserTenants = useCurrentUserTenants();

const receivingTenants = useMemo(() => {
if (receivingTenantIds.length) {
const currentUserTenantIds = currentUserTenants?.map(({ id: tenantId }) => tenantId);

// should get unique tenantIds from poLine locations and filter out tenantIds where the current user has assigned
return uniq([
...receivingTenantIds,
currentReceivingTenantId,
].filter((tenantId) => currentUserTenantIds?.includes(tenantId))
.filter(Boolean));
}

return [];
}, [receivingTenantIds, currentUserTenants, currentReceivingTenantId]);

const additionalLocations = useMemo(() => {
const locationIds = locationId ? [locationId] : [];
const tenantLocationIdsMap = currentReceivingTenantId ? { [currentReceivingTenantId]: locationIds } : {};

return {
additionalLocationIds: locationIds,
additionalTenantLocationIdsMap: tenantLocationIdsMap,
};
}, [locationId, currentReceivingTenantId]);

return {
receivingTenantIds: receivingTenants,
tenantId: currentReceivingTenantId,
...additionalLocations,
};
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { renderHook } from '@testing-library/react-hooks';

import { useReceivingTenantIdsAndLocations } from './useReceivingTenantIdsAndLocations';

jest.mock('../consortia', () => ({
useCurrentUserTenants: jest.fn(() => [{ id: 'central' }, { id: 'college' }]),
}));

describe('useReceivingTenantIdsAndLocations', () => {
it('should return receivingTenantIds', () => {
const tenants = ['central', 'college'];
const { result } = renderHook(() => useReceivingTenantIdsAndLocations({
receivingTenantIds: tenants,
currentReceivingTenantId: 'central',
}));

expect(result.current.receivingTenantIds).toEqual(tenants);
});

it('should return tenantId', () => {
const currentReceivingTenantId = 'central';

const { result } = renderHook(() => useReceivingTenantIdsAndLocations({ currentReceivingTenantId }));

expect(result.current.tenantId).toBe(currentReceivingTenantId);
});

it('should return additionalLocationIds', () => {
const { result } = renderHook(() => useReceivingTenantIdsAndLocations({}));

expect(result.current.additionalLocationIds).toEqual([]);
});
});
13 changes: 13 additions & 0 deletions lib/utils/extendKyWithTenant.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { OKAPI_TENANT_HEADER } from '../constants';

export const extendKyWithTenant = (ky, tenantId) => {
return ky.extend({
hooks: {
beforeRequest: [
request => {
request.headers.set(OKAPI_TENANT_HEADER, tenantId);
},
],
},
});
};
Loading
Loading