refactor: fix segment permission checks (#930)

* refactor: use DELETE_SEGMENT permission in segments list * refactor: clean up segment form mode prop * refactor: format file * refactor: fix ConstraintAccordion permission checks
2025-02-09 00:18:00 +01:00 · 2022-04-29 14:53:28 +02:00 · 2022-04-29 14:53:28 +02:00 · 774157b8d7
commit 774157b8d7
parent 34c4747cd4
11 changed files with 143 additions and 163 deletions
--- a/frontend/src/component/common/ConstraintAccordion/ConstraintAccordion.tsx
+++ b/frontend/src/component/common/ConstraintAccordion/ConstraintAccordion.tsx
@ -7,7 +7,6 @@ import { ConstraintAccordionView } from './ConstraintAccordionView/ConstraintAcc
 interface IConstraintAccordionProps {
    compact: boolean;
    editing: boolean;
-    environmentId?: string;
    constraint: IConstraint;
    onCancel: () => void;
    onEdit?: () => void;
@ -19,7 +18,6 @@ export const ConstraintAccordion = ({
    constraint,
    compact = false,
    editing,
-    environmentId,
    onEdit,
    onCancel,
    onDelete,
@ -43,7 +41,6 @@ export const ConstraintAccordion = ({
                    constraint={constraint}
                    onEdit={onEdit}
                    onDelete={onDelete}
-                    environmentId={environmentId}
                    compact={compact}
                />
            }
--- a/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionList/ConstraintAccordionList.tsx
+++ b/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionList/ConstraintAccordionList.tsx
@ -3,20 +3,14 @@ import React, { forwardRef, useImperativeHandle } from 'react';
 import { ConstraintAccordion } from 'component/common/ConstraintAccordion/ConstraintAccordion';
 import produce from 'immer';
 import useUnleashContext from 'hooks/api/getters/useUnleashContext/useUnleashContext';
-import PermissionButton from 'component/common/PermissionButton/PermissionButton';
-import {
-    CREATE_FEATURE_STRATEGY,
-    UPDATE_FEATURE_STRATEGY,
-} from 'component/providers/AccessProvider/permissions';
 import { useWeakMap } from 'hooks/useWeakMap';
 import { objectId } from 'utils/objectId';
 import { useStyles } from './ConstraintAccordionList.styles';
 import { createEmptyConstraint } from 'component/common/ConstraintAccordion/ConstraintAccordionList/createEmptyConstraint';
 import ConditionallyRender from 'component/common/ConditionallyRender';
+import { Button } from '@material-ui/core';

 interface IConstraintAccordionListProps {
-    projectId?: string;
-    environmentId?: string;
    constraints: IConstraint[];
    setConstraints?: React.Dispatch<React.SetStateAction<IConstraint[]>>;
    showCreateButton?: boolean;
@ -40,115 +34,97 @@ export const constraintAccordionListId = 'constraintAccordionListId';
 export const ConstraintAccordionList = forwardRef<
    IConstraintAccordionListRef | undefined,
    IConstraintAccordionListProps
->(
-    (
-        {
-            projectId,
-            environmentId,
-            constraints,
-            setConstraints,
-            showCreateButton,
-        },
-        ref
-    ) => {
-        const state = useWeakMap<
-            IConstraint,
-            IConstraintAccordionListItemState
-        >();
-        const { context } = useUnleashContext();
-        const styles = useStyles();
+>(({ constraints, setConstraints, showCreateButton }, ref) => {
+    const state = useWeakMap<IConstraint, IConstraintAccordionListItemState>();
+    const { context } = useUnleashContext();
+    const styles = useStyles();

-        const addConstraint =
-            setConstraints &&
-            ((contextName: string) => {
-                const constraint = createEmptyConstraint(contextName);
-                state.set(constraint, { editing: true, new: true });
-                setConstraints(prev => [...prev, constraint]);
-            });
+    const addConstraint =
+        setConstraints &&
+        ((contextName: string) => {
+            const constraint = createEmptyConstraint(contextName);
+            state.set(constraint, { editing: true, new: true });
+            setConstraints(prev => [...prev, constraint]);
+        });

-        useImperativeHandle(ref, () => ({
-            addConstraint,
-        }));
+    useImperativeHandle(ref, () => ({
+        addConstraint,
+    }));

-        const onAdd =
-            addConstraint &&
-            (() => {
-                addConstraint(context[0].name);
-            });
+    const onAdd =
+        addConstraint &&
+        (() => {
+            addConstraint(context[0].name);
+        });

-        const onEdit =
-            setConstraints &&
-            ((constraint: IConstraint) => {
-                state.set(constraint, { editing: true });
-            });
+    const onEdit =
+        setConstraints &&
+        ((constraint: IConstraint) => {
+            state.set(constraint, { editing: true });
+        });

-        const onRemove =
-            setConstraints &&
-            ((index: number) => {
-                const constraint = constraints[index];
-                state.set(constraint, {});
-                setConstraints(
-                    produce(draft => {
-                        draft.splice(index, 1);
-                    })
-                );
-            });
-
-        const onSave =
-            setConstraints &&
-            ((index: number, constraint: IConstraint) => {
-                state.set(constraint, {});
-                setConstraints(
-                    produce(draft => {
-                        draft[index] = constraint;
-                    })
-                );
-            });
-
-        const onCancel = (index: number) => {
+    const onRemove =
+        setConstraints &&
+        ((index: number) => {
            const constraint = constraints[index];
-            state.get(constraint)?.new && onRemove?.(index);
            state.set(constraint, {});
-        };
+            setConstraints(
+                produce(draft => {
+                    draft.splice(index, 1);
+                })
+            );
+        });

-        if (context.length === 0) {
-            return null;
-        }
+    const onSave =
+        setConstraints &&
+        ((index: number, constraint: IConstraint) => {
+            state.set(constraint, {});
+            setConstraints(
+                produce(draft => {
+                    draft[index] = constraint;
+                })
+            );
+        });

-        return (
-            <div className={styles.container} id={constraintAccordionListId}>
-                <ConditionallyRender
-                    condition={Boolean(showCreateButton && setConstraints)}
-                    show={
-                        <PermissionButton
+    const onCancel = (index: number) => {
+        const constraint = constraints[index];
+        state.get(constraint)?.new && onRemove?.(index);
+        state.set(constraint, {});
+    };
+
+    if (context.length === 0) {
+        return null;
+    }
+
+    return (
+        <div className={styles.container} id={constraintAccordionListId}>
+            <ConditionallyRender
+                condition={Boolean(showCreateButton && onAdd)}
+                show={
+                    <div>
+                        <Button
                            type="button"
                            onClick={onAdd}
                            variant="text"
-                            permission={[
-                                UPDATE_FEATURE_STRATEGY,
-                                CREATE_FEATURE_STRATEGY,
-                            ]}
-                            environmentId={environmentId}
-                            projectId={projectId}
+                            color="primary"
                        >
                            Add custom constraint
-                        </PermissionButton>
-                    }
+                        </Button>
+                    </div>
+                }
+            />
+            {constraints.map((constraint, index) => (
+                <ConstraintAccordion
+                    key={objectId(constraint)}
+                    constraint={constraint}
+                    onEdit={onEdit && onEdit.bind(null, constraint)}
+                    onCancel={onCancel.bind(null, index)}
+                    onDelete={onRemove && onRemove.bind(null, index)}
+                    onSave={onSave && onSave.bind(null, index)}
+                    editing={Boolean(state.get(constraint)?.editing)}
+                    compact
                />
-                {constraints.map((constraint, index) => (
-                    <ConstraintAccordion
-                        key={objectId(constraint)}
-                        environmentId={environmentId}
-                        constraint={constraint}
-                        onEdit={onEdit && onEdit.bind(null, constraint)}
-                        onCancel={onCancel.bind(null, index)}
-                        onDelete={onRemove && onRemove.bind(null, index)}
-                        onSave={onSave && onSave.bind(null, index)}
-                        editing={Boolean(state.get(constraint)?.editing)}
-                        compact
-                    />
-                ))}
-            </div>
-        );
-    }
-);
+            ))}
+        </div>
+    );
+});
--- a/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionView/ConstraintAccordionView.tsx
+++ b/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionView/ConstraintAccordionView.tsx
@ -17,7 +17,6 @@ import {

 import { useStyles } from '../ConstraintAccordion.styles';
 interface IConstraintAccordionViewProps {
-    environmentId?: string;
    constraint: IConstraint;
    onDelete?: () => void;
    onEdit?: () => void;
@ -26,7 +25,6 @@ interface IConstraintAccordionViewProps {

 export const ConstraintAccordionView = ({
    compact,
-    environmentId,
    constraint,
    onEdit,
    onDelete,
@ -49,7 +47,6 @@ export const ConstraintAccordionView = ({
            >
                <ConstraintAccordionViewHeader
                    compact={compact}
-                    environmentId={environmentId}
                    constraint={constraint}
                    onEdit={onEdit}
                    onDelete={onDelete}
--- a/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionView/ConstraintAccordionViewHeader/ConstraintAccordionViewHeader.tsx
+++ b/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionView/ConstraintAccordionViewHeader/ConstraintAccordionViewHeader.tsx
@ -1,15 +1,11 @@
 import StringTruncator from 'component/common/StringTruncator/StringTruncator';
-import { Chip, useMediaQuery } from '@material-ui/core';
+import { Chip, useMediaQuery, IconButton } from '@material-ui/core';
 import { ConstraintIcon } from 'component/common/ConstraintAccordion/ConstraintIcon';
 import { Delete, Edit } from '@material-ui/icons';
 import { IConstraint } from 'interfaces/strategy';

 import { useStyles } from 'component/common/ConstraintAccordion/ConstraintAccordion.styles';
 import ConditionallyRender from 'component/common/ConditionallyRender';
-import PermissionIconButton from 'component/common/PermissionIconButton/PermissionIconButton';
-import { UPDATE_FEATURE_STRATEGY } from 'component/providers/AccessProvider/permissions';
-import { useParams } from 'react-router-dom';
-import { IFeatureViewParams } from 'interfaces/params';
 import React from 'react';
 import { formatConstraintValue } from 'utils/formatConstraintValue';
 import { useLocationSettings } from 'hooks/useLocationSettings';
@ -21,7 +17,6 @@ interface IConstraintAccordionViewHeaderProps {
    onDelete?: () => void;
    onEdit?: () => void;
    singleValue: boolean;
-    environmentId?: string;
 }

 export const ConstraintAccordionViewHeader = ({
@ -30,11 +25,9 @@ export const ConstraintAccordionViewHeader = ({
    onEdit,
    onDelete,
    singleValue,
-    environmentId,
 }: IConstraintAccordionViewHeaderProps) => {
    const styles = useStyles();
    const { locationSettings } = useLocationSettings();
-    const { projectId } = useParams<IFeatureViewParams>();
    const smallScreen = useMediaQuery(`(max-width:${790}px)`);

    const minWidthHeader = compact || smallScreen ? '100px' : '175px';
@ -97,32 +90,19 @@ export const ConstraintAccordionViewHeader = ({
            <div className={styles.headerActions}>
                <ConditionallyRender
                    condition={Boolean(onEditClick)}
-                    show={
-                        <PermissionIconButton
-                            onClick={onEditClick!}
-                            permission={UPDATE_FEATURE_STRATEGY}
-                            projectId={projectId}
-                            environmentId={environmentId}
-                            hidden={!onEdit}
-                            tooltip="Edit constraint"
-                        >
-                            <Edit />
-                        </PermissionIconButton>
-                    }
+                    show={() => (
+                        <IconButton type="button" onClick={onEditClick}>
+                            <Edit titleAccess="Edit constraint" />
+                        </IconButton>
+                    )}
                />
                <ConditionallyRender
                    condition={Boolean(onDeleteClick)}
-                    show={
-                        <PermissionIconButton
-                            onClick={onDeleteClick!}
-                            permission={UPDATE_FEATURE_STRATEGY}
-                            projectId={projectId}
-                            tooltip="Delete constraint"
-                            environmentId={environmentId}
-                        >
-                            <Delete />
-                        </PermissionIconButton>
-                    }
+                    show={() => (
+                        <IconButton type="button" onClick={onDeleteClick}>
+                            <Delete titleAccess="Delete constraint" />
+                        </IconButton>
+                    )}
                />
            </div>
        </div>
--- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/FeatureStrategyConstraints.tsx
+++ b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/FeatureStrategyConstraints.tsx
@ -1,6 +1,11 @@
 import { IConstraint, IFeatureStrategy } from 'interfaces/strategy';
-import React, { useMemo } from 'react';
+import React, { useMemo, useContext } from 'react';
 import { ConstraintAccordionList } from 'component/common/ConstraintAccordion/ConstraintAccordionList/ConstraintAccordionList';
+import AccessContext from 'contexts/AccessContext';
+import {
+    UPDATE_FEATURE_STRATEGY,
+    CREATE_FEATURE_STRATEGY,
+} from 'component/providers/AccessProvider/permissions';

 interface IFeatureStrategyConstraintsProps {
    projectId: string;
@ -17,6 +22,8 @@ export const FeatureStrategyConstraints = ({
    strategy,
    setStrategy,
 }: IFeatureStrategyConstraintsProps) => {
+    const { hasAccess } = useContext(AccessContext);
+
    const constraints = useMemo(() => {
        return strategy.constraints ?? [];
    }, [strategy]);
@ -28,13 +35,23 @@ export const FeatureStrategyConstraints = ({
        }));
    };

+    const showCreateButton = hasAccess(
+        CREATE_FEATURE_STRATEGY,
+        projectId,
+        environmentId
+    );
+
+    const allowEditAndDelete = hasAccess(
+        UPDATE_FEATURE_STRATEGY,
+        projectId,
+        environmentId
+    );
+
    return (
        <ConstraintAccordionList
-            projectId={projectId}
-            environmentId={environmentId}
            constraints={constraints}
-            setConstraints={setConstraints}
-            showCreateButton
+            setConstraints={allowEditAndDelete ? setConstraints : undefined}
+            showCreateButton={showCreateButton}
        />
    );
 };
--- a/frontend/src/component/segments/CreateSegment/CreateSegment.tsx
+++ b/frontend/src/component/segments/CreateSegment/CreateSegment.tsx
@ -89,9 +89,9 @@ export const CreateSegment = () => {
                setDescription={setDescription}
                constraints={constraints}
                setConstraints={setConstraints}
-                mode="Create"
                errors={errors}
                clearErrors={clearErrors}
+                mode="create"
            >
                <CreateButton
                    name="segment"
--- a/frontend/src/component/segments/EditSegment/EditSegment.tsx
+++ b/frontend/src/component/segments/EditSegment/EditSegment.tsx
@ -92,9 +92,9 @@ export const EditSegment = () => {
                setDescription={setDescription}
                constraints={constraints}
                setConstraints={setConstraints}
-                mode="Edit"
                errors={errors}
                clearErrors={clearErrors}
+                mode="edit"
            >
                <UpdateButton
                    permission={UPDATE_SEGMENT}
--- a/frontend/src/component/segments/SegmentForm/SegmentForm.tsx
+++ b/frontend/src/component/segments/SegmentForm/SegmentForm.tsx
@ -7,6 +7,8 @@ import { SegmentFormStepList } from 'component/segments/SegmentFormStepList/Segm
 import ConditionallyRender from 'component/common/ConditionallyRender';

 export type SegmentFormStep = 1 | 2;
+export type SegmentFormMode = 'create' | 'edit';
+
 interface ISegmentProps {
    name: string;
    description: string;
@ -16,8 +18,8 @@ interface ISegmentProps {
    setConstraints: React.Dispatch<React.SetStateAction<IConstraint[]>>;
    handleSubmit: (e: any) => void;
    errors: { [key: string]: string };
-    mode: 'Create' | 'Edit';
    clearErrors: () => void;
+    mode: SegmentFormMode;
 }

 export const SegmentForm: React.FC<ISegmentProps> = ({
@ -31,6 +33,7 @@ export const SegmentForm: React.FC<ISegmentProps> = ({
    handleSubmit,
    errors,
    clearErrors,
+    mode,
 }) => {
    const styles = useStyles();
    const totalSteps = 2;
@ -61,6 +64,7 @@ export const SegmentForm: React.FC<ISegmentProps> = ({
                            constraints={constraints}
                            setConstraints={setConstraints}
                            setCurrentStep={setCurrentStep}
+                            mode={mode}
                        >
                            {children}
                        </SegmentFormStepTwo>
--- a/frontend/src/component/segments/SegmentFormStepTwo/SegmentFormStepTwo.tsx
+++ b/frontend/src/component/segments/SegmentFormStepTwo/SegmentFormStepTwo.tsx
@ -1,11 +1,15 @@
-import React, { useRef, useState } from 'react';
+import React, { useRef, useState, useContext } from 'react';
 import { Button } from '@material-ui/core';
 import { Add } from '@material-ui/icons';
 import ConditionallyRender from 'component/common/ConditionallyRender';
 import PermissionButton from 'component/common/PermissionButton/PermissionButton';
 import { SidebarModal } from 'component/common/SidebarModal/SidebarModal';
 import { CreateUnleashContext } from 'component/context/CreateUnleashContext/CreateUnleashContext';
-import { CREATE_CONTEXT_FIELD } from 'component/providers/AccessProvider/permissions';
+import {
+    CREATE_CONTEXT_FIELD,
+    CREATE_SEGMENT,
+    UPDATE_SEGMENT,
+} from 'component/providers/AccessProvider/permissions';
 import useUnleashContext from 'hooks/api/getters/useUnleashContext/useUnleashContext';
 import { IConstraint } from 'interfaces/strategy';
 import { useHistory } from 'react-router-dom';
@ -14,7 +18,7 @@ import {
    ConstraintAccordionList,
    IConstraintAccordionListRef,
 } from 'component/common/ConstraintAccordion/ConstraintAccordionList/ConstraintAccordionList';
-import { SegmentFormStep } from '../SegmentForm/SegmentForm';
+import { SegmentFormStep, SegmentFormMode } from '../SegmentForm/SegmentForm';
 import {
    AutocompleteBox,
    IAutocompleteBoxOption,
@ -25,11 +29,13 @@ import {
 } from 'component/segments/SegmentDocs/SegmentDocs';
 import { useSegmentValuesCount } from 'component/segments/hooks/useSegmentValuesCount';
 import { SEGMENT_VALUES_LIMIT } from 'utils/segmentLimits';
+import AccessContext from 'contexts/AccessContext';

 interface ISegmentFormPartTwoProps {
    constraints: IConstraint[];
    setConstraints: React.Dispatch<React.SetStateAction<IConstraint[]>>;
    setCurrentStep: React.Dispatch<React.SetStateAction<SegmentFormStep>>;
+    mode: SegmentFormMode;
 }

 export const SegmentFormStepTwo: React.FC<ISegmentFormPartTwoProps> = ({
@ -37,14 +43,17 @@ export const SegmentFormStepTwo: React.FC<ISegmentFormPartTwoProps> = ({
    constraints,
    setConstraints,
    setCurrentStep,
+    mode,
 }) => {
    const constraintsAccordionListRef = useRef<IConstraintAccordionListRef>();
    const history = useHistory();
    const styles = useStyles();
+    const { hasAccess } = useContext(AccessContext);
    const { context = [] } = useUnleashContext();
    const [open, setOpen] = useState(false);
    const segmentValuesCount = useSegmentValuesCount(constraints);
    const overSegmentValuesLimit = segmentValuesCount > SEGMENT_VALUES_LIMIT;
+    const modePermission = mode === 'create' ? CREATE_SEGMENT : UPDATE_SEGMENT;

    const autocompleteOptions = context.map(c => ({
        value: c.name,
@ -122,7 +131,11 @@ export const SegmentFormStepTwo: React.FC<ISegmentFormPartTwoProps> = ({
                    <ConstraintAccordionList
                        ref={constraintsAccordionListRef}
                        constraints={constraints}
-                        setConstraints={setConstraints}
+                        setConstraints={
+                            hasAccess(modePermission)
+                                ? setConstraints
+                                : undefined
+                        }
                    />
                </div>
            </div>
--- a/frontend/src/component/segments/SegmentList/SegmentList.tsx
+++ b/frontend/src/component/segments/SegmentList/SegmentList.tsx
@ -1,4 +1,4 @@
-import { useContext, useState } from 'react';
+import { useState } from 'react';
 import {
    Table,
    TableBody,
@ -7,12 +7,8 @@ import {
    TableRow,
    Typography,
 } from '@material-ui/core';
-import AccessContext from 'contexts/AccessContext';
 import usePagination from 'hooks/usePagination';
-import {
-    CREATE_SEGMENT,
-    UPDATE_SEGMENT,
-} from 'component/providers/AccessProvider/permissions';
+import { CREATE_SEGMENT } from 'component/providers/AccessProvider/permissions';
 import PaginateUI from 'component/common/PaginateUI/PaginateUI';
 import { SegmentListItem } from './SegmentListItem/SegmentListItem';
 import { ISegment } from 'interfaces/segment';
@ -32,7 +28,6 @@ import { NAVIGATE_TO_CREATE_SEGMENT } from 'utils/testIds';

 export const SegmentsList = () => {
    const history = useHistory();
-    const { hasAccess } = useContext(AccessContext);
    const { segments = [], refetchSegments } = useSegments();
    const { deleteSegment } = useSegmentsApi();
    const { page, pages, nextPage, prevPage, setPageIndex, pageIndex } =
@ -145,7 +140,7 @@ export const SegmentsList = () => {
                            classes={{ root: styles.cell }}
                            className={styles.lastHeader}
                        >
-                            {hasAccess(UPDATE_SEGMENT) ? 'Actions' : ''}
+                            Action
                        </TableCell>
                    </TableRow>
                </TableHead>
--- a/frontend/src/component/segments/SegmentList/SegmentListItem/SegmentListItem.tsx
+++ b/frontend/src/component/segments/SegmentList/SegmentListItem/SegmentListItem.tsx
@ -2,14 +2,15 @@ import { useStyles } from './SegmentListItem.styles';
 import { TableCell, TableRow, Typography } from '@material-ui/core';
 import { Delete, Edit } from '@material-ui/icons';
 import {
-    ADMIN,
    UPDATE_SEGMENT,
+    DELETE_SEGMENT,
 } from 'component/providers/AccessProvider/permissions';
 import PermissionIconButton from 'component/common/PermissionIconButton/PermissionIconButton';
 import TimeAgo from 'react-timeago';
 import { ISegment } from 'interfaces/segment';
 import { useHistory } from 'react-router-dom';
 import { SEGMENT_DELETE_BTN_ID } from 'utils/testIds';
+import React from 'react';

 interface ISegmentListItemProps {
    id: number;
@ -82,7 +83,7 @@ export const SegmentListItem = ({
                        });
                        setDelDialog(true);
                    }}
-                    permission={ADMIN}
+                    permission={DELETE_SEGMENT}
                    tooltip="Remove segment"
                    data-testid={`${SEGMENT_DELETE_BTN_ID}_${name}`}
                >