mirror of
https://github.com/Unleash/unleash.git
synced 2025-02-04 00:18:01 +01:00
feat: visualize variants diff in CR (#2979)
https://linear.app/unleash/issue/2-582/display-the-change-request-created-with-variants-in-the-ui ![image](https://user-images.githubusercontent.com/14320932/214341314-c4f1aefb-fada-4d59-9d40-86f8dce98b76.png) Includes a basic diff visualisation on variants change requests. It seems like components like `CodeSnippetPopover` and `PopoverDiff` are currently very tightly coupled together with strategies, so I preferred to follow my own approach and leave those alone for now instead of trying to refactor them. `patchVariant` could also be renamed to a more fitting name in the future as well, since we're now doing more of an override than applying a patch. `Diff` is a generic diff component that uses `EventDiff` internally and simply takes into account a "before" and "after" state, as `preData` and `data`. I made some changes to `EventDiff` that made some sense to me: - Cover edge cases where `path` is undefined and `.join` crashes, also fallback to the diff index (or undefined); - Leverage the key to correctly sort the change items in the diff;
This commit is contained in:
parent
80c444aa99
commit
c5fced89fb
@ -1,4 +1,4 @@
|
|||||||
import React, { FC, ReactNode } from 'react';
|
import { FC, ReactNode } from 'react';
|
||||||
import {
|
import {
|
||||||
hasNameField,
|
hasNameField,
|
||||||
IChange,
|
IChange,
|
||||||
@ -20,6 +20,7 @@ import {
|
|||||||
StrategyDeletedChange,
|
StrategyDeletedChange,
|
||||||
StrategyEditedChange,
|
StrategyEditedChange,
|
||||||
} from './StrategyChange';
|
} from './StrategyChange';
|
||||||
|
import { VariantPatch } from './VariantPatch/VariantPatch';
|
||||||
|
|
||||||
const StyledSingleChangeBox = styled(Box, {
|
const StyledSingleChangeBox = styled(Box, {
|
||||||
shouldForwardProp: (prop: string) => !prop.startsWith('$'),
|
shouldForwardProp: (prop: string) => !prop.startsWith('$'),
|
||||||
@ -138,6 +139,15 @@ export const Change: FC<{
|
|||||||
<StrategyExecution strategy={change.payload} />
|
<StrategyExecution strategy={change.payload} />
|
||||||
</>
|
</>
|
||||||
)}
|
)}
|
||||||
|
{change.action === 'patchVariant' && (
|
||||||
|
<VariantPatch
|
||||||
|
feature={feature.name}
|
||||||
|
project={changeRequest.project}
|
||||||
|
environment={changeRequest.environment}
|
||||||
|
change={change}
|
||||||
|
discard={discard}
|
||||||
|
/>
|
||||||
|
)}
|
||||||
</Box>
|
</Box>
|
||||||
</StyledSingleChangeBox>
|
</StyledSingleChangeBox>
|
||||||
);
|
);
|
||||||
|
@ -0,0 +1,29 @@
|
|||||||
|
import { styled } from '@mui/material';
|
||||||
|
import EventDiff from 'component/events/EventDiff/EventDiff';
|
||||||
|
|
||||||
|
const StyledCodeSection = styled('div')(({ theme }) => ({
|
||||||
|
overflowX: 'auto',
|
||||||
|
'& code': {
|
||||||
|
wordWrap: 'break-word',
|
||||||
|
whiteSpace: 'pre-wrap',
|
||||||
|
fontFamily: 'monospace',
|
||||||
|
lineHeight: 1.5,
|
||||||
|
fontSize: theme.fontSizes.smallBody,
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
|
||||||
|
interface IDiffProps {
|
||||||
|
preData: any;
|
||||||
|
data: any;
|
||||||
|
}
|
||||||
|
|
||||||
|
export const Diff = ({ preData, data }: IDiffProps) => (
|
||||||
|
<StyledCodeSection>
|
||||||
|
<EventDiff
|
||||||
|
entry={{
|
||||||
|
preData,
|
||||||
|
data,
|
||||||
|
}}
|
||||||
|
/>
|
||||||
|
</StyledCodeSection>
|
||||||
|
);
|
@ -0,0 +1,50 @@
|
|||||||
|
import { Box, styled, Typography } from '@mui/material';
|
||||||
|
import { IChangeRequestPatchVariant } from 'component/changeRequest/changeRequest.types';
|
||||||
|
import { useFeature } from 'hooks/api/getters/useFeature/useFeature';
|
||||||
|
import { ReactNode } from 'react';
|
||||||
|
import { Diff } from './Diff';
|
||||||
|
|
||||||
|
export const ChangeItemCreateEditWrapper = styled(Box)(({ theme }) => ({
|
||||||
|
display: 'flex',
|
||||||
|
justifyContent: 'space-between',
|
||||||
|
alignItems: 'center',
|
||||||
|
marginBottom: theme.spacing(2),
|
||||||
|
}));
|
||||||
|
|
||||||
|
const ChangeItemInfo = styled(Box)(({ theme }) => ({
|
||||||
|
display: 'flex',
|
||||||
|
flexDirection: 'column',
|
||||||
|
gap: theme.spacing(1),
|
||||||
|
}));
|
||||||
|
|
||||||
|
interface IVariantPatchProps {
|
||||||
|
feature: string;
|
||||||
|
project: string;
|
||||||
|
environment: string;
|
||||||
|
change: IChangeRequestPatchVariant;
|
||||||
|
discard?: ReactNode;
|
||||||
|
}
|
||||||
|
|
||||||
|
export const VariantPatch = ({
|
||||||
|
feature,
|
||||||
|
project,
|
||||||
|
environment,
|
||||||
|
change,
|
||||||
|
discard,
|
||||||
|
}: IVariantPatchProps) => {
|
||||||
|
const { feature: featureData } = useFeature(project, feature);
|
||||||
|
|
||||||
|
const preData = featureData.environments.find(
|
||||||
|
({ name }) => environment === name
|
||||||
|
)?.variants;
|
||||||
|
|
||||||
|
return (
|
||||||
|
<ChangeItemCreateEditWrapper>
|
||||||
|
<ChangeItemInfo>
|
||||||
|
<Typography>Updating variants:</Typography>
|
||||||
|
<Diff preData={preData} data={change.payload.variants} />
|
||||||
|
</ChangeItemInfo>
|
||||||
|
{discard}
|
||||||
|
</ChangeItemCreateEditWrapper>
|
||||||
|
);
|
||||||
|
};
|
@ -1,3 +1,4 @@
|
|||||||
|
import { IFeatureVariant } from 'interfaces/featureToggle';
|
||||||
import { IFeatureStrategy } from '../../interfaces/strategy';
|
import { IFeatureStrategy } from '../../interfaces/strategy';
|
||||||
import { IUser } from '../../interfaces/user';
|
import { IUser } from '../../interfaces/user';
|
||||||
|
|
||||||
@ -60,7 +61,8 @@ type ChangeRequestPayload =
|
|||||||
| ChangeRequestEnabled
|
| ChangeRequestEnabled
|
||||||
| ChangeRequestAddStrategy
|
| ChangeRequestAddStrategy
|
||||||
| ChangeRequestEditStrategy
|
| ChangeRequestEditStrategy
|
||||||
| ChangeRequestDeleteStrategy;
|
| ChangeRequestDeleteStrategy
|
||||||
|
| ChangeRequestVariantPatch;
|
||||||
|
|
||||||
export interface IChangeRequestAddStrategy extends IChangeRequestBase {
|
export interface IChangeRequestAddStrategy extends IChangeRequestBase {
|
||||||
action: 'addStrategy';
|
action: 'addStrategy';
|
||||||
@ -82,11 +84,21 @@ export interface IChangeRequestEnabled extends IChangeRequestBase {
|
|||||||
payload: ChangeRequestEnabled;
|
payload: ChangeRequestEnabled;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export interface IChangeRequestPatchVariant extends IChangeRequestBase {
|
||||||
|
action: 'patchVariant';
|
||||||
|
payload: ChangeRequestVariantPatch;
|
||||||
|
}
|
||||||
|
|
||||||
export type IChange =
|
export type IChange =
|
||||||
| IChangeRequestAddStrategy
|
| IChangeRequestAddStrategy
|
||||||
| IChangeRequestDeleteStrategy
|
| IChangeRequestDeleteStrategy
|
||||||
| IChangeRequestUpdateStrategy
|
| IChangeRequestUpdateStrategy
|
||||||
| IChangeRequestEnabled;
|
| IChangeRequestEnabled
|
||||||
|
| IChangeRequestPatchVariant;
|
||||||
|
|
||||||
|
type ChangeRequestVariantPatch = {
|
||||||
|
variants: IFeatureVariant[];
|
||||||
|
};
|
||||||
|
|
||||||
type ChangeRequestEnabled = { enabled: boolean };
|
type ChangeRequestEnabled = { enabled: boolean };
|
||||||
|
|
||||||
@ -106,7 +118,8 @@ export type ChangeRequestAction =
|
|||||||
| 'updateEnabled'
|
| 'updateEnabled'
|
||||||
| 'addStrategy'
|
| 'addStrategy'
|
||||||
| 'updateStrategy'
|
| 'updateStrategy'
|
||||||
| 'deleteStrategy';
|
| 'deleteStrategy'
|
||||||
|
| 'patchVariant';
|
||||||
|
|
||||||
export const hasNameField = (payload: unknown): payload is { name: string } =>
|
export const hasNameField = (payload: unknown): payload is { name: string } =>
|
||||||
typeof payload === 'object' && payload !== null && 'name' in payload;
|
typeof payload === 'object' && payload !== null && 'name' in payload;
|
||||||
|
@ -50,7 +50,7 @@ const EventDiff = ({ entry }: IEventDiffProps) => {
|
|||||||
|
|
||||||
const buildDiff = (diff: any, idx: number) => {
|
const buildDiff = (diff: any, idx: number) => {
|
||||||
let change;
|
let change;
|
||||||
const key = diff.path.join('.');
|
const key = diff.path?.join('.') ?? diff.index;
|
||||||
|
|
||||||
if (diff.item) {
|
if (diff.item) {
|
||||||
change = buildItemDiff(diff.item, key);
|
change = buildItemDiff(diff.item, key);
|
||||||
@ -74,13 +74,16 @@ const EventDiff = ({ entry }: IEventDiffProps) => {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
return <div key={idx}>{change}</div>;
|
return { key: key.toString(), value: <div key={idx}>{change}</div> };
|
||||||
};
|
};
|
||||||
|
|
||||||
let changes;
|
let changes;
|
||||||
|
|
||||||
if (diffs) {
|
if (diffs) {
|
||||||
changes = diffs.map(buildDiff);
|
changes = diffs
|
||||||
|
.map(buildDiff)
|
||||||
|
.sort((a, b) => a.key.localeCompare(b.key))
|
||||||
|
.map(({ value }) => value);
|
||||||
} else {
|
} else {
|
||||||
// Just show the data if there is no diff yet.
|
// Just show the data if there is no diff yet.
|
||||||
const data = entry.data || entry.preData;
|
const data = entry.data || entry.preData;
|
||||||
|
Loading…
Reference in New Issue
Block a user