feat: reorder step across approval branches

pregnantboy · pregnantboy · commit 09e543320df9 · 2025-11-30T16:56:25.000+08:00
diff --git a/packages/backend/src/graphql/mutations/update-step-positions.ts b/packages/backend/src/graphql/mutations/update-step-positions.ts
@@ -1,6 +1,9 @@
 import { IStep } from '@plumber/types'
 
+import { PartialModelObject, raw } from 'objection'
+
 import { BadUserInputError } from '@/errors/graphql-errors'
+import logger from '@/helpers/logger'
 import Step from '@/models/step'
 
 import type { MutationResolvers } from '../__generated__/types.generated'
@@ -76,9 +79,17 @@ const updateStepPositions: MutationResolvers['updateStepPositions'] = async (
 
     // Patch each step individually with its new position
     for (const stepPosition of stepPositions) {
-      await Step.query(trx).findById(stepPosition.id).patch({
+      const patchData: PartialModelObject<Step> = {
         position: stepPosition.position,
-      })
+      }
+      if (stepPosition.config) {
+        patchData.config = stepPosition.config.approval
+          ? raw(`jsonb_set(config, '{approval}', ?::jsonb, true)`, [
+              JSON.stringify(stepPosition.config?.approval),
+            ])
+          : raw(`config - 'approval'`)
+      }
+      await Step.query(trx).findById(stepPosition.id).patch(patchData)
     }
 
     // Update the flow's lastUpdatedAt timestamp
@@ -90,6 +101,21 @@ const updateStepPositions: MutationResolvers['updateStepPositions'] = async (
       .withGraphFetched('steps')
       .orderBy('steps.position', 'asc')
 
+    // sanity check that all step positions are contiguous
+    const contiguousPositions = updatedFlow.steps.map((step) => step.position)
+    if (
+      !contiguousPositions.every((position, index) => position === index + 1)
+    ) {
+      logger.error({
+        message: 'Updated positions are no longer contiguous',
+        stepPositions,
+        flowId: flow.id,
+      })
+      throw new BadUserInputError(
+        'Failed to update: updated positions are no longer contiguous',
+      )
+    }
+
     return updatedFlow.steps
   })
 
