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.

Sign up for GitHub

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

feat(rbac): upgrade to mui5 #1985

Merged
merged 1 commit into from
Nov 20, 2024
Merged

feat(rbac): upgrade to mui5 #1985

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions workspaces/rbac/.changeset/grumpy-foxes-smash.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@backstage-community/plugin-rbac': minor
---

Upgraded RBAC plugin from mui4 to mui5
4 changes: 1 addition & 3 deletions workspaces/rbac/plugins/rbac/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,9 @@
"@backstage/plugin-permission-react": "^0.4.27",
"@backstage/theme": "^0.6.0",
"@janus-idp/shared-react": "^2.13.1",
"@material-ui/core": "^4.9.13",
"@material-ui/icons": "^4.11.3",
"@material-ui/lab": "^4.0.0-alpha.45",
divyanshiGupta marked this conversation as resolved.
Show resolved Hide resolved
"@mui/icons-material": "5.16.4",
"@mui/material": "^5.14.18",
"@mui/styles": "^6.1.7",
Copy link
Contributor

Choose a reason for hiding this comment

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

6.1.7? Ain't this suppose to be 5?

Copy link
Contributor

Choose a reason for hiding this comment

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

@drodil Thanks for reviewing it . "^6.1.7" is the latest version of @mui/styles which is compatible with MUI v5 . Let us know the reason if you still think we should have used version 5

"@rjsf/core": "^5.21.2",
"@rjsf/mui": "^5.21.2",
"@rjsf/utils": "^5.21.2",
Expand Down
38 changes: 20 additions & 18 deletions workspaces/rbac/plugins/rbac/src/components/ConditionalAccess/AddNestedConditionButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,28 +15,30 @@
*/
import React from 'react';

import { Box, Typography } from '@material-ui/core';
import Tooltip from '@material-ui/core/Tooltip';
import HelpOutlineIcon from '@material-ui/icons/HelpOutline';
import HelpOutlineIcon from '@mui/icons-material/HelpOutline';
import Box from '@mui/material/Box';
import Tooltip from '@mui/material/Tooltip';
import Typography from '@mui/material/Typography';

export const tooltipTitle = () => (
<div>
<Typography variant="body1" component="p" align="center">
Nested conditions are <b>1 layer rules within a main condition</b>. It
lets you allow appropriate access by using detailed permissions based on
various conditions. You can add multiple nested conditions.
</Typography>
<Typography variant="body1" component="p" align="center">
For example, you can allow access to all entity types in the main
condition and use a nested condition to limit the access to entities owned
by the user.
</Typography>
</div>
);

