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

UIIN-2452: Enable/disable consortial holdings/item actions based on User permissions #2284

Merged
merged 45 commits into from
Oct 20, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
e9ff698
Consortial holdings acc
mariia-aloshyna Sep 22, 2023
f5c029c
UIIN-2410: Adjustments
mariia-aloshyna Sep 25, 2023
4a08c0c
UIIN-2410: Add new hook
mariia-aloshyna Sep 27, 2023
5dfcf0d
UIIN-2410: Add tests
mariia-aloshyna Sep 27, 2023
188acc3
UIIN-2452: Disable buttons when member tenant does not have permissions
OleksandrHladchenko1 Oct 3, 2023
58f4cd4
UIIN-2410: Instance 3rd pane: Add consortial holdings/item accordion
mariia-aloshyna Sep 22, 2023
dc2e0d0
UIIN-2452: Add unit tests & switching affiliation when view holdings …
OleksandrHladchenko1 Oct 3, 2023
108f852
UIIN-2410: Fix tests
mariia-aloshyna Oct 4, 2023
3f42476
Merge branch 'master' into UIIN-2410
mariia-aloshyna Oct 4, 2023
9e1b138
UIIN-2452: Add switching affiliation when click on item barcode & Add…
OleksandrHladchenko1 Oct 5, 2023
872f7be
Merge branch 'master' into UIIN-2452
OleksandrHladchenko1 Oct 5, 2023
c8e3f29
UIIN-2452: Add behaviour for non-consortial tenant
OleksandrHladchenko1 Oct 6, 2023
f57ce13
Merge branch 'UIIN-2452' of https://github.com/folio-org/ui-inventory…
OleksandrHladchenko1 Oct 6, 2023
5726f06
UIIN-2452: Fix tests
OleksandrHladchenko1 Oct 6, 2023
d25021e
Merge branch 'master' into UIIN-2452
OleksandrHladchenko1 Oct 6, 2023
767a4a1
Update HoldingAccordion.js
OleksandrHladchenko1 Oct 6, 2023
5769fca
Update HoldingButtonsGroup.js
OleksandrHladchenko1 Oct 6, 2023
fc4b05a
UIIN-2452: Fix tests
OleksandrHladchenko1 Oct 6, 2023
1c5bcd1
Merge branch 'UIIN-2452' of https://github.com/folio-org/ui-inventory…
OleksandrHladchenko1 Oct 6, 2023
081f18b
Update HoldingsListMovement.js
OleksandrHladchenko1 Oct 8, 2023
60b4011
Update HoldingContainer.js
OleksandrHladchenko1 Oct 9, 2023
9c397c8
UIIN-2452: Add returning to the previous affiliation
OleksandrHladchenko1 Oct 10, 2023
483d77e
Merge branch 'UIIN-2452' of https://github.com/folio-org/ui-inventory…
OleksandrHladchenko1 Oct 10, 2023
7894483
UIIN-2452: Add tenantId to props validation
OleksandrHladchenko1 Oct 10, 2023
a7efac1
UIIN-2452: Add behaviour for non-consortial tenant
OleksandrHladchenko1 Oct 10, 2023
42f16f0
Fix some comments
mariia-aloshyna Oct 13, 2023
22d2dc9
Fix perms handling
mariia-aloshyna Oct 13, 2023
ae50bd4
Merge branch 'master' into UIIN-2452
mariia-aloshyna Oct 13, 2023
69396b4
Fix warnings
mariia-aloshyna Oct 13, 2023
4244578
Adjust tests
mariia-aloshyna Oct 13, 2023
39c34b5
Merge branch 'master' into UIIN-2452
mariia-aloshyna Oct 13, 2023
27fc561
Fix tests
mariia-aloshyna Oct 13, 2023
2398063
Merge remote-tracking branch 'origin/UIIN-2452' into UIIN-2452
mariia-aloshyna Oct 13, 2023
a603d03
UIIN-2452: Switch user affiliation using validateUser
OleksandrHladchenko1 Oct 17, 2023
b313553
Merge branch 'master' into UIIN-2452
OleksandrHladchenko1 Oct 17, 2023
e18e43c
Update HoldingAccordion.js
OleksandrHladchenko1 Oct 17, 2023
f72699c
Supress Add holding & Add item & View holdings buttons if user doesn'…
OleksandrHladchenko1 Oct 18, 2023
10a2e3a
Resolve conflicts
OleksandrHladchenko1 Oct 18, 2023
773e3c1
UIIN-2452: Fix comments & add unit tests
OleksandrHladchenko1 Oct 18, 2023
76a05f7
UIIN-2452: Fixes after review
OleksandrHladchenko1 Oct 18, 2023
4d4d213
Merge branch 'master' into UIIN-2452
OleksandrHladchenko1 Oct 18, 2023
355c482
UIIN-2452: Fix code smells
OleksandrHladchenko1 Oct 18, 2023
00589ea
Merge branch 'UIIN-2452' of https://github.com/folio-org/ui-inventory…
OleksandrHladchenko1 Oct 18, 2023
51f91bf
Merge branch 'master' into UIIN-2452
OleksandrHladchenko1 Oct 19, 2023
e81a420
Merge branch 'master' into UIIN-2452
OleksandrHladchenko1 Oct 20, 2023
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
Prev Previous commit
Next Next commit
Fix some comments
  • Loading branch information
