Change requests - add multiple reviewers (#2448)

This PR adds implements the frontend and migrations part of multiple
reviewers.

2 UI parts:

1. Configuration to add the count of required approvals
2. Handle multiple approvers in review page.

frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestOverview.tsx

@@ -100,6 +100,9 @@ export const ChangeRequestOverview: FC = () => {
         changeRequest.state === 'In review' &&
         !isAdmin;
 
+    const hasApprovedAlready = changeRequest.approvals.some(
+        approval => approval.createdBy.id === user?.id
+    );
     return (
         <>
             <ChangeRequestHeader changeRequest={changeRequest} />
@@ -154,10 +157,14 @@ export const ChangeRequestOverview: FC = () => {
                         />
                         <ChangeRequestReviewStatus
                             state={changeRequest.state}
+                            environment={changeRequest.environment}
                         />
                         <StyledButtonBox>
                             <ConditionallyRender
-                                condition={changeRequest.state === 'In review'}
+                                condition={
+                                    changeRequest.state === 'In review' &&
+                                    !hasApprovedAlready
+                                }
                                 show={<ReviewButton />}
                             />
                             <ConditionallyRender

frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestReviewStatus/ChangeRequestReviewStatus.tsx

@@ -1,4 +1,4 @@
-import { FC } from 'react';
+import React, { FC } from 'react';
 import { Box, Theme, Typography, useTheme } from '@mui/material';
 import { ReactComponent as ChangesAppliedIcon } from 'assets/icons/merge.svg';
 import {
@@ -12,10 +12,35 @@ import {
     StyledDivider,
 } from './ChangeRequestReviewStatus.styles';
 import { ChangeRequestState } from 'component/changeRequest/changeRequest.types';
+import { useRequiredPathParam } from '../../../../hooks/useRequiredPathParam';
+import { useChangeRequestConfig } from '../../../../hooks/api/getters/useChangeRequestConfig/useChangeRequestConfig';
 interface ISuggestChangeReviewsStatusProps {
     state: ChangeRequestState;
+    environment: string;
 }
 
+export const useChangeRequestRequiredApprovals = (projectId: string) => {
+    const { data } = useChangeRequestConfig(projectId);
+
+    const getChangeRequestRequiredApprovals = React.useCallback(
+        (environment: string): number => {
+            const config = data.find(draft => {
+                return (
+                    draft.environment === environment &&
+                    draft.changeRequestEnabled
+                );
+            });
+
+            return config?.requiredApprovals || 1;
+        },
+        [data]
+    );
+
+    return {
+        getChangeRequestRequiredApprovals,
+    };
+};
+
 const resolveBorder = (state: ChangeRequestState, theme: Theme) => {
     if (state === 'Approved') {
         return `2px solid ${theme.palette.success.main}`;
@@ -51,9 +76,8 @@ const resolveIconColors = (state: ChangeRequestState, theme: Theme) => {
 
 export const ChangeRequestReviewStatus: FC<
     ISuggestChangeReviewsStatusProps
-> = ({ state }) => {
+> = ({ state, environment }) => {
     const theme = useTheme();
-
     return (
         <StyledOuterContainer>
             <StyledButtonContainer {...resolveIconColors(state, theme)}>
@@ -64,7 +88,7 @@ export const ChangeRequestReviewStatus: FC<
                 />
             </StyledButtonContainer>
             <StyledReviewStatusContainer border={resolveBorder(state, theme)}>
-                <ResolveComponent state={state} />
+                <ResolveComponent state={state} environment={environment} />
             </StyledReviewStatusContainer>
         </StyledOuterContainer>
     );
@@ -72,9 +96,10 @@ export const ChangeRequestReviewStatus: FC<
 
 interface IResolveComponentProps {
     state: ChangeRequestState;
+    environment: string;
 }
 
-const ResolveComponent = ({ state }: IResolveComponentProps) => {
+const ResolveComponent = ({ state, environment }: IResolveComponentProps) => {
     if (!state) {
         return null;
     }
@@ -91,7 +116,7 @@ const ResolveComponent = ({ state }: IResolveComponentProps) => {
         return <Cancelled />;
     }
 
-    return <ReviewRequired />;
+    return <ReviewRequired environment={environment} />;
 };
 
 const Approved = () => {
@@ -125,8 +150,16 @@ const Approved = () => {
     );
 };
 
-const ReviewRequired = () => {
+interface IReviewRequiredProps {
+    environment: string;
+}
+
+const ReviewRequired = ({ environment }: IReviewRequiredProps) => {
     const theme = useTheme();
+    const projectId = useRequiredPathParam('projectId');
+    const { getChangeRequestRequiredApprovals } =
+        useChangeRequestRequiredApprovals(projectId);
+    const approvals = getChangeRequestRequiredApprovals(environment);
 
     return (
         <>
@@ -137,7 +170,7 @@ const ReviewRequired = () => {
                         Review required
                     </StyledReviewTitle>
                     <Typography>
-                        At least 1 approving review must be submitted before
+                        At least {approvals} approvals must be submitted before
                         changes can be applied
                     </Typography>
                 </Box>

frontend/src/component/changeRequest/changeRequest.types.ts

@@ -18,6 +18,7 @@ export interface IChangeRequestEnvironmentConfig {
     environment: string;
     type: string;
     changeRequestEnabled: boolean;
+    requiredApprovals: number;
 }
 
 export interface IChangeRequestFeature {

frontend/src/component/common/GeneralSelect/GeneralSelect.tsx

@@ -9,12 +9,15 @@ import {
 } from '@mui/material';
 import { SELECT_ITEM_ID } from 'utils/testIds';
 import { KeyboardArrowDownOutlined } from '@mui/icons-material';
+import { SxProps } from '@mui/system';
+import { Theme } from '@mui/material/styles';
 
 export interface ISelectOption {
     key: string;
     title?: string;
     label?: string;
     disabled?: boolean;
+    sx?: SxProps<Theme>;
 }
 
 export interface IGeneralSelectProps extends Omit<SelectProps, 'onChange'> {
@@ -68,6 +71,7 @@ const GeneralSelect: React.FC<IGeneralSelectProps> = ({
             >
                 {options.map(option => (
                     <MenuItem
+                        sx={option.sx}
                         key={option.key}
                         value={option.key}
                         title={option.title || ''}

frontend/src/component/project/Project/ProjectSettings/ChangeRequestConfiguration/ChangeRequestConfiguration.tsx

@@ -1,6 +1,6 @@
-import { useMemo, useState, VFC } from 'react';
+import React, { useMemo, useState, VFC } from 'react';
 import { HeaderGroup, useGlobalFilter, useTable } from 'react-table';
-import { Alert, Box, Typography } from '@mui/material';
+import { Alert, Box, styled, Typography } from '@mui/material';
 import {
     SortableTableHeader,
     Table,
@@ -17,58 +17,114 @@ import { useRequiredPathParam } from 'hooks/useRequiredPathParam';
 import { Dialogue } from 'component/common/Dialogue/Dialogue';
 import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender';
 import { useChangeRequestConfig } from 'hooks/api/getters/useChangeRequestConfig/useChangeRequestConfig';
-import { useChangeRequestApi } from 'hooks/api/actions/useChangeRequestApi/useChangeRequestApi';
+import {
+    IChangeRequestConfig,
+    useChangeRequestApi,
+} from 'hooks/api/actions/useChangeRequestApi/useChangeRequestApi';
 import { UPDATE_PROJECT } from '@server/types/permissions';
 import useToast from 'hooks/useToast';
 import { formatUnknownError } from 'utils/formatUnknownError';
 import { ChangeRequestProcessHelp } from './ChangeRequestProcessHelp/ChangeRequestProcessHelp';
+import GeneralSelect from '../../../../common/GeneralSelect/GeneralSelect';
+import { KeyboardArrowDownOutlined } from '@mui/icons-material';
+import { useTheme } from '@mui/material/styles';
+
+const StyledBox = styled(Box)(({ theme }) => ({
+    padding: theme.spacing(1),
+    display: 'flex',
+    justifyContent: 'center',
+    '& .MuiInputBase-input': {
+        fontSize: theme.fontSizes.smallBody,
+    },
+}));
 
 export const ChangeRequestConfiguration: VFC = () => {
     const [dialogState, setDialogState] = useState<{
         isOpen: boolean;
-        enableEnvironment?: string;
+        enableEnvironment: string;
         isEnabled: boolean;
+        requiredApprovals: number;
     }>({
         isOpen: false,
         enableEnvironment: '',
         isEnabled: false,
+        requiredApprovals: 1,
     });
+    const theme = useTheme();
     const projectId = useRequiredPathParam('projectId');
     const { data, loading, refetchChangeRequestConfig } =
         useChangeRequestConfig(projectId);
     const { updateChangeRequestEnvironmentConfig } = useChangeRequestApi();
     const { setToastData, setToastApiError } = useToast();
 
-    const onClick = (enableEnvironment: string, isEnabled: boolean) => () => {
-        setDialogState({ isOpen: true, enableEnvironment, isEnabled });
-    };
+    const onRowChange =
+        (
+            enableEnvironment: string,
+            isEnabled: boolean,
+            requiredApprovals: number
+        ) =>
+        () => {
+            setDialogState({
+                isOpen: true,
+                enableEnvironment,
+                isEnabled,
+                requiredApprovals,
+            });
+        };
 
     const onConfirm = async () => {
         if (dialogState.enableEnvironment) {
-            try {
-                await updateChangeRequestEnvironmentConfig(
-                    projectId,
-                    dialogState.enableEnvironment,
-                    !dialogState.isEnabled
-                );
-                setToastData({
-                    type: 'success',
-                    title: 'Updated change request status',
-                    text: 'Successfully updated change request status.',
-                });
-                refetchChangeRequestConfig();
-            } catch (error) {
-                const message = formatUnknownError(error);
-                setToastApiError(message);
-            }
+            await updateConfiguration();
         }
         setDialogState({
             isOpen: false,
             enableEnvironment: '',
             isEnabled: false,
+            requiredApprovals: 1,
         });
     };
 
+    async function updateConfiguration(config?: IChangeRequestConfig) {
+        try {
+            await updateChangeRequestEnvironmentConfig(
+                config || {
+                    project: projectId,
+                    environment: dialogState.enableEnvironment,
+                    enabled: !dialogState.isEnabled,
+                    requiredApprovals: dialogState.requiredApprovals,
+                }
+            );
+            setToastData({
+                type: 'success',
+                title: 'Updated change request status',
+                text: 'Successfully updated change request status.',
+            });
+            await refetchChangeRequestConfig();
+        } catch (error) {
+            setToastApiError(formatUnknownError(error));
+        }
+    }
+
+    const approvalOptions = Array.from(Array(10).keys())
+        .map(key => String(key + 1))
+        .map(key => {
+            const labelText = key === '1' ? 'approval' : 'approvals';
+            return {
+                key,
+                label: `${key} ${labelText}`,
+                sx: { 'font-size': theme.fontSizes.smallBody },
+            };
+        });
+
+    function onRequiredApprovalsChange(original: any, approvals: string) {
+        updateConfiguration({
+            project: projectId,
+            environment: original.environment,
+            enabled: original.changeRequestEnabled,
+            requiredApprovals: Number(approvals),
+        });
+    }
+
     const columns = useMemo(
         () => [
             {
@@ -82,6 +138,34 @@ export const ChangeRequestConfiguration: VFC = () => {
                 disableGlobalFilter: true,
                 disableSortBy: true,
             },
+            {
+                Header: 'Required approvals',
+                align: 'center',
+                Cell: ({ row: { original } }: any) => (
+                    <ConditionallyRender
+                        condition={original.changeRequestEnabled}
+                        show={
+                            <StyledBox data-loading>
+                                <GeneralSelect
+                                    options={approvalOptions}
+                                    value={original.requiredApprovals || 1}
+                                    onChange={approvals => {
+                                        onRequiredApprovalsChange(
+                                            original,
+                                            approvals
+                                        );
+                                    }}
+                                    IconComponent={KeyboardArrowDownOutlined}
+                                    fullWidth
+                                />
+                            </StyledBox>
+                        }
+                    />
+                ),
+                width: 100,
+                disableGlobalFilter: true,
+                disableSortBy: true,
+            },
             {
                 Header: 'Status',
                 accessor: 'changeRequestEnabled',
@@ -89,22 +173,20 @@ export const ChangeRequestConfiguration: VFC = () => {
                 align: 'center',
 
                 Cell: ({ value, row: { original } }: any) => (
-                    <Box
-                        sx={{ display: 'flex', justifyContent: 'center' }}
-                        data-loading
-                    >
+                    <StyledBox data-loading>
                         <PermissionSwitch
                             checked={value}
                             environmentId={original.environment}
                             projectId={projectId}
                             permission={UPDATE_PROJECT}
                             inputProps={{ 'aria-label': original.environment }}
-                            onClick={onClick(
+                            onClick={onRowChange(
                                 original.environment,
-                                original.changeRequestEnabled
+                                original.changeRequestEnabled,
+                                original.requiredApprovals
                             )}
                         />
-                    </Box>
+                    </StyledBox>
                 ),
                 width: 100,
                 disableGlobalFilter: true,

frontend/src/hooks/api/actions/useChangeRequestApi/useChangeRequestApi.ts

@@ -10,6 +10,13 @@ export interface IChangeRequestsSchema {
     payload: string | boolean | object | number;
 }
 
+export interface IChangeRequestConfig {
+    project: string;
+    environment: string;
+    enabled: boolean;
+    requiredApprovals: number;
+}
+
 export const useChangeRequestApi = () => {
     const { makeRequest, createRequest, errors, loading } = useAPI({
         propagateErrors: true,
@@ -67,15 +74,19 @@ export const useChangeRequestApi = () => {
         }
     };
 
-    const updateChangeRequestEnvironmentConfig = async (
-        project: string,
-        environment: string,
-        enabled: boolean
-    ) => {
+    const updateChangeRequestEnvironmentConfig = async ({
+        project,
+        enabled,
+        environment,
+        requiredApprovals,
+    }: IChangeRequestConfig) => {
         const path = `api/admin/projects/${project}/environments/${environment}/change-requests/config`;
         const req = createRequest(path, {
             method: 'PUT',
-            body: JSON.stringify({ changeRequestsEnabled: enabled }),
+            body: JSON.stringify({
+                changeRequestsEnabled: enabled,
+                requiredApprovals,
+            }),
         });
 
         try {

src/lib/services/project-service.ts

@@ -16,8 +16,7 @@ import {
     ProjectGroupRemovedEvent,
     ProjectGroupUpdateRoleEvent,
 } from '../types/events';
-import { IUnleashStores } from '../types';
-import { IUnleashConfig } from '../types/option';
+import { IUnleashStores, IUnleashConfig } from '../types';
 import {
     FeatureToggle,
     IProject,
@@ -195,6 +194,13 @@ export default class ProjectService {
         );
     }
 
+    async addEnvironmentToProject(
+        project: string,
+        environment: string,
+    ): Promise<void> {
+        await this.store.addEnvironmentToProject(project, environment);
+    }
+
     async changeProject(
         newProjectId: string,
         featureName: string,

src/migrations/20221115072335-add-required-approvals.js

@@ -0,0 +1,25 @@
+'use strict';
+
+exports.up = function (db, callback) {
+    db.runSql(
+        `
+            ALTER TABLE change_request_settings ADD COLUMN required_approvals integer default 1;
+
+            ALTER TABLE change_request_settings
+                ADD CONSTRAINT change_request_settings_project_environment_key
+                    UNIQUE (project, environment)
+        `,
+        callback,
+    );
+};
+
+exports.down = function (db, callback) {
+    db.runSql(
+        `
+            ALTER TABLE change_request_settings DROP COLUMN IF EXISTS required_approvals;
+            ALTER TABLE change_request_settings
+                DROP CONSTRAINT IF EXISTS change_request_settings_project_environment_key;
+        `,
+        callback,
+    );
+};