export const AddNestedConditionButton = () => {
const tooltipTitle = () => (
<div>
<Typography variant="body1" component="p" align="center">
Nested conditions are <b>1 layer rules within a main condition</b>. It
lets you allow appropriate access by using detailed permissions based on
various conditions. You can add multiple nested conditions.
</Typography>
<Typography variant="body1" component="p" align="center">
For example, you can allow access to all entity types in the main
condition and use a nested condition to limit the access to entities
owned by the user.
</Typography>
</div>
);
return (
<Box
style={{
sx={{
display: 'flex',
justifyContent: 'center',
alignItems: 'center',
Expand Down
11 changes: 7 additions & 4 deletions workspaces/rbac/plugins/rbac/src/components/ConditionalAccess/ComplexConditionRow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ import React from 'react';

import { PermissionCondition } from '@backstage/plugin-permission-common';

import { IconButton } from '@material-ui/core';
import RemoveIcon from '@mui/icons-material/Remove';
import IconButton from '@mui/material/IconButton';

import {
getNestedRuleErrors,
Expand Down Expand Up @@ -51,7 +51,6 @@ type ComplexConditionRowProps = {
setRemoveAllClicked: React.Dispatch<React.SetStateAction<boolean>>;
conditionRulesData?: RulesData;
notConditionType?: NotConditionType;
classes: any;
currentCondition: Condition;
ruleIndex: number;
activeCriteria?: 'allOf' | 'anyOf';
Expand All @@ -70,7 +69,6 @@ export const ComplexConditionRow = ({
setRemoveAllClicked,
conditionRulesData,
notConditionType,
classes,
currentCondition,
ruleIndex,
activeCriteria,
Expand Down Expand Up @@ -252,7 +250,12 @@ export const ComplexConditionRow = ({
/>
<IconButton
title="Remove"
className={classes.removeRuleButton}
sx={{
color: theme => theme.palette.grey[500],
flexGrow: 0,
alignSelf: 'baseline',
marginTop: theme => theme.spacing(3.3),
}}
disabled={isNestedCondition ? nestedDisabled : disabled}
onClick={
isNestedCondition &&
Expand Down
38 changes: 29 additions & 9 deletions workspaces/rbac/plugins/rbac/src/components/ConditionalAccess/ComplexConditionRowButtons.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,20 @@
import React from 'react';

import AddIcon from '@mui/icons-material/Add';
import HelpOutlineIcon from '@mui/icons-material/HelpOutline';
import Box from '@mui/material/Box';
import Button from '@mui/material/Button';
import Tooltip from '@mui/material/Tooltip';

import { getDefaultRule } from '../../utils/conditional-access-utils';
import { AddNestedConditionButton } from './AddNestedConditionButton';
import { tooltipTitle } from './AddNestedConditionButton';
import { criterias } from './const';
import { Condition, ConditionsData } from './types';

type ComplexConditionRowButtonsProps = {
conditionRow: ConditionsData;
onRuleChange: (newCondition: ConditionsData) => void;
criteria: string;
classes: any;
selPluginResourceType: string;
updateErrors: (_index: number) => void;
isNestedConditionRule: (condition: Condition) => boolean;
Expand All @@ -39,7 +40,6 @@ export const ComplexConditionRowButtons = ({
conditionRow,
onRuleChange,
criteria,
classes,
selPluginResourceType,
updateErrors,
isNestedConditionRule,
Expand Down Expand Up @@ -72,22 +72,42 @@ export const ComplexConditionRowButtons = ({

return (
(criteria === criterias.allOf || criteria === criterias.anyOf) && (
<Box mt={1} mb={1}>
<Box
mt={1}
mb={1}
sx={{
display: 'flex',
flexDirection: 'column',
alignItems: 'flex-start',
}}
>
<Button
className={classes.addRuleButton}
sx={{
color: theme => theme.palette.primary.light,
}}
size="small"
onClick={handleAddRule}
startIcon={<AddIcon fontSize="small" />}
>
<AddIcon fontSize="small" />
Add rule
</Button>
<Button
className={classes.addNestedConditionButton}
sx={{
color: theme => theme.palette.primary.light,
}}
size="small"
onClick={() => handleAddNestedCondition(criteria)}
startIcon={<AddIcon fontSize="small" />}
endIcon={
<Tooltip title={tooltipTitle()} placement="top">
<HelpOutlineIcon
fontSize="small"
style={{ marginLeft: '0.25rem' }}
/>
</Tooltip>
}
>
<AddIcon fontSize="small" />
<AddNestedConditionButton />
Add Nested Condition
</Button>
</Box>
)
Expand Down
92 changes: 43 additions & 49 deletions workspaces/rbac/plugins/rbac/src/components/ConditionalAccess/ConditionalAccessSidebar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,50 +15,15 @@
*/
import React from 'react';

import { makeStyles } from '@material-ui/core';
import Drawer from '@material-ui/core/Drawer';
import CloseIcon from '@mui/icons-material/Close';
import Box from '@mui/material/Box';
import Drawer from '@mui/material/Drawer';
import IconButton from '@mui/material/IconButton';
import Typography from '@mui/material/Typography';

import { ConditionsForm } from './ConditionsForm';
import { ConditionsData, RulesData } from './types';

const useDrawerStyles = makeStyles(() => ({
paper: {
['@media (max-width: 960px)']: {
width: '-webkit-fill-available',
},
width: '50vw',
height: '100vh',
gap: '3%',
display: '-webkit-inline-box',
},
}));

const useDrawerContentStyles = makeStyles(theme => ({
sidebar: {
display: 'flex',
flexFlow: 'column',
justifyContent: 'space-between',
backgroundColor: `${theme.palette.background.default} !important`,
},
header: {
display: 'flex',
flexDirection: 'row',
justifyContent: 'space-between',
alignItems: 'baseline',
padding: theme.spacing(2.5),
fontFamily: theme.typography.fontFamily,
},
headerSubtitle: {
fontWeight: 400,
fontFamily: theme.typography.fontFamily,
paddingTop: theme.spacing(1),
},
}));

type ConditionalAccessSidebarProps = {
open: boolean;
onClose: () => void;
Expand All @@ -76,35 +41,64 @@ export const ConditionalAccessSidebar = ({
conditionRulesData,
conditionsFormVal,
}: ConditionalAccessSidebarProps) => {
const classes = useDrawerStyles();
const contentClasses = useDrawerContentStyles();
return (
<Drawer
anchor="right"
open={open}
data-testid="rules-sidebar"
classes={{
paper: classes.paper,
PaperProps={{
sx: {
['@media (max-width: 960px)']: {
width: '100%',
},
width: '50%',
height: '100%',
},
}}
>
<Box className={contentClasses.sidebar}>
<Box className={contentClasses.header}>
<Typography variant="h5">
<Typography component="span" sx={{ fontWeight: 500 }}>
Configure access for the
</Typography>{' '}
{selPluginResourceType}
<Box
sx={{
display: 'flex',
flexFlow: 'column',
justifyContent: 'space-between',
height: '100%',
backgroundColor: theme => `${theme.palette.background.paper}`,
}}
>
<Box
sx={{
display: 'flex',
flexDirection: 'row',
justifyContent: 'space-between',
alignItems: 'baseline',
padding: theme => theme.spacing(2.5),
fontFamily: theme => theme.typography.fontFamily,
}}
>
<Box>
<Box sx={{ display: 'flex', gap: '5px' }}>
<Typography sx={{ fontWeight: 500 }} variant="h5">
Configure access for the
</Typography>
<Typography style={{ fontWeight: 600 }} variant="h5">
{selPluginResourceType}
</Typography>
</Box>
<Typography
variant="body2"
className={contentClasses.headerSubtitle}
sx={{
fontWeight: 400,
fontFamily: theme => theme.typography.fontFamily,
paddingTop: theme => theme.spacing(1),
}}
align="left"
>
By default, the selected resource type will be visible to the
chosen users in step two. If you want to restrict or grant
permission to specific plugin resource type rule, select it and
add the required parameters.
</Typography>
</Typography>
</Box>
<IconButton
key="dismiss"
title="Close the drawer"
Expand Down
26 changes: 7 additions & 19 deletions workspaces/rbac/plugins/rbac/src/components/ConditionalAccess/ConditionsForm.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,22 @@
*/
import React from 'react';

import { fireEvent, render, screen } from '@testing-library/react';
import { fireEvent, render } from '@testing-library/react';

import { mockTransformedConditionRules } from '../../__fixtures__/mockTransformedConditionRules';
import { ConditionsForm } from './ConditionsForm';
import * as ConditionsFormRowFields from './ConditionsFormRowFields';

jest.mock('@material-ui/core', () => ({
...jest.requireActual('@material-ui/core'),
makeStyles: jest.fn().mockReturnValue(() => ({})),
}));
jest.mock('../ConditionalAccess/ConditionsFormRowFields');

describe('ConditionsForm', () => {
const selPluginResourceType = 'catalog-entity';
const onSaveMock = jest.fn();
const onCloseMock = jest.fn();

const spy = jest.spyOn(ConditionsFormRowFields, 'getTextFieldStyles');
spy.mockReturnValue({} as any);

const renderComponent = (props = {}) =>
render(
<ConditionsForm
Expand Down Expand Up @@ -79,21 +80,8 @@ describe('ConditionsForm', () => {
expect(onSaveMock).toHaveBeenCalledWith(undefined);
});

it('disables Save button if conditions are unchanged', () => {
const { getByTestId } = renderComponent();
expect(getByTestId('save-conditions')).toBeDisabled();
});

it('disables Save button if no rule is selected', () => {
it('disables Save button if conditions are unchanged or no rule selected', () => {
const { getByTestId } = renderComponent();

fireEvent.change(
screen.getByRole('textbox', {
name: /rule/i,
}),
{ target: { value: '' } },
);

expect(getByTestId('save-conditions')).toBeDisabled();
});

Expand Down
Loading
Loading