Add various fixes to the CR view (#10231)

Adds a number of small corrections and fixes to (mainly) CR-related component. Discovered as part of adding the new JSON diff view. Refer to the various inline comments for explanations. --------- Co-authored-by: Tymoteusz Czech <2625371+Tymek@users.noreply.github.com>
2025-10-27 11:02:16 +01:00 · 2025-06-27 12:16:11 +02:00 · 2025-06-27 12:16:11 +02:00 · f2766b6b3b
commit f2766b6b3b
parent 55b8941306
11 changed files with 95 additions and 71 deletions
--- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ArchiveFeatureChange.tsx
+++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ArchiveFeatureChange.tsx
@ -16,7 +16,7 @@ export const ArchiveFeatureChange: FC<IArchiveFeatureChange> = ({
    actions,
 }) => (
    <ChangeItemWrapper>
-        <ArchiveBox>Archiving feature</ArchiveBox>
+        <ArchiveBox>Archiving flag</ArchiveBox>
        {actions}
    </ChangeItemWrapper>
 );
--- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/EditChange.tsx
+++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/EditChange.tsx
@ -26,6 +26,7 @@ import { FeatureStrategyForm } from '../../../../feature/FeatureStrategy/Feature
 import { NewStrategyVariants } from 'component/feature/StrategyTypes/NewStrategyVariants';
 import { v4 as uuidv4 } from 'uuid';
 import { constraintId } from 'constants/constraintId.ts';
+import { apiPayloadConstraintReplacer } from 'utils/api-payload-constraint-replacer.ts';

 interface IEditChangeProps {
    change: IChangeRequestAddStrategy | IChangeRequestUpdateStrategy;
@ -208,7 +209,7 @@ export const formatUpdateStrategyApiCode = (
    }

    const url = `${unleashUrl}/api/admin/projects/${projectId}/change-requests/${changeRequestId}/changes/${changeId}`;
-    const payload = JSON.stringify(strategy, undefined, 2);
+    const payload = JSON.stringify(strategy, apiPayloadConstraintReplacer, 2);

    return `curl --location --request PUT '${url}' \\
    --header 'Authorization: INSERT_API_KEY' \\
--- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/EnvironmentStrategyExecutionOrder/EnvironmentStrategyExecutionOrder.tsx
+++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/EnvironmentStrategyExecutionOrder/EnvironmentStrategyExecutionOrder.tsx
@ -6,6 +6,7 @@ import { Box, styled } from '@mui/material';
 import { EnvironmentStrategyOrderDiff } from './EnvironmentStrategyOrderDiff.tsx';
 import { StrategyExecution } from 'component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyExecution/StrategyExecution';
 import { formatStrategyName } from '../../../../../../utils/strategyNames.tsx';
+import type { IFeatureStrategy } from 'interfaces/strategy.ts';

 const ChangeItemInfo = styled(Box)({
    display: 'flex',
@ -74,14 +75,14 @@ export const EnvironmentStrategyExecutionOrder = ({
                .map((strategy) => strategy.id) ?? [],
    };

-    const updatedStrategies = change.payload
+    const updatedStrategies: IFeatureStrategy[] = change.payload
        .map(({ id }) => {
            return environmentStrategies.find((s) => s.id === id);
        })
-        .filter(Boolean);
+        .filter((strategy): strategy is IFeatureStrategy => Boolean(strategy));

    const data = {
-        strategyIds: updatedStrategies.map((strategy) => strategy!.id),
+        strategyIds: updatedStrategies.map((strategy) => strategy.id),
    };

    return (
@ -105,7 +106,7 @@ export const EnvironmentStrategyExecutionOrder = ({
            </StyledChangeHeader>
            <StyledStrategyExecutionWrapper>
                {updatedStrategies.map((strategy, index) => (
-                    <StyledStrategyContainer>
+                    <StyledStrategyContainer key={strategy.id}>
                        {`${index + 1}: `}
                        {formatStrategyName(strategy?.name || '')}
                        {strategy?.title && ` - ${strategy.title}`}
--- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/SegmentChange.tsx
+++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/SegmentChange.tsx
@ -7,6 +7,7 @@ import type {
 } from '../../../changeRequest.types';
 import { SegmentChangeDetails } from './SegmentChangeDetails.tsx';
 import { ConflictWarning } from './ConflictWarning.tsx';
+import { useSegment } from 'hooks/api/getters/useSegment/useSegment.ts';

 interface ISegmentChangeProps {
    segmentChange: ISegmentChange;
@ -20,61 +21,65 @@ export const SegmentChange: FC<ISegmentChangeProps> = ({
    onNavigate,
    actions,
    changeRequestState,
-}) => (
-    <Card
-        elevation={0}
-        sx={(theme) => ({
-            marginTop: theme.spacing(2),
-            marginBottom: theme.spacing(2),
-            overflow: 'hidden',
-        })}
-    >
-        <Box
+}) => {
+    const { segment } = useSegment(segmentChange.payload.id);
+
+    return (
+        <Card
+            elevation={0}
            sx={(theme) => ({
-                backgroundColor: theme.palette.neutral.light,
-                borderRadius: (theme) =>
-                    `${theme.shape.borderRadiusLarge}px ${theme.shape.borderRadiusLarge}px 0 0`,
-                border: '1px solid',
-                borderColor: (theme) =>
-                    segmentChange.conflict
-                        ? theme.palette.warning.border
-                        : theme.palette.divider,
-                borderBottom: 'none',
+                marginTop: theme.spacing(2),
+                marginBottom: theme.spacing(2),
                overflow: 'hidden',
            })}
        >
-            <ConflictWarning conflict={segmentChange.conflict} />
            <Box
-                sx={{
-                    display: 'flex',
-                    pt: 2,
-                    pb: 2,
-                    px: 3,
-                }}
+                sx={(theme) => ({
+                    backgroundColor: theme.palette.neutral.light,
+                    borderRadius: `${theme.shape.borderRadiusLarge}px ${theme.shape.borderRadiusLarge}px 0 0`,
+                    border: '1px solid',
+                    borderColor: segmentChange.conflict
+                        ? theme.palette.warning.border
+                        : theme.palette.divider,
+                    borderBottom: 'none',
+                    overflow: 'hidden',
+                })}
            >
-                <Typography>Segment name: </Typography>
-
-                <Link
-                    component={RouterLink}
-                    to={`/segments/edit/${segmentChange.payload.id}`}
-                    color='primary'
-                    underline='hover'
+                <ConflictWarning conflict={segmentChange.conflict} />
+                <Box
                    sx={{
-                        marginLeft: 1,
-                        '& :hover': {
-                            textDecoration: 'underline',
-                        },
+                        display: 'flex',
+                        pt: 2,
+                        pb: 2,
+                        px: 3,
                    }}
-                    onClick={onNavigate}
                >
-                    <strong>{segmentChange.payload.name}</strong>
-                </Link>
+                    <Typography>Segment name:</Typography>
+
+                    <Link
+                        component={RouterLink}
+                        to={`/segments/edit/${segmentChange.payload.id}`}
+                        color='primary'
+                        underline='hover'
+                        sx={{
+                            marginLeft: 1,
+                            '& :hover': {
+                                textDecoration: 'underline',
+                            },
+                        }}
+                        onClick={onNavigate}
+                    >
+                        <strong>
+                            {segmentChange.payload.name || segment?.name}
+                        </strong>
+                    </Link>
+                </Box>
            </Box>
-        </Box>
-        <SegmentChangeDetails
-            change={segmentChange}
-            actions={actions}
-            changeRequestState={changeRequestState}
-        />
-    </Card>
-);
+            <SegmentChangeDetails
+                change={segmentChange}
+                actions={actions}
+                changeRequestState={changeRequestState}
+            />
+        </Card>
+    );
+};
--- a/frontend/src/component/changeRequest/ChangeRequest/StrategyTooltipLink/StrategyTooltipLink.tsx
+++ b/frontend/src/component/changeRequest/ChangeRequest/StrategyTooltipLink/StrategyTooltipLink.tsx
@ -9,13 +9,14 @@ import {
    formatStrategyName,
    GetFeatureStrategyIcon,
 } from 'utils/strategyNames';
-import EventDiff from 'component/events/EventDiff/EventDiff';
+import EventDiff, { NewEventDiff } from 'component/events/EventDiff/EventDiff';
 import omit from 'lodash.omit';
 import { TooltipLink } from 'component/common/TooltipLink/TooltipLink';
 import { Typography, styled } from '@mui/material';
 import type { IFeatureStrategy } from 'interfaces/strategy';
 import { textTruncated } from 'themes/themeStyles';
 import { NameWithChangeInfo } from '../NameWithChangeInfo/NameWithChangeInfo.tsx';
+import { useUiFlag } from 'hooks/useUiFlag.ts';

 const StyledCodeSection = styled('div')(({ theme }) => ({
    overflowX: 'auto',
@ -47,12 +48,22 @@ export const StrategyDiff: FC<{
        | IChangeRequestDeleteStrategy;
    currentStrategy?: IFeatureStrategy;
 }> = ({ change, currentStrategy }) => {
+    const useNewDiff = useUiFlag('improvedJsonDiff');
    const changeRequestStrategy =
        change.action === 'deleteStrategy' ? undefined : change.payload;

    const sortedCurrentStrategy = sortSegments(currentStrategy);
    const sortedChangeRequestStrategy = sortSegments(changeRequestStrategy);
-
+    if (useNewDiff) {
+        return (
+            <NewEventDiff
+                entry={{
+                    preData: omit(sortedCurrentStrategy, 'sortOrder'),
+                    data: omit(sortedChangeRequestStrategy, 'snapshot'),
+                }}
+            />
+        );
+    }
    return (
        <StyledCodeSection>
            <EventDiff
--- a/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestHeader/ChangeRequestHeader.styles.tsx
+++ b/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestHeader/ChangeRequestHeader.styles.tsx
@ -1,4 +1,4 @@
-import { styled } from '@mui/material';
+import { styled, type TypographyProps } from '@mui/material';
 import { Box, Card, Paper, Typography } from '@mui/material';
 import { UserAvatar } from 'component/common/UserAvatar/UserAvatar';

@ -18,12 +18,14 @@ export const StyledInnerContainer = styled(Box)(({ theme }) => ({
    alignItems: 'center',
 }));

-export const StyledHeader = styled(Typography)(({ theme }) => ({
-    display: 'flex',
-    alignItems: 'center',
-    marginRight: theme.spacing(1),
-    fontSize: theme.fontSizes.mainHeader,
-}));
+export const StyledHeader = styled(Typography)<TypographyProps>(
+    ({ theme }) => ({
+        display: 'flex',
+        alignItems: 'center',
+        marginRight: theme.spacing(1),
+        fontSize: theme.fontSizes.mainHeader,
+    }),
+);

 export const StyledCard = styled(Card)(({ theme }) => ({
    padding: theme.spacing(0.75, 1.5),
--- a/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestHeader/ChangeRequestHeader.tsx
+++ b/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestHeader/ChangeRequestHeader.tsx
@ -26,7 +26,7 @@ export const ChangeRequestHeader: FC<{ changeRequest: ChangeRequestType }> = ({
                title={title}
                setTitle={setTitle}
            >
-                <StyledHeader variant='h1' sx={{ mr: 1.5 }}>
+                <StyledHeader variant='h1' component='h2' sx={{ mr: 1.5 }}>
                    {title}
                </StyledHeader>
            </ChangeRequestTitle>
--- a/frontend/src/component/common/FeatureArchiveDialog/FeatureArchiveDialog.test.tsx
+++ b/frontend/src/component/common/FeatureArchiveDialog/FeatureArchiveDialog.test.tsx
@ -70,7 +70,7 @@ test('Add single archive feature change to change request', async () => {

    expect(screen.getByText('Archive feature flag')).toBeInTheDocument();
    await screen.findByText(
-        'Archiving features with dependencies will also remove those dependencies.',
+        'Archiving flags with dependencies will also remove those dependencies.',
    );
    const button = await screen.findByText('Add change to draft');

@ -100,7 +100,7 @@ test('Add multiple archive feature changes to change request', async () => {

    await screen.findByText('Archive feature flags');
    await screen.findByText(
-        'Archiving features with dependencies will also remove those dependencies.',
+        'Archiving flags with dependencies will also remove those dependencies.',
    );
    const button = await screen.findByText('Add to change request');

@ -163,7 +163,7 @@ test('Show error message when multiple parents of orphaned children are archived
    );
    expect(
        screen.queryByText(
-            'Archiving features with dependencies will also remove those dependencies.',
+            'Archiving flags with dependencies will also remove those dependencies.',
        ),
    ).not.toBeInTheDocument();
 });
@ -189,7 +189,7 @@ test('Show error message when 1 parent of orphaned children is archived', async
    );
    expect(
        screen.queryByText(
-            'Archiving features with dependencies will also remove those dependencies.',
+            'Archiving flags with dependencies will also remove those dependencies.',
        ),
    ).not.toBeInTheDocument();
 });
--- a/frontend/src/component/common/FeatureArchiveDialog/FeatureArchiveDialog.tsx
+++ b/frontend/src/component/common/FeatureArchiveDialog/FeatureArchiveDialog.tsx
@ -26,7 +26,7 @@ interface IFeatureArchiveDialogProps {
 const RemovedDependenciesAlert = () => {
    return (
        <Alert severity='warning' sx={{ m: (theme) => theme.spacing(2, 0) }}>
-            Archiving features with dependencies will also remove those
+            Archiving flags with dependencies will also remove those
            dependencies.
        </Alert>
    );
--- a/frontend/src/component/common/NewConstraintAccordion/ConstraintsList/ViewableConstraintsList.tsx
+++ b/frontend/src/component/common/NewConstraintAccordion/ConstraintsList/ViewableConstraintsList.tsx
@ -4,6 +4,7 @@ import useUnleashContext from 'hooks/api/getters/useUnleashContext/useUnleashCon
 import { ConstraintsList } from 'component/common/ConstraintsList/ConstraintsList';
 import { ConstraintAccordionView } from 'component/common/NewConstraintAccordion/ConstraintAccordionView/ConstraintAccordionView';
 import { constraintId } from 'constants/constraintId';
+import { objectId } from 'utils/objectId';

 export interface IViewableConstraintsListProps {
    constraints: IConstraint[];
@ -29,7 +30,7 @@ export const ViewableConstraintsList = ({
            <ConstraintsList>
                {constraints.map((constraint) => (
                    <ConstraintAccordionView
-                        key={constraint[constraintId]}
+                        key={constraint[constraintId] || objectId(constraint)}
                        constraint={constraint}
                    />
                ))}
--- a/frontend/src/component/events/EventDiff/EventDiff.tsx
+++ b/frontend/src/component/events/EventDiff/EventDiff.tsx
@ -78,7 +78,7 @@ const ButtonIcon = styled('span')(({ theme }) => ({
    marginInlineEnd: theme.spacing(0.5),
 }));

-const NewEventDiff: FC<IEventDiffProps> = ({ entry, excludeKeys }) => {
+export const NewEventDiff: FC<IEventDiffProps> = ({ entry, excludeKeys }) => {
    const changeType = entry.preData && entry.data ? 'edit' : 'replacement';
    const showExpandButton = changeType === 'edit';
    const [full, setFull] = useState(false);
@ -220,4 +220,7 @@ const EventDiff: FC<IEventDiffProps> = (props) => {
    return <OldEventDiff {...props} />;
 };

+/**
+ * @deprecated remove the default export with flag improvedJsonDiff. Switch imports in files that use this to the named import instead.
+ */
 export default EventDiff;