From b021e7cf851bfc05b5b73b7a16d768bf090f16bd Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Mon, 27 Nov 2023 11:34:34 +0100 Subject: [PATCH] feat: show strategies used by segments (#5407) This PR displays change request usage of segments when such usage is returned from the API. It expects at least #5406 to have been merged before it can be merged. ![image](https://github.com/Unleash/unleash/assets/17786332/c74bb1c9-07f9-4bca-95bb-4ca020398444) --- .../segments/SegmentDelete/SegmentDelete.tsx | 8 +- .../SegmentDeleteUsedSegment.test.tsx | 63 ++++++++++++ .../SegmentDeleteUsedSegment.tsx | 99 +++++++++++++++---- .../useStrategiesBySegment.ts | 19 ++++ 4 files changed, 168 insertions(+), 21 deletions(-) create mode 100644 frontend/src/component/segments/SegmentDelete/SegmentDeleteUsedSegment/SegmentDeleteUsedSegment.test.tsx diff --git a/frontend/src/component/segments/SegmentDelete/SegmentDelete.tsx b/frontend/src/component/segments/SegmentDelete/SegmentDelete.tsx index 64d804bbfa3e..e3944c57fede 100644 --- a/frontend/src/component/segments/SegmentDelete/SegmentDelete.tsx +++ b/frontend/src/component/segments/SegmentDelete/SegmentDelete.tsx @@ -18,9 +18,10 @@ export const SegmentDelete = ({ onClose, onRemove, }: ISegmentDeleteProps) => { - const { strategies, loading } = useStrategiesBySegment(segment.id); - const canDeleteSegment = strategies?.length === 0; - + const { strategies, changeRequestStrategies, loading } = + useStrategiesBySegment(segment.id); + const canDeleteSegment = + strategies?.length === 0 && changeRequestStrategies?.length === 0; if (loading) { return null; } @@ -42,6 +43,7 @@ export const SegmentDelete = ({ open={open} onClose={onClose} strategies={strategies} + changeRequestStrategies={changeRequestStrategies} /> } /> diff --git a/frontend/src/component/segments/SegmentDelete/SegmentDeleteUsedSegment/SegmentDeleteUsedSegment.test.tsx b/frontend/src/component/segments/SegmentDelete/SegmentDeleteUsedSegment/SegmentDeleteUsedSegment.test.tsx new file mode 100644 index 000000000000..c146d6eda601 --- /dev/null +++ b/frontend/src/component/segments/SegmentDelete/SegmentDeleteUsedSegment/SegmentDeleteUsedSegment.test.tsx @@ -0,0 +1,63 @@ +import React from 'react'; +import { render } from 'utils/testRenderer'; +import { screen } from '@testing-library/react'; +import { SegmentDeleteUsedSegment } from './SegmentDeleteUsedSegment'; + +describe('SegmentDeleteUsedSegment', () => { + it('should link to change requests for change request strategies', async () => { + const projectId = 'project1'; + + const crStrategies = [ + { + projectId, + featureName: 'feature1', + strategyName: 'flexible rollout', + environment: 'default', + changeRequest: { id: 1, title: null }, + }, + { + projectId, + featureName: 'feature1', + strategyName: 'flexible rollout', + environment: 'default', + changeRequest: { id: 2, title: 'My cool CR' }, + }, + ]; + + render( + {}} + />, + ); + + const links = await screen.findAllByRole('link'); + expect(links).toHaveLength(crStrategies.length); + + const [link1, link2] = links; + + expect(link1).toHaveTextContent('#1'); + expect(link1).toHaveAccessibleDescription('Change request 1'); + expect(link1).toHaveAttribute( + 'href', + `/projects/${projectId}/change-requests/1`, + ); + expect(link2).toHaveTextContent('#2 (My cool CR)'); + expect(link2).toHaveAccessibleDescription('Change request 2'); + expect(link2).toHaveAttribute( + 'href', + `/projects/${projectId}/change-requests/2`, + ); + }); +}); diff --git a/frontend/src/component/segments/SegmentDelete/SegmentDeleteUsedSegment/SegmentDeleteUsedSegment.tsx b/frontend/src/component/segments/SegmentDelete/SegmentDeleteUsedSegment/SegmentDeleteUsedSegment.tsx index 580bba9193f7..67ea0ce33bbf 100644 --- a/frontend/src/component/segments/SegmentDelete/SegmentDeleteUsedSegment/SegmentDeleteUsedSegment.tsx +++ b/frontend/src/component/segments/SegmentDelete/SegmentDeleteUsedSegment/SegmentDeleteUsedSegment.tsx @@ -5,6 +5,12 @@ import { Link } from 'react-router-dom'; import { formatEditStrategyPath } from 'component/feature/FeatureStrategy/FeatureStrategyEdit/FeatureStrategyEdit'; import { formatStrategyName } from 'utils/strategyNames'; import { styled } from '@mui/material'; +import { + ChangeRequestNewStrategy, + ChangeRequestStrategy, + ChangeRequestUpdatedStrategy, +} from 'hooks/api/getters/useStrategiesBySegment/useStrategiesBySegment'; +import { sortStrategiesByFeature } from './sort-strategies'; const StyledUl = styled('ul')({ marginBottom: 0, @@ -21,14 +27,29 @@ interface ISegmentDeleteUsedSegmentProps { open: boolean; onClose: () => void; strategies: IFeatureStrategy[] | undefined; + changeRequestStrategies: ChangeRequestStrategy[] | undefined; } +export const formatChangeRequestPath = ( + projectId: string, + changeRequestId: number, +): string => { + return `/projects/${projectId}/change-requests/${changeRequestId}`; +}; + export const SegmentDeleteUsedSegment = ({ segment, open, onClose, strategies, + changeRequestStrategies, }: ISegmentDeleteUsedSegmentProps) => { + const sortedStrategies = sortStrategiesByFeature< + IFeatureStrategy, + ChangeRequestUpdatedStrategy, + ChangeRequestNewStrategy + >(strategies ?? [], changeRequestStrategies ?? []); + return ( {segment.name} segment for their strategies:

- {strategies?.map((strategy) => ( -
  • - - {strategy.featureName!}{' '} - {formatStrategyNameParens(strategy)} - -
    • - ))} + {sortedStrategies.map((strategy, index) => + strategyListItem(strategy, index), + )}     ); }; -const formatStrategyNameParens = (strategy: IFeatureStrategy): string => { +const formatStrategyNameParens = (strategy: { + strategyName?: string; +}): string => { if (!strategy.strategyName) { return ''; } return `(${formatStrategyName(strategy.strategyName)})`; }; + +const strategyListItem = ( + strategy: + | IFeatureStrategy + | ChangeRequestUpdatedStrategy + | ChangeRequestNewStrategy, + index: number, +) => { + const isChangeRequest = ( + strategy: IFeatureStrategy | ChangeRequestStrategy, + ): strategy is ChangeRequestStrategy => 'changeRequest' in strategy; + + if (isChangeRequest(strategy)) { + const { id, title } = strategy.changeRequest; + + const text = title ? `#${id} (${title})` : `#${id}`; + return ( +
  • +

    + {strategy.featureName}{' '} + {`${formatStrategyNameParens( + strategy, + )} — in change request `} + + + {text} + +

    +
    • + ); + } else { + return ( +
  • + + {strategy.featureName!} {formatStrategyNameParens(strategy)} + +
    • + ); + } +}; diff --git a/frontend/src/hooks/api/getters/useStrategiesBySegment/useStrategiesBySegment.ts b/frontend/src/hooks/api/getters/useStrategiesBySegment/useStrategiesBySegment.ts index 8ffe12dbfa28..bfb9fa92aa8a 100644 --- a/frontend/src/hooks/api/getters/useStrategiesBySegment/useStrategiesBySegment.ts +++ b/frontend/src/hooks/api/getters/useStrategiesBySegment/useStrategiesBySegment.ts @@ -5,8 +5,26 @@ import handleErrorResponses from '../httpErrorResponseHandler'; import { IFeatureStrategy } from 'interfaces/strategy'; import { useConditionalSWR } from '../useConditionalSWR/useConditionalSWR'; +export type ChangeRequestInfo = { id: number; title: string | null }; +export type ChangeRequestNewStrategy = { + projectId: string; + featureName: string; + strategyName: string; + environment: string; + changeRequest: ChangeRequestInfo; +}; + +export type ChangeRequestUpdatedStrategy = ChangeRequestNewStrategy & { + id: string; +}; + +export type ChangeRequestStrategy = + | ChangeRequestNewStrategy + | ChangeRequestUpdatedStrategy; + export interface IUseStrategiesBySegmentOutput { strategies: IFeatureStrategy[]; + changeRequestStrategies: ChangeRequestStrategy[]; refetchUsedSegments: () => void; loading: boolean; error?: Error; @@ -26,6 +44,7 @@ export const useStrategiesBySegment = ( return { strategies: data?.strategies || [], + changeRequestStrategies: data?.changeRequestStrategies || [], refetchUsedSegments, loading: !error && !data, error,