mariia-aloshyna committed Oct 13, 2023
commit 42f16f012e0fa54037e867309006210cdf008a67
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@
* Instance 3rd pane: Add consortial holdings/item accordion. Refs UIIN-2410.
* Consortial Central Tenant: Handling Holdings and Item actions on the Instance detail view. Refs UIIN-2523.
* Hide Held by facet in Inventory contributor and subject browse. Refs UIIN-2591.
* Enable/disable consortial holdings/item actions based on User permissions. Refs UIIN-2452.

## [9.4.12](https://github.com/folio-org/ui-inventory/tree/v9.4.12) (2023-09-21)
[Full Changelog](https://github.com/folio-org/ui-inventory/compare/v9.4.11...v9.4.12)
Expand Down
4 changes: 2 additions & 2 deletions src/Holding/CreateHolding/CreateHolding.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
} from '../../common/hooks';
import useCallout from '../../hooks/useCallout';
import HoldingsForm from '../../edit/holdings/HoldingsForm';
import { updateAffiliation } from '../../utils';
import { switchAffiliation } from '../../utils';

const CreateHolding = ({
location,
Expand All @@ -38,7 +38,7 @@ const CreateHolding = ({
}, [location.search, instanceId]);

const onCancel = useCallback(() => {
updateAffiliation(stripes.okapi, tenantFrom, goBack);
switchAffiliation(stripes.okapi, tenantFrom, goBack).then();
}, [stripes.okapi, tenantFrom, goBack]);

const onSubmit = useCallback((newHolding) => {
Expand Down
6 changes: 3 additions & 3 deletions src/Instance/HoldingsList/Holding/HoldingButtonsGroup.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
Icon,
} from '@folio/stripes/components';

import { updateAffiliation } from '../../../utils';
import { switchAffiliation } from '../../../utils';

import { MoveToDropdown } from './MoveToDropdown';

Expand Down Expand Up @@ -47,7 +47,7 @@ const HoldingButtonsGroup = ({
<Button
id={`clickable-view-holdings-${holding.id}`}
data-test-view-holdings
onClick={() => updateAffiliation(stripes.okapi, tenantId, onViewHolding)}
onClick={() => switchAffiliation(stripes.okapi, tenantId, onViewHolding)}
disabled={isViewHoldingsDisabled}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we shouldn't display elements if a user doesn't have permission. I guess it's a FOLIO pattern.

Copy link
Member

Choose a reason for hiding this comment

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

I think we shouldn't display elements if a user doesn't have permission. I guess it's a FOLIO pattern.

Yep, that's the FOLIO UX I'm familiar with. I will follow up with the PO here as I see the story was written specifically with disabled elements, and I've seen similar stories in ui-users. We need to figure out if we are purposely abandoning that pattern or if this was just a goof.

Copy link
Contributor

Choose a reason for hiding this comment

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

@zburke and @Dmytro-Melnyshyn This is a requirement for this story, but we discussed with PO that suppressing these buttons from a user instead of disabling them will make the page kinda less noisy and will free up more space. We decided to go with the disabled buttons in the main release and after further testing and feedback from users, we might end up hiding these buttons.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I understand the story was written this way, but the point @Dmytro-Melnyshyn and I are making is that the story was written incorrectly. Given that we know the story was written incorrectly, why would deliberately do the wrong thing and add debt when we could just do it the right thing?

>
<FormattedMessage id="ui-inventory.viewHoldings" />
Expand All @@ -58,7 +58,7 @@ const HoldingButtonsGroup = ({
<Button
id={`clickable-new-item-${holding.id}`}
data-test-add-item
onClick={() => updateAffiliation(stripes.okapi, tenantId, onAddItem)}
onClick={() => switchAffiliation(stripes.okapi, tenantId, onAddItem)}
buttonStyle="primary paneHeaderNewButton"
disabled={isAddItemDisabled}
>
Expand Down
6 changes: 3 additions & 3 deletions src/Instance/HoldingsList/Holding/HoldingButtonsGroup.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import '../../../../test/jest/__mock__';
import renderWithIntl from '../../../../test/jest/helpers/renderWithIntl';
import translations from '../../../../test/jest/helpers/translationsProperties';

import { updateAffiliation } from '../../../utils';
import { switchAffiliation } from '../../../utils';

import HoldingButtonsGroup from './HoldingButtonsGroup';

Expand Down Expand Up @@ -85,7 +85,7 @@ describe('HoldingButtonsGroup', () => {

fireEvent.click(getByRole('button', { name: 'View holdings' }));

expect(updateAffiliation.mock.calls.length).toBe(1);
expect(switchAffiliation.mock.calls.length).toBe(1);
});
});

Expand All @@ -95,7 +95,7 @@ describe('HoldingButtonsGroup', () => {

fireEvent.click(getByRole('button', { name: 'Add item' }));

expect(updateAffiliation.mock.calls.length).toBe(1);
expect(switchAffiliation.mock.calls.length).toBe(1);
});
});
});
2 changes: 1 addition & 1 deletion src/Instance/InstanceDetails/InstanceDetails.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ const InstanceDetails = forwardRef(({
...rest
}, ref) => {
const stripes = useStripes();
const { okapi: { tenant: tenantId } } = stripes;
const intl = useIntl();
const location = useLocation();
const { okapi: { tenant: tenantId } } = useStripes();
const searchParams = new URLSearchParams(location.search);

const referenceData = useContext(DataContext);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
Button,
} from '@folio/stripes/components';

import { updateAffiliation } from '../../../utils';
import { switchAffiliation } from '../../../utils';

const InstanceNewHolding = ({
location,
Expand All @@ -39,7 +39,7 @@ const InstanceNewHolding = ({
aria-label={label}
buttonStyle="primary"
fullWidth
onClick={() => updateAffiliation(stripes.okapi, tenantId, onNewHolding)}
onClick={() => switchAffiliation(stripes.okapi, tenantId, onNewHolding)}
disabled={disabled}
>
{label}
Expand Down
4 changes: 2 additions & 2 deletions src/Instance/ItemsList/ItemBarcode.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
} from '@folio/stripes/components';
import css from '../../View.css';
import { QUERY_INDEXES } from '../../constants';
import { updateAffiliation } from '../../utils';
import { switchAffiliation } from '../../utils';

const ItemBarcode = ({
location,
Expand Down Expand Up @@ -67,7 +67,7 @@ const ItemBarcode = ({
<Button
buttonStyle="link"
buttonClass={css.linkWithoutBorder}
onClick={() => updateAffiliation(stripes.okapi, tenantId, onViewItem)}
onClick={() => switchAffiliation(stripes.okapi, tenantId, onViewItem)}
data-test-item-link
>
{itemBarcode}
Expand Down
4 changes: 2 additions & 2 deletions src/Item/CreateItem/CreateItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
import ItemForm from '../../edit/items/ItemForm';
import useCallout from '../../hooks/useCallout';
import { useItemMutation } from '../hooks';
import { updateAffiliation } from '../../utils';
import { switchAffiliation } from '../../utils';

const CreateItem = ({
referenceData,
Expand All @@ -44,7 +44,7 @@ const CreateItem = ({
}, [instanceId, location.search]);

const onCancel = useCallback(() => {
updateAffiliation(stripes.okapi, tenantFrom, goBack);
switchAffiliation(stripes.okapi, tenantFrom, goBack).then();
}, [stripes.okapi, tenantFrom, goBack]);

const onSuccess = useCallback(async (response) => {
Expand Down
4 changes: 2 additions & 2 deletions src/ViewHoldingsRecord.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ import {
getSortedNotes,
handleKeyCommand,
getDate,
updateAffiliation,
switchAffiliation,
} from './utils';
import withLocation from './withLocation';
import {
Expand Down Expand Up @@ -213,7 +213,7 @@ class ViewHoldingsRecord extends React.Component {

const { stripes, tenantFrom } = this.props;

updateAffiliation(stripes.okapi, tenantFrom, this.goBack);
switchAffiliation(stripes.okapi, tenantFrom, this.goBack);
}

// Edit Holdings records handlers
Expand Down
4 changes: 2 additions & 2 deletions src/ViewHoldingsRecord.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
translationsProperties,
} from '../test/jest/helpers';

import { updateAffiliation } from './utils';
import { switchAffiliation } from './utils';
import ViewHoldingsRecord from './ViewHoldingsRecord';

jest.mock('./withLocation', () => jest.fn(c => c));
Expand Down Expand Up @@ -118,7 +118,7 @@ describe('ViewHoldingsRecord actions', () => {
it('should close view holding page', async () => {
renderViewHoldingsRecord();
fireEvent.click(await screen.findByRole('button', { name: 'confirm' }));
expect(updateAffiliation).toHaveBeenCalled();
expect(switchAffiliation).toHaveBeenCalled();
});

it('should translate to edit holding form page', async () => {
Expand Down
4 changes: 2 additions & 2 deletions src/routes/ItemRoute.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { DataContext } from '../contexts';
import { getItem } from '../storage';
import {
TENANT_IDS_KEY,
updateAffiliation,
switchAffiliation,
} from '../utils';

const getRequestsPath = `circulation/requests?query=(itemId==:{itemid}) and status==(${requestsStatusString}) sortby requestDate desc&limit=1`;
Expand Down Expand Up @@ -175,7 +175,7 @@ class ItemRoute extends React.Component {

const tenantFrom = tenantIds ? tenantIds.tenantFrom : okapi.tenant;

updateAffiliation(okapi, tenantFrom, this.goBack);
switchAffiliation(okapi, tenantFrom, this.goBack).then();
}

isLoading = () => {
Expand Down
2 changes: 1 addition & 1 deletion src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -830,7 +830,7 @@ export const hasMemberTenantPermission = (permissions, permissionName, tenantId)

export const TENANT_IDS_KEY = 'tenantIds';

export const updateAffiliation = async (okapi, tenantId, move) => {
export const switchAffiliation = async (okapi, tenantId, move) => {
if (okapi.tenant !== tenantId) {
await updateTenant(okapi, tenantId);

Expand Down