From dba1c90db8c5792a5b74524b914256364a746d07 Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Tue, 28 Nov 2023 10:01:56 +0100 Subject: [PATCH] Feat: show change request data on segment project usage page (#5410) Show usage in change requests if that'd cause you to not be able to move the segment into a project. - [x] ~Relies on changes from #5407 (and #5405, #5406) to go through first.~ ![image](https://github.com/Unleash/unleash/assets/17786332/e6b84664-db86-457e-885f-a86c95bc46ec) --- .../component/segments/SegmentFormStepOne.tsx | 25 ++++- .../segments/SegmentProjectAlert.test.tsx | 60 ++++++++++++ .../segments/SegmentProjectAlert.tsx | 96 +++++++++++++++---- 3 files changed, 157 insertions(+), 24 deletions(-) create mode 100644 frontend/src/component/segments/SegmentProjectAlert.test.tsx diff --git a/frontend/src/component/segments/SegmentFormStepOne.tsx b/frontend/src/component/segments/SegmentFormStepOne.tsx index 04070098ca..d05474a2bf 100644 --- a/frontend/src/component/segments/SegmentFormStepOne.tsx +++ b/frontend/src/component/segments/SegmentFormStepOne.tsx @@ -12,8 +12,14 @@ import { ConditionallyRender } from 'component/common/ConditionallyRender/Condit import useProjects from 'hooks/api/getters/useProjects/useProjects'; import { useOptionalPathParam } from 'hooks/useOptionalPathParam'; import { GO_BACK } from 'constants/navigate'; -import { useStrategiesBySegment } from 'hooks/api/getters/useStrategiesBySegment/useStrategiesBySegment'; +import { + ChangeRequestNewStrategy, + ChangeRequestUpdatedStrategy, + useStrategiesBySegment, +} from 'hooks/api/getters/useStrategiesBySegment/useStrategiesBySegment'; import { SegmentProjectAlert } from './SegmentProjectAlert'; +import { sortStrategiesByFeature } from './SegmentDelete/SegmentDeleteUsedSegment/sort-strategies'; +import { IFeatureStrategy } from 'interfaces/strategy'; interface ISegmentFormPartOneProps { name: string; @@ -71,11 +77,20 @@ export const SegmentFormStepOne: React.FC = ({ const navigate = useNavigate(); const { projects, loading: loadingProjects } = useProjects(); - const { strategies, loading: loadingStrategies } = - useStrategiesBySegment(segmentId); + const { + strategies, + changeRequestStrategies, + loading: loadingStrategies, + } = useStrategiesBySegment(segmentId); + + const collectedStrategies = sortStrategiesByFeature< + IFeatureStrategy, + ChangeRequestUpdatedStrategy, + ChangeRequestNewStrategy + >(strategies ?? [], changeRequestStrategies ?? []); const projectsUsed = new Set( - strategies.map(({ projectId }) => projectId!).filter(Boolean), + collectedStrategies.map(({ projectId }) => projectId!).filter(Boolean), ); const availableProjects = projects.filter( ({ id }) => @@ -142,7 +157,7 @@ export const SegmentFormStepOne: React.FC = ({ /> diff --git a/frontend/src/component/segments/SegmentProjectAlert.test.tsx b/frontend/src/component/segments/SegmentProjectAlert.test.tsx new file mode 100644 index 0000000000..1dc37f9e3c --- /dev/null +++ b/frontend/src/component/segments/SegmentProjectAlert.test.tsx @@ -0,0 +1,60 @@ +import React from 'react'; +import { render } from 'utils/testRenderer'; +import { screen } from '@testing-library/react'; +import { SegmentProjectAlert } from './SegmentProjectAlert'; + +describe('SegmentDeleteUsedSegment', () => { + it('should link to change requests for change request strategies', async () => { + const projectId = 'project1'; + + const strategies = [ + { + 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' }, + }, + ]; + + const projectsUsed = [...new Set(strategies.map((s) => s.projectId))]; + + render( + , + ); + + const links = await screen.findAllByRole('link'); + expect(links).toHaveLength(strategies.length + projectsUsed.length); + + const [projectLink, crLink1, crLink2] = links; + + expect(projectLink).toHaveTextContent(projectId); + expect(projectLink).toHaveAttribute('href', `/projects/${projectId}`); + + expect(crLink1).toHaveTextContent('#1'); + expect(crLink1).toHaveAccessibleDescription('Change request 1'); + expect(crLink1).toHaveAttribute( + 'href', + `/projects/${projectId}/change-requests/1`, + ); + + expect(crLink2).toHaveTextContent('#2 (My cool CR)'); + expect(crLink2).toHaveAccessibleDescription('Change request 2'); + expect(crLink2).toHaveAttribute( + 'href', + `/projects/${projectId}/change-requests/2`, + ); + }); +}); diff --git a/frontend/src/component/segments/SegmentProjectAlert.tsx b/frontend/src/component/segments/SegmentProjectAlert.tsx index 6d3eb35dcd..92f5ac7afb 100644 --- a/frontend/src/component/segments/SegmentProjectAlert.tsx +++ b/frontend/src/component/segments/SegmentProjectAlert.tsx @@ -6,6 +6,11 @@ import { Link } from 'react-router-dom'; import { formatStrategyName } from 'utils/strategyNames'; import { usePlausibleTracker } from 'hooks/usePlausibleTracker'; import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; +import { + ChangeRequestNewStrategy, + ChangeRequestStrategy, + ChangeRequestUpdatedStrategy, +} from 'hooks/api/getters/useStrategiesBySegment/useStrategiesBySegment'; const StyledUl = styled('ul')(({ theme }) => ({ listStyle: 'none', @@ -18,7 +23,11 @@ const StyledAlert = styled(Alert)(({ theme }) => ({ interface ISegmentProjectAlertProps { projects: IProjectCard[]; - strategies: IFeatureStrategy[]; + strategies: ( + | IFeatureStrategy + | ChangeRequestUpdatedStrategy + | ChangeRequestNewStrategy + )[]; projectsUsed: string[]; availableProjects: IProjectCard[]; } @@ -56,23 +65,9 @@ export const SegmentProjectAlert = ({ ?.filter( (strategy) => strategy.projectId === projectId, ) - .map((strategy) => ( -
  • - - {strategy.featureName!}{' '} - {formatStrategyNameParens(strategy)} - -
  • - ))} + .map((strategy, index) => + strategyListItem(strategy, index), + )} ))} @@ -100,10 +95,73 @@ export const SegmentProjectAlert = ({ return null; }; -const formatStrategyNameParens = (strategy: IFeatureStrategy): string => { +const formatStrategyNameParens = (strategy: { + strategyName?: string; +}): string => { if (!strategy.strategyName) { return ''; } return `(${formatStrategyName(strategy.strategyName)})`; }; + +export const formatChangeRequestPath = ( + projectId: string, + changeRequestId: number, +): string => { + return `/projects/${projectId}/change-requests/${changeRequestId}`; +}; + +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)} + +
  • + ); + } +};