mirror of
				https://github.com/Unleash/unleash.git
				synced 2025-10-27 11:02:16 +01:00 
			
		
		
		
	fix: Only show strategy variant changes if there is a diff in the variants (#5353)
What it says on the box Closes # [1-1652](https://linear.app/unleash/issue/1-1652/remove-the-variants-from-change-request-page-when-not-modified)   --------- Signed-off-by: andreas-unleash <andreas@getunleash.ai>
This commit is contained in:
		
							parent
							
								
									9ac3d7511a
								
							
						
					
					
						commit
						8ffc92af5b
					
				@ -1,6 +1,6 @@
 | 
			
		||||
import { screen } from '@testing-library/react';
 | 
			
		||||
 | 
			
		||||
import { Routes, Route } from 'react-router-dom';
 | 
			
		||||
import { Route, Routes } from 'react-router-dom';
 | 
			
		||||
 | 
			
		||||
import { render } from 'utils/testRenderer';
 | 
			
		||||
import { ChangeRequest } from './ChangeRequest';
 | 
			
		||||
@ -9,6 +9,91 @@ import {
 | 
			
		||||
    IChangeRequestAddStrategy,
 | 
			
		||||
    IChangeRequestEnabled,
 | 
			
		||||
} from '../changeRequest.types';
 | 
			
		||||
import { testServerRoute, testServerSetup } from 'utils/testServer';
 | 
			
		||||
import { StrategyVariantSchema } from 'openapi';
 | 
			
		||||
 | 
			
		||||
const server = testServerSetup();
 | 
			
		||||
const uiConfigForEnterprise = () =>
 | 
			
		||||
    testServerRoute(server, '/api/admin/ui-config', {
 | 
			
		||||
        versionInfo: {
 | 
			
		||||
            current: { oss: 'version', enterprise: 'version' },
 | 
			
		||||
        },
 | 
			
		||||
        flags: {
 | 
			
		||||
            scheduledConfigurationChanges: true,
 | 
			
		||||
        },
 | 
			
		||||
    });
 | 
			
		||||
 | 
			
		||||
const featureWithStrategyVariants = () =>
 | 
			
		||||
    testServerRoute(server, `/api/admin/projects/default/features/feature1`, {
 | 
			
		||||
        name: 'feature1',
 | 
			
		||||
        impressionData: false,
 | 
			
		||||
        description: '',
 | 
			
		||||
        project: 'default',
 | 
			
		||||
        stale: false,
 | 
			
		||||
        variants: [],
 | 
			
		||||
        createdAt: '2022-11-14T08:16:33.338Z',
 | 
			
		||||
        lastSeenAt: null,
 | 
			
		||||
        type: 'release',
 | 
			
		||||
        archived: false,
 | 
			
		||||
        children: [],
 | 
			
		||||
        dependencies: [],
 | 
			
		||||
        environments: [
 | 
			
		||||
            {
 | 
			
		||||
                name: 'development',
 | 
			
		||||
                enabled: false,
 | 
			
		||||
                type: 'production',
 | 
			
		||||
                sortOrder: 1,
 | 
			
		||||
                strategies: [
 | 
			
		||||
                    {
 | 
			
		||||
                        id: '2e4f0555-518b-45b3-b0cd-a32cca388a92',
 | 
			
		||||
                        variants: [
 | 
			
		||||
                            {
 | 
			
		||||
                                name: 'variant1',
 | 
			
		||||
                                stickiness: 'default',
 | 
			
		||||
                                weight: 500,
 | 
			
		||||
                                weightType: 'fix',
 | 
			
		||||
                            },
 | 
			
		||||
                            {
 | 
			
		||||
                                name: 'variant2',
 | 
			
		||||
                                stickiness: 'default',
 | 
			
		||||
                                weight: 500,
 | 
			
		||||
                                weightType: 'fix',
 | 
			
		||||
                            },
 | 
			
		||||
                        ],
 | 
			
		||||
                    },
 | 
			
		||||
                ],
 | 
			
		||||
            },
 | 
			
		||||
        ],
 | 
			
		||||
    });
 | 
			
		||||
 | 
			
		||||
const feature = () =>
 | 
			
		||||
    testServerRoute(server, `/api/admin/projects/default/features/feature1`, {
 | 
			
		||||
        name: 'feature1',
 | 
			
		||||
        impressionData: false,
 | 
			
		||||
        description: '',
 | 
			
		||||
        project: 'default',
 | 
			
		||||
        stale: false,
 | 
			
		||||
        variants: [],
 | 
			
		||||
        createdAt: '2022-11-14T08:16:33.338Z',
 | 
			
		||||
        lastSeenAt: null,
 | 
			
		||||
        type: 'release',
 | 
			
		||||
        archived: false,
 | 
			
		||||
        children: [],
 | 
			
		||||
        dependencies: [],
 | 
			
		||||
        environments: [
 | 
			
		||||
            {
 | 
			
		||||
                name: 'development',
 | 
			
		||||
                enabled: false,
 | 
			
		||||
                type: 'production',
 | 
			
		||||
                sortOrder: 1,
 | 
			
		||||
                strategies: [
 | 
			
		||||
                    {
 | 
			
		||||
                        id: '2e4f0555-518b-45b3-b0cd-a32cca388a92',
 | 
			
		||||
                    },
 | 
			
		||||
                ],
 | 
			
		||||
            },
 | 
			
		||||
        ],
 | 
			
		||||
    });
 | 
			
		||||
 | 
			
		||||
const changeRequestWithDefaultChange = (
 | 
			
		||||
    defaultChange: IChangeRequestEnabled | IChangeRequestAddStrategy,
 | 
			
		||||
@ -26,7 +111,7 @@ const changeRequestWithDefaultChange = (
 | 
			
		||||
        segments: [],
 | 
			
		||||
        features: [
 | 
			
		||||
            {
 | 
			
		||||
                name: 'Feature Toggle Name',
 | 
			
		||||
                name: 'feature1',
 | 
			
		||||
                changes: [
 | 
			
		||||
                    {
 | 
			
		||||
                        id: 67,
 | 
			
		||||
@ -50,7 +135,51 @@ const changeRequestWithDefaultChange = (
 | 
			
		||||
        minApprovals: 1,
 | 
			
		||||
        state: 'Draft',
 | 
			
		||||
        title: 'My change request',
 | 
			
		||||
        project: 'project',
 | 
			
		||||
        project: 'default',
 | 
			
		||||
        environment: 'production',
 | 
			
		||||
    };
 | 
			
		||||
    return changeRequest;
 | 
			
		||||
};
 | 
			
		||||
 | 
			
		||||
const changeRequest = (variants: StrategyVariantSchema[]) => {
 | 
			
		||||
    const changeRequest: IChangeRequest = {
 | 
			
		||||
        approvals: [],
 | 
			
		||||
        rejections: [],
 | 
			
		||||
        comments: [],
 | 
			
		||||
        createdAt: new Date(),
 | 
			
		||||
        createdBy: {
 | 
			
		||||
            id: 1,
 | 
			
		||||
            username: 'author',
 | 
			
		||||
            imageUrl: '',
 | 
			
		||||
        },
 | 
			
		||||
        segments: [],
 | 
			
		||||
        features: [
 | 
			
		||||
            {
 | 
			
		||||
                name: 'feature1',
 | 
			
		||||
                changes: [
 | 
			
		||||
                    {
 | 
			
		||||
                        id: 0,
 | 
			
		||||
                        action: 'updateStrategy',
 | 
			
		||||
                        payload: {
 | 
			
		||||
                            id: '2e4f0555-518b-45b3-b0cd-a32cca388a92',
 | 
			
		||||
                            name: 'flexibleRollout',
 | 
			
		||||
                            constraints: [],
 | 
			
		||||
                            parameters: {
 | 
			
		||||
                                rollout: '100',
 | 
			
		||||
                                stickiness: 'default',
 | 
			
		||||
                                groupId: 'test123',
 | 
			
		||||
                            },
 | 
			
		||||
                            variants: variants,
 | 
			
		||||
                        },
 | 
			
		||||
                    },
 | 
			
		||||
                ],
 | 
			
		||||
            },
 | 
			
		||||
        ],
 | 
			
		||||
        id: 27,
 | 
			
		||||
        minApprovals: 1,
 | 
			
		||||
        state: 'Draft',
 | 
			
		||||
        title: 'My change request',
 | 
			
		||||
        project: 'default',
 | 
			
		||||
        environment: 'production',
 | 
			
		||||
    };
 | 
			
		||||
    return changeRequest;
 | 
			
		||||
@ -75,7 +204,7 @@ test('Display default add strategy', async () => {
 | 
			
		||||
        />,
 | 
			
		||||
    );
 | 
			
		||||
 | 
			
		||||
    expect(screen.getByText('Feature Toggle Name')).toBeInTheDocument();
 | 
			
		||||
    expect(screen.getByText('feature1')).toBeInTheDocument();
 | 
			
		||||
    expect(screen.getByText('Enabled')).toBeInTheDocument();
 | 
			
		||||
    expect(
 | 
			
		||||
        screen.getByText('Default strategy will be added'),
 | 
			
		||||
@ -95,12 +224,12 @@ test('Display default disable feature', async () => {
 | 
			
		||||
        />,
 | 
			
		||||
    );
 | 
			
		||||
 | 
			
		||||
    expect(screen.getByText('Feature Toggle Name')).toBeInTheDocument();
 | 
			
		||||
    expect(screen.getByText('feature1')).toBeInTheDocument();
 | 
			
		||||
    expect(screen.getByText('Disabled')).toBeInTheDocument();
 | 
			
		||||
    expect(screen.getByText('Feature status will change')).toBeInTheDocument();
 | 
			
		||||
});
 | 
			
		||||
 | 
			
		||||
test('Displays feature strategy variants table', async () => {
 | 
			
		||||
test('Displays feature strategy variants table when addStrategy action with variants', async () => {
 | 
			
		||||
    render(
 | 
			
		||||
        <Routes>
 | 
			
		||||
            <Route
 | 
			
		||||
@ -139,7 +268,7 @@ test('Displays feature strategy variants table', async () => {
 | 
			
		||||
            />
 | 
			
		||||
        </Routes>,
 | 
			
		||||
        {
 | 
			
		||||
            route: 'projects/default/features/colors/strategies/edit?environmentId=development&strategyId=2e4f0555-518b-45b3-b0cd-a32cca388a92',
 | 
			
		||||
            route: '/projects/default/features/feature1/strategies/edit',
 | 
			
		||||
        },
 | 
			
		||||
    );
 | 
			
		||||
 | 
			
		||||
@ -147,3 +276,59 @@ test('Displays feature strategy variants table', async () => {
 | 
			
		||||
        screen.getByText('Updating feature variants to:'),
 | 
			
		||||
    ).toBeInTheDocument();
 | 
			
		||||
});
 | 
			
		||||
 | 
			
		||||
test('Displays feature strategy variants table when there is a change in the variants array', async () => {
 | 
			
		||||
    uiConfigForEnterprise();
 | 
			
		||||
    featureWithStrategyVariants();
 | 
			
		||||
    render(
 | 
			
		||||
        <Routes>
 | 
			
		||||
            <Route
 | 
			
		||||
                path={'projects/:projectId/change-requests/:changeRequestId'}
 | 
			
		||||
                element={
 | 
			
		||||
                    <ChangeRequest
 | 
			
		||||
                        changeRequest={changeRequest([
 | 
			
		||||
                            {
 | 
			
		||||
                                name: 'variant1',
 | 
			
		||||
                                stickiness: 'default',
 | 
			
		||||
                                weight: 500,
 | 
			
		||||
                                weightType: 'fix',
 | 
			
		||||
                            },
 | 
			
		||||
                        ])}
 | 
			
		||||
                    />
 | 
			
		||||
                }
 | 
			
		||||
            />
 | 
			
		||||
        </Routes>,
 | 
			
		||||
        {
 | 
			
		||||
            route: '/projects/default/change-requests/27',
 | 
			
		||||
        },
 | 
			
		||||
    );
 | 
			
		||||
    await screen.findByText('Updating feature variants to:');
 | 
			
		||||
});
 | 
			
		||||
 | 
			
		||||
test('Displays feature strategy variants table when existing strategy does not have variants and change does', async () => {
 | 
			
		||||
    uiConfigForEnterprise();
 | 
			
		||||
    feature();
 | 
			
		||||
    render(
 | 
			
		||||
        <Routes>
 | 
			
		||||
            <Route
 | 
			
		||||
                path={'projects/:projectId/change-requests/:changeRequestId'}
 | 
			
		||||
                element={
 | 
			
		||||
                    <ChangeRequest
 | 
			
		||||
                        changeRequest={changeRequest([
 | 
			
		||||
                            {
 | 
			
		||||
                                name: 'variant1',
 | 
			
		||||
                                stickiness: 'default',
 | 
			
		||||
                                weight: 500,
 | 
			
		||||
                                weightType: 'fix',
 | 
			
		||||
                            },
 | 
			
		||||
                        ])}
 | 
			
		||||
                    />
 | 
			
		||||
                }
 | 
			
		||||
            />
 | 
			
		||||
        </Routes>,
 | 
			
		||||
        {
 | 
			
		||||
            route: '/projects/default/change-requests/27',
 | 
			
		||||
        },
 | 
			
		||||
    );
 | 
			
		||||
    await screen.findByText('Updating feature variants to:');
 | 
			
		||||
});
 | 
			
		||||
 | 
			
		||||
@ -127,20 +127,15 @@ 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 featureStrategyVariantsDisplay =
 | 
			
		||||
    const hasVariantDiff =
 | 
			
		||||
        isStrategyAction &&
 | 
			
		||||
        change.payload.variants &&
 | 
			
		||||
        change.payload.variants.length > 0 ? (
 | 
			
		||||
            <StyledBox>
 | 
			
		||||
                <StyledTypography>
 | 
			
		||||
                    Updating feature variants to:
 | 
			
		||||
                </StyledTypography>
 | 
			
		||||
                <EnvironmentVariantsTable variants={change.payload.variants} />
 | 
			
		||||
            </StyledBox>
 | 
			
		||||
        ) : null;
 | 
			
		||||
        hasDiff(currentStrategy?.variants || [], change.payload.variants || []);
 | 
			
		||||
 | 
			
		||||
    return (
 | 
			
		||||
        <>
 | 
			
		||||
@ -173,7 +168,21 @@ export const StrategyChange: VFC<{
 | 
			
		||||
                        <div>{actions}</div>
 | 
			
		||||
                    </ChangeItemCreateEditWrapper>
 | 
			
		||||
                    <StrategyExecution strategy={change.payload} />
 | 
			
		||||
                    {featureStrategyVariantsDisplay}
 | 
			
		||||
                    <ConditionallyRender
 | 
			
		||||
                        condition={Boolean(change.payload.variants)}
 | 
			
		||||
                        show={
 | 
			
		||||
                            change.payload.variants && (
 | 
			
		||||
                                <StyledBox>
 | 
			
		||||
                                    <StyledTypography>
 | 
			
		||||
                                        Updating feature variants to:
 | 
			
		||||
                                    </StyledTypography>
 | 
			
		||||
                                    <EnvironmentVariantsTable
 | 
			
		||||
                                        variants={change.payload.variants}
 | 
			
		||||
                                    />
 | 
			
		||||
                                </StyledBox>
 | 
			
		||||
                            )
 | 
			
		||||
                        }
 | 
			
		||||
                    />
 | 
			
		||||
                </>
 | 
			
		||||
            )}
 | 
			
		||||
            {change.action === 'deleteStrategy' && (
 | 
			
		||||
@ -240,7 +249,19 @@ export const StrategyChange: VFC<{
 | 
			
		||||
                        }
 | 
			
		||||
                    />
 | 
			
		||||
                    <StrategyExecution strategy={change.payload} />
 | 
			
		||||
                    {featureStrategyVariantsDisplay}
 | 
			
		||||
                    <ConditionallyRender
 | 
			
		||||
                        condition={Boolean(hasVariantDiff)}
 | 
			
		||||
                        show={
 | 
			
		||||
                            <StyledBox>
 | 
			
		||||
                                <StyledTypography>
 | 
			
		||||
                                    Updating feature variants to:
 | 
			
		||||
                                </StyledTypography>
 | 
			
		||||
                                <EnvironmentVariantsTable
 | 
			
		||||
                                    variants={change.payload.variants || []}
 | 
			
		||||
                                />
 | 
			
		||||
                            </StyledBox>
 | 
			
		||||
                        }
 | 
			
		||||
                    />
 | 
			
		||||
                </>
 | 
			
		||||
            )}
 | 
			
		||||
        </>
 | 
			
		||||
 | 
			
		||||
@ -14,8 +14,8 @@ export const useCurrentStrategy = (
 | 
			
		||||
    feature: string,
 | 
			
		||||
    environmentName: string,
 | 
			
		||||
) => {
 | 
			
		||||
    const currentFeature = useFeature(project, feature);
 | 
			
		||||
    const currentStrategy = currentFeature.feature?.environments
 | 
			
		||||
    const { feature: currentFeature } = useFeature(project, feature);
 | 
			
		||||
    const currentStrategy = currentFeature?.environments
 | 
			
		||||
        .find((environment) => environment.name === environmentName)
 | 
			
		||||
        ?.strategies.find(
 | 
			
		||||
            (strategy) =>
 | 
			
		||||
 | 
			
		||||
		Loading…
	
		Reference in New Issue
	
	Block a user