From 2e086161eb1e3ae3b59103aa2e4afe18c8ce6ae1 Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Wed, 5 Mar 2025 10:34:55 +0100 Subject: [PATCH] refactor: strategy draggable item is now proj/env agnostic (#9411) Updates `StrategyDraggableItem` (and `StrategyItem`) to be project/env agnostic. They now instead expect you to pass in the required header items (CR badges, strategy actions) at the call site. Updates their usage in the feature env accordion, and the release plan card. All components that have been updated are part of the new overview rework. The legacy components (which are used when the flag is off) remain untouched. Also makes a few small tweaks explained in inline comments. ## Rendered Milestone card (with flag on): ![image](https://github.com/user-attachments/assets/828d5fe4-4b07-4ebe-86cd-1ab24608ba31) Milestone card (with flag off): ![image](https://github.com/user-attachments/assets/10e37cc4-e5e4-4a07-a4f9-5e5f5c388915) Feature env accordion (flag on (no change)): ![image](https://github.com/user-attachments/assets/2e5db9e7-24b1-4b3e-9434-4705e5737157) Feature env accordion (flag off): ![image](https://github.com/user-attachments/assets/469970b6-ab57-4332-a99f-8f8e2e645230) --- .../ChangeRequestDraftStatusBadge.tsx | 28 ++++ .../StrategyItemContainer.tsx | 25 +--- .../EnvironmentAccordionBody.tsx | 6 +- ...rojectEnvironmentStrategyDraggableItem.tsx | 137 ++++++++++++++++++ .../StrategyDraggableItem.test.tsx | 4 +- .../StrategyDraggableItem.tsx | 120 ++------------- .../StrategyItem/LegacyStrategyItem.tsx | 78 ---------- .../StrategyItem/StrategyItem.tsx | 77 +--------- .../FeatureOverviewEnvironmentBody.tsx | 6 +- .../StrategyItem/FeatureStrategyItem.tsx | 2 +- .../ProjectEnvironmentDefaultStrategy.tsx | 2 +- .../MilestoneCard/MilestoneCard.tsx | 46 ++++-- 12 files changed, 241 insertions(+), 290 deletions(-) create mode 100644 frontend/src/component/changeRequest/ChangeRequestStatusBadge/ChangeRequestDraftStatusBadge.tsx create mode 100644 frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/ProjectEnvironmentStrategyDraggableItem.tsx diff --git a/frontend/src/component/changeRequest/ChangeRequestStatusBadge/ChangeRequestDraftStatusBadge.tsx b/frontend/src/component/changeRequest/ChangeRequestStatusBadge/ChangeRequestDraftStatusBadge.tsx new file mode 100644 index 0000000000..e8507bee80 --- /dev/null +++ b/frontend/src/component/changeRequest/ChangeRequestStatusBadge/ChangeRequestDraftStatusBadge.tsx @@ -0,0 +1,28 @@ +import { Badge } from 'component/common/Badge/Badge'; +import type { IFeatureChange } from '../changeRequest.types'; +import type { SxProps } from '@mui/material'; + +export const ChangeRequestDraftStatusBadge = ({ + changeAction, + sx, +}: { + changeAction: IFeatureChange['action']; + sx?: SxProps; +}) => { + switch (changeAction) { + case 'updateStrategy': + return ( + + Modified in draft + + ); + case 'deleteStrategy': + return ( + + Deleted in draft + + ); + default: + return null; + } +}; diff --git a/frontend/src/component/common/StrategyItemContainer/StrategyItemContainer.tsx b/frontend/src/component/common/StrategyItemContainer/StrategyItemContainer.tsx index 79bdc5d04d..1f5bf69134 100644 --- a/frontend/src/component/common/StrategyItemContainer/StrategyItemContainer.tsx +++ b/frontend/src/component/common/StrategyItemContainer/StrategyItemContainer.tsx @@ -3,27 +3,23 @@ import type { DragEventHandler, FC, ReactNode } from 'react'; import DragIndicator from '@mui/icons-material/DragIndicator'; import { Box, IconButton, styled } from '@mui/material'; import type { IFeatureStrategy } from 'interfaces/strategy'; -import { - formatStrategyName, - getFeatureStrategyIcon, -} from 'utils/strategyNames'; +import { formatStrategyName } from 'utils/strategyNames'; import StringTruncator from 'component/common/StringTruncator/StringTruncator'; import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; import type { PlaygroundStrategySchema } from 'openapi'; import { Badge } from '../Badge/Badge'; import { Link } from 'react-router-dom'; -interface IStrategyItemContainerProps { +type StrategyItemContainerProps = { strategy: IFeatureStrategy | PlaygroundStrategySchema; onDragStart?: DragEventHandler; onDragEnd?: DragEventHandler; - actions?: ReactNode; - orderNumber?: number; + headerItemsRight?: ReactNode; className?: string; style?: React.CSSProperties; description?: string; children?: React.ReactNode; -} +}; const DragIcon = styled(IconButton)({ padding: 0, @@ -76,17 +72,15 @@ const NewStyledHeader = styled('div', { }), ); -export const StrategyItemContainer: FC = ({ +export const StrategyItemContainer: FC = ({ strategy, onDragStart, onDragEnd, - actions, + headerItemsRight, children, style = {}, description, }) => { - const Icon = getFeatureStrategyIcon(strategy.name); - const StrategyHeaderLink: React.FC<{ children?: React.ReactNode }> = 'links' in strategy ? ({ children }) => {children} @@ -118,11 +112,6 @@ export const StrategyItemContainer: FC = ({ )} /> - theme.palette.action.disabled, - }} - /> = ({ alignItems: 'center', }} > - {actions} + {headerItemsRight} {children} diff --git a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/EnvironmentAccordionBody.tsx b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/EnvironmentAccordionBody.tsx index f712504cb9..db4665ad96 100644 --- a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/EnvironmentAccordionBody.tsx +++ b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/EnvironmentAccordionBody.tsx @@ -20,9 +20,9 @@ import type { IFeatureStrategy } from 'interfaces/strategy'; import { usePlausibleTracker } from 'hooks/usePlausibleTracker'; import { useUiFlag } from 'hooks/useUiFlag'; import { useReleasePlans } from 'hooks/api/getters/useReleasePlans/useReleasePlans'; -import { StrategyDraggableItem } from './StrategyDraggableItem/StrategyDraggableItem'; import { ReleasePlan } from '../../../ReleasePlan/ReleasePlan'; import { StrategySeparator } from 'component/common/StrategySeparator/StrategySeparator'; +import { ProjectEnvironmentStrategyDraggableItem } from './StrategyDraggableItem/ProjectEnvironmentStrategyDraggableItem'; interface IEnvironmentAccordionBodyProps { isDisabled: boolean; @@ -252,7 +252,7 @@ export const EnvironmentAccordionBody = ({ ) : null} - ) : null} - , + index: number, + ) => DragEventHandler; + onDragOver?: ( + ref: RefObject, + index: number, + ) => DragEventHandler; + onDragEnd?: () => void; +}; + +const onDragNoOp = () => () => {}; + +export const ProjectEnvironmentStrategyDraggableItem = ({ + strategy, + index, + environmentName, + otherEnvironments, + isDragging, + onDragStartRef = onDragNoOp, + onDragOver = onDragNoOp, + onDragEnd = onDragNoOp, +}: ProjectEnvironmentStrategyDraggableItemProps) => { + const projectId = useRequiredPathParam('projectId'); + const featureId = useRequiredPathParam('featureId'); + const ref = useRef(null); + const strategyChangesFromRequest = useStrategyChangesFromRequest( + projectId, + featureId, + environmentName, + strategy.id, + ); + + const { changeRequests: scheduledChanges } = + useScheduledChangeRequestsWithStrategy(projectId, strategy.id); + + const editStrategyPath = formatEditStrategyPath( + projectId, + featureId, + environmentName, + strategy.id, + ); + + const draftChange = strategyChangesFromRequest?.find( + ({ isScheduledChange }) => !isScheduledChange, + ); + + const theme = useTheme(); + const isSmallScreen = useMediaQuery(theme.breakpoints.down('sm')); + + return ( + + + {draftChange && !isSmallScreen ? ( + + ) : null} + + {scheduledChanges && + scheduledChanges.length > 0 && + !isSmallScreen ? ( + scheduledChange.id)} + /> + ) : null} + {otherEnvironments && otherEnvironments?.length > 0 ? ( + + ) : null} + + + + + + } + /> + + ); +}; diff --git a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyDraggableItem.test.tsx b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyDraggableItem.test.tsx index e69c17ec23..725ca7f3ef 100644 --- a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyDraggableItem.test.tsx +++ b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyDraggableItem.test.tsx @@ -1,6 +1,5 @@ import { testServerRoute, testServerSetup } from 'utils/testServer'; import { render } from 'utils/testRenderer'; -import { StrategyDraggableItem } from './StrategyDraggableItem'; import { vi } from 'vitest'; import { ADMIN } from 'component/providers/AccessProvider/permissions'; import { screen } from '@testing-library/react'; @@ -9,6 +8,7 @@ import type { ChangeRequestType, ChangeRequestAction, } from 'component/changeRequest/changeRequest.types'; +import { ProjectEnvironmentStrategyDraggableItem } from './ProjectEnvironmentStrategyDraggableItem'; const server = testServerSetup(); @@ -211,7 +211,7 @@ const Component = () => { () => {}; + +type StrategyDraggableItemProps = { + headerItemsRight: ReactNode; strategy: IFeatureStrategy; - environmentName: string; index: number; - otherEnvironments?: IFeatureEnvironment['name'][]; isDragging?: boolean; onDragStartRef?: ( ref: RefObject, @@ -32,32 +24,18 @@ interface IStrategyDraggableItemProps { index: number, ) => DragEventHandler; onDragEnd?: () => void; -} - -const onDragNoOp = () => () => {}; +}; export const StrategyDraggableItem = ({ strategy, index, - environmentName, - otherEnvironments, isDragging, onDragStartRef = onDragNoOp, onDragOver = onDragNoOp, onDragEnd = onDragNoOp, -}: IStrategyDraggableItemProps) => { - const projectId = useRequiredPathParam('projectId'); - const featureId = useRequiredPathParam('featureId'); + headerItemsRight, +}: StrategyDraggableItemProps) => { const ref = useRef(null); - const strategyChangesFromRequest = useStrategyChangesFromRequest( - projectId, - featureId, - environmentName, - strategy.id, - ); - - const { changeRequests: scheduledChangesUsingStrategy } = - useScheduledChangeRequestsWithStrategy(projectId, strategy.id); return ( ); }; - -const ChangeRequestStatusBadge = ({ - change, -}: { - change: IFeatureChange | undefined; -}) => { - const theme = useTheme(); - const isSmallScreen = useMediaQuery(theme.breakpoints.down('sm')); - - if (isSmallScreen) { - return null; - } - - return ( - - Modified in draft} - /> - Deleted in draft} - /> - - ); -}; - -const renderHeaderChildren = ( - changes?: UseStrategyChangeFromRequestResult, - scheduledChanges?: ScheduledChangeRequestViewModel[], -): JSX.Element[] => { - const badges: JSX.Element[] = []; - if (changes?.length === 0 && scheduledChanges?.length === 0) { - return []; - } - - const draftChange = changes?.find( - ({ isScheduledChange }) => !isScheduledChange, - ); - - if (draftChange) { - badges.push( - , - ); - } - - if (scheduledChanges && scheduledChanges.length > 0) { - badges.push( - scheduledChange.id, - )} - />, - ); - } - - return badges; -}; diff --git a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/LegacyStrategyItem.tsx b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/LegacyStrategyItem.tsx index b0b3efbe05..cae14a3ee8 100644 --- a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/LegacyStrategyItem.tsx +++ b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/LegacyStrategyItem.tsx @@ -15,7 +15,6 @@ import { StrategyItemContainer } from 'component/common/StrategyItemContainer/Le import MenuStrategyRemove from './MenuStrategyRemove/MenuStrategyRemove'; import SplitPreviewSlider from 'component/feature/StrategyTypes/SplitPreviewSlider/SplitPreviewSlider'; import { Box } from '@mui/material'; -import { StrategyItemContainer as NewStrategyItemContainer } from 'component/common/StrategyItemContainer/StrategyItemContainer'; interface IStrategyItemProps { environmentId: string; strategy: IFeatureStrategy; @@ -102,80 +101,3 @@ export const StrategyItem: FC = ({ ); }; - -export const NewStrategyItem: FC = ({ - environmentId, - strategy, - onDragStart, - onDragEnd, - otherEnvironments, - orderNumber, - headerChildren, -}) => { - const projectId = useRequiredPathParam('projectId'); - const featureId = useRequiredPathParam('featureId'); - - const editStrategyPath = formatEditStrategyPath( - projectId, - featureId, - environmentId, - strategy.id, - ); - - return ( - - {headerChildren} - 0, - )} - show={() => ( - - )} - /> - - - - - - } - > - - - {strategy.variants && - strategy.variants.length > 0 && - (strategy.disabled ? ( - - - - ) : ( - - ))} - - ); -}; diff --git a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyItem.tsx b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyItem.tsx index af927059df..123c984dd8 100644 --- a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyItem.tsx +++ b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyItem.tsx @@ -1,90 +1,29 @@ -import type { DragEventHandler, FC } from 'react'; -import Edit from '@mui/icons-material/Edit'; -import { Link } from 'react-router-dom'; -import type { IFeatureEnvironment } from 'interfaces/featureToggle'; +import type { DragEventHandler, FC, ReactNode } from 'react'; import type { IFeatureStrategy } from 'interfaces/strategy'; -import PermissionIconButton from 'component/common/PermissionIconButton/PermissionIconButton'; -import { UPDATE_FEATURE_STRATEGY } from 'component/providers/AccessProvider/permissions'; -import { formatEditStrategyPath } from 'component/feature/FeatureStrategy/FeatureStrategyEdit/FeatureStrategyEdit'; -import { useRequiredPathParam } from 'hooks/useRequiredPathParam'; import { StrategyExecution } from './StrategyExecution/StrategyExecution'; -import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; -import { CopyStrategyIconMenu } from './CopyStrategyIconMenu/CopyStrategyIconMenu'; -import MenuStrategyRemove from './MenuStrategyRemove/MenuStrategyRemove'; import SplitPreviewSlider from 'component/feature/StrategyTypes/SplitPreviewSlider/SplitPreviewSlider'; import { Box } from '@mui/material'; import { StrategyItemContainer as NewStrategyItemContainer } from 'component/common/StrategyItemContainer/StrategyItemContainer'; -interface IStrategyItemProps { - environmentId: string; + +type StrategyItemProps = { + headerItemsRight: ReactNode; strategy: IFeatureStrategy; onDragStart?: DragEventHandler; onDragEnd?: DragEventHandler; - otherEnvironments?: IFeatureEnvironment['name'][]; - orderNumber?: number; - headerChildren?: JSX.Element[] | JSX.Element; -} +}; -export const StrategyItem: FC = ({ - environmentId, +export const StrategyItem: FC = ({ strategy, onDragStart, onDragEnd, - otherEnvironments, - orderNumber, - headerChildren, + headerItemsRight, }) => { - const projectId = useRequiredPathParam('projectId'); - const featureId = useRequiredPathParam('featureId'); - - const editStrategyPath = formatEditStrategyPath( - projectId, - featureId, - environmentId, - strategy.id, - ); - return ( - {headerChildren} - 0, - )} - show={() => ( - - )} - /> - - - - - - } + headerItemsRight={headerItemsRight} > diff --git a/frontend/src/component/feature/FeatureView/FeatureOverview/NewFeatureOverviewEnvironment/FeatureOverviewEnvironmentBody.tsx b/frontend/src/component/feature/FeatureView/FeatureOverview/NewFeatureOverviewEnvironment/FeatureOverviewEnvironmentBody.tsx index 93e4422260..8d296686f3 100644 --- a/frontend/src/component/feature/FeatureView/FeatureOverview/NewFeatureOverviewEnvironment/FeatureOverviewEnvironmentBody.tsx +++ b/frontend/src/component/feature/FeatureView/FeatureOverview/NewFeatureOverviewEnvironment/FeatureOverviewEnvironmentBody.tsx @@ -8,7 +8,6 @@ import { Alert, Pagination, styled } from '@mui/material'; import useFeatureStrategyApi from 'hooks/api/actions/useFeatureStrategyApi/useFeatureStrategyApi'; import { formatUnknownError } from 'utils/formatUnknownError'; import useToast from 'hooks/useToast'; -import { StrategyDraggableItem } from '../FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyDraggableItem'; import type { IFeatureEnvironment } from 'interfaces/featureToggle'; import { useRequiredPathParam } from 'hooks/useRequiredPathParam'; import { useFeature } from 'hooks/api/getters/useFeature/useFeature'; @@ -24,6 +23,7 @@ import { useReleasePlans } from 'hooks/api/getters/useReleasePlans/useReleasePla import { ReleasePlan } from '../ReleasePlan/ReleasePlan'; import { SectionSeparator } from '../FeatureOverviewEnvironments/FeatureOverviewEnvironment/SectionSeparator/SectionSeparator'; import { Badge } from 'component/common/Badge/Badge'; +import { ProjectEnvironmentStrategyDraggableItem } from '../FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/ProjectEnvironmentStrategyDraggableItem'; interface IEnvironmentAccordionBodyProps { isDisabled: boolean; @@ -256,7 +256,7 @@ export const FeatureOverviewEnvironmentBody = ({ !manyStrategiesPagination ? ( <> {strategiesToDisplay.map((strategy, index) => ( -
{page.map((strategy, index) => ( - {index > 0 ? : null} - - milestoneStrategyDeleted(strg.id) - } - onEditClick={() => { - openAddUpdateStrategyForm(strg, true); - }} isDragging={dragItem?.id === strg.id} - strategy={strg} + strategy={{ + ...strg, + name: + strg.name || + strg.strategyName || + '', + }} + headerItemsRight={ + <> + { + openAddUpdateStrategyForm( + strg, + true, + ); + }} + > + + + + milestoneStrategyDeleted( + strg.id, + ) + } + > + + + + } /> ))}