Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
52 changes: 52 additions & 0 deletions src/authz-module/components/PermissionTable.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { Check, Close } from '@openedx/paragon/icons';
import { Card, Icon } from '@openedx/paragon';
import { PermissionsResourceGrouped, Role } from '@src/types';
import { actionsDictionary } from './RoleCard/constants';
import ResourceTooltip from './ResourceTooltip';

type PermissionTableProps = {
roles: Role[];
permissionsTable: PermissionsResourceGrouped[];
};

const PermissionTable = ({ permissionsTable, roles }: PermissionTableProps) => (
<Card>
<table className="permission-table w-100">
<thead>
<tr>
<th className="" aria-hidden="true" />
{roles.map(role => (
<th key={role.name} className="text-center py-3">{role.name}</th>
))}
</tr>
</thead>
<tbody>
{permissionsTable.map(resourceGroup => (
<>
<tr className="bg-info-100 text-primary">
<td colSpan={roles.length + 1} className="text-start py-3 px-4">
<strong>{resourceGroup.label}</strong>
<ResourceTooltip resourceGroup={resourceGroup} />
</td>
</tr>
{resourceGroup.permissions.map(permission => (
<tr key={permission.key} className="border-top">
<td className="text-start d-flex align-items-center small px-4 py-3">
<Icon className="d-inline-block mr-2" size="sm" src={actionsDictionary[permission.actionKey]} />
{permission.label}
</td>
{roles.map(role => (
<td key={role.name} className="text-center">
{permission.roles[role.name] ? <Icon className="d-inline-block" src={Check} /> : <Icon className="text-danger d-inline-block" src={Close} />}

Choose a reason for hiding this comment

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

Just curious as to what color we want the checkmarks to be

Current

Image

"Success"

Suggested change
{permission.roles[role.name] ? <Icon className="d-inline-block" src={Check} /> : <Icon className="text-danger d-inline-block" src={Close} />}
{permission.roles[role.name] ? <Icon className="text-success d-inline-block" src={Check} /> : <Icon className="text-danger d-inline-block" src={Close} />}
Image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The original design has no specific color https://www.figma.com/design/q3Knq0BKoVTBbtaxb81n9R/RBAC---Console---Wireframes?node-id=2750-9661&t=hu3T5aVg659Dg0J2-4

Do you consider the green is a better indicative?

Choose a reason for hiding this comment

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

I'm perfectly happy with it either way. I just saw "text-danger" for the red X and my mind jumped to using text-success for a green check, but if the designs don't have it as green then it's good as is.

</td>
))}
</tr>
))}
</>
))}
</tbody>
</table>
</Card>
);

export default PermissionTable;
31 changes: 31 additions & 0 deletions src/authz-module/components/ResourceTooltip.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { Icon, OverlayTrigger, Popover } from '@openedx/paragon';
import { Info } from '@openedx/paragon/icons';
import { PermissionsResourceGrouped, RoleResourceGroup } from '@src/types';

type ResourceTooltipProps = {
resourceGroup: PermissionsResourceGrouped | RoleResourceGroup;
};

const ResourceTooltip = ({ resourceGroup }:ResourceTooltipProps) => (
<OverlayTrigger
key={`overlay-${resourceGroup.key}`}
placement="auto"
overlay={(
<Popover id={`tooltip-${resourceGroup.label}`}>
<Popover.Content className="p-3">
<h4 className="text-primary">{resourceGroup.label}</h4>
<p className="small">{resourceGroup.description}</p>
<ul className="small">
{resourceGroup.permissions.map(permission => (
<li><b>{permission.label.trim()}:</b> {permission.description}</li>
))}
</ul>
</Popover.Content>
</Popover>
)}
>
<Icon className="d-inline-block text-gray ml-2 my-auto" size="inline" src={Info} />
</OverlayTrigger>
);

export default ResourceTooltip;
26 changes: 11 additions & 15 deletions src/authz-module/components/RoleCard/PermissionsRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,38 +2,34 @@ import { ComponentType } from 'react';
import {
Chip, Col, Row,
} from '@openedx/paragon';
import { RoleResourceGroup } from '@src/types';
import { actionsDictionary, ActionKey } from './constants';
import ResourceTooltip from '../ResourceTooltip';

interface Action {
key: string;
label?: string;
disabled?: boolean;
}
type PermissionRowProps = {
resource: RoleResourceGroup;
};

interface PermissionRowProps {
resourceLabel: string;
actions: Action[];
}

