From 7b2f86b7a6449457e052405a3d635ae7330b2acf Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Fri, 9 Feb 2024 10:40:56 +0900 Subject: [PATCH] fix: don't show strategy change updates if the CR is closed This PR hides the strategy changes that would be overwritten if the CR is closed (applied, rejected, or canceled) This should, however, be merged into the recent changes that add support for showing this for segments too. --- .../Changes/Change/FeatureChange.tsx | 1 + .../Changes/Change/StrategyChange.tsx | 12 ++- .../StrategyChangeOverwriteWarning.test.tsx | 85 +++++++++++++++++++ .../Change/StrategyChangeOverwriteWarning.tsx | 38 +++++++-- 4 files changed, 127 insertions(+), 9 deletions(-) create mode 100644 frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChangeOverwriteWarning.test.tsx diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/FeatureChange.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/FeatureChange.tsx index d111f3218a..5657ddbe99 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/FeatureChange.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/FeatureChange.tsx @@ -163,6 +163,7 @@ export const FeatureChange: FC<{ featureName={feature.name} environmentName={changeRequest.environment} projectId={changeRequest.project} + changeRequestState={changeRequest.state} /> ) : null} {change.action === 'patchVariant' && ( diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.tsx index c7b71a4c9e..0a327c019f 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.tsx @@ -8,6 +8,7 @@ import { } from '../../StrategyTooltipLink/StrategyTooltipLink'; import { StrategyExecution } from 'component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyExecution/StrategyExecution'; import { + ChangeRequestState, IChangeRequestAddStrategy, IChangeRequestDeleteStrategy, IChangeRequestUpdateStrategy, @@ -120,7 +121,15 @@ export const StrategyChange: VFC<{ environmentName: string; featureName: string; projectId: string; -}> = ({ actions, change, featureName, environmentName, projectId }) => { + changeRequestState: ChangeRequestState; +}> = ({ + actions, + change, + featureName, + environmentName, + projectId, + changeRequestState, +}) => { const currentStrategy = useCurrentStrategy( change, projectId, @@ -211,6 +220,7 @@ export const StrategyChange: VFC<{ {change.action === 'updateStrategy' && ( <> diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChangeOverwriteWarning.test.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChangeOverwriteWarning.test.tsx new file mode 100644 index 0000000000..82a490e27f --- /dev/null +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChangeOverwriteWarning.test.tsx @@ -0,0 +1,85 @@ +import { render } from 'utils/testRenderer'; +import { screen } from '@testing-library/react'; +import { ChangesToOverwriteInternal } from './StrategyChangeOverwriteWarning'; +import { IFeatureStrategy } from 'interfaces/strategy'; +import { + ChangeRequestState, + IChangeRequestUpdateStrategy, +} from 'component/changeRequest/changeRequest.types'; + +const existingStrategy: IFeatureStrategy = { + name: 'flexibleRollout', + constraints: [], + variants: [], + parameters: { + groupId: 'aaa', + rollout: '71', + stickiness: 'default', + }, + id: '31572930-2db7-461f-813b-3eedc200cb33', + title: '', + disabled: false, + segments: [], +}; + +const snapshot: IFeatureStrategy = { + id: '31572930-2db7-461f-813b-3eedc200cb33', + name: 'flexibleRollout', + title: '', + disabled: true, + segments: [], + variants: [], + parameters: { + groupId: 'aaa', + rollout: '72', + stickiness: 'default', + }, + constraints: [], +}; + +const change: IChangeRequestUpdateStrategy = { + id: 39, + action: 'updateStrategy' as const, + payload: { + id: '31572930-2db7-461f-813b-3eedc200cb33', + name: 'flexibleRollout', + title: '', + disabled: false, + segments: [], + snapshot, + variants: [], + parameters: { + groupId: 'baa', + rollout: '38', + stickiness: 'default', + }, + constraints: [], + }, + createdAt: new Date('2024-01-18T07:58:36.314Z'), + createdBy: { + id: 1, + username: 'admin', + imageUrl: '', + }, +}; + +test.each([ + ['Draft', true], + ['Approved', true], + ['In review', true], + ['Applied', false], + ['Scheduled', true], + ['Cancelled', false], + ['Rejected', false], +])('Shows warnings for CRs in the "%s" state: %s', (status, showWarning) => { + render( + , + ); + + const hasRendered = screen.queryByText('Heads up!') !== null; + expect(hasRendered).toBe(showWarning); +}); diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChangeOverwriteWarning.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChangeOverwriteWarning.tsx index 8c6bf2ae42..0bcd53eeed 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChangeOverwriteWarning.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChangeOverwriteWarning.tsx @@ -1,5 +1,8 @@ import { Box, styled } from '@mui/material'; -import { IChangeRequestUpdateStrategy } from 'component/changeRequest/changeRequest.types'; +import { + ChangeRequestState, + IChangeRequestUpdateStrategy, +} from 'component/changeRequest/changeRequest.types'; import { useChangeRequestPlausibleContext } from 'component/changeRequest/ChangeRequestContext'; import { useUiFlag } from 'hooks/useUiFlag'; import { IFeatureStrategy } from 'interfaces/strategy'; @@ -70,24 +73,29 @@ const OverwriteTable = styled('table')(({ theme }) => ({ }, })); -export const ChangesToOverwrite: React.FC<{ +export const ChangesToOverwriteInternal: React.FC<{ currentStrategy?: IFeatureStrategy; change: IChangeRequestUpdateStrategy; -}> = ({ change, currentStrategy }) => { - const checkForChanges = useUiFlag('changeRequestConflictHandling'); - const changesThatWouldBeOverwritten = checkForChanges - ? getChangesThatWouldBeOverwritten(currentStrategy, change) - : null; + changeRequestState: ChangeRequestState; +}> = ({ change, currentStrategy, changeRequestState }) => { + const changesThatWouldBeOverwritten = getChangesThatWouldBeOverwritten( + currentStrategy, + change, + ); const { registerWillOverwriteStrategyChanges } = useChangeRequestPlausibleContext(); + const changeRequestIsClosed = ['Applied', 'Cancelled', 'Rejected'].includes( + changeRequestState, + ); + useEffect(() => { if (changesThatWouldBeOverwritten) { registerWillOverwriteStrategyChanges(); } }, [changesThatWouldBeOverwritten]); - if (!changesThatWouldBeOverwritten) { + if (!changesThatWouldBeOverwritten || changeRequestIsClosed) { return null; } @@ -162,3 +170,17 @@ export const ChangesToOverwrite: React.FC<{ ); }; + +export const ChangesToOverwrite: React.FC<{ + currentStrategy?: IFeatureStrategy; + change: IChangeRequestUpdateStrategy; + changeRequestState: ChangeRequestState; +}> = ({ change, currentStrategy, changeRequestState }) => { + return useUiFlag('changeRequestConflictHandling') ? ( + + ) : null; +};