Skip to content

Commit

Permalink
Tags policy permissions (#2738)
Browse files Browse the repository at this point in the history
* Pill component disabled state

* Extracted tag delete button into separate component with disabled state

* Tags abilities migration

* Tag policy

* Tags controller protected with policy

* DisabledGuard tooltip wrapping as props

* Frontend tag component supports abilities

* ClusterList with tag abilities

* DatabaseOverview tags with abilities

* HostsList with tags with abilities

* SapSytemOverview tags with abilities

* Tests refactor

* Addressing review feedbacks
  • Loading branch information
CDimonaco authored Jul 4, 2024
1 parent 5af8fac commit b9414b7
Show file tree
Hide file tree
Showing 26 changed files with 527 additions and 81 deletions.
3 changes: 2 additions & 1 deletion assets/js/common/DisabledGuard/DisabledGuard.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ function DisabledGuard({
userAbilities,
permitted,
withTooltip = true,
tooltipWrap = false,
tooltipMessage = DEFAULT_TOOLTIP_MESSAGE,
tooltipPlace = 'bottom',
children,
Expand All @@ -41,7 +42,7 @@ function DisabledGuard({
isEnabled={withTooltip}
content={tooltipMessage}
place={tooltipPlace}
wrap={false}
wrap={tooltipWrap}
>
{disabledElement}
</Tooltip>
Expand Down
7 changes: 6 additions & 1 deletion assets/js/common/Pill/Pill.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ function Pill({
size = 'sm',
roundedMode = 'rounded-full',
display = 'inline-flex',
disabled,
}) {
return (
<span
Expand All @@ -22,13 +23,17 @@ function Pill({
'bg-green-100': !className,
'text-green-800': !className,
},
{ 'opacity-50 pointer-events-none': disabled },
roundedMode,
sizeClasses[size],
display,
className
)}
aria-hidden="true"
onClick={onClick}
onClick={(e) => {
if (disabled) return;
onClick(e);
}}
>
{children}
</span>
Expand Down
9 changes: 9 additions & 0 deletions assets/js/common/Pill/Pill.stories.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@ export function ExtraSmall() {
return <Pill size="xs">Extra small!</Pill>;
}

export function Disabled() {
return (
<Pill size="xs" disabled>
{' '}
Disabled
</Pill>
);
}

export function WithIcon() {
return (
<Pill className="bg-green-100 text-green-800 group flex items-center">
Expand Down
24 changes: 24 additions & 0 deletions assets/js/common/Pill/Pill.test.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import '@testing-library/jest-dom';

import Pill from '.';
Expand All @@ -12,6 +13,29 @@ describe('Pill', () => {
);
});

it('should display a pill with the disabled behaviour when disabled is passed as props', async () => {
const onClick = jest.fn();
const user = userEvent.setup();
render(
<Pill
className="some-class"
roundedMode="not-rounded"
size="xs"
onClick={onClick}
disabled
display="inline-block"
>
Content
</Pill>
);
expect(screen.getByText('Content')).toHaveClass(
'opacity-50 pointer-events-none'
);

await user.click(screen.getByText('Content'));
expect(onClick).not.toHaveBeenCalled();
});

it('should display a pill with provided props', () => {
render(
<Pill
Expand Down
93 changes: 61 additions & 32 deletions assets/js/common/Tags/Tags.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import classNames from 'classnames';
import { EOS_NEW_LABEL, EOS_CLOSE } from 'eos-icons-react';
import Pill from '@common/Pill';
import Tooltip from '@common/Tooltip';
import DisabledGuard from '@common/DisabledGuard';
import useOnClickOutside from '@hooks/useOnClickOutside';
// eslint-disable-next-line
const tagRegexValidation = /^[\+\-=.,_:@\p{L}\w]*$/u;
Expand All @@ -15,13 +16,33 @@ const tagValidationDefaultMessage = (
</>
);

function ExistingTag({ onClick, disabled }) {
return (
<span
aria-hidden="true"
className={classNames('cursor-pointer group-hover:opacity-60', {
'opacity-50 pointer-events-none': disabled,
})}
onClick={() => {
if (disabled) return;
onClick();
}}
>
<EOS_CLOSE className="ml-2" color="#276749" size="base" />
</span>
);
}

function Tags({
className,
tags,
onChange,
onAdd,
onRemove,
resourceId,
userAbilities,
tagAdditionPermittedFor = [],
tagDeletionPermittedFor = [],
validationMessage = tagValidationDefaultMessage,
}) {
const [renderedTags, setTags] = useState(tags);
Expand Down Expand Up @@ -75,21 +96,23 @@ function Tags({
}}
>
{tag}
<span
aria-hidden="true"
className="ml-2 cursor-pointer group-hover:opacity-60"
onClick={() => {
const newTagsList = renderedTags.reduce(
(acc, current) => (current === tag ? acc : [...acc, current]),
[]
);
setTags(newTagsList);
onChange(newTagsList);
onRemove(tag);
}}
<DisabledGuard
userAbilities={userAbilities}
permitted={tagDeletionPermittedFor}
tooltipWrap
>
<EOS_CLOSE color="#276749" size="base" />
</span>
<ExistingTag
onClick={() => {
const newTagsList = renderedTags.reduce(
(acc, current) => (current === tag ? acc : [...acc, current]),
[]
);
setTags(newTagsList);
onChange(newTagsList);
onRemove(tag);
}}
/>
</DisabledGuard>
</Pill>
))}
{addingTag ? (
Expand Down Expand Up @@ -123,25 +146,31 @@ function Tags({
</Pill>
</Tooltip>
) : (
<Pill
className={classNames({
'text-green-800': true,
'bg-green-100': true,
flex: true,
'items-center': true,
'cursor-pointer': true,
'hover:scale-110': true,
transition: true,
'ease-in-out': true,
'delay-50': true,
})}
onClick={(e) => {
e.stopPropagation();
setAddingTag(true);
}}
<DisabledGuard
userAbilities={userAbilities}
permitted={tagAdditionPermittedFor}
tooltipWrap
>
<EOS_NEW_LABEL color="#276749" size="base" /> Add Tag
</Pill>
<Pill
className={classNames({
'text-green-800': true,
'bg-green-100': true,
flex: true,
'items-center': true,
'cursor-pointer': true,
'hover:scale-110': true,
transition: true,
'ease-in-out': true,
'delay-50': true,
})}
onClick={(e) => {
e.stopPropagation();
setAddingTag(true);
}}
>
<EOS_NEW_LABEL color="#276749" size="base" /> Add Tag
</Pill>
</DisabledGuard>
)}
</span>
);
Expand Down
47 changes: 38 additions & 9 deletions assets/js/common/Tags/Tags.stories.jsx
Original file line number Diff line number Diff line change
@@ -1,17 +1,46 @@
import React from 'react';

import Tags from '.';

export default {
title: 'Components/Tags',
component: Tags,
argTypes: { onChange: { action: 'tag changed' } },
args: {
tags: ['carbonara', 'Amatriciana'],
userAbilities: [{ name: 'all', resource: 'all' }],
tagAdditionPermittedFor: ['all:all'],
tagDeletionPermittedFor: ['all:all'],
},
argTypes: {
onChange: { action: 'tag changed' },
tagAdditionPermittedFor: {
control: 'array',
description: 'Abilities that allow tag creation',
},
tagDeletionPermittedFor: {
control: 'array',
description: 'Abilities that allow tag deletion',
},
},
};

export function Populated(args) {
return <Tags tags={['carbonara', 'Amatriciana']} {...args} />;
}
export const Default = {
args: {
tags: ['carbonara', 'Amatriciana'],
userAbilities: [{ name: 'all', resource: 'all' }],
tagAdditionPermittedFor: ['all:all'],
tagDeletionPermittedFor: ['all:all'],
},
};

export function Empty(args) {
return <Tags tags={[]} {...args} />;
}
export const Empty = {
args: {
...Default.args,
tags: [],
},
};

export const Disabled = {
args: {
...Default.args,
userAbilities: [],
},
};
31 changes: 29 additions & 2 deletions assets/js/common/Tags/Tags.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,34 @@ import '@testing-library/jest-dom';
import Tags from '.';

describe('Tags', () => {
it('should disable creation and deletion when the proper user abilities are not present', async () => {
render(
<Tags
tags={['thetag']}
userAbilities={[{ name: 'all', resource: 'resource' }]}
tagAdditionPermittedFor={['all:host_tags']}
tagDeletionPermittedFor={['all:host_tags']}
/>
);

expect(screen.queryByText('Add Tag')).toHaveClass('opacity-50');
// grab the X
expect(
screen.queryByText('thetag').children.item(0).children.item(0)
).toHaveClass('opacity-50');
});

it('should show validation tooltip when inserting an unsupported character in a tag', async () => {
const user = userEvent.setup();
const msg = 'invalid character';

render(<Tags tags={[]} validationMessage={msg} />);
render(
<Tags
tags={[]}
validationMessage={msg}
userAbilities={[{ name: 'all', resource: 'all' }]}
/>
);

expect(screen.queryByText(msg)).toBeNull();

Expand Down Expand Up @@ -39,7 +62,11 @@ describe('Tags', () => {
render(
<>
<div>sibling</div>
<Tags tags={[]} validationMessage={msg} />
<Tags
tags={[]}
validationMessage={msg}
userAbilities={[{ name: 'all', resource: 'all' }]}
/>
</>
);

Expand Down
4 changes: 3 additions & 1 deletion assets/js/lib/test-utils/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ const middlewares = [];
const mockStore = configureStore(middlewares);

export const defaultInitialState = {
user: {
abilities: [{ name: 'all', resource: 'all' }],
},
hostsList: { hosts },
clustersList: { clusters },
sapSystemsList: {
Expand All @@ -35,7 +38,6 @@ export const defaultInitialState = {
},
checksSelection: { host: {}, cluster: {} },
catalog: { loading: false, data: [], error: null },
user: { abilities: [{ name: 'all', resource: 'all' }] },
};

export const withState = (component, initialState = {}) => {
Expand Down
6 changes: 6 additions & 0 deletions assets/js/pages/ClusterDetails/ClustersList.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import { addTagToCluster, removeTagFromCluster } from '@state/clusters';
import { getAllSAPInstances } from '@state/selectors/sapSystem';
import { getInstanceID } from '@state/instances';

import { getUserProfile } from '@state/selectors/user';

import PageHeader from '@common/PageHeader';
import SapSystemLink from '@common/SapSystemLink';
import Table from '@common/Table';
Expand Down Expand Up @@ -38,6 +40,7 @@ function ClustersList() {
const allInstances = useSelector(getAllSAPInstances);
const dispatch = useDispatch();
const [searchParams, setSearchParams] = useSearchParams();
const { abilities } = useSelector(getUserProfile);

const config = {
pagination: true,
Expand Down Expand Up @@ -118,6 +121,9 @@ function ClustersList() {
element[key].some((tag) => filter.includes(tag)),
render: (content, item) => (
<Tags
userAbilities={abilities}
tagAdditionPermittedFor={['all:cluster_tags']}
tagDeletionPermittedFor={['all:cluster_tags']}
tags={content}
resourceId={item.id}
onChange={() => {}}
Expand Down
Loading

0 comments on commit b9414b7

Please sign in to comment.