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

Author: pregnantboy

packages/backend/src/graphql/schema.graphql

@@ -669,6 +669,7 @@ type Step {
   executionSteps: [ExecutionStep]
   config: StepConfig
   createdAt: String
+  flowId: String
 }
 
 type StepConfig {

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 && {
@@ -96,7 +50,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

@@ -134,7 +134,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,
@@ -118,10 +120,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 &&
@@ -140,6 +143,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/contexts/Editor.tsx

@@ -117,52 +117,6 @@ type EditorProviderProps = {
   resetFormRef?: React.MutableRefObject<(() => void) | null>
 }
 
-/**
- * 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: null,
-        templateConfig: {
-          appEventKey: createdStep.config?.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 const EditorProvider = ({
   readOnly,
   flow,
@@ -260,36 +214,17 @@ export const EditorProvider = ({
 
       const createdStep = await createStep({
         variables: { input: mutationInput },
-        update: updateHandlerFactory(flowId, previousStepId),
       })
 
-      const newStep = createdStep.data.createStep
+      let newStep = createdStep.data.createStep
       setCurrentStepId(newStep.id)
 
       // account for the for-each and if-then
-      if (appKey === TOOLBOX_APP_KEY) {
-        // Get the complete step data from the cache
-        const { getFlow: updatedFlow } = client.readQuery({
-          query: GET_FLOW,
-          variables: { id: flowId },
-        })
-
-        const completeStep = updatedFlow.steps.find(
-          (s: IStep) => s.id === newStep.id,
-        )
-
-        if (completeStep) {
-          const completeStepWithFlow = {
-            ...completeStep,
-            flowId: flowId,
-          }
-          if (eventKey === TOOLBOX_ACTIONS.IfThen) {
-            return (await initializeIfThen(
-              completeStepWithFlow,
-            )) as unknown as IStep
-          }
-        }
+      if (appKey === TOOLBOX_APP_KEY && eventKey === TOOLBOX_ACTIONS.IfThen) {
+        newStep = await initializeIfThen(newStep)
       }
+      // we refetch GET_FLOW after everything is completed
+      await client.refetchQueries({ include: [GET_FLOW] })
 
       return newStep as IStep
     },

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

@@ -10,6 +10,8 @@ export const CREATE_STEP = gql`
       parameters
       position
       status
+      flowId
+      createdAt
       connection {
         id
       }

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

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

packages/frontend/src/graphql/queries/get-flow.ts

@@ -15,6 +15,7 @@ export const GET_FLOW = gql`
         webhookUrl
         status
         position
+        flowId
         createdAt
         connection {
           id