From ecb85a8a4503640d12906df4f33433ea3dba60ba Mon Sep 17 00:00:00 2001 From: FredrikOseberg Date: Tue, 21 Oct 2025 14:59:35 +0200 Subject: [PATCH] refactor: release plan change --- .../Changes/Change/MilestoneListRenderer.tsx | 97 ++++ .../Changes/Change/ProgressionChange.tsx | 126 +++++ .../Changes/Change/ReleasePlanChange.tsx | 510 +++--------------- .../Changes/Change/useModifiedReleasePlan.ts | 71 +++ .../MilestoneAutomation.tsx | 2 +- 5 files changed, 355 insertions(+), 451 deletions(-) create mode 100644 frontend/src/component/changeRequest/ChangeRequest/Changes/Change/MilestoneListRenderer.tsx create mode 100644 frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ProgressionChange.tsx create mode 100644 frontend/src/component/changeRequest/ChangeRequest/Changes/Change/useModifiedReleasePlan.ts diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/MilestoneListRenderer.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/MilestoneListRenderer.tsx new file mode 100644 index 0000000000..da14293e35 --- /dev/null +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/MilestoneListRenderer.tsx @@ -0,0 +1,97 @@ +import { styled } from '@mui/material'; +import type { IReleasePlan } from 'interfaces/releasePlans'; +import type { UpdateMilestoneProgressionSchema } from 'openapi'; +import type { ChangeRequestState } from 'component/changeRequest/changeRequest.types'; +import { ReleasePlanMilestone } from 'component/feature/FeatureView/FeatureOverview/ReleasePlan/ReleasePlanMilestone/ReleasePlanMilestone'; +import { MilestoneAutomationSection } from 'component/feature/FeatureView/FeatureOverview/ReleasePlan/ReleasePlanMilestone/MilestoneAutomationSection.tsx'; +import { MilestoneTransitionDisplay } from 'component/feature/FeatureView/FeatureOverview/ReleasePlan/ReleasePlanMilestone/MilestoneTransitionDisplay.tsx'; +import type { MilestoneStatus } from 'component/feature/FeatureView/FeatureOverview/ReleasePlan/ReleasePlanMilestone/ReleasePlanMilestoneStatus.tsx'; + +const StyledConnection = styled('div')(({ theme }) => ({ + width: 2, + height: theme.spacing(2), + backgroundColor: theme.palette.divider, + marginLeft: theme.spacing(3.25), +})); + +interface MilestoneListRendererProps { + plan: IReleasePlan; + changeRequestState: ChangeRequestState; + milestonesWithAutomation?: Set; + milestonesWithDeletedAutomation?: Set; + onUpdateAutomation?: ( + sourceMilestoneId: string, + payload: UpdateMilestoneProgressionSchema, + ) => Promise; + onDeleteAutomation?: (sourceMilestoneId: string) => void; +} + +export const MilestoneListRenderer = ({ + plan, + changeRequestState, + milestonesWithAutomation = new Set(), + milestonesWithDeletedAutomation = new Set(), + onUpdateAutomation, + onDeleteAutomation, +}: MilestoneListRendererProps) => { + const readonly = + changeRequestState === 'Applied' || changeRequestState === 'Cancelled'; + const status: MilestoneStatus = 'not-started'; + + return ( + <> + {plan.milestones.map((milestone, index) => { + const isNotLastMilestone = index < plan.milestones.length - 1; + const shouldShowAutomation = + milestonesWithAutomation.has(milestone.id) || + milestonesWithDeletedAutomation.has(milestone.id); + const showAutomation = + isNotLastMilestone && + shouldShowAutomation && + milestone.transitionCondition; + + const hasPendingDelete = milestonesWithDeletedAutomation.has( + milestone.id, + ); + + const automationSection = + showAutomation && milestone.transitionCondition ? ( + + { + await onUpdateAutomation?.( + milestone.id, + payload, + ); + return { shouldReset: true }; + }} + onDelete={() => + onDeleteAutomation?.(milestone.id) + } + milestoneName={milestone.name} + status={status} + hasPendingUpdate={false} + hasPendingDelete={hasPendingDelete} + /> + + ) : undefined; + + return ( +
+ + {isNotLastMilestone && } +
+ ); + })} + + ); +}; diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ProgressionChange.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ProgressionChange.tsx new file mode 100644 index 0000000000..c23aea943d --- /dev/null +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ProgressionChange.tsx @@ -0,0 +1,126 @@ +import type { FC, ReactNode } from 'react'; +import { Typography } from '@mui/material'; +import type { + ChangeRequestState, + IChangeRequestCreateMilestoneProgression, + IChangeRequestUpdateMilestoneProgression, +} from 'component/changeRequest/changeRequest.types'; +import type { IReleasePlan } from 'interfaces/releasePlans'; +import type { UpdateMilestoneProgressionSchema } from 'openapi'; +import { EventDiff } from 'component/events/EventDiff/EventDiff'; +import { Tab, TabList, TabPanel, Tabs } from './ChangeTabComponents.tsx'; +import { + Action, + Added, + ChangeItemInfo, + ChangeItemWrapper, +} from './Change.styles.tsx'; +import { styled } from '@mui/material'; +import { MilestoneListRenderer } from './MilestoneListRenderer.tsx'; +import { useModifiedReleasePlan } from './useModifiedReleasePlan.ts'; + +const StyledTabs = styled(Tabs)(({ theme }) => ({ + display: 'flex', + flexFlow: 'column', + gap: theme.spacing(1), +})); + +interface ProgressionChangeProps { + change: + | IChangeRequestCreateMilestoneProgression + | IChangeRequestUpdateMilestoneProgression; + currentReleasePlan?: IReleasePlan; + actions?: ReactNode; + changeRequestState: ChangeRequestState; + onUpdateChangeRequestSubmit?: ( + sourceMilestoneId: string, + payload: UpdateMilestoneProgressionSchema, + ) => Promise; + onDeleteChangeRequestSubmit?: (sourceMilestoneId: string) => void; +} + +export const ProgressionChange: FC = ({ + change, + currentReleasePlan, + actions, + changeRequestState, + onUpdateChangeRequestSubmit, + onDeleteChangeRequestSubmit, +}) => { + const basePlan = change.payload.snapshot || currentReleasePlan; + if (!basePlan) return null; + + const isCreate = change.action === 'createMilestoneProgression'; + + const sourceId = isCreate + ? change.payload.sourceMilestone + : change.payload.sourceMilestoneId || change.payload.sourceMilestone; + + if (!sourceId) return null; + + const sourceMilestone = basePlan.milestones.find( + (milestone) => milestone.id === sourceId, + ); + const sourceMilestoneName = sourceMilestone?.name || sourceId; + + const targetMilestoneName = isCreate + ? basePlan.milestones.find( + (milestone) => milestone.id === change.payload.targetMilestone, + )?.name || change.payload.targetMilestone + : undefined; + + const modifiedPlan = useModifiedReleasePlan(basePlan, [change]); + + const previousMilestone = sourceMilestone; + const newMilestone = modifiedPlan.milestones.find( + (milestone) => milestone.id === sourceId, + ); + + return ( + + + + {isCreate ? ( + <> + Adding automation to release plan + + {sourceMilestoneName} → {targetMilestoneName} + + + ) : ( + <> + Updating automation in release plan + + {sourceMilestoneName} + + + )} + +
+ + View change + View diff + + {actions} +
+ + + + + + + + + ); +}; diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ReleasePlanChange.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ReleasePlanChange.tsx index 35f7a9221b..d29d8c23d7 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ReleasePlanChange.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ReleasePlanChange.tsx @@ -29,9 +29,9 @@ import { useChangeRequestApi } from 'hooks/api/actions/useChangeRequestApi/useCh import { usePendingChangeRequests } from 'hooks/api/getters/usePendingChangeRequests/usePendingChangeRequests'; import useToast from 'hooks/useToast'; import type { UpdateMilestoneProgressionSchema } from 'openapi'; -import { MilestoneAutomationSection } from 'component/feature/FeatureView/FeatureOverview/ReleasePlan/ReleasePlanMilestone/MilestoneAutomationSection.tsx'; -import { MilestoneTransitionDisplay } from 'component/feature/FeatureView/FeatureOverview/ReleasePlan/ReleasePlanMilestone/MilestoneTransitionDisplay.tsx'; -import type { MilestoneStatus } from 'component/feature/FeatureView/FeatureOverview/ReleasePlan/ReleasePlanMilestone/ReleasePlanMilestoneStatus.tsx'; +import { MilestoneListRenderer } from './MilestoneListRenderer.tsx'; +import { useModifiedReleasePlan } from './useModifiedReleasePlan.ts'; +import { ProgressionChange } from './ProgressionChange.tsx'; const StyledTabs = styled(Tabs)(({ theme }) => ({ display: 'flex', @@ -39,13 +39,6 @@ const StyledTabs = styled(Tabs)(({ theme }) => ({ gap: theme.spacing(1), })); -const StyledConnection = styled('div')(({ theme }) => ({ - width: 2, - height: theme.spacing(2), - backgroundColor: theme.palette.divider, - marginLeft: theme.spacing(3.25), -})); - const DeleteReleasePlan: FC<{ change: IChangeRequestDeleteReleasePlan; currentReleasePlan?: IReleasePlan; @@ -246,289 +239,6 @@ const AddReleasePlan: FC<{ ); }; -const CreateMilestoneProgression: FC<{ - change: IChangeRequestCreateMilestoneProgression; - currentReleasePlan?: IReleasePlan; - actions?: ReactNode; - changeRequestState: ChangeRequestState; - onUpdateChangeRequestSubmit?: ( - sourceMilestoneId: string, - payload: UpdateMilestoneProgressionSchema, - ) => void; - onDeleteChangeRequestSubmit?: (sourceMilestoneId: string) => void; -}> = ({ - change, - currentReleasePlan, - actions, - changeRequestState, - onUpdateChangeRequestSubmit, - onDeleteChangeRequestSubmit, -}) => { - // Use snapshot if available (for Applied state) or if the change has a snapshot - const basePlan = change.payload.snapshot || currentReleasePlan; - if (!basePlan) return null; - - // Create a modified release plan with the progression added - const modifiedPlan: IReleasePlan = { - ...basePlan, - milestones: basePlan.milestones.map((milestone) => { - if (milestone.id === change.payload.sourceMilestone) { - return { - ...milestone, - transitionCondition: change.payload.transitionCondition, - }; - } - return milestone; - }), - }; - - const sourceMilestone = basePlan.milestones.find( - (milestone) => milestone.id === change.payload.sourceMilestone, - ); - - const sourceMilestoneName = - sourceMilestone?.name || change.payload.sourceMilestone; - - const targetMilestoneName = - basePlan.milestones.find( - (milestone) => milestone.id === change.payload.targetMilestone, - )?.name || change.payload.targetMilestone; - - // Get the milestone before and after for diff - const previousMilestone = sourceMilestone; - const newMilestone = modifiedPlan.milestones.find( - (milestone) => milestone.id === change.payload.sourceMilestone, - ); - - return ( - - - - Adding automation to release plan - - {sourceMilestoneName} → {targetMilestoneName} - - -
- - View change - View diff - - {actions} -
- - - {modifiedPlan.milestones.map((milestone, index) => { - const isNotLastMilestone = - index < modifiedPlan.milestones.length - 1; - const isTargetMilestone = - milestone.id === change.payload.sourceMilestone; - const hasProgression = Boolean( - milestone.transitionCondition, - ); - const showAutomation = - isTargetMilestone && - isNotLastMilestone && - hasProgression; - - const readonly = - changeRequestState === 'Applied' || - changeRequestState === 'Cancelled'; - const status: MilestoneStatus = 'not-started'; // In change request view, always not-started - - // Build automation section for this milestone - const automationSection = - showAutomation && milestone.transitionCondition ? ( - - { - onUpdateChangeRequestSubmit?.( - milestone.id, - payload, - ); - return { shouldReset: true }; - }} - onDelete={() => - onDeleteChangeRequestSubmit?.( - milestone.id, - ) - } - milestoneName={milestone.name} - status={status} - hasPendingUpdate={false} - hasPendingDelete={false} - /> - - ) : undefined; - - return ( -
- - {isNotLastMilestone && } -
- ); - })} - - - - - - ); -}; - -const UpdateMilestoneProgression: FC<{ - change: IChangeRequestUpdateMilestoneProgression; - currentReleasePlan?: IReleasePlan; - actions?: ReactNode; - changeRequestState: ChangeRequestState; - onUpdateChangeRequestSubmit?: ( - sourceMilestoneId: string, - payload: UpdateMilestoneProgressionSchema, - ) => void; - onDeleteChangeRequestSubmit?: (sourceMilestoneId: string) => void; -}> = ({ - change, - currentReleasePlan, - actions, - changeRequestState, - onUpdateChangeRequestSubmit, - onDeleteChangeRequestSubmit, -}) => { - // Use snapshot if available (for Applied state) or if the change has a snapshot - const basePlan = change.payload.snapshot || currentReleasePlan; - if (!basePlan) return null; - - const sourceId = - change.payload.sourceMilestoneId || change.payload.sourceMilestone; - const sourceMilestone = basePlan.milestones.find( - (milestone) => milestone.id === sourceId, - ); - const sourceMilestoneName = sourceMilestone?.name || sourceId; - - // Create a modified release plan with the updated progression - const modifiedPlan: IReleasePlan = { - ...basePlan, - milestones: basePlan.milestones.map((milestone) => { - if (milestone.id === sourceId) { - return { - ...milestone, - transitionCondition: change.payload.transitionCondition, - }; - } - return milestone; - }), - }; - - // Get the milestone before and after for diff - const previousMilestone = sourceMilestone; - const newMilestone = modifiedPlan.milestones.find( - (milestone) => milestone.id === change.payload.sourceMilestoneId, - ); - - return ( - - - - Updating automation in release plan - - {sourceMilestoneName} - - -
- - View change - View diff - - {actions} -
- - - {modifiedPlan.milestones.map((milestone, index) => { - const isNotLastMilestone = - index < modifiedPlan.milestones.length - 1; - const showAutomation = - milestone.id === sourceId && - isNotLastMilestone && - Boolean(milestone.transitionCondition); - - const readonly = - changeRequestState === 'Applied' || - changeRequestState === 'Cancelled'; - const status: MilestoneStatus = 'not-started'; - - // Build automation section for this milestone - const automationSection = - showAutomation && milestone.transitionCondition ? ( - - { - onUpdateChangeRequestSubmit?.( - milestone.id, - payload, - ); - return { shouldReset: true }; - }} - onDelete={() => - onDeleteChangeRequestSubmit?.( - milestone.id, - ) - } - milestoneName={milestone.name} - status={status} - hasPendingUpdate={false} - hasPendingDelete={false} - /> - - ) : undefined; - - return ( -
- - {isNotLastMilestone && } -
- ); - })} - - - - - - ); -}; const ConsolidatedProgressionChanges: FC<{ feature: IChangeRequestFeature; @@ -562,7 +272,6 @@ const ConsolidatedProgressionChanges: FC<{ if (progressionChanges.length === 0) return null; // Use snapshot from first change if available, otherwise use current release plan - // Prioritize create/update changes over delete changes for snapshot selection const firstChangeWithSnapshot = progressionChanges.find( (change) => @@ -589,61 +298,36 @@ const ConsolidatedProgressionChanges: FC<{ ); } - // Apply all progression changes to the release plan - const modifiedPlan: IReleasePlan = { - ...basePlan, - milestones: basePlan.milestones.map((milestone) => { - // Find if there's a progression change for this milestone - const createChange = progressionChanges.find( - (change): change is IChangeRequestCreateMilestoneProgression => - change.action === 'createMilestoneProgression' && - change.payload.sourceMilestone === milestone.id, - ); - const updateChange = progressionChanges.find( - (change): change is IChangeRequestUpdateMilestoneProgression => - change.action === 'updateMilestoneProgression' && - (change.payload.sourceMilestoneId === milestone.id || - change.payload.sourceMilestone === milestone.id), - ); - const deleteChange = progressionChanges.find( - (change): change is IChangeRequestDeleteMilestoneProgression => - change.action === 'deleteMilestoneProgression' && - (change.payload.sourceMilestoneId === milestone.id || - change.payload.sourceMilestone === milestone.id), - ); + // Apply all progression changes using our hook + const modifiedPlan = useModifiedReleasePlan(basePlan, progressionChanges); - // Check for conflicting changes (delete + create/update for same milestone) - if (deleteChange && (createChange || updateChange)) { - console.warn( - '[ConsolidatedProgressionChanges] Conflicting changes detected for milestone:', - { - milestone: milestone.name, - hasCreate: !!createChange, - hasUpdate: !!updateChange, - hasDelete: !!deleteChange, - }, - ); - } + // Collect milestone IDs with automation (modified or original) + const milestonesWithAutomation = new Set( + progressionChanges + .filter( + (change) => + change.action === 'createMilestoneProgression' || + change.action === 'updateMilestoneProgression', + ) + .map((change) => + change.action === 'createMilestoneProgression' + ? change.payload.sourceMilestone + : change.payload.sourceMilestoneId || + change.payload.sourceMilestone, + ) + .filter((id): id is string => Boolean(id)), + ); - // If there's a delete change, remove the transition condition - // Delete takes precedence over create/update - if (deleteChange) { - return { - ...milestone, - transitionCondition: null, - }; - } - - const change = updateChange || createChange; - if (change) { - return { - ...milestone, - transitionCondition: change.payload.transitionCondition, - }; - } - return milestone; - }), - }; + const milestonesWithDeletedAutomation = new Set( + progressionChanges + .filter((change) => change.action === 'deleteMilestoneProgression') + .map( + (change) => + change.payload.sourceMilestoneId || + change.payload.sourceMilestone, + ) + .filter((id): id is string => Boolean(id)), + ); const changeDescriptions = progressionChanges.map((change) => { const sourceId = @@ -663,6 +347,18 @@ const ConsolidatedProgressionChanges: FC<{ return `${action} automation for ${sourceName}`; }); + // For diff view, we need to use basePlan with deleted automations shown + const basePlanWithDeletedAutomations: IReleasePlan = { + ...basePlan, + milestones: basePlan.milestones.map((milestone) => { + // If this milestone is being deleted, ensure it has its transition condition + if (milestonesWithDeletedAutomation.has(milestone.id)) { + return milestone; + } + return milestone; + }), + }; + return ( @@ -687,93 +383,20 @@ const ConsolidatedProgressionChanges: FC<{ - {modifiedPlan.milestones.map((milestone, index) => { - const isNotLastMilestone = - index < modifiedPlan.milestones.length - 1; - - // Check if there's a delete change for this milestone - const deleteChange = progressionChanges.find( - ( - change, - ): change is IChangeRequestDeleteMilestoneProgression => - change.action === 'deleteMilestoneProgression' && - (change.payload.sourceMilestoneId === - milestone.id || - change.payload.sourceMilestone === - milestone.id), - ); - - // If there's a delete change, use the original milestone from basePlan - const originalMilestone = deleteChange - ? basePlan.milestones.find( - (baseMilestone) => - baseMilestone.id === milestone.id, - ) - : null; - - const displayMilestone = - deleteChange && originalMilestone - ? originalMilestone - : milestone; - - // Show automation section for any milestone that has a transition condition - // or if there's a delete change (to show what's being deleted) - const shouldShowAutomationSection = - Boolean(displayMilestone.transitionCondition) || - Boolean(deleteChange); - const showAutomation = - isNotLastMilestone && shouldShowAutomationSection; - - const readonly = - changeRequestState === 'Applied' || - changeRequestState === 'Cancelled'; - const status: MilestoneStatus = 'not-started'; - - // Build automation section for this milestone - const automationSection = - showAutomation && - displayMilestone.transitionCondition ? ( - - { - onUpdateChangeRequestSubmit?.( - displayMilestone.id, - payload, - ); - return { shouldReset: true }; - }} - onDelete={() => - onDeleteChangeRequestSubmit?.( - displayMilestone.id, - ) - } - milestoneName={displayMilestone.name} - status={status} - hasPendingUpdate={false} - hasPendingDelete={Boolean(deleteChange)} - /> - - ) : undefined; - - return ( -
- - {isNotLastMilestone && } -
- ); - })} + 0 + ? basePlanWithDeletedAutomations + : modifiedPlan + } + changeRequestState={changeRequestState} + milestonesWithAutomation={milestonesWithAutomation} + milestonesWithDeletedAutomation={ + milestonesWithDeletedAutomation + } + onUpdateAutomation={onUpdateChangeRequestSubmit} + onDeleteAutomation={onDeleteChangeRequestSubmit} + /> )} - {change.action === 'createMilestoneProgression' && ( - - )} - {change.action === 'updateMilestoneProgression' && ( - { + return { + ...basePlan, + milestones: basePlan.milestones.map((milestone) => { + // Find if there's a progression change for this milestone + const createChange = progressionChanges.find( + (change): change is IChangeRequestCreateMilestoneProgression => + change.action === 'createMilestoneProgression' && + change.payload.sourceMilestone === milestone.id, + ); + const updateChange = progressionChanges.find( + (change): change is IChangeRequestUpdateMilestoneProgression => + change.action === 'updateMilestoneProgression' && + (change.payload.sourceMilestoneId === milestone.id || + change.payload.sourceMilestone === milestone.id), + ); + const deleteChange = progressionChanges.find( + (change): change is IChangeRequestDeleteMilestoneProgression => + change.action === 'deleteMilestoneProgression' && + (change.payload.sourceMilestoneId === milestone.id || + change.payload.sourceMilestone === milestone.id), + ); + + // Check for conflicting changes (delete + create/update for same milestone) + if (deleteChange && (createChange || updateChange)) { + console.warn( + '[useModifiedReleasePlan] Conflicting changes detected for milestone:', + { + milestone: milestone.name, + hasCreate: !!createChange, + hasUpdate: !!updateChange, + hasDelete: !!deleteChange, + }, + ); + } + + // If there's a delete change, remove the transition condition + // Delete takes precedence over create/update + if (deleteChange) { + return { + ...milestone, + transitionCondition: null, + }; + } + + const change = updateChange || createChange; + if (change) { + return { + ...milestone, + transitionCondition: change.payload.transitionCondition, + }; + } + return milestone; + }), + }; +}; diff --git a/frontend/src/component/feature/FeatureView/FeatureOverview/ReleasePlan/ReleasePlanMilestoneItem/MilestoneAutomation.tsx b/frontend/src/component/feature/FeatureView/FeatureOverview/ReleasePlan/ReleasePlanMilestoneItem/MilestoneAutomation.tsx index c46c1e97ce..bda5e4b0a0 100644 --- a/frontend/src/component/feature/FeatureView/FeatureOverview/ReleasePlan/ReleasePlanMilestoneItem/MilestoneAutomation.tsx +++ b/frontend/src/component/feature/FeatureView/FeatureOverview/ReleasePlan/ReleasePlanMilestoneItem/MilestoneAutomation.tsx @@ -10,7 +10,7 @@ import { MilestoneAutomationSection } from '../ReleasePlanMilestone/MilestoneAut import { MilestoneTransitionDisplay } from '../ReleasePlanMilestone/MilestoneTransitionDisplay.tsx'; import type { MilestoneStatus } from '../ReleasePlanMilestone/ReleasePlanMilestoneStatus.tsx'; import { MilestoneProgressionForm } from '../MilestoneProgressionForm/MilestoneProgressionForm.tsx'; -import type { PendingProgressionChange } from './ReleasePlanMilestoneItem'; +import type { PendingProgressionChange } from './ReleasePlanMilestoneItem.tsx'; const StyledAddAutomationButton = styled(Button)(({ theme }) => ({ textTransform: 'none',