[EXCEL-MULTI-LOOKUP-2] feat: patch parameters

packages/backend/src/apps/m365-excel/__tests__/actions/get-table-rows.itest.ts

@@ -5,6 +5,8 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
 import { FOR_EACH_INPUT_SOURCE } from '@/apps/toolbox/common/constants'
 import StepError from '@/errors/step'
 import { generateMockContext } from '@/graphql/__tests__/mutations/tiles/table.mock'
+import Flow from '@/models/flow'
+import Step from '@/models/step'
 import Context from '@/types/express/context'
 
 import m365Excel from '../..'
@@ -69,6 +71,9 @@ describe('getTableRowsAction', () => {
       http: {
         request: vi.fn(),
       },
+      execution: {
+        testRun: false,
+      },
     } as unknown as IGlobalVariable
 
     // Setup default mock implementations
@@ -397,4 +402,62 @@ describe('getTableRowsAction', () => {
       'data500',
     )
   })
+
+  /**
+   * TODO (kevinkim-ogp): remove this entire describe block when
+   * patchParameters is removed from get-table-rows.
+   * These tests only guard the temporary DB patch during test runs.
+   */
+  describe('patchParameters (temporary patch-parameters-temp)', () => {
+    it('does not call Step.query when execution.testRun is false', async () => {
+      const stepQuerySpy = vi.spyOn(Step, 'query')
+      await getTableRowsAction.run($)
+      expect(stepQuerySpy).not.toHaveBeenCalled()
+    })
+
+    it('when execution.testRun is true, patches the step row with filters from lookup fields', async () => {
+      const context = await generateMockContext()
+      const flow = await Flow.query().insert({
+        userId: context.currentUser.id,
+        name: 'get-table-rows patchParameters test flow',
+      })
+
+      const parameters = {
+        fileId: 'test-file-id',
+        tableId: '{test-table-id}',
+        lookupColumn: 'Column1',
+        lookupValue: 'patch-test-lookup',
+      }
+
+      const dbStep = await Step.query().insert({
+        flowId: flow.id,
+        key: getTableRowsAction.key,
+        appKey: 'm365-excel',
+        type: 'action',
+        position: 2,
+        status: 'completed',
+        parameters: { ...parameters },
+      })
+
+      $.flow.id = flow.id
+      $.flow.userId = context.currentUser.id
+      $.step.id = dbStep.id
+      $.step.parameters = { ...parameters }
+      $.execution.testRun = true
+
+      await getTableRowsAction.run($)
+
+      const updated = await Step.query().findById(dbStep.id).throwIfNotFound()
+
+      expect(updated.parameters).toMatchObject({
+        ...parameters,
+        filters: [
+          {
+            lookupColumn: parameters.lookupColumn,
+            lookupValue: parameters.lookupValue,
+          },
+        ],
+      })
+    })
+  })
 })

packages/backend/src/apps/m365-excel/actions/get-table-row/index.ts

@@ -4,6 +4,8 @@ import z from 'zod'
 
 import StepError from '@/errors/step'
 
+import { LOOKUP_CONDITIONS_SUBFIELDS } from '../../common/constants'
+import patchParameters from '../../common/patch-parameters-temp'
 import { convertRowToHexEncodedRowRecord } from '../../common/workbook-helpers/tables'
 import WorkbookSession from '../../common/workbook-session'
 import { RATE_LIMIT_FOR_RELEASE_ONLY_REMOVE_AFTER_JULY_2024 } from '../../FOR_RELEASE_PERIOD_ONLY'
@@ -112,14 +114,32 @@ const action: IRawAction = {
         op: 'is_empty',
       },
     },
+    {
+      label: 'Lookup conditions',
+      description:
+        'If multiple rows meet the conditions, the topmost entry will be returned. Lookup values are case sensitive and should not include units (e.g., $5.20 → 5.2). Leave blank to search for empty cells.',
+      key: 'filters',
+      type: 'multirow-multicol' as const,
+      required: true,
+      subFields: LOOKUP_CONDITIONS_SUBFIELDS,
+      maxRows: 1,
+      hiddenIf: {
+        op: 'always_true',
+      },
+    },
   ],