diff --git a/packages/backend/src/graphql/schema.graphql b/packages/backend/src/graphql/schema.graphql
@@ -578,6 +578,7 @@ input StepPositionInput {
   id: String!
   position: Int!
   type: StepEnumType!
+  config: StepConfigInput
 }
 
 input UpdateStepPositionsInput {
@@ -683,8 +684,8 @@ type StepTemplateConfig {
 }
 
 type StepApprovalConfig {
-  branch: String
-  stepId: String
+  branch: String!
+  stepId: String!
 }
 
 input StepConnectionInput {
diff --git a/packages/frontend/src/components/Editor/components/StepsList.tsx b/packages/frontend/src/components/Editor/components/StepsList.tsx
@@ -1,14 +1,14 @@
 import { IStep } from '@plumber/types'
 
-import { useContext } from 'react'
+import { useCallback, useContext } from 'react'
 import { Center, Flex } from '@chakra-ui/react'
 
 import PrimarySpinner from '@/components/PrimarySpinner'
 import { SortableList } from '@/components/SortableList'
 import { EditorContext } from '@/contexts/Editor'
+import { MrfContext } from '@/contexts/MrfContext'
 import { StepsToDisplayContext } from '@/contexts/StepsToDisplay'
 import { FlowStepGroup } from '@/exports/components'
-import { StepEnumType } from '@/graphql/__generated__/graphql'
 import { TOOLBOX_ACTIONS } from '@/helpers/toolbox'
 import useReorderSteps from '@/hooks/useReorderSteps'
 
@@ -30,26 +30,43 @@ export function StepsList({ isNested }: StepsListProps) {
     groupingActions,
   } = useContext(StepsToDisplayContext)
   const { flow, isDrawerOpen, isMobile, readOnly } = useContext(EditorContext)
+  const { mrfSteps, mrfApprovalSteps, approvalBranches } =
+    useContext(MrfContext)
 
-  const { handleReorderUpdate } = useReorderSteps(flow.id)
+  const { calculateReorderedSteps, handleReorderUpdate } = useReorderSteps(
+    flow.id,
+  )
 
-  const handleReorderSteps = async (reorderedSteps: IStep[]) => {
-    const stepPositions = reorderedSteps.map((step, index) => ({
-      id: step.id,
-      position: index + 2, // trigger position is 1
-      type: step.type as StepEnumType,
-    }))
+  const handleReorderSteps = useCallback(
+    async (reorderedSteps: IStep[]) => {
+      const allSteps = flow.steps
+      const allReorderedSteps = calculateReorderedSteps({
+        reorderedSteps,
+        allSteps,
+        mrfSteps,
+        mrfApprovalSteps,
+        approvalBranches,
+      })
 
-    try {
-      await handleReorderUpdate(stepPositions)
-    } catch (error) {
-      console.error(
-        'Error updating step positions: ',
-        error,
-        JSON.stringify(stepPositions),
-      )
-    }
-  }
+      try {
+        await handleReorderUpdate(allReorderedSteps)
+      } catch (error) {
+        console.error(
+          'Error updating step positions: ',
+          error,
+          JSON.stringify(allReorderedSteps),
+        )
+      }
+    },
+    [
+      flow.steps,
+      calculateReorderedSteps,
+      mrfSteps,
+      mrfApprovalSteps,
+      approvalBranches,
+      handleReorderUpdate,
+    ],
+  )
 
   const nonIfThenActionSteps = actionStepsBeforeGroup.filter(
     (step) => step.key !== TOOLBOX_ACTIONS.IfThen,
diff --git a/packages/frontend/src/hooks/useReorderSteps.ts b/packages/frontend/src/hooks/useReorderSteps.ts
@@ -1,22 +1,114 @@
-import { IStep } from '@plumber/types'
+import { IStep, IStepApprovalBranch, IStepApprovalConfig } from '@plumber/types'
 
+import { useCallback } from 'react'
 import { useMutation } from '@apollo/client'
 import { useToast } from '@opengovsg/design-system-react'
 
-import { StepEnumType } from '@/graphql/__generated__/graphql'
+import {
+  StepConfigInput,
+  StepEnumType,
+  StepPositionInput,
+} from '@/graphql/__generated__/graphql'
 import { UPDATE_STEP_POSITIONS } from '@/graphql/mutations/update-step-positions'
 import { GET_FLOW } from '@/graphql/queries/get-flow'
 
-interface StepPositionInput {
-  id: string
-  position: number
-  type: StepEnumType
-}
-
 const useReorderSteps = (flowId: string) => {
   const toast = useToast()
   const [updateStepPositions] = useMutation(UPDATE_STEP_POSITIONS)
 
+  // TODO: write unit test for this function
+  const calculateReorderedSteps = useCallback(
+    ({
+      reorderedSteps,
+      allSteps,
+      mrfSteps,
+      mrfApprovalSteps,
+      approvalBranches,
+    }: {
+      reorderedSteps: IStep[]
+      allSteps: IStep[]
+      mrfSteps: IStep[]
+      mrfApprovalSteps: IStep[]
+      approvalBranches: { [stepId: string]: IStepApprovalBranch }
+    }): StepPositionInput[] => {
+      // we start from 2 because the first step is the trigger step and not part of the sortable list
+      let nextPosition = 2
+      let currentApprovalconfig: IStepApprovalConfig | null = null
+      const allReorderedSteps: StepPositionInput[] = []
+      reorderedSteps.forEach((step) => {
+        const isMrfStep = mrfSteps.some((mrfStep) => mrfStep.id === step.id)
+
+        if (isMrfStep) {
+          // if the previous approval config is in the approve branch
+          // we need to add the steps in the reject branch that was hidden
+          if (currentApprovalconfig?.branch === 'approve') {
+            const stepsInRejectBranch = allSteps.filter(
+              (step) =>
+                step.config?.approval?.branch === 'reject' &&
+                step.config?.approval?.stepId === currentApprovalconfig?.stepId,
+            ) as IStep[]
+            stepsInRejectBranch.forEach((step) => {
+              allReorderedSteps.push({
+                id: step.id,
+                position: nextPosition++,
+                type: step.type as StepEnumType,
+                config: {
+                  approval: step.config.approval,
+                } as StepConfigInput,
+              })
+            })
+          }
+          currentApprovalconfig = null
+        }
+
+        // add the current step to the list
+        allReorderedSteps.push({
+          id: step.id,
+          position: nextPosition++,
+          type: step.type as StepEnumType,
+          config: {
+            approval: currentApprovalconfig,
+          } as StepConfigInput,
+        })
+
+        const isMrfApprovalStep = mrfApprovalSteps.some(
+          (mrfApprovalStep) => mrfApprovalStep.id === step.id,
+        )
+        if (isMrfApprovalStep) {
+          currentApprovalconfig = {
+            branch: approvalBranches[step.id],
+            stepId: step.id,
+          }
+          // if the current approval config is in the reject branch
+          // we need to add the steps in the approve branch that was hidden
+          if (currentApprovalconfig?.branch === 'reject') {
+            const stepsInApproveBranch = allSteps.filter(
+              (step) =>
+                step.config?.approval?.branch === 'approve' &&
+                step.config?.approval?.stepId === currentApprovalconfig?.stepId,
+            ) as IStep[]
+            stepsInApproveBranch.forEach((step) => {
+              allReorderedSteps.push({
+                id: step.id,
+                position: nextPosition++,
+                type: step.type as StepEnumType,
+                config: {
+                  approval: step.config.approval,
+                } as StepConfigInput,
+              })
+            })
+          }
+        }
+      })
+
+      // all later steps not in the sortable list need not be updated since
+      // reordering of visible steps will not affect them
+
+      return allReorderedSteps
+    },
+    [],
+  )
+
   const handleReorderUpdate = async (stepPositions: StepPositionInput[]) => {
     try {
       await updateStepPositions({
@@ -25,6 +117,7 @@ const useReorderSteps = (flowId: string) => {
           updateStepPositions: stepPositions.map((sp) => ({
             id: sp.id,
             position: sp.position,
+            config: sp.config ? { approval: sp.config.approval } : undefined,
             __typename: 'Step' as const,
           })),
         },
@@ -37,15 +130,22 @@ const useReorderSteps = (flowId: string) => {
 
           if (flow) {
             // Create a map of step positions for quick lookup
-            const positionMap = new Map(
-              stepPositions.map((sp) => [sp.id, sp.position]),
+            const updatedStepMap = new Map(
+              stepPositions.map((sp) => [
+                sp.id,
+                { position: sp.position, config: sp.config },
+              ]),
             )
 
             // Update steps with new positions
             const updatedSteps = flow.steps.map((step: IStep) => {
-              const newPosition = positionMap.get(step.id)
-              return newPosition !== undefined
-                ? { ...step, position: newPosition }
+              const updatedStep = updatedStepMap.get(step.id)
+              return updatedStep !== undefined
+                ? {
+                    ...step,
+                    position: updatedStep.position,
+                    config: { ...step.config, ...updatedStep.config },
+                  }
                 : step
             })
 
@@ -91,7 +191,7 @@ const useReorderSteps = (flowId: string) => {
     }
   }
 
-  return { handleReorderUpdate }
+  return { handleReorderUpdate, calculateReorderedSteps }
 }
 
 export default useReorderSteps

Original file line number	Diff line number	Diff line change
`@@ -578,6 +578,7 @@ input StepPositionInput {`
`578`	`578`	`id: String!`
`579`	`579`	`position: Int!`
`580`	`580`	`type: StepEnumType!`
	`581`	`+ config: StepConfigInput`
`581`	`582`	`}`
`582`	`583`
`583`	`584`	`input UpdateStepPositionsInput {`
`@@ -683,8 +684,8 @@ type StepTemplateConfig {`
`683`	`684`	`}`
`684`	`685`
`685`	`686`	`type StepApprovalConfig {`
`686`		`- branch: String`
`687`		`- stepId: String`
	`687`	`+ branch: String!`
	`688`	`+ stepId: String!`
`688`	`689`	`}`
`689`	`690`
`690`	`691`	`input StepConnectionInput {`