From c5fced89fbbb337301722279ffa328ba9ab5449d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nuno=20G=C3=B3is?= Date: Tue, 24 Jan 2023 16:06:53 +0000 Subject: [PATCH] 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; --- .../ChangeRequest/Changes/Change/Change.tsx | 12 ++++- .../Changes/Change/VariantPatch/Diff.tsx | 29 +++++++++++ .../Change/VariantPatch/VariantPatch.tsx | 50 +++++++++++++++++++ .../changeRequest/changeRequest.types.ts | 19 +++++-- .../component/events/EventDiff/EventDiff.tsx | 9 ++-- 5 files changed, 112 insertions(+), 7 deletions(-) create mode 100644 frontend/src/component/changeRequest/ChangeRequest/Changes/Change/VariantPatch/Diff.tsx create mode 100644 frontend/src/component/changeRequest/ChangeRequest/Changes/Change/VariantPatch/VariantPatch.tsx diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/Change.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/Change.tsx index 0a760a9bf1..dd03080771 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/Change.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/Change.tsx @@ -1,4 +1,4 @@ -import React, { FC, ReactNode } from 'react'; +import { FC, ReactNode } from 'react'; import { hasNameField, IChange, @@ -20,6 +20,7 @@ import { StrategyDeletedChange, StrategyEditedChange, } from './StrategyChange'; +import { VariantPatch } from './VariantPatch/VariantPatch'; const StyledSingleChangeBox = styled(Box, { shouldForwardProp: (prop: string) => !prop.startsWith('$'), @@ -138,6 +139,15 @@ export const Change: FC<{ )} + {change.action === 'patchVariant' && ( + + )} ); diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/VariantPatch/Diff.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/VariantPatch/Diff.tsx new file mode 100644 index 0000000000..d0caf4aaff --- /dev/null +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/VariantPatch/Diff.tsx @@ -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) => ( + + + +); diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/VariantPatch/VariantPatch.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/VariantPatch/VariantPatch.tsx new file mode 100644 index 0000000000..d9b1d0e335 --- /dev/null +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/VariantPatch/VariantPatch.tsx @@ -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 ( + + + Updating variants: + + + {discard} + + ); +}; diff --git a/frontend/src/component/changeRequest/changeRequest.types.ts b/frontend/src/component/changeRequest/changeRequest.types.ts index 992cb5feca..8d9fdb49e2 100644 --- a/frontend/src/component/changeRequest/changeRequest.types.ts +++ b/frontend/src/component/changeRequest/changeRequest.types.ts @@ -1,3 +1,4 @@ +import { IFeatureVariant } from 'interfaces/featureToggle'; import { IFeatureStrategy } from '../../interfaces/strategy'; import { IUser } from '../../interfaces/user'; @@ -60,7 +61,8 @@ type ChangeRequestPayload = | ChangeRequestEnabled | ChangeRequestAddStrategy | ChangeRequestEditStrategy - | ChangeRequestDeleteStrategy; + | ChangeRequestDeleteStrategy + | ChangeRequestVariantPatch; export interface IChangeRequestAddStrategy extends IChangeRequestBase { action: 'addStrategy'; @@ -82,11 +84,21 @@ export interface IChangeRequestEnabled extends IChangeRequestBase { payload: ChangeRequestEnabled; } +export interface IChangeRequestPatchVariant extends IChangeRequestBase { + action: 'patchVariant'; + payload: ChangeRequestVariantPatch; +} + export type IChange = | IChangeRequestAddStrategy | IChangeRequestDeleteStrategy | IChangeRequestUpdateStrategy - | IChangeRequestEnabled; + | IChangeRequestEnabled + | IChangeRequestPatchVariant; + +type ChangeRequestVariantPatch = { + variants: IFeatureVariant[]; +}; type ChangeRequestEnabled = { enabled: boolean }; @@ -106,7 +118,8 @@ export type ChangeRequestAction = | 'updateEnabled' | 'addStrategy' | 'updateStrategy' - | 'deleteStrategy'; + | 'deleteStrategy' + | 'patchVariant'; export const hasNameField = (payload: unknown): payload is { name: string } => typeof payload === 'object' && payload !== null && 'name' in payload; diff --git a/frontend/src/component/events/EventDiff/EventDiff.tsx b/frontend/src/component/events/EventDiff/EventDiff.tsx index e5b46c0cea..d4f5a205dd 100644 --- a/frontend/src/component/events/EventDiff/EventDiff.tsx +++ b/frontend/src/component/events/EventDiff/EventDiff.tsx @@ -50,7 +50,7 @@ const EventDiff = ({ entry }: IEventDiffProps) => { const buildDiff = (diff: any, idx: number) => { let change; - const key = diff.path.join('.'); + const key = diff.path?.join('.') ?? diff.index; if (diff.item) { change = buildItemDiff(diff.item, key); @@ -74,13 +74,16 @@ const EventDiff = ({ entry }: IEventDiffProps) => { ); } - return
{change}
; + return { key: key.toString(), value:
{change}
}; }; let changes; if (diffs) { - changes = diffs.map(buildDiff); + changes = diffs + .map(buildDiff) + .sort((a, b) => a.key.localeCompare(b.key)) + .map(({ value }) => value); } else { // Just show the data if there is no diff yet. const data = entry.data || entry.preData;