fix: resolve issues around changing a toggle's project (#978)

* refactor: show save button before using the dropdown

* refactor: simplify FeatureSettingsProject toast message

* refactor: fix FeatureProjectSelect filter prop type

* refactor: hide change project page for non-enterprise

* refactor: derive move targets from projects list instead of from permissions

* refactor: align frontend project compat check with backend

* refactor: fix useProject object stability

* refactor: disable the save button for the current project

* refactor: require equal environments when moving toggles

* refactor: improve arraysHaveSameItems name

frontend/src/component/feature/FeatureView/FeatureSettings/FeatureSettings.tsx

@@ -6,6 +6,7 @@ import { ConditionallyRender } from 'component/common/ConditionallyRender/Condit
 import FeatureSettingsProject from './FeatureSettingsProject/FeatureSettingsProject';
 import { FeatureSettingsInformation } from './FeatureSettingsInformation/FeatureSettingsInformation';
 import { useRequiredPathParam } from 'hooks/useRequiredPathParam';
+import useUiConfig from 'hooks/api/getters/useUiConfig/useUiConfig';
 
 const METADATA = 'metadata';
 const PROJECT = 'project';
@@ -15,6 +16,7 @@ export const FeatureSettings = () => {
     const projectId = useRequiredPathParam('projectId');
     const featureId = useRequiredPathParam('featureId');
     const [settings, setSettings] = useState(METADATA);
+    const { uiConfig } = useUiConfig();
 
     return (
         <PageContent header="Settings" bodyClass={styles.bodyContainer}>
@@ -36,6 +38,7 @@ export const FeatureSettings = () => {
                             button
                             onClick={() => setSettings(PROJECT)}
                             selected={settings === PROJECT}
+                            hidden={!uiConfig.flags.P}
                         >
                             Project
                         </ListItem>
@@ -52,7 +55,7 @@ export const FeatureSettings = () => {
                         }
                     />
                     <ConditionallyRender
-                        condition={settings === PROJECT}
+                        condition={settings === PROJECT && uiConfig.flags.P}
                         show={<FeatureSettingsProject />}
                     />
                 </div>

frontend/src/component/feature/FeatureView/FeatureSettings/FeatureSettingsProject/FeatureProjectSelect/FeatureProjectSelect.tsx

@@ -10,7 +10,7 @@ interface IFeatureProjectSelectProps
     extends Omit<IGeneralSelectProps, 'options'> {
     enabled: boolean;
     value: string;
-    filter: (project: string) => void;
+    filter: (projectId: string) => boolean;
 }
 
 const FeatureProjectSelect = ({

frontend/src/component/feature/FeatureView/FeatureSettings/FeatureSettingsProject/FeatureSettingsProject.tsx

@@ -1,121 +1,79 @@
-import { useContext, useEffect, useState } from 'react';
+import { useContext, useState, useMemo } from 'react';
 import { useNavigate } from 'react-router';
 import AccessContext from 'contexts/AccessContext';
 import useFeatureApi from 'hooks/api/actions/useFeatureApi/useFeatureApi';
 import { useFeature } from 'hooks/api/getters/useFeature/useFeature';
 import useToast from 'hooks/useToast';
 import { MOVE_FEATURE_TOGGLE } from 'component/providers/AccessProvider/permissions';
-import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender';
 import PermissionButton from 'component/common/PermissionButton/PermissionButton';
 import FeatureProjectSelect from './FeatureProjectSelect/FeatureProjectSelect';
 import FeatureSettingsProjectConfirm from './FeatureSettingsProjectConfirm/FeatureSettingsProjectConfirm';
-import { IPermission } from 'interfaces/user';
-import { useAuthPermissions } from 'hooks/api/getters/useAuth/useAuthPermissions';
 import { formatUnknownError } from 'utils/formatUnknownError';
 import { useRequiredPathParam } from 'hooks/useRequiredPathParam';
+import useProjects from 'hooks/api/getters/useProjects/useProjects';
 
 const FeatureSettingsProject = () => {
     const { hasAccess } = useContext(AccessContext);
     const projectId = useRequiredPathParam('projectId');
     const featureId = useRequiredPathParam('featureId');
     const { feature, refetchFeature } = useFeature(projectId, featureId);
-    const [project, setProject] = useState(feature.project);
-    const [dirty, setDirty] = useState(false);
     const [showConfirmDialog, setShowConfirmDialog] = useState(false);
-    const editable = hasAccess(MOVE_FEATURE_TOGGLE, projectId);
-    const { permissions = [] } = useAuthPermissions();
     const { changeFeatureProject } = useFeatureApi();
     const { setToastData, setToastApiError } = useToast();
+    const [project, setProject] = useState(projectId);
+    const { projects } = useProjects();
     const navigate = useNavigate();
 
-    useEffect(() => {
-        if (project !== feature.project) {
-            setDirty(true);
-            return;
-        }
-        setDirty(false);
-        /* eslint-disable-next-line react-hooks/exhaustive-deps */
-    }, [project]);
-
-    useEffect(() => {
-        const movableTargets = createMoveTargets();
-
-        if (!movableTargets[project]) {
-            setDirty(false);
-            setProject(projectId);
-        }
-        /* eslint-disable-next-line */
-    }, [permissions.length]);
-
-    const updateProject = async () => {
-        const newProject = project;
+    const onConfirm = async () => {
         try {
-            await changeFeatureProject(projectId, featureId, newProject);
-            refetchFeature();
-            setToastData({
-                title: 'Updated project',
-                confetti: true,
-                type: 'success',
-                text: 'Successfully updated toggle project.',
-            });
-            setDirty(false);
-            setShowConfirmDialog(false);
-            navigate(`/projects/${newProject}/features/${featureId}/settings`, {
-                replace: true,
-            });
+            if (project) {
+                await changeFeatureProject(projectId, featureId, project);
+                refetchFeature();
+                setToastData({ title: 'Project changed', type: 'success' });
+                setShowConfirmDialog(false);
+                navigate(
+                    `/projects/${project}/features/${featureId}/settings`,
+                    { replace: true }
+                );
+            }
         } catch (error: unknown) {
             setToastApiError(formatUnknownError(error));
         }
     };
 
-    const createMoveTargets = () => {
-        return permissions.reduce(
-            (acc: { [key: string]: boolean }, p: IPermission) => {
-                if (p.project && p.permission === MOVE_FEATURE_TOGGLE) {
-                    acc[p.project] = true;
-                }
-                return acc;
-            },
-            {}
-        );
-    };
+    const targetProjectIds = useMemo(() => {
+        return projects
+            .map(project => project.id)
+            .filter(projectId => hasAccess(MOVE_FEATURE_TOGGLE, projectId));
+    }, [projects, hasAccess]);
 
-    const filterProjects = () => {
-        const validTargets = createMoveTargets();
+    if (targetProjectIds.length === 0) {
+        return null;
+    }
 
-        return (project: string) => {
-            if (validTargets[project]) {
-                return project;
-            }
-        };
-    };
     return (
         <>
             <FeatureProjectSelect
                 value={project}
                 onChange={setProject}
                 label="Project"
-                enabled={editable}
-                filter={filterProjects()}
-            />
-            <ConditionallyRender
-                condition={dirty}
-                show={
-                    <PermissionButton
-                        permission={MOVE_FEATURE_TOGGLE}
-                        onClick={() => setShowConfirmDialog(true)}
-                        projectId={projectId}
-                    >
-                        Save changes
-                    </PermissionButton>
-                }
+                filter={projectId => targetProjectIds.includes(projectId)}
+                enabled
             />
+            <PermissionButton
+                permission={MOVE_FEATURE_TOGGLE}
+                onClick={() => setShowConfirmDialog(true)}
+                disabled={project === projectId}
+                projectId={projectId}
+            >
+                Save
+            </PermissionButton>
             <FeatureSettingsProjectConfirm
                 projectId={project}
                 open={showConfirmDialog}
                 feature={feature}
                 onClose={() => setShowConfirmDialog(false)}
-                onClick={updateProject}
+                onClick={onConfirm}
             />
         </>
     );

frontend/src/component/feature/FeatureView/FeatureSettings/FeatureSettingsProject/FeatureSettingsProjectConfirm/FeatureSettingsProjectConfirm.styles.ts

@@ -1,46 +1,9 @@
 import { makeStyles } from 'tss-react/mui';
 
 export const useStyles = makeStyles()(theme => ({
-    compatability: {
-        padding: '1rem',
-        border: `1px solid ${theme.palette.grey[300]}`,
-        marginTop: '1rem',
-        display: 'flex',
-        alignItems: 'center',
-    },
-    iconContainer: {
-        width: '50px',
-        height: '50px',
-        backgroundColor: theme.palette.success.main,
-        borderRadius: '50%',
-        display: 'flex',
-        alignItems: 'center',
-        justifyContent: 'center',
-    },
-    errorIconContainer: {
-        width: '50px',
-        height: '50px',
-        backgroundColor: theme.palette.error.main,
-        borderRadius: '50%',
-        display: 'flex',
-        alignItems: 'center',
-        justifyContent: 'center',
-    },
-    topContent: {
-        display: 'flex',
-        alignItems: 'center',
-        justifyContent: 'center',
-    },
-    check: {
-        fill: '#fff',
-        width: '30px',
-        height: '30px',
-    },
-    paragraph: {
-        marginTop: '1rem',
-    },
-    cloud: {
-        fill: theme.palette.grey[500],
-        marginRight: '0.5rem',
+    container: {
+        display: 'grid',
+        gap: theme.spacing(2),
+        paddingTop: theme.spacing(2),
     },
 }));

frontend/src/component/feature/FeatureView/FeatureSettings/FeatureSettingsProject/FeatureSettingsProjectConfirm/FeatureSettingsProjectConfirm.tsx

@@ -1,11 +1,11 @@
-import { List, ListItem } from '@mui/material';
-import { Check, Error, Cloud } from '@mui/icons-material';
-import { useState, useEffect } from 'react';
+import { useMemo } from 'react';
 import useProject from 'hooks/api/getters/useProject/useProject';
-import { IFeatureEnvironment, IFeatureToggle } from 'interfaces/featureToggle';
+import { IFeatureToggle } from 'interfaces/featureToggle';
 import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender';
 import { Dialogue } from 'component/common/Dialogue/Dialogue';
 import { useStyles } from './FeatureSettingsProjectConfirm.styles';
+import { arraysHaveSameItems } from 'utils/arraysHaveSameItems';
+import { Alert } from '@mui/material';
 
 interface IFeatureSettingsProjectConfirm {
     projectId: string;
@@ -23,37 +23,18 @@ const FeatureSettingsProjectConfirm = ({
     feature,
 }: IFeatureSettingsProjectConfirm) => {
     const { project } = useProject(projectId);
-    const [incompatibleEnvs, setIncompatibleEnvs] = useState([]);
     const { classes: styles } = useStyles();
 
-    useEffect(() => {
-        calculateCompatability();
-        /* eslint-disable-next-line react-hooks/exhaustive-deps */
-    }, [projectId, project.name]);
-
-    const calculateCompatability = () => {
-        const featureEnvWithStrategies = feature.environments
-            .filter((env: IFeatureEnvironment) => {
-                return env.strategies.length > 0;
-            })
-            .map((env: IFeatureEnvironment) => env.name);
-
-        const destinationProjectActiveEnvironments = project.environments;
-
-        let incompatible: string[] = [];
-
-        featureEnvWithStrategies.forEach((env: string) => {
-            if (destinationProjectActiveEnvironments.indexOf(env) === -1) {
-                incompatible = [...incompatible, env];
-            }
-        });
-        // @ts-expect-error
-        setIncompatibleEnvs(incompatible);
-    };
+    const hasSameEnvironments: boolean = useMemo(() => {
+        return arraysHaveSameItems(
+            feature.environments.map(env => env.name),
+            project.environments
+        );
+    }, [feature, project]);
 
     return (
         <ConditionallyRender
-            condition={incompatibleEnvs?.length === 0}
+            condition={hasSameEnvironments}
             show={
                 <Dialogue
                     open={open}
@@ -63,14 +44,15 @@ const FeatureSettingsProjectConfirm = ({
                     primaryButtonText="Change project"
                     secondaryButtonText="Cancel"
                 >
-                    Are you sure you want to change the project for this feature
-                    toggle?
-                    <div className={styles.compatability}>
-                        This feature toggle is 100% compatible with the new
-                        project.
-                        <div className={styles.iconContainer}>
-                            <Check className={styles.check} />
-                        </div>
+                    <div className={styles.container}>
+                        <Alert severity="success">
+                            This feature toggle is compatible with the new
+                            project.
+                        </Alert>
+                        <p>
+                            Are you sure you want to change the project for this
+                            toggle?
+                        </p>
                     </div>
                 </Dialogue>
             }
@@ -81,44 +63,16 @@ const FeatureSettingsProjectConfirm = ({
                     title="Confirm change project"
                     secondaryButtonText="OK"
                 >
-                    <div className={styles.topContent}>
+                    <div className={styles.container}>
+                        <Alert severity="warning">
+                            Incompatible project environments
+                        </Alert>
                         <p>
-                            {' '}
-                            This feature toggle is not compatible with the new
-                            project destination.
+                            In order to move a feature toggle between two
+                            projects, both projects must have the exact same
+                            environments enabled.
                         </p>
-                        <div className={styles.iconContainer}>
-                            <div className={styles.errorIconContainer}>
-                                <Error
-                                    className={styles.check}
-                                    titleAccess="Error"
-                                />
-                            </div>
-                        </div>
                     </div>
-                    <div className={styles.compatability}>
-                        <div>
-                            <p className={styles.paragraph}>
-                                This feature toggle has strategy configuration
-                                in an environment that is not activated in the
-                                target project:
-                            </p>
-                            <List>
-                                {incompatibleEnvs.map(env => {
-                                    return (
-                                        <ListItem key={env}>
-                                            <Cloud className={styles.cloud} />
-                                            {env}
-                                        </ListItem>
-                                    );
-                                })}
-                            </List>
-                        </div>
-                    </div>
-                    <p className={styles.paragraph}>
-                        In order to move this feature toggle, make sure you
-                        enable the required environments in the target project.
-                    </p>
                 </Dialogue>
             }
         />

frontend/src/hooks/api/getters/useProject/useProject.ts

@@ -1,5 +1,5 @@
-import useSWR, { mutate, SWRConfiguration } from 'swr';
-import { useState, useEffect } from 'react';
+import useSWR, { SWRConfiguration } from 'swr';
+import { useCallback } from 'react';
 import { getProjectFetcher } from './getProjectFetcher';
 import { IProject } from 'interfaces/project';
 
@@ -15,22 +15,16 @@ const fallbackProject: IProject = {
 
 const useProject = (id: string, options: SWRConfiguration = {}) => {
     const { KEY, fetcher } = getProjectFetcher(id);
+    const { data, error, mutate } = useSWR<IProject>(KEY, fetcher, options);
 
-    const { data, error } = useSWR<IProject>(KEY, fetcher, options);
-    const [loading, setLoading] = useState(!error && !data);
-
-    const refetch = () => {
-        mutate(KEY);
-    };
-
-    useEffect(() => {
-        setLoading(!error && !data);
-    }, [data, error]);
+    const refetch = useCallback(() => {
+        mutate();
+    }, [mutate]);
 
     return {
         project: data || fallbackProject,
+        loading: !error && !data,
         error,
-        loading,
         refetch,
     };
 };

frontend/src/hooks/useSort.ts

@@ -1,4 +1,4 @@
-import { useState } from 'react';
+import { useState, useCallback } from 'react';
 import {
     sortFeaturesByNameAscending,
     sortFeaturesByNameDescending,
@@ -63,25 +63,29 @@ const useSort = () => {
         return sortFeaturesByStatusDescending(features);
     };
 
-    const sort = (features: IFeatureToggleListItem[]) => {
-        switch (sortData.sortKey) {
-            case 'name':
-                return handleSortName(features);
-            case 'last-seen':
-                return handleSortLastSeen(features);
-            case 'created':
-                return handleSortCreatedAt(features);
-            case 'expired':
-            case 'report':
-                return handleSortExpiredAt(features);
-            case 'status':
-                return handleSortStatus(features);
-            default:
-                return features;
-        }
-    };
+    const sort = useCallback(
+        (features: IFeatureToggleListItem[]): IFeatureToggleListItem[] => {
+            switch (sortData.sortKey) {
+                case 'name':
+                    return handleSortName(features);
+                case 'last-seen':
+                    return handleSortLastSeen(features);
+                case 'created':
+                    return handleSortCreatedAt(features);
+                case 'expired':
+                case 'report':
+                    return handleSortExpiredAt(features);
+                case 'status':
+                    return handleSortStatus(features);
+                default:
+                    return features;
+            }
+        },
+        // eslint-disable-next-line react-hooks/exhaustive-deps
+        [sortData]
+    );
 
-    return [sort, setSortData];
+    return [sort, setSortData] as const;
 };
 
 export default useSort;

frontend/src/utils/arraysHaveSameItems.test.ts

@@ -0,0 +1,17 @@
+import { arraysHaveSameItems } from 'utils/arraysHaveSameItems';
+
+test('arraysHaveSameItems', () => {
+    expect(arraysHaveSameItems([], [])).toEqual(true);
+    expect(arraysHaveSameItems([1], [1])).toEqual(true);
+    expect(arraysHaveSameItems([1], [1, 1])).toEqual(true);
+    expect(arraysHaveSameItems([1, 1], [1])).toEqual(true);
+    expect(arraysHaveSameItems([1, 2], [1, 2])).toEqual(true);
+    expect(arraysHaveSameItems([1, 2], [2, 1])).toEqual(true);
+    expect(arraysHaveSameItems([1, 2], [2, 2, 1])).toEqual(true);
+    expect(arraysHaveSameItems([1], [])).toEqual(false);
+    expect(arraysHaveSameItems([1], [2])).toEqual(false);
+    expect(arraysHaveSameItems([1, 2], [1, 3])).toEqual(false);
+    expect(arraysHaveSameItems([1, 2], [1, 2, 3])).toEqual(false);
+    expect(arraysHaveSameItems([1, 2, 3], [1, 2])).toEqual(false);
+    expect(arraysHaveSameItems([1, 2, 3], [1, 2, 4])).toEqual(false);
+});

frontend/src/utils/arraysHaveSameItems.ts

@@ -0,0 +1,12 @@
+export const arraysHaveSameItems = <T>(a: T[], b: T[]): boolean => {
+    const setA = new Set(a);
+    const setB = new Set(b);
+
+    if (setA.size !== setB.size) {
+        return false;
+    }
+
+    return [...setA].every(itemA => {
+        return setB.has(itemA);
+    });
+};

frontend/src/utils/projectFilterGenerator.ts

@@ -1,5 +1,5 @@
-import { ADMIN } from '../component/providers/AccessProvider/permissions';
-import { IPermission } from '../interfaces/user';
+import { ADMIN } from 'component/providers/AccessProvider/permissions';
+import { IPermission } from 'interfaces/user';
 
 type objectIdx = {
     [key: string]: string;
@@ -8,8 +8,9 @@ type objectIdx = {
 export const projectFilterGenerator = (
     permissions: IPermission[] = [],
     matcherPermission: string
-) => {
+): ((projectId: string) => boolean) => {
     let admin = false;
+
     const permissionMap: objectIdx = permissions.reduce(
         (acc: objectIdx, p: IPermission) => {
             if (p.permission === ADMIN) {
@@ -24,7 +25,8 @@ export const projectFilterGenerator = (
         },
         {}
     );
+
     return (projectId: string) => {
-        return admin || permissionMap[projectId];
+        return admin || Boolean(permissionMap[projectId]);
     };
 };