diff --git a/CHANGELOG.md b/CHANGELOG.md index ffc69b18..2898dc86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ * Suppress banking information management for unauthorized user. Refs UIORGS-418. * Add validation for `Day` field in `Monthly` scheduler for export method. Refs UIORGS-417. * Support `data-export-spring` interface `v2.0`. Refs UXPROD-3903. +* Accurately handle user permissions for banking information operations. Refs UIORGS-424. ## [5.0.0](https://github.com/folio-org/ui-organizations/tree/v5.0.0) (2023-10-12) [Full Changelog](https://github.com/folio-org/ui-organizations/compare/v4.0.0...v5.0.0) diff --git a/src/Organizations/useBankingInformationManager.js b/src/Organizations/useBankingInformationManager.js index 127b40ff..1318e8d3 100644 --- a/src/Organizations/useBankingInformationManager.js +++ b/src/Organizations/useBankingInformationManager.js @@ -10,13 +10,21 @@ import { } from '../common/hooks'; import { getArrayItemsChanges } from '../common/utils'; -const executeSequentially = (fn, arr) => arr.reduce((acc, curr) => { - return acc.then(() => fn({ bankingInformation: curr })); -}, Promise.resolve()); +const executeSequentially = (fn, arr, hasPerm) => { + if (!hasPerm) return Promise.resolve(); -const executeParallel = (fn, arr) => chunk(arr, 5).reduce((acc, chunked) => { - return acc.then(() => Promise.all(chunked.map((bankingInformation) => fn({ bankingInformation })))); -}, Promise.resolve()); + return arr.reduce((acc, curr) => { + return acc.then(() => fn({ bankingInformation: curr })); + }, Promise.resolve()); +}; + +const executeParallel = (fn, arr, hasPerm) => { + if (!hasPerm) return Promise.resolve(); + + return chunk(arr, 5).reduce((acc, chunked) => { + return acc.then(() => Promise.all(chunked.map((bankingInformation) => fn({ bankingInformation })))); + }, Promise.resolve()); +}; export const useBankingInformationManager = () => { const stripes = useStripes(); @@ -36,13 +44,7 @@ export const useBankingInformationManager = () => { bankingInformation, organization, }) => { - const isOperationRestricted = !( - organization.isVendor - && isBankingInformationEnabled - && stripes.hasPerm('organizations.banking-information.item.post') - && stripes.hasPerm('organizations.banking-information.item.put') - && stripes.hasPerm('organizations.banking-information.item.delete') - ); + const isOperationRestricted = !(organization.isVendor && isBankingInformationEnabled); if (isOperationRestricted) return Promise.resolve(); @@ -53,12 +55,13 @@ export const useBankingInformationManager = () => { } = getArrayItemsChanges(initBankingInformation, bankingInformation); return Promise.all([ - executeSequentially(createBankingInformation, created.map((item) => ({ - organizationId: organization.id, - ...item, - }))), - executeParallel(updateBankingInformation, updated), - executeParallel(deleteBankingInformation, deleted), + executeSequentially( + createBankingInformation, + created.map((item) => ({ organizationId: organization.id, ...item })), + stripes.hasPerm('organizations.banking-information.item.post'), + ), + executeParallel(updateBankingInformation, updated, stripes.hasPerm('organizations.banking-information.item.put')), + executeParallel(deleteBankingInformation, deleted, stripes.hasPerm('organizations.banking-information.item.delete')), ]).catch(() => { showCallout({ type: 'error', diff --git a/src/Organizations/useBankingInformationManager.test.js b/src/Organizations/useBankingInformationManager.test.js index d88aa39b..a3f22e97 100644 --- a/src/Organizations/useBankingInformationManager.test.js +++ b/src/Organizations/useBankingInformationManager.test.js @@ -1,4 +1,5 @@ import { renderHook } from '@folio/jest-config-stripes/testing-library/react'; +import { useStripes } from '@folio/stripes/core'; import { organization } from 'fixtures'; import { @@ -13,6 +14,11 @@ const mutationsMock = { deleteBankingInformation: jest.fn(), }; +jest.mock('@folio/stripes/core', () => ({ + ...jest.requireActual('@folio/stripes/core'), + useStripes: jest.fn(), +})); + jest.mock('../common/hooks', () => ({ ...jest.requireActual('../common/hooks'), useBankingInformationMutation: jest.fn(), @@ -29,7 +35,15 @@ const bankingInformation = [ { id: 3, bankName: 'Bank name 3' }, ]; -describe('useBankingAccountTypes', () => { +const hasPermFactory = (permsMap) => (perm) => { + return permsMap ? permsMap[perm] : true; +}; + +const stripesStub = { + hasPerm: hasPermFactory(), +}; + +describe('useBankingInformationManager', () => { beforeEach(() => { Object .values(mutationsMock) @@ -40,6 +54,9 @@ describe('useBankingAccountTypes', () => { useBankingInformationSettings .mockClear() .mockReturnValue({ enabled: true }); + useStripes + .mockClear() + .mockReturnValue(stripesStub); }); it('should handle banking information fields change', async () => { @@ -100,4 +117,79 @@ describe('useBankingAccountTypes', () => { expect(mutationsMock.deleteBankingInformation).not.toHaveBeenCalled(); expect(mutationsMock.updateBankingInformation).not.toHaveBeenCalled(); }); + + describe('Permissions handling', () => { + it('edit banking information item', async () => { + useStripes.mockReturnValue({ hasPerm: hasPermFactory({ 'organizations.banking-information.item.put': true }) }); + + const { result } = renderHook(() => useBankingInformationManager()); + const { manageBankingInformation } = result.current; + + await manageBankingInformation({ + initBankingInformation, + bankingInformation, + organization, + }); + + expect(mutationsMock.createBankingInformation).not.toHaveBeenCalled(); + expect(mutationsMock.deleteBankingInformation).not.toHaveBeenCalled(); + expect(mutationsMock.updateBankingInformation).toHaveBeenCalled(); + }); + + it('create banking information item', async () => { + useStripes.mockReturnValue({ hasPerm: hasPermFactory({ 'organizations.banking-information.item.post': true }) }); + + const { result } = renderHook(() => useBankingInformationManager()); + const { manageBankingInformation } = result.current; + + await manageBankingInformation({ + initBankingInformation, + bankingInformation, + organization, + }); + + expect(mutationsMock.createBankingInformation).toHaveBeenCalled(); + expect(mutationsMock.deleteBankingInformation).not.toHaveBeenCalled(); + expect(mutationsMock.updateBankingInformation).not.toHaveBeenCalled(); + }); + + it('delete banking information item', async () => { + useStripes.mockReturnValue({ hasPerm: hasPermFactory({ 'organizations.banking-information.item.delete': true }) }); + + const { result } = renderHook(() => useBankingInformationManager()); + const { manageBankingInformation } = result.current; + + await manageBankingInformation({ + initBankingInformation, + bankingInformation, + organization, + }); + + expect(mutationsMock.createBankingInformation).not.toHaveBeenCalled(); + expect(mutationsMock.deleteBankingInformation).toHaveBeenCalled(); + expect(mutationsMock.updateBankingInformation).not.toHaveBeenCalled(); + }); + + it('create and edit banking information item', async () => { + const permsMap = { + 'organizations.banking-information.item.post': true, + 'organizations.banking-information.item.put': true, + }; + + useStripes.mockReturnValue({ hasPerm: hasPermFactory(permsMap) }); + + const { result } = renderHook(() => useBankingInformationManager()); + const { manageBankingInformation } = result.current; + + await manageBankingInformation({ + initBankingInformation, + bankingInformation, + organization, + }); + + expect(mutationsMock.createBankingInformation).toHaveBeenCalled(); + expect(mutationsMock.deleteBankingInformation).not.toHaveBeenCalled(); + expect(mutationsMock.updateBankingInformation).toHaveBeenCalled(); + }); + }); });