From 9e15f7a2d0683ecb21d424ae345f451f00f38f4d Mon Sep 17 00:00:00 2001
From: Aldo Giambelluca <xoen@users.noreply.github.com>
Date: Thu, 5 Dec 2024 12:05:54 +0000
Subject: [PATCH] Questions controller reduce errors (#286)

* Refactor: Set steps/fields in `populateReport()`

This is so that these are available from the
`QuestionsController` as well.

* Don't look up conditional fields as questions

This is to avoid unnecessary false errors in the logs.

When questions/answers are submitted the submitted
values' keys are fields names. However these field
names can be either a question ID or a conditional
field name (e.g. comment/date)

This change only look up question IDs in the report
config.

NOTE: This does not change what we post to the API.
Conditional fields values are still submitted when
the question's answers require date/comment.
---
 server/controllers/wip/questionsController.ts | 14 ++++++++++++++
 server/middleware/populateReport.ts           |  4 ++++
 server/routes/questions/router.ts             |  6 +-----
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/server/controllers/wip/questionsController.ts b/server/controllers/wip/questionsController.ts
index 92ea1c75..06807c67 100644
--- a/server/controllers/wip/questionsController.ts
+++ b/server/controllers/wip/questionsController.ts
@@ -108,9 +108,23 @@ export default class QuestionsController extends BaseController<FormWizard.Multi
 
         const report = res.locals.report as ReportWithDetails
         const reportConfig = res.locals.reportConfig as IncidentTypeConfiguration
+        const reportFields = res.locals.reportFields as FormWizard.Fields
 
         const questionsResponses = []
         for (const [fieldName, values] of Object.entries(submittedValues)) {
+          // Skip conditional fields
+          if (reportFields[fieldName]?.dependent) {
+            // Conditional fields don't have their own question config,
+            // submitted values are included later if the question
+            // requires a date or comment.
+            // Skipping these so that we don't get false errors
+            // when questions with ID like `{Q.id}-{A.id}-(date|comment)`
+            // are not found in the config
+
+            // eslint-disable-next-line no-continue
+            continue
+          }
+
           const questionConfig = reportConfig.questions[fieldName]
           if (questionConfig === undefined) {
             logger.error(
diff --git a/server/middleware/populateReport.ts b/server/middleware/populateReport.ts
index c67c07b3..1f85857d 100644
--- a/server/middleware/populateReport.ts
+++ b/server/middleware/populateReport.ts
@@ -2,6 +2,7 @@ import type { NextFunction, Request, Response } from 'express'
 
 import logger from '../../logger'
 import { getIncidentTypeConfiguration } from '../reportConfiguration/types'
+import { generateFields, generateSteps } from '../data/incidentTypeConfiguration/formWizard'
 
 // eslint-disable-next-line import/prefer-default-export
 export function populateReport() {
@@ -25,6 +26,9 @@ export function populateReport() {
       return
     }
 
+    res.locals.reportSteps = generateSteps(res.locals.reportConfig)
+    res.locals.reportFields = generateFields(res.locals.reportConfig)
+
     next()
   }
 }
diff --git a/server/routes/questions/router.ts b/server/routes/questions/router.ts
index b7de4525..3ffc13e0 100644
--- a/server/routes/questions/router.ts
+++ b/server/routes/questions/router.ts
@@ -1,7 +1,6 @@
 import express from 'express'
 import wizard from 'hmpo-form-wizard'
 
-import { generateFields, generateSteps } from '../../data/incidentTypeConfiguration/formWizard'
 import asyncMiddleware from '../../middleware/asyncMiddleware'
 import { populateReport } from '../../middleware/populateReport'
 
@@ -12,10 +11,7 @@ router.use(
   asyncMiddleware(async (req, res, next) => {
     const reportId = req.params.id
 
-    const steps = generateSteps(res.locals.reportConfig)
-    const fields = generateFields(res.locals.reportConfig)
-
-    const wizardRouter = wizard(steps, fields, {
+    const wizardRouter = wizard(res.locals.reportSteps, res.locals.reportFields, {
       name: `${reportId}-questions`,
       templatePath: 'pages/wip/questions',
       // Needs to be false, session already handled by application