diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ff4e1896..3b28dad54 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ * Create new instance success toast no longer shows the instance HRID. Fixes UIIN-2635. * Optimistic locking message not working for instances in non-consortial tenant. Fixes UIIN-2628. * Add immediate warning message when a local instance is shared. Refs UIIN-2617. +* ECS: Show info message when user in member tenant tries to view shared instance details without permission. Refs UIIN-2590. ## [10.0.1](https://github.com/folio-org/ui-inventory/tree/v10.0.1) (2023-11-03) [Full Changelog](https://github.com/folio-org/ui-inventory/compare/v10.0.0...v10.0.1) diff --git a/src/Instance/InstanceDetails/InstanceDetails.js b/src/Instance/InstanceDetails/InstanceDetails.js index 5f41c0c18..ee39f3d1f 100644 --- a/src/Instance/InstanceDetails/InstanceDetails.js +++ b/src/Instance/InstanceDetails/InstanceDetails.js @@ -19,7 +19,6 @@ import { PaneMenu, Row, MessageBanner, - Icon, } from '@folio/stripes/components'; import { InstanceTitle } from './InstanceTitle'; @@ -71,8 +70,6 @@ const InstanceDetails = forwardRef(({ tagsEnabled, userTenantPermissions, isShared, - isLoading, - isInstanceSharing, ...rest }, ref) => { const intl = useIntl(); @@ -86,15 +83,12 @@ const InstanceDetails = forwardRef(({ const accordionState = useMemo(() => getAccordionState(instance, accordions), [instance]); const [helperApp, setHelperApp] = useState(); - const isBasicPane = isInstanceSharing || isLoading; + const canCreateHoldings = stripes.hasPerm('ui-inventory.holdings.create'); const tags = instance?.tags?.tagList; const isUserInCentralTenant = checkIfUserInCentralTenant(stripes); - - const canCreateHoldings = stripes.hasPerm('ui-inventory.holdings.create'); + const isConsortialHoldingsVisible = instance?.shared || isInstanceShadowCopy(instance?.source); const detailsLastMenu = useMemo(() => { - if (isBasicPane) return null; - return ( { @@ -110,15 +104,9 @@ const InstanceDetails = forwardRef(({ } ); - }, [isBasicPane, tagsEnabled, tags, intl]); - const detailsActionMenu = useMemo( - () => (isBasicPane ? null : actionMenu), - [isBasicPane, actionMenu], - ); + }, [tagsEnabled, tags, intl]); const renderPaneTitle = () => { - if (isBasicPane) return intl.formatMessage({ id: 'ui-inventory.edit' }); - const isInstanceShared = Boolean(isShared || isInstanceShadowCopy(instance?.source)); return ( @@ -134,8 +122,6 @@ const InstanceDetails = forwardRef(({ }; const renderPaneSubtitle = () => { - if (isBasicPane) return null; - return ( { - if (isInstanceSharing) { - return ( -
- - - -
- ); - } - - if (isLoading) { - return ( -
- -
- ); - } - - const isConsortialHoldingsVisible = instance?.shared || isInstanceShadowCopy(instance?.source); - - return ( - <> + return ( + <> + } + paneTitle={renderPaneTitle()} + paneSub={renderPaneSubtitle()} + actionMenu={actionMenu} + lastMenu={detailsLastMenu} + dismissible + onClose={onClose} + defaultWidth="fill" + > @@ -293,25 +267,6 @@ const InstanceDetails = forwardRef(({ /> - - ); - }; - - return ( - <> - } - paneTitle={renderPaneTitle()} - paneSub={renderPaneSubtitle()} - actionMenu={detailsActionMenu} - lastMenu={detailsLastMenu} - dismissible - onClose={onClose} - defaultWidth="fill" - > - {renderDetails()} { helperApp && } @@ -326,16 +281,12 @@ InstanceDetails.propTypes = { tagsToggle: PropTypes.func, tagsEnabled: PropTypes.bool, userTenantPermissions: PropTypes.arrayOf(PropTypes.object), - isLoading: PropTypes.bool, - isInstanceSharing: PropTypes.bool, isShared: PropTypes.bool, }; InstanceDetails.defaultProps = { instance: {}, tagsEnabled: false, - isLoading: false, - isInstanceSharing: false, isShared: false, }; diff --git a/src/Instance/InstanceDetails/InstanceDetails.test.js b/src/Instance/InstanceDetails/InstanceDetails.test.js index e8cd22f7e..daf483dcd 100644 --- a/src/Instance/InstanceDetails/InstanceDetails.test.js +++ b/src/Instance/InstanceDetails/InstanceDetails.test.js @@ -112,13 +112,6 @@ describe('InstanceDetails', () => { expect(screen.getByText('Instance relationship (analytics and bound-with)')).toBeInTheDocument(); }); - it('should show a correct Warning message banner when instance sharing is in progress', () => { - renderInstanceDetails({ isInstanceSharing: true }); - - expect(screen.getByText('Sharing this local instance will take a few moments.' + - ' A success message and updated details will be displayed upon completion.')).toBeInTheDocument(); - }); - it('should show a correct Warning message banner when staff suppressed', () => { const staffSuppressedInstance = { ...instance, diff --git a/src/Instance/InstanceDetails/InstanceLoadingPane/InstanceLoadingPane.js b/src/Instance/InstanceDetails/InstanceLoadingPane/InstanceLoadingPane.js new file mode 100644 index 000000000..28d33c5eb --- /dev/null +++ b/src/Instance/InstanceDetails/InstanceLoadingPane/InstanceLoadingPane.js @@ -0,0 +1,33 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { useIntl } from 'react-intl'; + +import { AppIcon } from '@folio/stripes/core'; +import { + Pane, + Icon, +} from '@folio/stripes/components'; + +const InstanceLoadingPane = ({ onClose }) => { + const intl = useIntl(); + + return ( + } + paneTitle={intl.formatMessage({ id: 'ui-inventory.edit' })} + dismissible + onClose={onClose} + defaultWidth="fill" + > + + + ); +}; + +InstanceLoadingPane.propTypes = { onClose: PropTypes.func.isRequired }; + +export default InstanceLoadingPane; diff --git a/src/Instance/InstanceDetails/InstanceLoadingPane/InstanceLoadingPane.test.js b/src/Instance/InstanceDetails/InstanceLoadingPane/InstanceLoadingPane.test.js new file mode 100644 index 000000000..a8f1e7bc8 --- /dev/null +++ b/src/Instance/InstanceDetails/InstanceLoadingPane/InstanceLoadingPane.test.js @@ -0,0 +1,53 @@ +import { + fireEvent, + screen, +} from '@folio/jest-config-stripes/testing-library/react'; + +import '../../../../test/jest/__mock__'; + +import { Icon } from '@folio/stripes/components'; + +import { + renderWithIntl, + translationsProperties, +} from '../../../../test/jest/helpers'; + +import InstanceLoadingPane from './InstanceLoadingPane'; + +Icon.mockClear().mockImplementation(({ icon }) => {icon}); + +const mockOnClose = jest.fn(); + +const renderInstanceLoadingPane = () => { + const component = ; + + return renderWithIntl(component, translationsProperties); +}; + +describe('InstanceLoadingPane', () => { + it('should render loading spinner', () => { + renderInstanceLoadingPane(); + + expect(screen.getByText('spinner-ellipsis')).toBeInTheDocument(); + }); + + it('should render correct header', () => { + renderInstanceLoadingPane(); + + expect(screen.getByText('Edit')).toBeInTheDocument(); + }); + + it('should not render action menu', () => { + renderInstanceLoadingPane(); + + expect(screen.queryByRole('button', { name: 'Actions' })).not.toBeInTheDocument(); + }); + + it('should call onClose cb when pane is closed', () => { + renderInstanceLoadingPane(); + + fireEvent.click(screen.getByRole('button', { name: 'Close Edit' })); + + expect(mockOnClose).toHaveBeenCalled(); + }); +}); diff --git a/src/Instance/InstanceDetails/InstanceLoadingPane/index.js b/src/Instance/InstanceDetails/InstanceLoadingPane/index.js new file mode 100644 index 000000000..29e7c58ee --- /dev/null +++ b/src/Instance/InstanceDetails/InstanceLoadingPane/index.js @@ -0,0 +1 @@ +export { default } from './InstanceLoadingPane'; diff --git a/src/Instance/InstanceDetails/InstanceWarningPane/InstanceWarningPane.js b/src/Instance/InstanceDetails/InstanceWarningPane/InstanceWarningPane.js new file mode 100644 index 000000000..8762ddf3e --- /dev/null +++ b/src/Instance/InstanceDetails/InstanceWarningPane/InstanceWarningPane.js @@ -0,0 +1,38 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { useIntl } from 'react-intl'; + +import { AppIcon } from '@folio/stripes/core'; +import { + Pane, + MessageBanner, +} from '@folio/stripes/components'; + +const InstanceWarningPane = ({ + onClose, + messageBannerText, +}) => { + const intl = useIntl(); + + return ( + } + paneTitle={intl.formatMessage({ id: 'ui-inventory.edit' })} + dismissible + onClose={onClose} + defaultWidth="fill" + > + + {messageBannerText} + + + ); +}; + +InstanceWarningPane.propTypes = { + onClose: PropTypes.func.isRequired, + messageBannerText: PropTypes.oneOfType([PropTypes.element, PropTypes.string]).isRequired, +}; + +export default InstanceWarningPane; diff --git a/src/Instance/InstanceDetails/InstanceWarningPane/InstanceWarningPane.test.js b/src/Instance/InstanceDetails/InstanceWarningPane/InstanceWarningPane.test.js new file mode 100644 index 000000000..bb4603a2d --- /dev/null +++ b/src/Instance/InstanceDetails/InstanceWarningPane/InstanceWarningPane.test.js @@ -0,0 +1,58 @@ +import { + fireEvent, + screen, +} from '@folio/jest-config-stripes/testing-library/react'; + +import '../../../../test/jest/__mock__'; + +import { Icon } from '@folio/stripes/components'; + +import { + renderWithIntl, + translationsProperties, +} from '../../../../test/jest/helpers'; + +import InstanceWarningPane from './InstanceWarningPane'; + +Icon.mockClear().mockImplementation(({ icon }) => {icon}); + +const mockOnClose = jest.fn(); + +const renderInstanceLoadingPane = (messageBannerText = 'warning text') => { + const component = ( + + ); + + return renderWithIntl(component, translationsProperties); +}; + +describe('InstanceWarningPane', () => { + it('should render warning banner', () => { + renderInstanceLoadingPane(); + + expect(screen.getByText('warning text')).toBeInTheDocument(); + }); + + it('should render correct header', () => { + renderInstanceLoadingPane(); + + expect(screen.getByText('Edit')).toBeInTheDocument(); + }); + + it('should not render action menu', () => { + renderInstanceLoadingPane(); + + expect(screen.queryByRole('button', { name: 'Actions' })).not.toBeInTheDocument(); + }); + + it('should call onClose cb when pane is closed', () => { + renderInstanceLoadingPane(); + + fireEvent.click(screen.getByRole('button', { name: 'Close Edit' })); + + expect(mockOnClose).toHaveBeenCalled(); + }); +}); diff --git a/src/Instance/InstanceDetails/InstanceWarningPane/index.js b/src/Instance/InstanceDetails/InstanceWarningPane/index.js new file mode 100644 index 000000000..ab6da1637 --- /dev/null +++ b/src/Instance/InstanceDetails/InstanceWarningPane/index.js @@ -0,0 +1 @@ +export { default } from './InstanceWarningPane'; diff --git a/src/Instance/InstanceDetails/index.js b/src/Instance/InstanceDetails/index.js index 065a56d8f..be1ebdbf8 100644 --- a/src/Instance/InstanceDetails/index.js +++ b/src/Instance/InstanceDetails/index.js @@ -11,6 +11,8 @@ export * from './InstanceTitle'; export * from './InstanceNotesView'; export * from './InstanceNewHolding'; +export { default as InstanceLoadingPane } from './InstanceLoadingPane'; +export { default as InstanceWarningPane } from './InstanceWarningPane'; export { default as SubInstanceList } from './SubInstanceList'; export { default as InstanceDetails } from './InstanceDetails'; export { default as SubInstanceGroup } from './SubInstanceGroup'; diff --git a/src/ViewInstance.js b/src/ViewInstance.js index f687c918a..2ae2cd82a 100644 --- a/src/ViewInstance.js +++ b/src/ViewInstance.js @@ -41,6 +41,7 @@ import { } from './utils'; import { AUTHORITY_LINKED_FIELDS, + HTTP_RESPONSE_STATUS_CODES, indentifierTypeNames, INSTANCE_SHARING_STATUSES, layers, @@ -53,6 +54,8 @@ import { HoldingsListContainer, MoveItemsContext, InstanceDetails, + InstanceWarningPane, + InstanceLoadingPane, } from './Instance'; import { ActionItem, @@ -884,11 +887,18 @@ class ViewInstance extends React.Component { isCentralTenantPermissionsLoading, isShared, isLoading, + isError, + error, } = this.props; const { linkedAuthoritiesLength, isInstanceSharing, } = this.state; + + const isUserLacksPermToViewSharedInstance = isError + && error?.response?.status === HTTP_RESPONSE_STATUS_CODES.FORBIDDEN + && isShared; + const ci = makeConnectedInstance(this.props, stripes.logger); const instance = ci.instance(); @@ -925,6 +935,28 @@ class ViewInstance extends React.Component { const isInstanceLoading = isLoading || !instance || isCentralTenantPermissionsLoading; const keyInStorageToHoldingsAccsState = ['holdings']; + if (isUserLacksPermToViewSharedInstance) { + return ( + } + /> + ); + } + + if (isInstanceSharing) { + return ( + } + /> + ); + } + + if (isInstanceLoading) { + return ; + } + return ( {data => ( @@ -941,8 +973,6 @@ class ViewInstance extends React.Component { tagsEnabled={tagsEnabled} ref={this.accordionStatusRef} userTenantPermissions={this.state.userTenantPermissions} - isLoading={isInstanceLoading} - isInstanceSharing={isInstanceSharing} isShared={isShared} > { @@ -1099,6 +1129,8 @@ ViewInstance.propTypes = { tagsEnabled: PropTypes.bool, updateLocation: PropTypes.func.isRequired, isLoading: PropTypes.bool, + isError: PropTypes.bool, + error: PropTypes.object, }; export default flowRight( diff --git a/src/ViewInstance.test.js b/src/ViewInstance.test.js index 188eb8000..0a6aa4779 100644 --- a/src/ViewInstance.test.js +++ b/src/ViewInstance.test.js @@ -18,7 +18,10 @@ import { checkIfUserInMemberTenant, checkIfUserInCentralTenant, } from '@folio/stripes/core'; -import { ConfirmationModal } from '@folio/stripes/components'; +import { + ConfirmationModal, + Icon, +} from '@folio/stripes/components'; import { instances } from '../test/fixtures/instances'; import { DataContext } from './contexts'; @@ -103,6 +106,8 @@ jest .mockImplementation(() => instance) .mockImplementationOnce(() => {}); +Icon.mockClear().mockImplementation(({ children, icon }) => (children || {icon})); + ConfirmationModal.mockImplementation(({ open, onCancel, @@ -286,6 +291,35 @@ describe('ViewInstance', () => { }); useStripes().hasInterface.mockReturnValue(true); }); + + it('should show a correct Warning message banner when instance sharing is in progress', async () => { + defaultProp.mutator.shareInstance.POST.mockResolvedValue({}); + checkIfUserInMemberTenant.mockClear().mockReturnValue(true); + + renderViewInstance({ isShared: false }); + + fireEvent.click(screen.getByRole('button', { name: 'Actions' })); + fireEvent.click(screen.getByRole('button', { name: 'Share local instance' })); + fireEvent.click(screen.getByRole('button', { name: 'Confirm' })); + + await waitFor(() => expect(screen.getByText('Sharing this local instance will take a few moments.' + + ' A success message and updated details will be displayed upon completion.')).toBeInTheDocument()); + }); + it('should show a correct Warning message banner when user lacks permission to vew shared instance', () => { + renderViewInstance({ + isError: true, + isShared: true, + error: { response: { status: 403 } }, + }); + + expect(screen.getByText('You do not currently have permission to access details of shared instances. ' + + 'Contact your FOLIO administrator for more information.')).toBeInTheDocument(); + }); + it('should show loading spinner when instance is loading', () => { + renderViewInstance({ isLoading: true }); + + expect(screen.getByText('spinner-ellipsis')).toBeInTheDocument(); + }); it('should display action menu items', () => { renderViewInstance(); expect(screen.getByText('Move items within an instance')).toBeInTheDocument(); diff --git a/src/ViewInstanceWrapper.js b/src/ViewInstanceWrapper.js index 170ed5ef1..fddc7c78a 100644 --- a/src/ViewInstanceWrapper.js +++ b/src/ViewInstanceWrapper.js @@ -17,7 +17,13 @@ const ViewInstanceWrapper = (props) => { const userId = stripes?.user?.user?.id; const centralTenantId = stripes.user.user?.consortium?.centralTenantId; const consortiumId = stripes.user.user?.consortium?.id; - const { instance, isLoading, refetch } = useInstance(id); + const { + instance, + isLoading, + refetch, + isError, + error, + } = useInstance(id); const isShared = Boolean(instance?.shared); const tenantId = instance?.tenantId ?? stripes.okapi.tenant; @@ -42,6 +48,8 @@ const ViewInstanceWrapper = (props) => { refetchInstance={refetch} selectedInstance={instance} isLoading={isLoading} + isError={isError} + error={error} centralTenantPermissions={centralTenantPermissions} isCentralTenantPermissionsLoading={isCentralTenantPermissionsLoading} /> diff --git a/src/common/hooks/useInstance.js b/src/common/hooks/useInstance.js index 686408d17..585cb136d 100644 --- a/src/common/hooks/useInstance.js +++ b/src/common/hooks/useInstance.js @@ -4,12 +4,20 @@ import useSearchInstanceByIdQuery from './useSearchInstanceByIdQuery'; import useInstanceQuery from './useInstanceQuery'; const useInstance = (id) => { - const { isLoading: isSearchInstanceByIdLoading, instance: _instance } = useSearchInstanceByIdQuery(id); + const { + isLoading: isSearchInstanceByIdLoading, + instance: _instance, + } = useSearchInstanceByIdQuery(id); const instanceTenantId = _instance?.tenantId; const isShared = _instance?.shared; - const { isLoading: isInstanceLoading, instance: data, refetch } = useInstanceQuery( + const { + isLoading: isInstanceLoading, + instance: data, + refetch, + ...rest + } = useInstanceQuery( id, { tenantId: instanceTenantId }, { enabled: Boolean(id && !isSearchInstanceByIdLoading) } @@ -32,6 +40,7 @@ const useInstance = (id) => { instance, isLoading, refetch, + ...rest, }; }; diff --git a/src/common/hooks/useInstanceQuery/useInstanceQuery.js b/src/common/hooks/useInstanceQuery/useInstanceQuery.js index 1dc296331..4e478348a 100644 --- a/src/common/hooks/useInstanceQuery/useInstanceQuery.js +++ b/src/common/hooks/useInstanceQuery/useInstanceQuery.js @@ -8,7 +8,12 @@ const useInstanceQuery = (instanceId, { tenantId = '' } = {}, options = {}) => { const ky = useTenantKy({ tenantId }); const [namespace] = useNamespace({ key: 'instance' }); - const { isLoading, data: instance = {}, refetch } = useQuery( + const { + isLoading, + data: instance = {}, + refetch, + ...rest + } = useQuery( [namespace, instanceId, tenantId], () => ky.get(`inventory/instances/${instanceId}`).json(), { @@ -21,6 +26,7 @@ const useInstanceQuery = (instanceId, { tenantId = '' } = {}, options = {}) => { isLoading, instance, refetch, + ...rest, }); }; diff --git a/src/constants.js b/src/constants.js index 2aaa0852c..2897e5e19 100644 --- a/src/constants.js +++ b/src/constants.js @@ -677,3 +677,7 @@ export const INSTANCE_SHARING_STATUSES = { ERROR: 'ERROR', IN_PROGRESS: 'IN_PROGRESS', }; + +export const HTTP_RESPONSE_STATUS_CODES = { + FORBIDDEN: 403, +}; diff --git a/test/jest/__mock__/stripesIcon.mock.js b/test/jest/__mock__/stripesIcon.mock.js index d50082820..ffcd3589b 100644 --- a/test/jest/__mock__/stripesIcon.mock.js +++ b/test/jest/__mock__/stripesIcon.mock.js @@ -1,7 +1,7 @@ import React from 'react'; jest.mock('@folio/stripes-components/lib/Icon', () => { - return props => { + return jest.fn(props => { return props.children ? props.children : ; - }; + }); }); diff --git a/translations/ui-inventory/en.json b/translations/ui-inventory/en.json index 91e1ac580..68a67612c 100644 --- a/translations/ui-inventory/en.json +++ b/translations/ui-inventory/en.json @@ -799,6 +799,7 @@ "warning.instance.staffSuppressed": "Warning: Instance is marked staff suppressed", "warning.instance.suppressedFromDiscoveryAndStaffSuppressed": "Warning: Instance is marked suppressed from discovery and staff suppressed", "warning.instance.sharingLocalInstance": "Sharing this local instance will take a few moments. A success message and updated details will be displayed upon completion.", + "warning.instance.accessSharedInstance": "You do not currently have permission to access details of shared instances. Contact your FOLIO administrator for more information.", "warning.holdingsRecord.suppressedFromDiscovery": "Warning: Holdings is marked suppressed from discovery", "warning.item.suppressedFromDiscovery": "Warning: Item is marked suppressed from discovery", "discoverySuppressed": "Suppressed from discovery",