feat: Change request applied diff for update strategy (#8859)

frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.test.tsx

@@ -0,0 +1,197 @@
+import { render } from 'utils/testRenderer';
+import { StrategyChange } from './StrategyChange';
+import { testServerRoute, testServerSetup } from 'utils/testServer';
+import { screen } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+
+const server = testServerSetup();
+
+const feature = 'my_feature';
+const projectId = 'default';
+const environmentName = 'production';
+const snapshotRollout = '70';
+const currentRollout = '80';
+const changeRequestRollout = '90';
+const strategy = {
+    name: 'flexibleRollout',
+    constraints: [],
+    variants: [],
+    parameters: {
+        groupId: 'child_1',
+        stickiness: 'default',
+    },
+    segments: [],
+    id: '8e25e369-6424-4dad-b17f-5d32cceb2fbe',
+    disabled: false,
+};
+
+const setupApi = () => {
+    testServerRoute(
+        server,
+        `/api/admin/projects/${projectId}/features/${feature}`,
+        {
+            environments: [
+                {
+                    name: environmentName,
+                    strategies: [
+                        {
+                            ...strategy,
+                            title: 'current_title',
+                            parameters: {
+                                ...strategy.parameters,
+                                rollout: currentRollout,
+                            },
+                        },
+                    ],
+                },
+            ],
+        },
+    );
+};
+
+beforeEach(setupApi);
+
+test('Editing strategy before change request is applied diffs against current strategy', async () => {
+    render(
+        <StrategyChange
+            featureName={feature}
+            environmentName={environmentName}
+            projectId={projectId}
+            changeRequestState='Approved'
+            change={{
+                action: 'updateStrategy',
+                id: 1,
+                payload: {
+                    ...strategy,
+                    title: 'change_request_title',
+                    parameters: {
+                        ...strategy.parameters,
+                        rollout: changeRequestRollout,
+                    },
+                    snapshot: {
+                        ...strategy,
+                        title: 'snapshot_title',
+                        parameters: {
+                            ...strategy.parameters,
+                            rollout: snapshotRollout,
+                        },
+                    },
+                },
+            }}
+        />,
+    );
+
+    await screen.findByText('Editing strategy:');
+    await screen.findByText('change_request_title');
+    await screen.findByText('current_title');
+    expect(screen.queryByText('snapshot_title')).not.toBeInTheDocument();
+
+    const viewDiff = await screen.findByText('View Diff');
+    await userEvent.hover(viewDiff);
+    await screen.findByText(`- parameters.rollout: "${currentRollout}"`);
+    await screen.findByText(`+ parameters.rollout: "${changeRequestRollout}"`);
+});
+
+test('Editing strategy after change request is applied diffs against the snapshot', async () => {
+    render(
+        <StrategyChange
+            featureName='my_feature'
+            environmentName='production'
+            projectId='default'
+            changeRequestState='Applied'
+            change={{
+                action: 'updateStrategy',
+                id: 1,
+                payload: {
+                    ...strategy,
+                    title: 'change_request_title',
+                    parameters: {
+                        ...strategy.parameters,
+                        rollout: changeRequestRollout,
+                    },
+                    snapshot: {
+                        ...strategy,
+                        title: 'snapshot_title',
+                        parameters: {
+                            ...strategy.parameters,
+                            rollout: snapshotRollout,
+                        },
+                    },
+                },
+            }}
+        />,
+    );
+
+    await screen.findByText('Editing strategy:');
+    await screen.findByText('change_request_title');
+    await screen.findByText('snapshot_title');
+    expect(screen.queryByText('current_title')).not.toBeInTheDocument();
+
+    const viewDiff = await screen.findByText('View Diff');
+    await userEvent.hover(viewDiff);
+    await screen.findByText(`- parameters.rollout: "${snapshotRollout}"`);
+    await screen.findByText(`+ parameters.rollout: "${changeRequestRollout}"`);
+});
+
+test('Deleting strategy before change request is applied diffs against current strategy', async () => {
+    render(
+        <StrategyChange
+            featureName={feature}
+            environmentName={environmentName}
+            projectId={projectId}
+            changeRequestState='Approved'
+            change={{
+                action: 'deleteStrategy',
+                id: 1,
+                payload: {
+                    id: strategy.id,
+                    name: strategy.name,
+                },
+            }}
+        />,
+    );
+
+    await screen.findByText('- Deleting strategy:');
+    await screen.findByText('Gradual rollout');
+    await screen.findByText('current_title');
+
+    const viewDiff = await screen.findByText('View Diff');
+    await userEvent.hover(viewDiff);
+    await screen.findByText('- constraints (deleted)');
+});
+
+test('Deleting strategy after change request is applied diffs against the snapshot', async () => {
+    render(
+        <StrategyChange
+            featureName={feature}
+            environmentName={environmentName}
+            projectId={projectId}
+            changeRequestState='Applied'
+            change={{
+                action: 'deleteStrategy',
+                id: 1,
+                payload: {
+                    id: strategy.id,
+                    // name is gone
+                    snapshot: {
+                        ...strategy,
+                        title: 'snapshot_title',
+                        parameters: {
+                            ...strategy.parameters,
+                            rollout: snapshotRollout,
+                        },
+                    },
+                },
+            }}
+        />,
+    );
+
+    await screen.findByText('- Deleting strategy:');
+    await screen.findByText('Gradual rollout');
+    await screen.findByText('snapshot_title');
+    expect(screen.queryByText('current_title')).not.toBeInTheDocument();
+
+    const viewDiff = await screen.findByText('View Diff');
+    await userEvent.hover(viewDiff);
+    await screen.findByText('- constraints (deleted)');
+});

frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.tsx

@@ -1,5 +1,5 @@
 import type React from 'react';
-import type { VFC, FC, ReactNode } from 'react';
+import type { FC, ReactNode } from 'react';
 import { Box, styled, Tooltip, Typography } from '@mui/material';
 import BlockIcon from '@mui/icons-material/Block';
 import TrackChangesIcon from '@mui/icons-material/TrackChanges';
@@ -20,6 +20,7 @@ import { ConditionallyRender } from 'component/common/ConditionallyRender/Condit
 import { flexRow } from 'themes/themeStyles';
 import { EnvironmentVariantsTable } from 'component/feature/FeatureView/FeatureVariants/FeatureEnvironmentVariants/EnvironmentVariantsCard/EnvironmentVariantsTable/EnvironmentVariantsTable';
 import { ChangeOverwriteWarning } from './ChangeOverwriteWarning/ChangeOverwriteWarning';
+import type { IFeatureStrategy } from 'interfaces/strategy';
 
 export const ChangeItemWrapper = styled(Box)({
     display: 'flex',
@@ -60,10 +61,7 @@ const StyledTypography: FC<{ children?: React.ReactNode }> = styled(Typography)(
     }),
 );
 
-const hasNameField = (payload: unknown): payload is { name: string } =>
-    typeof payload === 'object' && payload !== null && 'name' in payload;
-
-const DisabledEnabledState: VFC<{ show?: boolean; disabled: boolean }> = ({
+const DisabledEnabledState: FC<{ show?: boolean; disabled: boolean }> = ({
     show = true,
     disabled,
 }) => {
@@ -98,7 +96,7 @@ const DisabledEnabledState: VFC<{ show?: boolean; disabled: boolean }> = ({
     );
 };
 
-const EditHeader: VFC<{
+const EditHeader: FC<{
     wasDisabled?: boolean;
     willBeDisabled?: boolean;
 }> = ({ wasDisabled = false, willBeDisabled = false }) => {
@@ -119,7 +117,157 @@ const EditHeader: VFC<{
     return <Typography>Editing strategy:</Typography>;
 };
 
-export const StrategyChange: VFC<{
+const hasDiff = (object: unknown, objectToCompare: unknown) =>
+    JSON.stringify(object) !== JSON.stringify(objectToCompare);
+
+const DeleteStrategy: FC<{
+    change: IChangeRequestDeleteStrategy;
+    changeRequestState: ChangeRequestState;
+    currentStrategy: IFeatureStrategy | undefined;
+    actions?: ReactNode;
+}> = ({ change, changeRequestState, currentStrategy, actions }) => {
+    const name =
+        changeRequestState === 'Applied'
+            ? change.payload?.snapshot?.name
+            : currentStrategy?.name;
+    const title =
+        changeRequestState === 'Applied'
+            ? change.payload?.snapshot?.title
+            : currentStrategy?.title;
+    const referenceStrategy =
+        changeRequestState === 'Applied'
+            ? change.payload.snapshot
+            : currentStrategy;
+
+    return (
+        <>
+            <ChangeItemCreateEditDeleteWrapper className='delete-strategy-information-wrapper'>
+                <ChangeItemInfo>
+                    <Typography
+                        sx={(theme) => ({
+                            color: theme.palette.error.main,
+                        })}
+                    >
+                        - Deleting strategy:
+                    </Typography>
+                    <StrategyTooltipLink name={name || ''} title={title}>
+                        <StrategyDiff
+                            change={change}
+                            currentStrategy={referenceStrategy}
+                        />
+                    </StrategyTooltipLink>
+                </ChangeItemInfo>
+                <div>{actions}</div>
+            </ChangeItemCreateEditDeleteWrapper>
+            {referenceStrategy && (
+                <StrategyExecution strategy={referenceStrategy} />
+            )}
+            <ConditionallyRender
+                condition={Boolean(referenceStrategy?.variants?.length)}
+                show={
+                    referenceStrategy?.variants && (
+                        <StyledBox>
+                            <StyledTypography>
+                                Deleting strategy variants:
+                            </StyledTypography>
+                            <EnvironmentVariantsTable
+                                variants={referenceStrategy.variants}
+                            />
+                        </StyledBox>
+                    )
+                }
+            />
+        </>
+    );
+};
+
+const UpdateStrategy: FC<{
+    change: IChangeRequestUpdateStrategy;
+    changeRequestState: ChangeRequestState;
+    currentStrategy: IFeatureStrategy | undefined;
+    actions?: ReactNode;
+}> = ({ change, changeRequestState, currentStrategy, actions }) => {
+    const hasVariantDiff = hasDiff(
+        currentStrategy?.variants || [],
+        change.payload.variants || [],
+    );
+    const previousTitle =
+        changeRequestState === 'Applied'
+            ? change.payload.snapshot?.title
+            : currentStrategy?.title;
+    const referenceStrategy =
+        changeRequestState === 'Applied'
+            ? change.payload.snapshot
+            : currentStrategy;
+
+    return (
+        <>
+            <ChangeOverwriteWarning
+                data={{
+                    current: currentStrategy,
+                    change,
+                    changeType: 'strategy',
+                }}
+                changeRequestState={changeRequestState}
+            />
+            <ChangeItemCreateEditDeleteWrapper>
+                <ChangeItemInfo>
+                    <EditHeader
+                        wasDisabled={currentStrategy?.disabled}
+                        willBeDisabled={change.payload?.disabled}
+                    />
+                    <StrategyTooltipLink
+                        name={change.payload.name}
+                        title={change.payload.title}
+                        previousTitle={previousTitle}
+                    >
+                        <StrategyDiff
+                            change={change}
+                            currentStrategy={referenceStrategy}
+                        />
+                    </StrategyTooltipLink>
+                </ChangeItemInfo>
+                <div>{actions}</div>
+            </ChangeItemCreateEditDeleteWrapper>
+            <ConditionallyRender
+                condition={
+                    change.payload?.disabled !== currentStrategy?.disabled
+                }
+                show={
+                    <Typography
+                        sx={{
+                            marginTop: (theme) => theme.spacing(2),
+                            marginBottom: (theme) => theme.spacing(2),
+                            ...flexRow,
+                            gap: (theme) => theme.spacing(1),
+                        }}
+                    >
+                        This strategy will be{' '}
+                        <DisabledEnabledState
+                            disabled={change.payload?.disabled || false}
+                        />
+                    </Typography>
+                }
+            />
+            <StrategyExecution strategy={change.payload} />
+            <ConditionallyRender
+                condition={Boolean(hasVariantDiff)}
+                show={
+                    <StyledBox>
+                        <StyledTypography>
+                            Updating feature variants to:
+                        </StyledTypography>
+                        <EnvironmentVariantsTable
+                            variants={change.payload.variants || []}
+                        />
+                    </StyledBox>
+                }
+            />
+        </>
+    );
+};
+
+export const StrategyChange: FC<{
     actions?: ReactNode;
     change:
         | IChangeRequestAddStrategy
@@ -144,16 +292,6 @@ export const StrategyChange: VFC<{
         environmentName,
     );
 
-    const hasDiff = (object: unknown, objectToCompare: unknown) =>
-        JSON.stringify(object) !== JSON.stringify(objectToCompare);
-
-    const isStrategyAction =
-        change.action === 'addStrategy' || change.action === 'updateStrategy';
-
-    const hasVariantDiff =
-        isStrategyAction &&
-        hasDiff(currentStrategy?.variants || [], change.payload.variants || []);
-
     return (
         <>
             {change.action === 'addStrategy' && (
@@ -169,7 +307,10 @@ export const StrategyChange: VFC<{
                             >
                                 + Adding strategy:
                             </Typography>
-                            <StrategyTooltipLink change={change}>
+                            <StrategyTooltipLink
+                                name={change.payload.name}
+                                title={change.payload.title}
+                            >
                                 <StrategyDiff
                                     change={change}
                                     currentStrategy={currentStrategy}
@@ -185,139 +326,35 @@ export const StrategyChange: VFC<{
                         <div>{actions}</div>
                     </ChangeItemCreateEditDeleteWrapper>
                     <StrategyExecution strategy={change.payload} />
-                    <ConditionallyRender
-                        condition={hasVariantDiff}
-                        show={
-                            change.payload.variants && (
-                                <StyledBox>
-                                    <StyledTypography>
-                                        Updating feature variants to:
-                                    </StyledTypography>
-                                    <EnvironmentVariantsTable
-                                        variants={change.payload.variants}
-                                    />
-                                </StyledBox>
-                            )
-                        }
-                    />
-                </>
-            )}
-            {change.action === 'deleteStrategy' && (
-                <>
-                    <ChangeItemCreateEditDeleteWrapper className='delete-strategy-information-wrapper'>
-                        <ChangeItemInfo>
-                            <Typography
-                                sx={(theme) => ({
-                                    color: theme.palette.error.main,
-                                })}
-                            >
-                                - Deleting strategy:
-                            </Typography>
-                            {hasNameField(change.payload) && (
-                                <StrategyTooltipLink change={change}>
-                                    <StrategyDiff
-                                        change={change}
-                                        currentStrategy={currentStrategy}
-                                    />
-                                </StrategyTooltipLink>
-                            )}
-                        </ChangeItemInfo>
-                        <div>{actions}</div>
-                    </ChangeItemCreateEditDeleteWrapper>
-                    <ConditionallyRender
-                        condition={Boolean(currentStrategy)}
-                        show={
-                            <Typography>
-                                {
-                                    <StrategyExecution
-                                        strategy={currentStrategy!}
-                                    />
-                                }
-                            </Typography>
-                        }
-                    />
-                    <ConditionallyRender
-                        condition={Boolean(currentStrategy?.variants?.length)}
-                        show={
-                            currentStrategy?.variants && (
-                                <StyledBox>
-                                    <StyledTypography>
-                                        Deleting strategy variants:
-                                    </StyledTypography>
-                                    <EnvironmentVariantsTable
-                                        variants={currentStrategy.variants}
-                                    />
-                                </StyledBox>
-                            )
-                        }
-                    />
-                </>
-            )}
-            {change.action === 'updateStrategy' && (
-                <>
-                    <ChangeOverwriteWarning
-                        data={{
-                            current: currentStrategy,
-                            change,
-                            changeType: 'strategy',
-                        }}
-                        changeRequestState={changeRequestState}
-                    />
-                    <ChangeItemCreateEditDeleteWrapper>
-                        <ChangeItemInfo>
-                            <EditHeader
-                                wasDisabled={currentStrategy?.disabled}
-                                willBeDisabled={change.payload?.disabled}
-                            />
-                            <StrategyTooltipLink
-                                change={change}
-                                previousTitle={currentStrategy?.title}
-                            >
-                                <StrategyDiff
-                                    change={change}
-                                    currentStrategy={currentStrategy}
-                                />
-                            </StrategyTooltipLink>
-                        </ChangeItemInfo>
-                        <div>{actions}</div>
-                    </ChangeItemCreateEditDeleteWrapper>
-                    <ConditionallyRender
-                        condition={
-                            change.payload?.disabled !==
-                            currentStrategy?.disabled
-                        }
-                        show={
-                            <Typography
-                                sx={{
-                                    marginTop: (theme) => theme.spacing(2),
-                                    marginBottom: (theme) => theme.spacing(2),
-                                    ...flexRow,
-                                    gap: (theme) => theme.spacing(1),
-                                }}
-                            >
-                                This strategy will be{' '}
-                                <DisabledEnabledState
-                                    disabled={change.payload?.disabled || false}
-                                />
-                            </Typography>
-                        }
-                    />
-                    <StrategyExecution strategy={change.payload} />
-                    <ConditionallyRender
-                        condition={Boolean(hasVariantDiff)}
-                        show={
+                    {change.payload.variants &&
+                        change.payload.variants.length > 0 && (
                             <StyledBox>
                                 <StyledTypography>
                                     Updating feature variants to:
                                 </StyledTypography>
                                 <EnvironmentVariantsTable
-                                    variants={change.payload.variants || []}
+                                    variants={change.payload.variants}
                                 />
                             </StyledBox>
-                        }
-                    />
+                        )}
                 </>
             )}
+            {change.action === 'deleteStrategy' && (
+                <DeleteStrategy
+                    change={change}
+                    changeRequestState={changeRequestState}
+                    currentStrategy={currentStrategy}
+                    actions={actions}
+                />
+            )}
+            {change.action === 'updateStrategy' && (
+                <UpdateStrategy
+                    change={change}
+                    changeRequestState={changeRequestState}
+                    currentStrategy={currentStrategy}
+                    actions={actions}
+                />
+            )}
         </>
     );
 };

frontend/src/component/changeRequest/ChangeRequest/StrategyTooltipLink/StrategyTooltipLink.tsx

@@ -51,10 +51,8 @@ export const StrategyDiff: FC<{
 };
 
 interface IStrategyTooltipLinkProps {
-    change:
-        | IChangeRequestAddStrategy
-        | IChangeRequestUpdateStrategy
-        | IChangeRequestDeleteStrategy;
+    name: string;
+    title?: string;
     previousTitle?: string;
     children?: React.ReactNode;
 }
@@ -80,29 +78,32 @@ const Truncated = styled('div')(() => ({
 }));
 
 export const StrategyTooltipLink: FC<IStrategyTooltipLinkProps> = ({
-    change,
+    name,
+    title,
     previousTitle,
     children,
-}) => (
-    <StyledContainer>
-        <GetFeatureStrategyIcon strategyName={change.payload.name} />
-        <Truncated>
-            <Typography component='span'>
-                {formatStrategyName(change.payload.name)}
-            </Typography>
-            <TooltipLink
-                tooltip={children}
-                tooltipProps={{
-                    maxWidth: 500,
-                    maxHeight: 600,
-                }}
-            >
-                <ViewDiff>View Diff</ViewDiff>
-            </TooltipLink>
-            <NameWithChangeInfo
-                newName={change.payload.title}
-                previousName={previousTitle}
-            />
-        </Truncated>
-    </StyledContainer>
-);
+}) => {
+    return (
+        <StyledContainer>
+            <GetFeatureStrategyIcon strategyName={name} />
+            <Truncated>
+                <Typography component='span'>
+                    {formatStrategyName(name)}
+                </Typography>
+                <TooltipLink
+                    tooltip={children}
+                    tooltipProps={{
+                        maxWidth: 500,
+                        maxHeight: 600,
+                    }}
+                >
+                    <ViewDiff>View Diff</ViewDiff>
+                </TooltipLink>
+                <NameWithChangeInfo
+                    newName={title}
+                    previousName={previousTitle}
+                />
+            </Truncated>
+        </StyledContainer>
+    );
+};

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

@@ -241,14 +241,15 @@ export type ChangeRequestAddStrategy = Pick<
 
 export type ChangeRequestEditStrategy = ChangeRequestAddStrategy & {
     id: string;
-    snapshot?: Omit<IFeatureStrategy, 'title'> & { title?: string | null };
+    snapshot?: IFeatureStrategy;
 };
 
 type ChangeRequestDeleteStrategy = {
     id: string;
-    name: string;
+    name?: string;
     title?: string;
     disabled?: boolean;
+    snapshot?: IFeatureStrategy;
 };
 
 export type ChangeRequestAction =

frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyExecution/StrategyExecution.tsx

@@ -1,4 +1,4 @@
-import { Fragment, useMemo, type VFC } from 'react';
+import { type FC, Fragment, useMemo } from 'react';
 import { Alert, Box, Chip, Link, styled } from '@mui/material';
 import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender';
 import PercentageCircle from 'component/common/PercentageCircle/PercentageCircle';
@@ -56,7 +56,7 @@ const CustomStrategyDeprecationWarning = () => (
     </Alert>
 );
 
-const NoItems: VFC = () => (
+const NoItems: FC = () => (
     <Box sx={{ px: 3, color: 'text.disabled' }}>
         This strategy does not have constraints or parameters.
     </Box>
@@ -73,7 +73,7 @@ const StyledValueSeparator = styled('span')(({ theme }) => ({
     color: theme.palette.neutral.main,
 }));
 
-export const StrategyExecution: VFC<IStrategyExecutionProps> = ({
+export const StrategyExecution: FC<IStrategyExecutionProps> = ({
     strategy,
 }) => {
     const { parameters, constraints = [] } = strategy;