+  // TODO (kevinkim-ogp): update this to ['lookupColumn', 'lookupValue'] once we have moved to the new multi lookup
+  ignoredArgs: ['filters'],
 
   getDataOutMetadata,
 
   async run($) {
     // FOR RELEASE ONLY TO STEM ANY THUNDERING HERDS; REMOVE AFTER 21 Jul 2024.
     if ($.execution.testRun) {
       await RATE_LIMIT_FOR_RELEASE_ONLY_REMOVE_AFTER_JULY_2024($.user?.email, $)
+
+      // TODO (kevinkim-ogp): remove this once we have moved to the
+      await patchParameters($)
     }
 
     const parametersParseResult = parametersSchema.safeParse($.step.parameters)

packages/backend/src/apps/m365-excel/actions/get-table-rows/index.ts

@@ -5,8 +5,12 @@ import z from 'zod'
 import { FOR_EACH_INPUT_SOURCE } from '@/apps/toolbox/common/constants'
 import StepError from '@/errors/step'
 
-import { GET_TABLE_ROWS_LIMIT } from '../../common/constants'
+import {
+  GET_TABLE_ROWS_LIMIT,
+  LOOKUP_CONDITIONS_SUBFIELDS,
+} from '../../common/constants'
 import getTopNTableRows from '../../common/get-top-n-table-rows'
+import patchParameters from '../../common/patch-parameters-temp'
 import { convertRowToHexKeyedObject } from '../../common/workbook-helpers/tables/convert-row-to-hex-encoded-row-record'
 import WorkbookSession from '../../common/workbook-session'
 import { MAX_ROWS } from '../get-table-row/implementation'
@@ -107,11 +111,31 @@ const action: IRawAction = {
       required: false,
       variables: true,
     },
+    {
+      key: 'filters',
+      label: 'Lookup conditions',
+      description:
+        'Lookup values are case sensitive and should not include units (e.g., $5.20 → 5.2). Leave blank to search for empty cells.',
+      type: 'multirow-multicol' as const,
+      required: true,
+      subFields: LOOKUP_CONDITIONS_SUBFIELDS,
+      maxRows: 1, // Phase 1: Restrict to single filter
+      hiddenIf: {
+        op: 'always_true',
+      },
+    },
   ],