const PermissionRow = ({ resourceLabel, actions }: PermissionRowProps) => (
const PermissionRow = ({ resource }: PermissionRowProps) => (
<Row className="row align-items-center border px-2 py-2">
<Col md={2}>
<span className="small font-weight-bold">{resourceLabel}</span>
<span className="small font-weight-bold">{resource.label}</span>
<ResourceTooltip resourceGroup={resource} />
</Col>
<Col>
<div className="w-100 d-flex flex-wrap align-items-center">
{actions.map((action, index) => (
{resource.permissions.map((action, index) => (
<>
<Chip
key={action.key}
iconBefore={actionsDictionary[action.key as ActionKey] as ComponentType}
iconBefore={actionsDictionary[action.actionKey as ActionKey] as ComponentType}
disabled={action.disabled}
className="mx-3 my-2 px-3 bg-primary-100 border-0 permission-chip"
variant="light"
>
{action.label}
</Chip>
{(index === actions.length - 1) ? null
{(index === resource.permissions.length - 1) ? null
: (<hr className="border-right mx-2" style={{ height: '24px' }} />)}
</>
))}
Expand Down
14 changes: 9 additions & 5 deletions src/authz-module/components/RoleCard/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,17 @@ describe('RoleCard', () => {
description: 'Can manage everything',
showDelete: true,
userCounter: 2,
permissions: [
permissionsByResource: [
{
key: 'library',
label: 'Library Resource',
actions: [
{ key: 'view', label: 'View' },
{ key: 'manage', label: 'Manage', disabled: true },
permissions: [
{
key: 'view', label: 'View', actionKey: 'view', disabled: false,
},
{
key: 'manage', label: 'Manage', actionKey: 'manage', disabled: true,
},
],
},
],
Expand Down Expand Up @@ -83,7 +87,7 @@ describe('RoleCard', () => {
});

it('handles empty permissions gracefully', () => {
renderWrapper(<RoleCard {...defaultProps} permissions={[]} />);
renderWrapper(<RoleCard {...defaultProps} permissionsByResource={[]} />);
expect(screen.queryByText('Library Resource')).not.toBeInTheDocument();
});
});
12 changes: 5 additions & 7 deletions src/authz-module/components/RoleCard/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ interface RoleCardProps extends CardTitleProps {
objectName?: string | null;
description: string;
showDelete?: boolean;
permissions: any[];
permissionsByResource: any[];
}

const CardTitle = ({ title, userCounter = null }: CardTitleProps) => (
Expand All @@ -31,7 +31,7 @@ const CardTitle = ({ title, userCounter = null }: CardTitleProps) => (
);

const RoleCard = ({
title, objectName, description, showDelete, permissions, userCounter,
title, objectName, description, showDelete, permissionsByResource, userCounter,
}: RoleCardProps) => {
const intl = useIntl();

Expand All @@ -51,13 +51,11 @@ const RoleCard = ({
title={intl.formatMessage(messages['authz.permissions.title'])}
>
<Container>
{permissions.map(({ key, label, actions }) => (
{permissionsByResource.map((resourceGroup) => (
<PermissionRow
key={`${title}-${key}`}
resourceLabel={label}
actions={actions}
key={`${title}-${resourceGroup.key}`}
resource={resourceGroup}
/>

))}
</Container>
</Collapsible>
Expand Down
6 changes: 6 additions & 0 deletions src/authz-module/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@
height: var(--pgn-size-icon-xs);
}
}

.permission-table {
td {
line-height: 24px;
}
}
}


Expand Down
38 changes: 30 additions & 8 deletions src/authz-module/libraries-manager/LibrariesTeamManager.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { screen } from '@testing-library/react';
import { screen, within } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { renderWrapper } from '@src/setupTest';
import { initializeMockApp } from '@edx/frontend-platform/testing';
Expand Down Expand Up @@ -32,11 +32,15 @@ jest.mock('./components/AddNewTeamMemberModal', () => ({

jest.mock('../components/RoleCard', () => ({
__esModule: true,
default: ({ title, description, permissions }: { title: string, description: string, permissions: any[] }) => (
default: ({ title, description, permissionsByResource }: {
title: string,
description: string,
permissionsByResource: any[]
}) => (
<div data-testid="role-card">
<div>{title}</div>
<div>{description}</div>
<div>{permissions.length} permissions</div>
<div>{permissionsByResource.length} permissions</div>
</div>
),
}));
Expand All @@ -63,9 +67,9 @@ describe('LibrariesTeamManager', () => {
],
permissions: [
{ key: 'view_library', label: 'view', resource: 'library' },
{ key: 'edit_library', name: 'edit', resource: 'library' },
{ key: 'edit_library', label: 'edit', resource: 'library' },
],
resources: [{ key: 'library', displayName: 'Library' }],
resources: [{ key: 'library', label: 'Library' }],
canManageTeam: true,
});

Expand Down Expand Up @@ -106,10 +110,28 @@ describe('LibrariesTeamManager', () => {
await user.click(rolesTab);

const roleCards = await screen.findAllByTestId('role-card');

expect(roleCards.length).toBeGreaterThan(0);
expect(screen.getByText('Instructor')).toBeInTheDocument();
const rolesScope = within(roleCards[0]);
expect(roleCards.length).toBe(1);
expect(rolesScope.getByText('Instructor')).toBeInTheDocument();
expect(screen.getByText(/Can manage content/i)).toBeInTheDocument();
expect(screen.getByText(/1 permissions/i)).toBeInTheDocument();
});

it('renders role matrix when "Permissions" tab is selected', async () => {
const user = userEvent.setup();

renderWrapper(<LibrariesTeamManager />);

// Click on "Permissions" tab
const permissionsTab = await screen.findByRole('tab', { name: /permissions/i });
await user.click(permissionsTab);

const tablePermissionMatrix = await screen.getByRole('table');
const matrixScope = within(tablePermissionMatrix);

expect(matrixScope.getByText('Library')).toBeInTheDocument();
expect(matrixScope.getByText('Instructor')).toBeInTheDocument();
expect(matrixScope.getByText('edit')).toBeInTheDocument();
expect(matrixScope.getByText('view')).toBeInTheDocument();
});
});
46 changes: 28 additions & 18 deletions src/authz-module/libraries-manager/LibrariesTeamManager.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ import { useLocation } from 'react-router-dom';
import TeamTable from './components/TeamTable';
import AuthZLayout from '../components/AuthZLayout';
import RoleCard from '../components/RoleCard';
import PermissionTable from '../components/PermissionTable';
import { useLibraryAuthZ } from './context';
import { AddNewTeamMemberTrigger } from './components/AddNewTeamMemberModal';
import { buildPermissionsByRoleMatrix } from './utils';
import { buildPermissionMatrixByResource, buildPermissionMatrixByRole } from './utils';

import messages from './messages';

Expand All @@ -23,12 +24,18 @@ const LibrariesTeamManager = () => {
const { data: library } = useLibrary(libraryId);
const rootBradecrumb = intl.formatMessage(messages['library.authz.breadcrumb.root']) || '';
const pageTitle = intl.formatMessage(messages['library.authz.manage.page.title']);
const libraryRoles = useMemo(() => roles.map(role => ({
...role,
permissions: buildPermissionsByRoleMatrix({
rolePermissions: role.permissions, permissions, resources, intl,
}),
})), [roles, permissions, resources, intl]);

const [libraryPermissionsByRole, libraryPermissionsByResource] = useMemo(() => {
if (!roles && !permissions && !resources) { return [null, null]; }
const permissionsByRole = buildPermissionMatrixByRole({
roles, permissions, resources, intl,
});
const permissionsByResource = buildPermissionMatrixByResource({
roles, permissions, resources, intl,
});

return [permissionsByRole, permissionsByResource];
}, [roles, permissions, resources, intl]);

return (
<div className="authz-libraries">
Expand All @@ -54,20 +61,23 @@ const LibrariesTeamManager = () => {
</Tab>
<Tab eventKey="roles" title={intl.formatMessage(messages['library.authz.tabs.roles'])}>
<Container className="p-5">
{!libraryRoles ? <Skeleton count={2} height={200} /> : null}
{libraryRoles && libraryRoles.map(role => (
<RoleCard
key={`${role.role}-description`}
title={role.name}
userCounter={role.userCount}
description={role.description}
permissions={role.permissions as any[]}
/>
))}
{!libraryPermissionsByRole ? <Skeleton count={2} height={200} />
: libraryPermissionsByRole.map(role => (
<RoleCard
key={`${role.role}-description`}
title={role.name}
userCounter={role.userCount}
description={role.description}
permissionsByResource={role.resources as any[]}
/>
))}
</Container>
</Tab>
<Tab id="libraries-permissions-tab" eventKey="permissions" title={intl.formatMessage(messages['library.authz.tabs.permissions'])}>
Permissions tab.
<Container className="p-5 container-mw-lg">
{!libraryPermissionsByResource ? <Skeleton count={2} height={200} />
: <PermissionTable permissionsTable={libraryPermissionsByResource} roles={roles} />}
</Container>
</Tab>
</Tabs>
</AuthZLayout>
Expand Down
16 changes: 6 additions & 10 deletions src/authz-module/libraries-manager/LibrariesUserManager.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { useLibraryAuthZ } from './context';
import RoleCard from '../components/RoleCard';
import { AssignNewRoleTrigger } from './components/AssignNewRoleModal';
import { useLibrary, useTeamMembers } from '../data/hooks';
import { buildPermissionsByRoleMatrix } from './utils';
import { buildPermissionMatrixByRole } from './utils';

import messages from './messages';

Expand All @@ -25,14 +25,10 @@ const LibrariesUserManager = () => {
const { data: teamMembers, isLoading } = useTeamMembers(libraryId);
const user = teamMembers?.find(member => member.username === username);
const userRoles = useMemo(() => {
const assignedRoles = roles.filter(role => user?.roles.includes(role.role))
.map(role => ({
...role,
permissions: buildPermissionsByRoleMatrix({
rolePermissions: role.permissions, permissions, resources, intl,
}),
}));
return assignedRoles;
const assignedRoles = roles.filter(role => user?.roles.includes(role.role));
return buildPermissionMatrixByRole({
roles: assignedRoles, permissions, resources, intl,
});
}, [roles, user?.roles, permissions, resources, intl]);

return (
Expand Down Expand Up @@ -60,7 +56,7 @@ const LibrariesUserManager = () => {
objectName={library.title}
description={role.description}
showDelete
permissions={role.permissions as any[]}
permissionsByResource={role.resources as any[]}
/>
))}
</Container>
Expand Down
Loading