Skip to content

Commit

Permalink
feat: display potential conflicts in existing change requests (#5521)
Browse files Browse the repository at this point in the history
This update displays schedule conflicts in active change requests (any
CR that isn't applied, canceled, or rejected).


![image](https://github.com/Unleash/unleash/assets/17786332/181c6c0a-f5de-4eed-9093-ac0109e1e8f3)
  • Loading branch information
thomasheartman committed Dec 4, 2023
1 parent a9363ef commit a0a1541
Show file tree
Hide file tree
Showing 3 changed files with 188 additions and 2 deletions.
@@ -0,0 +1,135 @@
import React from 'react';
import { render } from 'utils/testRenderer';
import { screen } from '@testing-library/react';
import { FeatureChange } from './FeatureChange';
import {
ChangeRequestState,
IChangeRequestFeature,
IFeatureChange,
} from 'component/changeRequest/changeRequest.types';

describe('Schedule conflicts', () => {
const change = {
id: 15,
action: 'deleteStrategy' as const,
payload: {
id: 'b3ac8595-8ad3-419e-aa18-4d82f2b6bc4c',
name: 'flexibleRollout',
},
createdAt: new Date(),
createdBy: {
id: 1,
username: 'admin',
imageUrl: '',
},
scheduleConflicts: {
changeRequests: [
{
id: 73,
},
{
id: 80,
title: 'Adjust rollout percentage',
},
],
},
};

const feature = (change: IFeatureChange): IChangeRequestFeature => ({
name: 'conflict-test',
changes: [change],
});

const changeRequest =
(feature: IChangeRequestFeature) => (state: ChangeRequestState) => ({
id: 1,
state,
title: '',
project: 'default',
environment: 'default',
minApprovals: 1,
createdBy: { id: 1, username: 'user1', imageUrl: '' },
createdAt: new Date(),
features: [feature],
segments: [],
approvals: [],
rejections: [],
comments: [],
});

it.each(['Draft', 'Scheduled', 'In review', 'Approved'])(
'should show schedule conflicts (when they exist) for change request in the %s state',
async (changeRequestState) => {
const flag = feature(change);
render(
<FeatureChange
actions={null}
index={0}
changeRequest={changeRequest(flag)(
changeRequestState as ChangeRequestState,
)}
change={change}
feature={flag}
/>,
);

const alert = await screen.findByRole('alert');

expect(
alert.textContent!.startsWith('Potential conflict'),
).toBeTruthy();

const links = await screen.findAllByRole('link');

expect(links).toHaveLength(
change.scheduleConflicts.changeRequests.length,
);

const [link1, link2] = links;

expect(link1).toHaveTextContent('#73');
expect(link1).toHaveAccessibleDescription('Change request 73');
expect(link1).toHaveAttribute(
'href',
`/projects/default/change-requests/73`,
);

expect(link2).toHaveTextContent('#80 (Adjust rollout percentage)');
expect(link2).toHaveAccessibleDescription('Change request 80');
expect(link2).toHaveAttribute(
'href',
`/projects/default/change-requests/80`,
);
},
);

it.each(['Draft', 'Scheduled', 'In review', 'Approved'])(
'should not show schedule conflicts when they do not exist for change request in the %s state',
async (changeRequestState) => {
const { scheduleConflicts, ...changeWithNoScheduleConflicts } =
change;

const flag = feature(changeWithNoScheduleConflicts);

render(
<FeatureChange
actions={null}
index={0}
changeRequest={changeRequest(flag)(
changeRequestState as ChangeRequestState,
)}
change={changeWithNoScheduleConflicts}
feature={flag}
/>,
);

const links = screen.queryByRole('link');

expect(links).toBe(null);

const alert = screen.queryByRole('alert');

expect(alert).toBe(null);
},
);
});
Expand Up @@ -13,6 +13,7 @@ import { VariantPatch } from './VariantPatch/VariantPatch';
import { EnvironmentStrategyExecutionOrder } from './EnvironmentStrategyExecutionOrder/EnvironmentStrategyExecutionOrder';
import { ArchiveFeatureChange } from './ArchiveFeatureChange';
import { DependencyChange } from './DependencyChange';
import { Link } from 'react-router-dom';

const StyledSingleChangeBox = styled(Box, {
shouldForwardProp: (prop: string) => !prop.startsWith('$'),
Expand Down Expand Up @@ -56,6 +57,15 @@ const StyledAlert = styled(Alert)(({ theme }) => ({
},
}));

const InlineList = styled('ul')(({ theme }) => ({
display: 'inline',
padding: 0,
li: { display: 'inline' },
'li + li::before': {
content: '", "',
},
}));

export const FeatureChange: FC<{
actions: ReactNode;
index: number;
Expand All @@ -71,9 +81,12 @@ export const FeatureChange: FC<{
return (
<StyledSingleChangeBox
key={objectId(change)}
$hasConflict={Boolean(change.conflict)}
$hasConflict={Boolean(change.conflict || change.scheduleConflicts)}
$isInConflictFeature={Boolean(feature.conflict)}
$isAfterWarning={Boolean(feature.changes[index - 1]?.conflict)}
$isAfterWarning={Boolean(
feature.changes[index - 1]?.conflict ||
feature.changes[index - 1]?.scheduleConflicts,
)}
$isLast={index + 1 === lastIndex}
>
<ConditionallyRender
Expand All @@ -86,6 +99,41 @@ export const FeatureChange: FC<{
}
/>

<ConditionallyRender
condition={Boolean(change.scheduleConflicts)}
show={
<StyledAlert severity='warning'>
<strong>Potential conflict!</strong> This change would
create conflicts with the following scheduled change
request(s):{' '}
<InlineList>
{(
change.scheduleConflicts ?? {
changeRequests: [],
}
).changeRequests.map(({ id, title }) => {
const text = title
? `#${id} (${title})`
: `#${id}`;
return (
<li key={id}>
<Link
to={`/projects/${changeRequest.project}/change-requests/${id}`}
target='_blank'
rel='noopener noreferrer'
title={`Change request ${id}`}
>
{text}
</Link>
</li>
);
})}
.
</InlineList>
</StyledAlert>
}
/>

<Box sx={(theme) => ({ padding: theme.spacing(3) })}>
{(change.action === 'addDependency' ||
change.action === 'deleteDependency') && (
Expand Down
3 changes: 3 additions & 0 deletions frontend/src/component/changeRequest/changeRequest.types.ts
Expand Up @@ -66,6 +66,9 @@ export interface IChangeRequestChangeBase {
conflict?: string;
createdBy?: Pick<IUser, 'id' | 'username' | 'imageUrl'>;
createdAt?: Date;
scheduleConflicts?: {
changeRequests: { id: number; title?: string }[];
};
}

export type ChangeRequestState =
Expand Down

0 comments on commit a0a1541

Please sign in to comment.