refactor(frontend): fetch flowId directly from backend, remove manual cache manipulation for createStep and duplicate step

Author: pregnantboy

packages/frontend/src/components/Editor/index.tsx

@@ -30,19 +30,7 @@ export default function Editor(props: EditorProps): React.ReactElement {
     useContext(EditorContext)
 
   const { handleReorderUpdate } = useReorderSteps(flow.id)
-  const rawSteps = flow.steps
-  const steps = useMemo(
-    // Populate each step's flowId so that IStep isn't LYING about flowId being
-    // non-undefined. We do it here instead of fetching in GraphQL since all
-    // steps have same pipe, so a bit wasteful to repeat this data over the wire.
-    () =>
-      rawSteps.map((step) => ({
-        ...step,
-        flow,
-        flowId: flow.id,
-      })),
-    [flow, rawSteps],
-  )
+  const steps = flow.steps
 
   const currentStep = useMemo(() => {
     return steps.find((step) => step.id === currentStepId)

packages/frontend/src/components/FlowStep/components/DuplicateStepButton.tsx

@@ -17,59 +17,13 @@ interface DuplicateStepButtonProps {
   step: IStep
 }
 
-/**
- * Helper function to update the flow in the cache
- */
-function updateHandlerFactory(flowId: string, previousStepId: string) {
-  return function createStepUpdateHandler(cache: any, mutationResult: any) {
-    const { data } = mutationResult
-    const { createStep: createdStep } = data
-    const { getFlow: flow } = cache.readQuery({
-      query: GET_FLOW,
-      variables: { id: flowId },
-    })
-
-    // getFlow requires certain attributes to be returned
-    const completeCreatedStep = {
-      ...createdStep,
-      iconUrl: null,
-      webhookUrl: null,
-      config: {
-        stepName: createdStep?.config?.stepName || null,
-        templateConfig: {
-          appEventKey: null,
-        },
-        approval: {
-          branch: createdStep?.config?.approval?.branch ?? null,
-          stepId: createdStep?.config?.approval?.stepId ?? null,
-        },
-      },
-      createdAt: new Date().toISOString(),
-    }
-
-    const steps = flow.steps.reduce((steps: any[], currentStep: any) => {
-      if (currentStep.id === previousStepId) {
-        return [...steps, currentStep, completeCreatedStep]
-      }
-
-      return [...steps, currentStep]
-    }, [])
-
-    cache.writeQuery({
-      query: GET_FLOW,
-      variables: { id: flowId },
-      data: { getFlow: { ...flow, steps } },
-    })
-  }
-}
-
 export default function DuplicateStepButton(props: DuplicateStepButtonProps) {
   const { isNested, step } = props
 
   const { flow, isMobile, onDrawerOpen, setCurrentStepId } =
     useContext(EditorContext)
 
-  const [createStep] = useMutation(CREATE_STEP)
+  const [createStep] = useMutation(CREATE_STEP, { refetchQueries: [GET_FLOW] })
   const onDuplicateStep = useCallback(async () => {
     const duplicateConfig = {
       ...(step.config?.approval && {
@@ -97,7 +51,6 @@ export default function DuplicateStepButton(props: DuplicateStepButtonProps) {
 
     const createdStep = await createStep({
       variables: { input: mutationInput },
-      update: updateHandlerFactory(flow.id, step.id),
     })
 
     const newStep = createdStep.data.createStep

packages/frontend/src/components/FlowStepConfigurationModal/ChooseAndAddConnection/index.tsx

@@ -139,7 +139,7 @@ export default function ChooseAndAddConnection(
             selectedApp.key,
             selectedEvent.key,
             connectionId,
-            { approval: approvalConfig },
+            approvalConfig && { approval: approvalConfig },
           )
           newStepId = createdStep.id
         } else if (step) {

packages/frontend/src/components/FlowStepConfigurationModal/ChooseAppAndEvent/index.tsx

@@ -5,7 +5,9 @@ import { useQuery } from '@apollo/client'
 
 import { EditorContext } from '@/contexts/Editor'
 import { MrfContext } from '@/contexts/MrfContext'
+import client from '@/graphql/client'
 import { GET_APP_CONNECTIONS } from '@/graphql/queries/get-app-connections'
+import { GET_FLOW } from '@/graphql/queries/get-flow'
 import { getMrfApprovalConfig } from '@/helpers/formsg'
 import {
   TOOLBOX_ACTIONS,
@@ -120,10 +122,11 @@ export default function ChooseAppAndEvent(props: ChooseAppAndEventProps) {
             app.key,
             triggerOrAction.key,
             excelConnection?.id || undefined,
-            { approval: approvalConfig },
+            approvalConfig && { approval: approvalConfig },
           )
           newStepId = createdStep.id
         } else if (step) {
+          // This part of the code happens when updating an empty step
           // account for the if-then edge case
           if (
             app.key === TOOLBOX_APP_KEY &&
@@ -142,6 +145,8 @@ export default function ChooseAppAndEvent(props: ChooseAppAndEventProps) {
             })
             newStepId = updatedStep.id
           }
+          // we refetch GET_FLOW after everything is completed
+          await client.refetchQueries({ include: [GET_FLOW] })
         }
         onClose()
         onDrawerOpen()

packages/frontend/src/graphql/link.ts

@@ -1,6 +1,7 @@
 import type { ApolloLink } from '@apollo/client'
 import { from, HttpLink } from '@apollo/client'
 import { onError } from '@apollo/client/link/error'
+import { removeTypenameFromVariables } from '@apollo/client/link/remove-typename'
 
 import { INVALID_TILE_VIEW_KEY, NOT_AUTHORISED } from '@/config/errors'
 import * as URLS from '@/config/urls'
@@ -62,7 +63,13 @@ const createLink = (options: CreateLinkOptions): ApolloLink => {
 
   const httpOptions = { uri, token }
 
-  return from([createErrorLink(onError), createHttpLink(httpOptions)])
+  return from([
+    // this removes the __typename from variables before sending to the server,
+    // which prevents validation errors especially when duplicating steps
+    removeTypenameFromVariables(),
+    createErrorLink(onError),
+    createHttpLink(httpOptions),
+  ])
 }
 
 export default createLink

packages/frontend/src/graphql/mutations/create-step.ts

@@ -11,6 +11,7 @@ export const CREATE_STEP = gql`
       position
       status
       flowId
+      createdAt
       connection {
         id
       }

packages/frontend/src/graphql/mutations/update-step.ts

@@ -12,6 +12,7 @@ export const UPDATE_STEP = graphql(`
       status
       position
       flowId
+      createdAt
       connection {
         id
       }

packages/frontend/src/helpers/formsg.ts

@@ -29,10 +29,7 @@ export function getMrfApprovalConfig({
     previousStep.config?.approval?.branch &&
     previousStep.config?.approval?.stepId
   ) {
-    return {
-      branch: previousStep.config?.approval?.branch,
-      stepId: previousStep.config?.approval?.stepId,
-    }
+    return previousStep.config?.approval
   }
 
   /**

packages/frontend/src/helpers/toolbox.ts

@@ -4,10 +4,8 @@ import { useCallback, useContext, useState } from 'react'
 import { useMutation } from '@apollo/client'
 
 import { BranchContext } from '@/components/FlowStepGroup/Content/IfThen/BranchContext'
-import client from '@/graphql/client'
 import { CREATE_STEP } from '@/graphql/mutations/create-step'
 import { UPDATE_STEP } from '@/graphql/mutations/update-step'
-import { GET_FLOW } from '@/graphql/queries/get-flow'
 
 export const TOOLBOX_APP_KEY = 'toolbox'
 
@@ -241,10 +239,10 @@ export function useIfThenInitializer(): [
         },
       })
 
-      // Refetch only after completion of all initialization steps.
-      await client.refetchQueries({ include: [GET_FLOW] })
-
       setIsInitializing(false)
+
+      // we dont refetch GET_FLOW here but leave it to the caller to refetch
+
       return currStep
     },
     [createStep, depth, updateStep],