+  // TODO (kevinkim-ogp): update this to ['lookupColumn', 'lookupValue'] once we have moved to the new multi lookup
+  ignoredArgs: ['filters'],
 
   getDataOutMetadata,
 
   async run($) {
+    if ($.execution.testRun) {
+      // TODO (kevinkim-ogp): remove this once we have moved to the new multi lookup
+      await patchParameters($)
+    }
+
     const parametersParseResult = parametersSchema.safeParse($.step.parameters)
     if (parametersParseResult.success === false) {
       throw new StepError(

packages/backend/src/apps/m365-excel/actions/update-table-row/index.ts

@@ -4,6 +4,7 @@ import z from 'zod'
 
 import StepError from '@/errors/step'
 
+import patchParameters from '../../common/patch-parameters-temp'
 import { sanitiseInputValue } from '../../common/sanitise-formula-input'
 import {
   constructMsGraphValuesArrayForRowWrite,
@@ -81,13 +82,18 @@ const action: IRawAction = {
       ],
     },
   ],
+  // TODO (kevinkim-ogp): update this to ['lookupColumn', 'lookupValue'] once we have moved to the new multi lookup
+  ignoredArgs: ['filters'],
 
   getDataOutMetadata,
 
   async run($) {
     // FOR RELEASE ONLY TO STEM ANY THUNDERING HERDS; REMOVE AFTER 21 Jul 2024.
     if ($.execution.testRun) {
       await RATE_LIMIT_FOR_RELEASE_ONLY_REMOVE_AFTER_JULY_2024($.user?.email, $)
+
+      // TODO (kevinkim-ogp): remove this once we have moved to the new multi lookup
+      await patchParameters($)
     }
 
     const parametersParseResult = parametersSchema.safeParse($.step.parameters)

packages/backend/src/apps/m365-excel/common/constants.ts

@@ -5,3 +5,44 @@ export const APP_KEY = 'm365-excel'
 export const MS_GRAPH_OAUTH_BASE_URL = 'https://login.microsoftonline.com'
 
 export const GET_TABLE_ROWS_LIMIT = 500
+
+export const LOOKUP_CONDITIONS_SUBFIELDS = [
+  {
+    placeholder: 'Lookup column',
+    key: 'lookupColumn',
+    type: 'dropdown' as const,
+    required: true,
+    variables: false,
+    showOptionValue: false,
+    source: {
+      type: 'query' as const,
+      name: 'getDynamicData' as const,
+      arguments: [
+        {
+          name: 'key',
+          value: 'listTableColumns',
+        },
+        {
+          name: 'parameters.fileId',
+          value: '{parameters.fileId}',
+        },
+        {
+          name: 'parameters.tableId',
+          value: '{parameters.tableId}',
+        },
+      ],
+    },
+    customStyle: { flex: 2 },
+  },
+  {
+    key: 'lookupValue' as const,
+    placeholder: 'Lookup value',
+    // We don't support matching on Excel-formatted text because it's very
+    // weird (e.g. currency cells have a trailing space), and will lead to too
+    // much user confusion.
+    type: 'string' as const,
+    required: false,
+    variables: true,
+    customStyle: { flex: 3, minWidth: 0, maxWidth: '60%' },
+  },
+]

packages/backend/src/apps/m365-excel/common/patch-parameters-temp.ts

@@ -0,0 +1,40 @@
+import { IGlobalVariable } from '@plumber/types'
+
+import logger from '@/helpers/logger'
+import Step from '@/models/step'
+
+/**
+ * TODO(kevinkim-ogp): remove this after patching the database for all remaining steps
+ *
+ * We manually patch the step's parameter's so that it contains the new 'filters' field.
+ * this ensures that the 'filters' field is automatically populated when we move to this.
+ *
+ * We also update $.step.parameters in memory so that the filters are automatically populated
+ * in the ExecutionStep's dataIn when the test run is executed.
+ */
+const patchParameters = async ($: IGlobalVariable) => {
+  try {
+    // Transform in memory - add filters
+    $.step.parameters.filters = [
+      {
+        lookupColumn: $.step.parameters.lookupColumn,
+        lookupValue: $.step.parameters.lookupValue,
+      },
+    ]
+
+    // Persist to database
+    await Step.query()
+      .patch({ parameters: $.step.parameters })
+      .where({ 'steps.id': $.step.id })
+      .throwIfNotFound()
+  } catch (error) {
+    // NOTE: we do not want to throw error here so that the test run can continue
+    // if this failed, we can still patch it later on
+    logger.error(
+      'Failed to patch the executionStep and step parameters during test run',
+      error,
+    )
+  }
+}
+
+export default patchParameters

packages/backend/src/graphql/schema.graphql

@@ -214,6 +214,8 @@ type Action {
   isNew: Boolean
   linkToGuide: String
   substeps: [ActionSubstep]
+
+  ignoredArgs: [String]
 }
 
 type ActionSubstep {
@@ -837,6 +839,8 @@ type Trigger {
   substeps: [TriggerSubstep]
   isNew: Boolean
   noAuthRequired: Boolean
+
+  ignoredArgs: [String]
 }
 
 type TriggerInstructions {

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

@@ -134,12 +134,25 @@ export default function FlowStepTestController(
   // isLastTestExecutionCurrent is false if the form values are saved but not tested
   const isLastTestExecutionCurrent = useMemo(() => {
     const formValues = formContext.getValues()
+
+    /**
+     * hiddenKeys are used to filter out ignored arguments from the form values and dataIn
+     * so that the comparison is not affected by ignored arguments
+     */
+    const hiddenKeys = selectedActionOrTrigger?.ignoredArgs || []
+
     return matchParamsToDataIn(
       currentTestExecutionStep?.dataIn,
       formValues.parameters,
       varInfoMap,
+      hiddenKeys,
     )
-  }, [currentTestExecutionStep, formContext, varInfoMap])
+  }, [
+    currentTestExecutionStep?.dataIn,
+    formContext,
+    selectedActionOrTrigger,
+    varInfoMap,
+  ])
 
   const [infoBoxVariant, infoBoxText] = getInfoBoxDetails({
     isDirty,

packages/frontend/src/components/FlowStepTestController/utils.tsx

@@ -125,14 +125,18 @@ const deepCompare = (a: any, b: any, varInfoMap: VariableInfoMap): boolean => {
  * NOTE: it also validates the variable value as dataIn is based on the extracted value
  */
 export const matchParamsToDataIn = (
-  dataIn?: IJSONObject,
-  params?: IJSONObject,
+  rawDataIn?: IJSONObject,
+  rawParams?: IJSONObject,
   varInfoMap?: VariableInfoMap,
+  hiddenKeys?: string[],
 ) => {
-  if (!dataIn || !params || !varInfoMap) {
+  if (!rawDataIn || !rawParams || !varInfoMap) {
     return false
   }
 
+  const dataIn = omitHiddenParameterKeys(rawDataIn, hiddenKeys)
+  const params = omitHiddenParameterKeys(rawParams, hiddenKeys)
+
   // If both are empty objects, return true
   if (Object.keys(dataIn).length === 0 && Object.keys(params).length === 0) {
     return true
@@ -351,3 +355,26 @@ export function getForEachDataMessage(
   const numberOfItems = getForEachIterationCount(testExecutionSteps, step.id)
   return `This for-each action will run on ${numberOfItems} items. The first item is shown below.`
 }
+
+/**
+ * * TODO (kevinkim-ogp): remove this once the Excel multi lookup is done
+ * as we should not have fields that are always hidden.
+ *
+ * Drops keys (e.g. hidden setUpAction fields) while keeping IJSONObject typing.
+ */
+export function omitHiddenParameterKeys(
+  parameters: IJSONObject,
+  hiddenKeys: string[] | undefined,
+): IJSONObject {
+  if (!hiddenKeys?.length) {
+    return { ...parameters }
+  }
+  const visible: IJSONObject = {}
+  for (const key of Object.keys(parameters)) {
+    if (hiddenKeys.includes(key)) {
+      continue
+    }
+    visible[key] = parameters[key]
+  }
+  return visible
+}

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

@@ -178,6 +178,7 @@ export const GET_APPS = gql`
         isNew
         linkToGuide
         hiddenFromUser
+        ignoredArgs
         substeps {
           key
           name