From d6ddc95c1ebc0f5c2a8cf1b0db310a93c9b6cab8 Mon Sep 17 00:00:00 2001 From: "unleash-bot[bot]" <194219037+unleash-bot[bot]@users.noreply.github.com> Date: Thu, 21 Aug 2025 09:50:48 +0000 Subject: [PATCH] chore(AI): improvedJsonDiff flag cleanup (#10486) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR cleans up the improvedJsonDiff flag. These changes were automatically generated by AI and should be reviewed carefully. Fixes #10483 ## ๐Ÿงน AI Flag Cleanup Summary This PR removes the `improvedJsonDiff` feature flag, making the enhanced JSON diffing component the default and only option. The now-unused legacy diff component and all related feature flag logic have been removed to streamline the codebase. ### ๐Ÿšฎ Removed - **Components** - `OldEventDiff` component was removed, along with its helper types and constants. - **Flag Logic** - All conditional rendering based on the `improvedJsonDiff` flag was removed. - The `sort` prop from `EventDiff` was removed as it was only used by the legacy component. - **Configuration** - `improvedJsonDiff` flag definition was removed from `uiConfig.ts`, `experimental.ts`, and `server-dev.ts`. - **Tests** - Mock configuration for `improvedJsonDiff` in tests was removed. ### ๐Ÿ›  Kept - **Components** - `NewEventDiff` was renamed to `EventDiff` and is now the standard implementation. ### ๐Ÿ“ Why The `improvedJsonDiff` feature flag was marked as completed with its outcome being "kept". This cleanup finalizes the feature rollout by removing the flag and associated legacy code, simplifying the implementation and reducing code complexity. --------- Co-authored-by: unleash-bot <194219037+unleash-bot[bot]@users.noreply.github.com> Co-authored-by: Thomas Heartman --- .../EnvironmentStrategyOrderDiff.tsx | 32 +--- .../Change/LegacyReleasePlanChange.tsx | 2 +- .../Changes/Change/StrategyChange.test.tsx | 6 - .../Change/VariantPatch/VariantDiff.tsx | 3 +- .../StaleDataNotification.tsx | 2 +- .../component/events/EventCard/EventCard.tsx | 2 +- .../events/EventDiff/EventDiff.test.tsx | 28 ++-- .../component/events/EventDiff/EventDiff.tsx | 137 +----------------- frontend/src/interfaces/uiConfig.ts | 1 - pijul/identities/publickey.json | 1 + .../metrics/impact/metrics-translator.ts | 2 +- src/lib/types/experimental.ts | 5 - src/server-dev.ts | 1 - 13 files changed, 33 insertions(+), 189 deletions(-) create mode 100644 pijul/identities/publickey.json diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/EnvironmentStrategyExecutionOrder/EnvironmentStrategyOrderDiff.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/EnvironmentStrategyExecutionOrder/EnvironmentStrategyOrderDiff.tsx index 24e11195c6..299fea453a 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/EnvironmentStrategyExecutionOrder/EnvironmentStrategyOrderDiff.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/EnvironmentStrategyExecutionOrder/EnvironmentStrategyOrderDiff.tsx @@ -1,18 +1,4 @@ -import { styled } from '@mui/material'; import { EventDiff } from 'component/events/EventDiff/EventDiff'; -import { useUiFlag } from 'hooks/useUiFlag'; -import { Fragment } from 'react'; - -const StyledCodeSection = styled('div')(({ theme }) => ({ - overflowX: 'auto', - '& code': { - wordWrap: 'break-word', - whiteSpace: 'pre-wrap', - fontFamily: 'monospace', - lineHeight: 1.5, - fontSize: theme.fontSizes.smallBody, - }, -})); type StrategyIds = { strategyIds: string[] }; interface IDiffProps { preData: StrategyIds; @@ -20,18 +6,12 @@ interface IDiffProps { } export const EnvironmentStrategyOrderDiff = ({ preData, data }: IDiffProps) => { - const useNewDiff = useUiFlag('improvedJsonDiff'); - const Wrapper = useNewDiff ? Fragment : StyledCodeSection; - return ( - - a.index - b.index} - /> - + ); }; diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/LegacyReleasePlanChange.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/LegacyReleasePlanChange.tsx index 2a819076f2..6fc60d460f 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/LegacyReleasePlanChange.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/LegacyReleasePlanChange.tsx @@ -10,7 +10,7 @@ import type { import { useReleasePlanPreview } from 'hooks/useReleasePlanPreview'; import { useReleasePlans } from 'hooks/api/getters/useReleasePlans/useReleasePlans'; import { TooltipLink } from 'component/common/TooltipLink/TooltipLink'; -import EventDiff from 'component/events/EventDiff/EventDiff'; +import { EventDiff } from 'component/events/EventDiff/EventDiff'; import { ReleasePlan } from 'component/feature/FeatureView/FeatureOverview/ReleasePlan/ReleasePlan'; import { ReleasePlanMilestone } from 'component/feature/FeatureView/FeatureOverview/ReleasePlan/ReleasePlanMilestone/ReleasePlanMilestone'; import type { IReleasePlan } from 'interfaces/releasePlans'; diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.test.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.test.tsx index 8121ec7f28..93ae9b59e8 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.test.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.test.tsx @@ -27,12 +27,6 @@ const strategy = { }; const setupApi = () => { - testServerRoute(server, '/api/admin/ui-config', { - flags: { - improvedJsonDiff: true, - }, - }); - testServerRoute( server, `/api/admin/projects/${projectId}/features/${feature}`, diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/VariantPatch/VariantDiff.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/VariantPatch/VariantDiff.tsx index b4c217af65..4c2fb4f893 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/VariantPatch/VariantDiff.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/VariantPatch/VariantDiff.tsx @@ -1,5 +1,5 @@ import { styled } from '@mui/material'; -import EventDiff from 'component/events/EventDiff/EventDiff'; +import { EventDiff } from 'component/events/EventDiff/EventDiff'; import type { IFeatureVariant } from 'interfaces/featureToggle'; const StyledCodeSection = styled('div')(({ theme }) => ({ @@ -31,7 +31,6 @@ export const VariantDiff = ({ preData, data }: IDiffProps) => ( preData: variantsArrayToObject(preData), data: variantsArrayToObject(data), }} - sort={(a, b) => a.index - b.index} /> ); diff --git a/frontend/src/component/common/StaleDataNotification/StaleDataNotification.tsx b/frontend/src/component/common/StaleDataNotification/StaleDataNotification.tsx index 484a6f2271..e141311563 100644 --- a/frontend/src/component/common/StaleDataNotification/StaleDataNotification.tsx +++ b/frontend/src/component/common/StaleDataNotification/StaleDataNotification.tsx @@ -1,5 +1,5 @@ import { Typography, Button, useTheme, useMediaQuery } from '@mui/material'; -import EventDiff from 'component/events/EventDiff/EventDiff'; +import { EventDiff } from 'component/events/EventDiff/EventDiff'; import { fadeInBottomEnter, fadeInBottomStartWithoutFixed, diff --git a/frontend/src/component/events/EventCard/EventCard.tsx b/frontend/src/component/events/EventCard/EventCard.tsx index 0a9b346ce4..b1cff2047a 100644 --- a/frontend/src/component/events/EventCard/EventCard.tsx +++ b/frontend/src/component/events/EventCard/EventCard.tsx @@ -1,4 +1,4 @@ -import EventDiff from 'component/events/EventDiff/EventDiff'; +import { EventDiff } from 'component/events/EventDiff/EventDiff'; import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; import { useLocationSettings } from 'hooks/useLocationSettings'; import { formatDateYMDHMS } from 'utils/formatDate'; diff --git a/frontend/src/component/events/EventDiff/EventDiff.test.tsx b/frontend/src/component/events/EventDiff/EventDiff.test.tsx index f4c3bb6962..3aef6f89b1 100644 --- a/frontend/src/component/events/EventDiff/EventDiff.test.tsx +++ b/frontend/src/component/events/EventDiff/EventDiff.test.tsx @@ -1,23 +1,27 @@ import { render } from 'utils/testRenderer'; import { screen } from '@testing-library/react'; -import EventDiff from './EventDiff.tsx'; +import { EventDiff } from './EventDiff.tsx'; test('Show no changes', async () => { - render(); - - expect(screen.getByText('(no changes)')).toBeInTheDocument(); + const { container } = render( + , + ); + const diff = container.querySelector('.diff'); + expect(diff).toBeEmptyDOMElement(); }); test('Show new data added diff', async () => { render(); - expect(screen.getByText('+ segments: []')).toBeInTheDocument(); + const element = await screen.findByText(/segments:.*/); + expect(element).toHaveClass('addition'); }); test('Show new data removed diff', async () => { render(); - expect(screen.getByText('- segments (deleted)')).toBeInTheDocument(); + const element = await screen.findByText(/segments:.*/); + expect(element).toHaveClass('deletion'); }); test('Show new data changes diff', async () => { @@ -27,8 +31,10 @@ test('Show new data changes diff', async () => { />, ); - expect(screen.getByText('- segments: "a"')).toBeInTheDocument(); - expect(screen.getByText('+ segments: "b"')).toBeInTheDocument(); + const deleted = await screen.findByText(/segments: "a".*/); + expect(deleted).toHaveClass('deletion'); + const added = await screen.findByText(/segments: "b".*/); + expect(added).toHaveClass('addition'); }); test('Show new data only', async () => { @@ -36,7 +42,8 @@ test('Show new data only', async () => { , ); - expect(screen.getByText('{ "segments": [] }')).toBeInTheDocument(); + const element = await screen.findByText(/segments:.*/); + expect(element).toHaveClass('addition'); }); test('Show old data only', async () => { @@ -44,5 +51,6 @@ test('Show old data only', async () => { , ); - expect(screen.getByText('{ "segments": [] }')).toBeInTheDocument(); + const element = await screen.findByText(/segments:.*/); + expect(element).toHaveClass('deletion'); }); diff --git a/frontend/src/component/events/EventDiff/EventDiff.tsx b/frontend/src/component/events/EventDiff/EventDiff.tsx index 3e4c75bdf2..07e312fc96 100644 --- a/frontend/src/component/events/EventDiff/EventDiff.tsx +++ b/frontend/src/component/events/EventDiff/EventDiff.tsx @@ -1,28 +1,9 @@ -import { diff } from 'deep-diff'; -import { type JSX, type CSSProperties, useState, type FC, useId } from 'react'; +import { useState, type FC, useId } from 'react'; import { JsonDiffComponent, type JsonValue } from 'json-diff-react'; -import { Button, styled, useTheme } from '@mui/material'; -import { useUiFlag } from 'hooks/useUiFlag'; - -const DIFF_PREFIXES: Record = { - A: ' ', - E: ' ', - D: '-', - N: '+', -}; - -interface IEventDiffResult { - key: string; - value: JSX.Element; - index: number; -} +import { Button, styled } from '@mui/material'; interface IEventDiffProps { entry: { data?: unknown; preData?: unknown }; - /** - * @deprecated remove with flag improvedJsonDiff - */ - sort?: (a: IEventDiffResult, b: IEventDiffResult) => number; excludeKeys?: string[]; } @@ -78,7 +59,7 @@ const ButtonIcon = styled('span')(({ theme }) => ({ marginInlineEnd: theme.spacing(0.5), })); -export const NewEventDiff: FC = ({ entry, excludeKeys }) => { +export const EventDiff: FC = ({ entry, excludeKeys }) => { const changeType = entry.preData && entry.data ? 'edit' : 'replacement'; const showExpandButton = changeType === 'edit'; const [full, setFull] = useState(false); @@ -112,115 +93,3 @@ export const NewEventDiff: FC = ({ entry, excludeKeys }) => { ); }; - -const OldEventDiff: FC = ({ - entry, - sort = (a, b) => a.key.localeCompare(b.key), -}: IEventDiffProps) => { - const theme = useTheme(); - - const styles: Record = { - A: { color: theme.palette.eventLog.edited }, // array edited - E: { color: theme.palette.eventLog.edited }, // edited - D: { color: theme.palette.eventLog.diffSub }, // deleted - N: { color: theme.palette.eventLog.diffAdd }, // added - }; - - const diffs = - entry.data && entry.preData - ? diff(entry.preData, entry.data) - : undefined; - - const buildItemDiff = (diff: any, key: string) => { - let change: JSX.Element | undefined; - if (diff.lhs !== undefined) { - change = ( -
- - {key}: {JSON.stringify(diff.lhs)} -
- ); - } else if (diff.rhs !== undefined) { - change = ( -
- + {key}: {JSON.stringify(diff.rhs)} -
- ); - } - - return change; - }; - - const buildDiff = (diff: any, index: number): IEventDiffResult => { - let change: JSX.Element | undefined; - const key = diff.path?.join('.') ?? diff.index; - - if (diff.item) { - change = buildItemDiff(diff.item, key); - } else if (diff.lhs !== undefined && diff.rhs !== undefined) { - change = ( -
-
- - {key}: {JSON.stringify(diff.lhs)} -
-
- + {key}: {JSON.stringify(diff.rhs)} -
-
- ); - } else { - const changeValue = JSON.stringify(diff.rhs || diff.item); - change = ( -
- {DIFF_PREFIXES[diff.kind]} {key} - {changeValue - ? `: ${changeValue}` - : diff.kind === 'D' - ? ' (deleted)' - : ''} -
- ); - } - - return { - key: key.toString(), - value:
{change}
, - index, - }; - }; - - let changes: any[] = []; - - if (diffs) { - changes = diffs - .map(buildDiff) - .sort(sort) - .map(({ value }) => value); - } else if (entry.data == null || entry.preData == null) { - // Just show the data if there is no diff yet. - const data = entry.data || entry.preData; - changes = [ -
- {JSON.stringify(data, null, 2)} -
, - ]; - } - - return ( - 
-            {changes.length === 0 ? '(no changes)' : changes}
-
- ); -}; - -export const EventDiff: FC = (props) => { - const useNewJsonDiff = useUiFlag('improvedJsonDiff'); - if (useNewJsonDiff) { - return ; - } - return ; -}; - -/** - * @deprecated remove the default export with flag improvedJsonDiff. Switch imports in files that use this to the named import instead. - */ -export default EventDiff; diff --git a/frontend/src/interfaces/uiConfig.ts b/frontend/src/interfaces/uiConfig.ts index fc6d3576b6..2c85786f61 100644 --- a/frontend/src/interfaces/uiConfig.ts +++ b/frontend/src/interfaces/uiConfig.ts @@ -88,7 +88,6 @@ export type UiFlags = { customMetrics?: boolean; lifecycleMetrics?: boolean; createFlagDialogCache?: boolean; - improvedJsonDiff?: boolean; impactMetrics?: boolean; crDiffView?: boolean; changeRequestApproverEmails?: boolean; diff --git a/pijul/identities/publickey.json b/pijul/identities/publickey.json new file mode 100644 index 0000000000..0967ef424b --- /dev/null +++ b/pijul/identities/publickey.json @@ -0,0 +1 @@ +{} diff --git a/src/lib/features/metrics/impact/metrics-translator.ts b/src/lib/features/metrics/impact/metrics-translator.ts index 0e343f0caf..dcd577f141 100644 --- a/src/lib/features/metrics/impact/metrics-translator.ts +++ b/src/lib/features/metrics/impact/metrics-translator.ts @@ -54,7 +54,7 @@ export class MetricsTranslator { ): Record { return { ...(sample.labels || {}), - origin: (sample.labels && sample.labels.origin) || 'sdk', + origin: sample.labels?.origin || 'sdk', }; } diff --git a/src/lib/types/experimental.ts b/src/lib/types/experimental.ts index 852c72a84b..6378a9cb4e 100644 --- a/src/lib/types/experimental.ts +++ b/src/lib/types/experimental.ts @@ -58,7 +58,6 @@ export type IFlagKey = | 'customMetrics' | 'impactMetrics' | 'createFlagDialogCache' - | 'improvedJsonDiff' | 'crDiffView' | 'changeRequestApproverEmails' | 'paygTrialEvents' @@ -275,10 +274,6 @@ const flags: IFlags = { process.env.UNLEASH_EXPERIMENTAL_CHANGE_REQUEST_APPROVER_EMAILS, false, ), - improvedJsonDiff: parseEnvVarBoolean( - process.env.UNLEASH_EXPERIMENTAL_IMPROVED_JSON_DIFF, - false, - ), crDiffView: parseEnvVarBoolean( process.env.UNLEASH_EXPERIMENTAL_CR_DIFF_VIEW, false, diff --git a/src/server-dev.ts b/src/server-dev.ts index 893af00cfe..eefb22fd36 100644 --- a/src/server-dev.ts +++ b/src/server-dev.ts @@ -54,7 +54,6 @@ process.nextTick(async () => { reportUnknownFlags: true, customMetrics: true, lifecycleMetrics: true, - improvedJsonDiff: true, impactMetrics: true, crDiffView: true, paygTrialEvents: true,