chore: update getDataOutMetadata and tests

Author: pregnantboy

packages/backend/src/apps/formsg/__tests__/triggers/new-submission.test.ts

@@ -8,6 +8,8 @@ import {
 
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 
+import apps from '@/apps'
+
 import { ADDRESS_LABELS } from '../../common/constants'
 import trigger from '../../triggers/new-submission'
 
@@ -24,6 +26,7 @@ const mocks = vi.hoisted(() => ({
       },
     }
   }),
+  removeMrfSteps: vi.fn(),
 }))
 
 vi.mock('../../triggers/new-submission/get-mock-data', () => ({
@@ -34,12 +37,26 @@ vi.mock('../../triggers/new-submission/fetch-form-schema', () => ({
   fetchFormSchema: mocks.fetchFormSchema,
 }))
 
+vi.mock('../../triggers/new-submission/remove-mrf-steps', () => ({
+  removeMrfSteps: mocks.removeMrfSteps,
+}))
+
 vi.mock('../../common/webhook-settings', () => ({
   registerWebhookUrl: vi.fn(),
   verifyWebhookUrl: vi.fn(),
   getFormDetailsFromGlobalVariable: mocks.getFormDetailsFromGlobalVariable,
 }))
 
+vi.mock('../../../../models/step', () => ({
+  default: {
+    query: vi.fn(() => ({
+      findById: vi.fn(() => ({
+        throwIfNotFound: vi.fn(() => ({ parameters: {} })),
+      })),
+    })),
+  },
+}))
+
 describe('new submission trigger', () => {
   let executionStep: IExecutionStep
 
@@ -72,12 +89,24 @@ describe('new submission trigger', () => {
     } as unknown as IExecutionStep
   })
 
+  afterEach(() => {
+    vi.clearAllMocks()
+  })
+
   describe('testRun', () => {
     const $ = {
       auth: { data: { formId: '123' } },
       user: { email: '[email protected]' },
+      step: {
+        id: '123',
+        position: 1,
+      },
+      flow: {
+        id: 'flow-id',
+      },
       pushTriggerItem: pushTriggerItemMock,
       getLastExecutionStep: getLastExecutionStepMock,
+      app: apps.formsg,
     } as unknown as IGlobalVariable
 
     const mockData = {
@@ -108,10 +137,6 @@ describe('new submission trigger', () => {
       })
     })
 
-    afterEach(() => {
-      vi.clearAllMocks()
-    })
-
     it('should use mock data if preferMock is true and there is no past submission', async () => {
       getLastExecutionStepMock.mockResolvedValue(null)
       await trigger.testRun($, { preferMock: true })
@@ -223,6 +248,12 @@ describe('new submission trigger', () => {
         },
       })
     })
+
+    it('should call remove MRF steps function if the form is storage mode', async () => {
+      getLastExecutionStepMock.mockResolvedValue(null)
+      await trigger.testRun($, { preferMock: false })
+      expect(mocks.removeMrfSteps).toHaveBeenCalledOnce()
+    })
   })
   describe('dataOut metadata', () => {
     it('ensures that only question, answer and answerArray props are visible', async () => {
@@ -355,7 +386,7 @@ describe('new submission trigger', () => {
       )
     })
 
-    it('sets label to the associated question for attachment answers', async () => {
+    it('sets label to the associated question for attachment answers with question number', async () => {
       executionStep.dataOut.fields = {
         fileFieldId: {
           question: 'Attach a file.',
@@ -365,14 +396,14 @@ describe('new submission trigger', () => {
       }
 
       const metadata = await trigger.getDataOutMetadata(executionStep)
-      expect(metadata.fields.fileFieldId.answer.label).toEqual('Attach a file.')
+      expect(metadata.fields.fileFieldId.answer.label).toEqual(
+        '1. Attach a file.',
+      )
     })
 
-    it('collapses header fields', async () => {
+    it('hides header fields', async () => {
       const metadata = await trigger.getDataOutMetadata(executionStep)
-      expect(
-        metadata.fields.headerFieldId.question.isCollapsedByDefault,
-      ).toEqual(true)
+      expect(metadata.fields.headerFieldId.isHidden).toEqual(true)
     })
 
     it('collapses question variables', async () => {

packages/backend/src/apps/formsg/common/get-data-out-metadata.ts

@@ -9,8 +9,10 @@ import {
 
 import logger from '@/helpers/logger'
 import { parseS3Id } from '@/helpers/s3'
+import Step from '@/models/step'
 
 import { ADDRESS_LABELS } from './constants'
+import { ParsedMrfWorkflowStep } from './types'
 
 function buildQuestionMetadatum(fieldData: IJSONObject): IDataOutMetadatum {
   const question: IDataOutMetadatum = {
@@ -44,7 +46,7 @@ function buildAnswerMetadatum(fieldData: IJSONObject): IDataOutMetadatum {
       answer['type'] = 'file'
       // We encode the question as the label because we hide the actual question
       // as a variable.
-      answer['label'] = fieldData.question as string
+      answer['label'] = `${fieldData.order}. ${fieldData.question}` as string
       // For attachments, answer is one of:
       // 1. An S3 ID (if we stored the attachment into S3).
       // 2. An empty string (if attachment field is optional).
@@ -327,6 +329,18 @@ async function getDataOutMetadata(
     return null
   }
 
+  const { parameters } = await Step.query()
+    .findById(executionStep.stepId)
+    .throwIfNotFound()
+
+  let questionIdsToShowForMrf: Set<string> | undefined
+
+  if (parameters.mrf) {
+    questionIdsToShowForMrf = new Set(
+      (parameters.mrf as ParsedMrfWorkflowStep).fields,
+    )
+  }
+
   const fieldMetadata: IDataOutMetadata = Object.create(null)
 
   const fields = Object.entries(data.fields).sort((a, b) => {
@@ -339,35 +353,35 @@ async function getDataOutMetadata(
   })
 
   /**
-   * This is a hack to match the question number to the form as closely as possible.
-   * In formsg, the headers are not numbered, so we need to exclude them from the question number.
-   * But we also need to keep track of the headers between questions, so we can order them correctly.
-   * The regenerated order will be like so:
-   * Example given form:
-   * Header 0.9991
-   * Header 0.9992
-   * Question 1
-   * Question 2
-   * Sub Heading 2.9991
-   * Question 3
-   * Header 3.9991
-   * Sub Heading 3.9992
-   * Question 4
-   * Sub Heading 4.9991
-   * Question 4
+   * We ignore all 'image' and 'section' (aka headers) field types
+   * Paragraphs are not returned in encryptedContent
    */
   let questionOrder = 0
-  let headerOrderBetweenQuestions = 0
   for (const [fieldId, fieldData] of fields) {
-    if (fieldData.fieldType !== 'section') {
-      // reset order between questions (altho not necessary)
-      headerOrderBetweenQuestions = 0
-      questionOrder++
-      fieldData.order = questionOrder
-    } else {
-      headerOrderBetweenQuestions++
-      fieldData.order = questionOrder + +`0.999${headerOrderBetweenQuestions}`
+    // ignore image fields, dont even increment question order
+    if (fieldData.fieldType === 'image' || fieldData.fieldType === 'section') {
+      fieldMetadata[fieldId] = { isHidden: true }
+      continue
     }
+
+    // increment question order for each field
+    questionOrder++
+    fieldData.order = questionOrder
+
+    // if this is an MRF step, we only show the fields that are editable
+    if (questionIdsToShowForMrf && !questionIdsToShowForMrf.has(fieldId)) {
+      fieldMetadata[fieldId] = {
+        question: { isHidden: true },
+        answer: { isHidden: true },
+        fieldType: { isHidden: true },
+        order: { isHidden: true },
+        myInfo: { attr: { isHidden: true } },
+        isVisible: { isHidden: true },
+        isHeader: { isHidden: true },
+      }
+      continue
+    }
+
     fieldMetadata[fieldId] = {
       question: buildQuestionMetadatum(fieldData),
       answer: buildAnswerMetadatum(fieldData),