diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.test.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.test.tsx new file mode 100644 index 000000000000..90c7f8a4565f --- /dev/null +++ b/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( + , + ); + + 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( + , + ); + + 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( + , + ); + + 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( + , + ); + + 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)'); +}); diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.tsx index 03321f4f4984..fd03701a168b 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.tsx +++ b/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 Editing strategy:; }; -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 ( + <> + + + ({ + color: theme.palette.error.main, + })} + > + - Deleting strategy: + + + + + +
{actions}
+ + {referenceStrategy && ( + + )} + + + Deleting strategy variants: + + + + ) + } + /> + + ); +}; + +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 ( + <> + + + + + + + + +
{actions}
+ + theme.spacing(2), + marginBottom: (theme) => theme.spacing(2), + ...flexRow, + gap: (theme) => theme.spacing(1), + }} + > + This strategy will be{' '} + + + } + /> + + + + Updating feature variants to: + + + + } + /> + + ); +}; + +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: - + {actions} - - - Updating feature variants to: - - - - ) - } - /> - - )} - {change.action === 'deleteStrategy' && ( - <> - - - ({ - color: theme.palette.error.main, - })} - > - - Deleting strategy: - - {hasNameField(change.payload) && ( - - - - )} - -
{actions}
- - - { - - } - - } - /> - - - Deleting strategy variants: - - - - ) - } - /> - - )} - {change.action === 'updateStrategy' && ( - <> - - - - - - - - -
{actions}
- - theme.spacing(2), - marginBottom: (theme) => theme.spacing(2), - ...flexRow, - gap: (theme) => theme.spacing(1), - }} - > - This strategy will be{' '} - - - } - /> - - 0 && ( Updating feature variants to: - } - /> + )} )} + {change.action === 'deleteStrategy' && ( + + )} + {change.action === 'updateStrategy' && ( + + )} ); }; diff --git a/frontend/src/component/changeRequest/ChangeRequest/StrategyTooltipLink/StrategyTooltipLink.tsx b/frontend/src/component/changeRequest/ChangeRequest/StrategyTooltipLink/StrategyTooltipLink.tsx index 7295dcf614e2..8719d91e2470 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/StrategyTooltipLink/StrategyTooltipLink.tsx +++ b/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 = ({ - change, + name, + title, previousTitle, children, -}) => ( - - - - - {formatStrategyName(change.payload.name)} - - - View Diff - - - - -); +}) => { + return ( + + + + + {formatStrategyName(name)} + + + View Diff + + + + + ); +}; diff --git a/frontend/src/component/changeRequest/changeRequest.types.ts b/frontend/src/component/changeRequest/changeRequest.types.ts index 61ccaeeecd64..e2e742b8027a 100644 --- a/frontend/src/component/changeRequest/changeRequest.types.ts +++ b/frontend/src/component/changeRequest/changeRequest.types.ts @@ -241,14 +241,15 @@ export type ChangeRequestAddStrategy = Pick< export type ChangeRequestEditStrategy = ChangeRequestAddStrategy & { id: string; - snapshot?: Omit & { title?: string | null }; + snapshot?: IFeatureStrategy; }; type ChangeRequestDeleteStrategy = { id: string; - name: string; + name?: string; title?: string; disabled?: boolean; + snapshot?: IFeatureStrategy; }; export type ChangeRequestAction = diff --git a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyExecution/StrategyExecution.tsx b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyExecution/StrategyExecution.tsx index 9f7bf3dbe801..38dbc9bca533 100644 --- a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyExecution/StrategyExecution.tsx +++ b/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 = () => ( ); -const NoItems: VFC = () => ( +const NoItems: FC = () => ( This strategy does not have constraints or parameters. @@ -73,7 +73,7 @@ const StyledValueSeparator = styled('span')(({ theme }) => ({ color: theme.palette.neutral.main, })); -export const StrategyExecution: VFC = ({ +export const StrategyExecution: FC = ({ strategy, }) => { const { parameters, constraints = [] } = strategy;