feat: show scheduled CRs using strategies when removing it (#5560)

Show a warning about how deleting a strategy might mess up scheduled change requests. If there are change requests, list them. If there are no conflicts, show nothing. If we don't know (because of no successful response from the API), say that it might cause issues. ![image](https://github.com/Unleash/unleash/assets/17786332/2c6a4257-69f5-458a-ab6f-9b2ea2f5d550)
2024-12-22 19:07:54 +01:00 · 2023-12-06 15:39:17 +01:00 · 2023-12-06 15:39:17 +01:00 · a228f54344
commit a228f54344
parent 87ebbb0fa2
4 changed files with 297 additions and 21 deletions
--- a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyDraggableItem.tsx
+++ b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyDraggableItem.tsx
@ -117,7 +117,12 @@ const renderHeaderChildren = (
    );

    if (draftChange) {
-        badges.push(<ChangeRequestStatusBadge change={draftChange.change} />);
+        badges.push(
+            <ChangeRequestStatusBadge
+                key={`draft-change#${draftChange.change.id}`}
+                change={draftChange.change}
+            />,
+        );
    }

    const scheduledChanges = changes.filter(
@ -127,6 +132,7 @@ const renderHeaderChildren = (
    if (scheduledChanges.length > 0) {
        badges.push(
            <ChangesScheduledBadge
+                key='scheduled-changes'
                scheduledChangeRequestIds={scheduledChanges.map(
                    (scheduledChange) => scheduledChange.changeRequestId,
                )}
--- a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/MenuStrategyRemove/DialogStrategyRemove.test.tsx
+++ b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/MenuStrategyRemove/DialogStrategyRemove.test.tsx
@ -0,0 +1,141 @@
+import React from 'react';
+import { render } from 'utils/testRenderer';
+import { screen } from '@testing-library/react';
+import {
+    FeatureStrategyRemoveDialogue,
+    SuggestFeatureStrategyRemoveDialogue,
+} from './DialogStrategyRemove';
+import {
+    ChangeRequestState,
+    IChangeRequestFeature,
+    IFeatureChange,
+} from 'component/changeRequest/changeRequest.types';
+
+const strategyId = 'c81e3a1d-e91c-4083-bd0f-75bb8a9e32a2';
+const projectId = 'default';
+const environment = 'development';
+const featureId = 'bb1d79e0-95b0-4393-b248-64d1e0294ee3';
+
+describe('Use in scheduled change requests', () => {
+    it.each(['enabled', 'disabled'])(
+        'should show usage in scheduled change requests with change requests %s for the project',
+        async (changeRequestsEnabled) => {
+            const changeRequestWithTitle = { id: 1, title: 'My CR' };
+            const changeRequestWithoutTitle = { id: 2 };
+            const scheduledChangeRequests = [
+                changeRequestWithTitle,
+                changeRequestWithoutTitle,
+            ];
+
+            if (changeRequestsEnabled === 'enabled') {
+                render(
+                    <SuggestFeatureStrategyRemoveDialogue
+                        onRemove={async () => {}}
+                        onClose={() => {}}
+                        isOpen={true}
+                        scheduledChangeRequestsForStrategy={{
+                            projectId,
+                            changeRequests: scheduledChangeRequests,
+                        }}
+                    />,
+                );
+            } else {
+                render(
+                    <FeatureStrategyRemoveDialogue
+                        onRemove={async () => {}}
+                        onClose={() => {}}
+                        isOpen={true}
+                        scheduledChangeRequestsForStrategy={{
+                            projectId,
+                            changeRequests: scheduledChangeRequests,
+                        }}
+                    />,
+                );
+            }
+
+            const alerts = await screen.findAllByRole('alert');
+
+            expect(
+                alerts.find((alert) =>
+                    alert.textContent!.startsWith('This strategy'),
+                ),
+            ).toBeTruthy();
+
+            const links = await screen.findAllByRole('link');
+
+            expect(links).toHaveLength(scheduledChangeRequests.length);
+
+            const [link1, link2] = links;
+
+            expect(link1).toHaveTextContent('#1 (My CR)');
+            expect(link1).toHaveAccessibleDescription('Change request 1');
+            expect(link1).toHaveAttribute(
+                'href',
+                `/projects/default/change-requests/1`,
+            );
+
+            expect(link2).toHaveTextContent('#2');
+            expect(link2).toHaveAccessibleDescription('Change request 2');
+            expect(link2).toHaveAttribute(
+                'href',
+                `/projects/default/change-requests/2`,
+            );
+        },
+    );
+
+    it('should not render scheduled change requests warning when there are no scheduled change requests', async () => {
+        render(
+            <SuggestFeatureStrategyRemoveDialogue
+                onRemove={async () => {}}
+                onClose={() => {}}
+                isOpen={true}
+                scheduledChangeRequestsForStrategy={{
+                    projectId,
+                    changeRequests: [],
+                }}
+            />,
+        );
+
+        const alerts = await screen.findAllByRole('alert');
+
+        expect(
+            alerts.find((alert) =>
+                alert.textContent!.startsWith('This strategy'),
+            ),
+        ).toBeFalsy();
+
+        expect(alerts).toHaveLength(1);
+
+        const link = screen.queryByRole('link');
+
+        expect(link).toBe(null);
+    });
+
+    it("It should render a warning saying there might be scheduled change requests if it doesn't get a successful API response", async () => {
+        render(
+            <SuggestFeatureStrategyRemoveDialogue
+                onRemove={async () => {}}
+                onClose={() => {}}
+                isOpen={true}
+                scheduledChangeRequestsForStrategy={{
+                    projectId,
+                    changeRequests: undefined,
+                }}
+            />,
+        );
+
+        const alerts = await screen.findAllByRole('alert');
+
+        expect(
+            alerts.find((alert) =>
+                alert.textContent!.startsWith('This strategy'),
+            ),
+        ).toBeTruthy();
+
+        expect(alerts).toHaveLength(2);
+
+        const link = screen.queryByRole('link');
+
+        expect(link).toBe(null);
+    });
+});
--- a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/MenuStrategyRemove/DialogStrategyRemove.tsx
+++ b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/MenuStrategyRemove/DialogStrategyRemove.tsx
@ -1,7 +1,7 @@
 import React, { FC } from 'react';
 import useFeatureStrategyApi from 'hooks/api/actions/useFeatureStrategyApi/useFeatureStrategyApi';
 import { formatUnknownError } from 'utils/formatUnknownError';
-import { useNavigate } from 'react-router-dom';
+import { Link, useNavigate } from 'react-router-dom';
 import useToast from 'hooks/useToast';
 import { formatFeaturePath } from '../../../../../../../../FeatureStrategy/FeatureStrategyEdit/FeatureStrategyEdit';
 import { Dialogue } from 'component/common/Dialogue/Dialogue';
@ -10,7 +10,8 @@ import { useFeature } from 'hooks/api/getters/useFeature/useFeature';
 import { useChangeRequestApi } from 'hooks/api/actions/useChangeRequestApi/useChangeRequestApi';
 import { useChangeRequestsEnabled } from 'hooks/useChangeRequestsEnabled';
 import { usePendingChangeRequests } from 'hooks/api/getters/usePendingChangeRequests/usePendingChangeRequests';
-
+import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender';
+import { useScheduledChangeRequestsWithStrategy } from 'hooks/api/getters/useScheduledChangeRequestsWithStrategy/useScheduledChangeRequestsWithStrategy';
 interface IFeatureStrategyRemoveProps {
    projectId: string;
    featureId: string;
@ -18,13 +19,23 @@ interface IFeatureStrategyRemoveProps {
    strategyId: string;
    disabled?: boolean;
    icon?: boolean;
-    text?: boolean;
 }

+type ChangeRequest = {
+    id: number;
+    title?: string;
+};
+
+type ScheduledChangeRequestData = {
+    changeRequests?: ChangeRequest[];
+    projectId: string;
+};
+
 interface IFeatureStrategyRemoveDialogueProps {
    onRemove: (event: React.FormEvent) => Promise<void>;
    onClose: () => void;
    isOpen: boolean;
+    scheduledChangeRequestsForStrategy: ScheduledChangeRequestData;
 }

 const RemoveAlert: FC = () => (
@ -34,30 +45,103 @@ const RemoveAlert: FC = () => (
    </Alert>
 );

-const FeatureStrategyRemoveDialogue: FC<IFeatureStrategyRemoveDialogueProps> =
-    ({ onRemove, onClose, isOpen }) => {
+const AlertContainer = styled('div')(({ theme }) => ({
+    '> * + *': {
+        marginTop: theme.spacing(1),
+    },
+}));
+
+const StrategyInScheduledChangeRequestsWarning: FC<{
+    changeRequests?: ChangeRequest[];
+    projectId: string;
+}> = ({ changeRequests, projectId }) => {
+    if (changeRequests && changeRequests.length > 0) {
        return (
-            <Dialogue
-                title='Are you sure you want to delete this strategy?'
-                open={isOpen}
-                primaryButtonText='Remove strategy'
-                secondaryButtonText='Cancel'
-                onClick={onRemove}
-                onClose={onClose}
-            >
-                <RemoveAlert />
-            </Dialogue>
+            <Alert severity='warning'>
+                <p>
+                    This strategy is in use by at least one scheduled change
+                    request. If you remove it, those change requests can no
+                    longer be applied.
+                </p>
+                <p>
+                    The following scheduled change requests use this strategy:
+                </p>
+                <ul>
+                    {changeRequests.map(({ id, title }) => {
+                        const text = title ? `#${id} (${title})` : `#${id}`;
+                        return (
+                            <li key={id}>
+                                <Link
+                                    to={`/projects/${projectId}/change-requests/${id}`}
+                                    target='_blank'
+                                    rel='noopener noreferrer'
+                                    title={`Change request ${id}`}
+                                >
+                                    {text}
+                                </Link>
+                            </li>
+                        );
+                    })}
+                </ul>
+            </Alert>
        );
-    };
+    } else if (changeRequests === undefined) {
+        return (
+            <Alert severity='warning'>
+                <p>
+                    This strategy may be in use by one or more scheduled change
+                    requests. If you remove it, those change requests can no
+                    longer be applied.
+                </p>
+            </Alert>
+        );
+    }
+
+    // all good, we have nothing to show
+    return null;
+};
+
+const Alerts: FC<{
+    scheduledChangeRequestsForStrategy: ScheduledChangeRequestData;
+}> = ({ scheduledChangeRequestsForStrategy }) => (
+    <AlertContainer>
+        <RemoveAlert />
+        <StrategyInScheduledChangeRequestsWarning
+            projectId={scheduledChangeRequestsForStrategy.projectId}
+            changeRequests={scheduledChangeRequestsForStrategy.changeRequests}
+        />
+    </AlertContainer>
+);
+
+export const FeatureStrategyRemoveDialogue: FC<
+    IFeatureStrategyRemoveDialogueProps
+> = ({ onRemove, onClose, isOpen, scheduledChangeRequestsForStrategy }) => {
+    return (
+        <Dialogue
+            title='Are you sure you want to delete this strategy?'
+            open={isOpen}
+            primaryButtonText='Remove strategy'
+            secondaryButtonText='Cancel'
+            onClick={onRemove}
+            onClose={onClose}
+        >
+            <Alerts
+                scheduledChangeRequestsForStrategy={
+                    scheduledChangeRequestsForStrategy
+                }
+            />
+        </Dialogue>
+    );
+};

 const MsgContainer = styled('div')(({ theme }) => ({
    marginTop: theme.spacing(3),
    marginBottom: theme.spacing(1),
 }));

-const SuggestFeatureStrategyRemoveDialogue: FC<
+export const SuggestFeatureStrategyRemoveDialogue: FC<
    IFeatureStrategyRemoveDialogueProps
-> = ({ onRemove, onClose, isOpen }) => {
+> = ({ onRemove, onClose, isOpen, scheduledChangeRequestsForStrategy }) => {
    return (
        <Dialogue
            title='Suggest changes'
@ -67,7 +151,11 @@ const SuggestFeatureStrategyRemoveDialogue: FC<
            onClick={onRemove}
            onClose={onClose}
        >
-            <RemoveAlert />
+            <Alerts
+                scheduledChangeRequestsForStrategy={
+                    scheduledChangeRequestsForStrategy
+                }
+            />
            <MsgContainer>
                <Typography variant='body2' color='text.secondary'>
                    Your suggestion:
@ -155,7 +243,6 @@ export const DialogStrategyRemove = ({
    featureId,
    environmentId,
    strategyId,
-    text,
    isOpen,
    onClose,
 }: IFeatureStrategyRemoveProps & {
@ -164,6 +251,16 @@ export const DialogStrategyRemove = ({
 }) => {
    const { isChangeRequestConfigured } = useChangeRequestsEnabled(projectId);

+    const { changeRequests } = useScheduledChangeRequestsWithStrategy(
+        projectId,
+        strategyId,
+    );
+
+    const changeRequestData = {
+        changeRequests,
+        projectId,
+    };
+
    const onRemove = useOnRemove({
        featureId,
        projectId,
@ -186,6 +283,7 @@ export const DialogStrategyRemove = ({
                    await onSuggestRemove(e);
                    onClose();
                }}
+                scheduledChangeRequestsForStrategy={changeRequestData}
            />
        );
    }
@ -195,6 +293,7 @@ export const DialogStrategyRemove = ({
            isOpen={isOpen}
            onClose={() => onClose()}
            onRemove={onRemove}
+            scheduledChangeRequestsForStrategy={changeRequestData}
        />
    );
 };
--- a/frontend/src/hooks/api/getters/useScheduledChangeRequestsWithStrategy/useScheduledChangeRequestsWithStrategy.ts
+++ b/frontend/src/hooks/api/getters/useScheduledChangeRequestsWithStrategy/useScheduledChangeRequestsWithStrategy.ts
@ -0,0 +1,30 @@
+import { formatApiPath } from 'utils/formatPath';
+import handleErrorResponses from '../httpErrorResponseHandler';
+import { IChangeRequest } from 'component/changeRequest/changeRequest.types';
+import { useEnterpriseSWR } from '../useEnterpriseSWR/useEnterpriseSWR';
+
+const fetcher = (path: string) => {
+    return fetch(path)
+        .then(handleErrorResponses('ChangeRequest'))
+        .then((res) => res.json());
+};
+
+export const useScheduledChangeRequestsWithStrategy = (
+    project: string,
+    strategyId: string,
+) => {
+    const { data, error, mutate } = useEnterpriseSWR<IChangeRequest[]>(
+        [],
+        formatApiPath(
+            `api/admin/projects/${project}/change-requests/scheduled/with-strategy/${strategyId}`,
+        ),
+        fetcher,
+    );
+
+    return {
+        changeRequests: data,
+        loading: !error && !data,
+        refetch: mutate,
+        error,
+    };
+};