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

Tags policy permissions #2738

Merged
merged 13 commits into from
Jul 4, 2024
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', {
arbulu89 marked this conversation as resolved.
Show resolved Hide resolved
'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
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the comment !

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: {
Copy link
Contributor

Choose a reason for hiding this comment

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

This will conflict with my PR as well 🙈

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
Loading