1
0
mirror of https://github.com/Unleash/unleash.git synced 2025-01-25 00:07:47 +01:00

fix: use the correct hook for permissions and CR (#3050)

## About the changes
When checking for permissions we have 2 methods, one that is not change
request aware and one that is. We were using the one that is not aware
of CR while the feature we developed is aware of CR.

This PR switches to the correct method and documents the hooks
This commit is contained in:
Gastón Fournier 2023-02-06 16:00:58 +01:00 committed by GitHub
parent e7f68eedd5
commit fbd2d99d97
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 53 additions and 28 deletions

View File

@ -0,0 +1,28 @@
import { FC } from 'react';
import { useHasProjectEnvironmentAccess } from 'hooks/useHasAccess';
import { Checkbox, MenuItem } from '@mui/material';
interface PermissionCheckboxMenuItemProps {
permission: string | string[];
projectId: string;
environment: string;
checked: boolean;
onClick: () => void;
}
export const PermissionCheckboxMenuItem: FC<
PermissionCheckboxMenuItemProps
> = ({ permission, projectId, environment, checked, onClick, ...props }) => {
const hasPermissions = useHasProjectEnvironmentAccess(
permission,
projectId,
environment
);
return (
<MenuItem disabled={!hasPermissions} onClick={onClick} {...props}>
<Checkbox checked={checked} />
{environment}
</MenuItem>
);
};

View File

@ -1,15 +1,8 @@
import {
Button,
Checkbox,
Divider,
Menu,
MenuItem,
styled,
} from '@mui/material';
import { Button, Divider, Menu, styled } from '@mui/material';
import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender';
import { IFeatureEnvironmentWithCrEnabled } from 'interfaces/featureToggle';
import { useState } from 'react';
import { useCheckProjectAccess } from 'hooks/useHasAccess';
import { PermissionCheckboxMenuItem } from './PermissionCheckboxMenuItem';
const StyledMenu = styled(Menu)(({ theme }) => ({
'& > div > ul': {
@ -54,12 +47,6 @@ export const PushVariantsButton = ({
IFeatureEnvironmentWithCrEnabled[]
>([]);
const hasAccess = useCheckProjectAccess(projectId);
const hasAccessTo = environments.reduce((acc, env) => {
acc[env.name] = hasAccess(permission, env.name);
return acc;
}, {} as Record<string, boolean>);
const addSelectedEnvironment = (
environment: IFeatureEnvironmentWithCrEnabled
) => {
@ -127,25 +114,20 @@ export const PushVariantsButton = ({
{environments
.filter(environment => environment.name !== current)
.map(otherEnvironment => (
<MenuItem
<PermissionCheckboxMenuItem
projectId={projectId}
permission={permission}
environment={otherEnvironment.name}
key={otherEnvironment.name}
disabled={
!hasAccessTo[otherEnvironment.name] ??
false
}
checked={selectedEnvironments.includes(
otherEnvironment
)}
onClick={() =>
toggleSelectedEnvironment(
otherEnvironment
)
}
>
<Checkbox
checked={selectedEnvironments.includes(
otherEnvironment
)}
/>
{otherEnvironment.name}
</MenuItem>
/>
))}
<StyledActions>
<Divider />

View File

@ -6,8 +6,13 @@ import {
UPDATE_FEATURE_STRATEGY,
DELETE_FEATURE_STRATEGY,
UPDATE_FEATURE_ENVIRONMENT,
UPDATE_FEATURE_ENVIRONMENT_VARIANTS,
} from '../component/providers/AccessProvider/permissions';
/**
* This is for features not integrated with change request.
* If the feature is integrated with change request, use useCheckProjectAccess instead.
*/
const useCheckProjectPermissions = (projectId?: string) => {
const { hasAccess } = useContext(AccessContext);
@ -44,6 +49,11 @@ const useCheckProjectPermissions = (projectId?: string) => {
};
};
/**
* This is for features integrated with change request.
* If the feature is not integrated with change request, use useCheckProjectPermissions instead.
* When change request is enabled, the user will have access to the feature because permissions will be checked later
*/
export const useCheckProjectAccess = (projectId: string) => {
const { isChangeRequestConfigured } = useChangeRequestsEnabled(projectId);
const checkAccess = useCheckProjectPermissions(projectId);
@ -61,12 +71,17 @@ const ALLOWED_CHANGE_REQUEST_PERMISSIONS = [
UPDATE_FEATURE_STRATEGY,
DELETE_FEATURE_STRATEGY,
UPDATE_FEATURE_ENVIRONMENT,
UPDATE_FEATURE_ENVIRONMENT_VARIANTS,
];
const intersect = (array1: string[], array2: string[]) => {
return array1.filter(value => array2.includes(value)).length > 0;
};
/**
* This methods does the same as useCheckProjectAccess but also checks if the permission is in ALLOWED_CHANGE_REQUEST_PERMISSIONS
* If you know what you're doing you can skip that check and call useCheckProjectAccess
*/
export const useHasProjectEnvironmentAccess = (
permission: string | string[],
projectId: string,