From f2766b6b3b2ad0c3fc3d7e58c999d15edcab47c2 Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Fri, 27 Jun 2025 12:16:11 +0200 Subject: [PATCH] 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> --- .../Changes/Change/ArchiveFeatureChange.tsx | 2 +- .../Changes/Change/EditChange.tsx | 3 +- .../EnvironmentStrategyExecutionOrder.tsx | 9 +- .../Changes/Change/SegmentChange.tsx | 101 +++++++++--------- .../StrategyTooltipLink.tsx | 15 ++- .../ChangeRequestHeader.styles.tsx | 16 +-- .../ChangeRequestHeader.tsx | 2 +- .../FeatureArchiveDialog.test.tsx | 8 +- .../FeatureArchiveDialog.tsx | 2 +- .../ViewableConstraintsList.tsx | 3 +- .../component/events/EventDiff/EventDiff.tsx | 5 +- 11 files changed, 95 insertions(+), 71 deletions(-) diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ArchiveFeatureChange.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ArchiveFeatureChange.tsx index a96a034e76..9f62e99db3 100644 --- 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 = ({ actions, }) => ( - Archiving feature + Archiving flag {actions} ); diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/EditChange.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/EditChange.tsx index 3cac329927..bb60ca07b9 100644 --- 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' \\ diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/EnvironmentStrategyExecutionOrder/EnvironmentStrategyExecutionOrder.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/EnvironmentStrategyExecutionOrder/EnvironmentStrategyExecutionOrder.tsx index e122e40331..61315cb836 100644 --- 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 = ({ {updatedStrategies.map((strategy, index) => ( - + {`${index + 1}: `} {formatStrategyName(strategy?.name || '')} {strategy?.title && ` - ${strategy.title}`} diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/SegmentChange.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/SegmentChange.tsx index fadd3b334f..ad3bb088b0 100644 --- 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 = ({ onNavigate, actions, changeRequestState, -}) => ( - ({ - marginTop: theme.spacing(2), - marginBottom: theme.spacing(2), - overflow: 'hidden', - })} - > - { + const { segment } = useSegment(segmentChange.payload.id); + + return ( + ({ - 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', })} > - ({ + 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', + })} > - Segment name: - - + - {segmentChange.payload.name} - + Segment name: + + + + {segmentChange.payload.name || segment?.name} + + + - - - -); + + + ); +}; diff --git a/frontend/src/component/changeRequest/ChangeRequest/StrategyTooltipLink/StrategyTooltipLink.tsx b/frontend/src/component/changeRequest/ChangeRequest/StrategyTooltipLink/StrategyTooltipLink.tsx index 9b75425959..5d8584fb5e 100644 --- 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 ( + + ); + } return ( ({ 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)( + ({ 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), diff --git a/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestHeader/ChangeRequestHeader.tsx b/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestHeader/ChangeRequestHeader.tsx index 25c3b19940..9563c1df3e 100644 --- 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} > - + {title} diff --git a/frontend/src/component/common/FeatureArchiveDialog/FeatureArchiveDialog.test.tsx b/frontend/src/component/common/FeatureArchiveDialog/FeatureArchiveDialog.test.tsx index 65e7a755d7..139a63e8ea 100644 --- 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(); }); diff --git a/frontend/src/component/common/FeatureArchiveDialog/FeatureArchiveDialog.tsx b/frontend/src/component/common/FeatureArchiveDialog/FeatureArchiveDialog.tsx index 954febd1db..a9df36b147 100644 --- 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 ( theme.spacing(2, 0) }}> - Archiving features with dependencies will also remove those + Archiving flags with dependencies will also remove those dependencies. ); diff --git a/frontend/src/component/common/NewConstraintAccordion/ConstraintsList/ViewableConstraintsList.tsx b/frontend/src/component/common/NewConstraintAccordion/ConstraintsList/ViewableConstraintsList.tsx index fd2e0d7d13..48cb79ab1e 100644 --- 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 = ({ {constraints.map((constraint) => ( ))} diff --git a/frontend/src/component/events/EventDiff/EventDiff.tsx b/frontend/src/component/events/EventDiff/EventDiff.tsx index f5bc7bf8fd..93968bc36f 100644 --- 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 = ({ entry, excludeKeys }) => { +export const NewEventDiff: FC = ({ 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 = (props) => { return ; }; +/** + * @deprecated remove the default export with flag improvedJsonDiff. Switch imports in files that use this to the named import instead. + */ export default EventDiff;