Refactor question type cleaning & added unit tests

Author: Bharath-K-Shetty

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@openmrs/esm-form-builder-app",
-  "version": "3.0.0",
+  "version": "3.0.1",
   "license": "MPL-2.0",
   "description": "Form Builder for O3",
   "browser": "dist/openmrs-esm-form-builder-app.js",

src/components/interactive-builder/modals/question/question-form/question/question.component.tsx

@@ -5,6 +5,7 @@ import RenderTypeComponent from '../rendering-types/rendering-type.component';
 import QuestionTypeComponent from '../question-types/question-type.component';
 import RequiredLabel from '../common/required-label/required-label.component';
 import { useFormField } from '../../form-field-context';
+import { cleanFormFieldForType } from '../../question.modal';
 import type { FormField, RenderType } from '@openmrs/esm-form-engine-lib';
 import { questionTypes, renderTypeOptions, renderingTypes } from '@constants';
 import styles from './question.scss';
@@ -59,11 +60,14 @@ const Question: React.FC<QuestionProps> = ({ checkIfQuestionIdExists }) => {
         )}
         required
       />
+
       <Select
         value={formField?.type ?? 'control'}
-        onChange={(event: React.ChangeEvent<HTMLSelectElement>) =>
-          setFormField({ ...formField, type: event.target.value })
-        }
+        onChange={(event) => {
+          const newType = event.target.value;
+          const cleaned = cleanFormFieldForType({ ...formField, type: newType }, newType);
+          setFormField(cleaned);
+        }}
         id="questionType"
         invalidText={t('typeRequired', 'Type is required')}
         labelText={t('questionType', 'Question type')}
@@ -148,6 +152,7 @@ const Question: React.FC<QuestionProps> = ({ checkIfQuestionIdExists }) => {
           </RadioButtonGroup>
         </>
       )}
+
       {formField.type && <QuestionTypeComponent />}
       {formField.questionOptions?.rendering && <RenderTypeComponent />}
     </>

src/components/interactive-builder/modals/question/question-form/question/question.test.tsx

@@ -0,0 +1,45 @@
+import { cleanFormFieldForType } from '../../question.modal';
+import type { FormField } from '@openmrs/esm-form-engine-lib';
+
+describe('cleanFormFieldForType', () => {
+  it('should remove irrelevant top-level and nested properties when changing the question type', () => {
+    const formField: FormField = {
+      id: 'testQuestion',
+      label: 'Test Question',
+      type: 'obs',
+      questionOptions: {
+        rendering: 'radio',
+        concept: '12345',
+        answers: [
+          { concept: '111', label: 'Yes' },
+          { concept: '222', label: 'No' },
+        ],
+        irrelevantOption: 'remove me',
+      },
+      required: true,
+      extraProp: 'should be removed',
+    } as any;
+
+    // Change type from 'obs' to 'control'
+    const newType = 'control';
+    const cleaned = cleanFormFieldForType(formField, newType);
+
+    // Assert that the type is updated.
+    expect(cleaned.type).toBe('control');
+
+    // Assert that required properties remain.
+    expect(cleaned.id).toBe('testQuestion');
+    expect(cleaned.label).toBe('Test Question');
+
+    // Irrelevant top-level properties should be removed.
+    expect(cleaned.required).toBeUndefined();
+    expect(cleaned).not.toHaveProperty('extraProp');
+
+    // For nested questionOptions, only allowed keys remain for 'control'.
+    // Allowed keys for 'control' in questionOptions are: rendering, minLength, and maxLength.
+    expect(cleaned.questionOptions).toHaveProperty('rendering', 'radio');
+    expect(cleaned.questionOptions).not.toHaveProperty('concept');
+    expect(cleaned.questionOptions).not.toHaveProperty('answers');
+    expect(cleaned.questionOptions).not.toHaveProperty('irrelevantOption');
+  });
+});

src/components/interactive-builder/modals/question/question.modal.tsx

@@ -1,4 +1,4 @@
-import React, { useCallback, useEffect } from 'react';
+import React, { useCallback } from 'react';
 import { useTranslation } from 'react-i18next';
 import flattenDeep from 'lodash-es/flattenDeep';
 import {
@@ -82,18 +82,15 @@ const allowedQuestionOptionsMapping: Record<string, string[]> = {
 function cleanQuestionOptionsForType(options: any, newType: string): any {
   const allowedOpts = allowedQuestionOptionsMapping[newType] || [];
   const cleanedOpts = Object.fromEntries(Object.entries(options).filter(([optKey]) => allowedOpts.includes(optKey)));
-  // Ensure required property 'rendering' exists if present in the original options.
-  if (!('rendering' in cleanedOpts) && options.rendering) {
-    cleanedOpts.rendering = options.rendering;
-  }
+  cleanedOpts.rendering = options.rendering;
   return cleanedOpts;
 }
 
 /**
  * Cleans the given form field by retaining only allowed top‑level properties for the new type.
  * Also cleans nested questionOptions using the nested mapping.
  */
-function cleanFormFieldForType(field: FormField, newType: string): FormField {
+export function cleanFormFieldForType(field: FormField, newType: string): FormField {
   const allowedKeys = allowedPropertiesMapping[newType] || [];
   const cleaned: Partial<FormField> = {};
 
@@ -109,10 +106,14 @@ function cleanFormFieldForType(field: FormField, newType: string): FormField {
   }
 
   cleaned.type = newType;
-
   return cleaned as FormField;
 }
 
+const getAllQuestionIds = (questions?: FormField[]): string[] => {
+  if (!questions) return [];
+  return flattenDeep(questions.map((question) => [question.id, getAllQuestionIds(question.questions)]));
+};
+
 const QuestionModalContent: React.FC<QuestionModalProps> = ({
   formField: formFieldProp,
   closeModal,
@@ -124,39 +125,41 @@ const QuestionModalContent: React.FC<QuestionModalProps> = ({
 }) => {
   const { t } = useTranslation();
   const { formField, setFormField } = useFormField();
-  useEffect(() => {
-    if (formField && formField.type) {
-      const cleaned = cleanFormFieldForType(formField, formField.type);
-      if (JSON.stringify(cleaned) !== JSON.stringify(formField)) {
-        setFormField(cleaned);
-      }
-    }
-  }, [formField?.type, formField, setFormField]);
-
-  const getAllQuestionIds = useCallback((questions?: FormField[]): string[] => {
-    if (!questions) return [];
-    return flattenDeep(questions.map((question) => [question.id, getAllQuestionIds(question.questions)]));
-  }, []);
-
   const checkIfQuestionIdExists = useCallback(
     (idToTest: string): boolean => {
       // Get all IDs from the schema
       const schemaIds: string[] =
         schema?.pages?.flatMap((page) => page?.sections?.flatMap((section) => getAllQuestionIds(section.questions))) ||
         [];
 
-      // Get all IDs from the current formField's questions array
+      // Get all IDs from the obsGroup questions
       const formFieldIds: string[] = formField?.questions ? getAllQuestionIds(formField.questions) : [];
 
+      // The main question's id
+      formFieldIds.push(formField.id);
+
+      const originalFormFieldQuestionIds =
+        formFieldProp && formFieldProp.id !== '' ? getAllQuestionIds(formFieldProp.questions) : [];
+
+      if (formFieldProp && formFieldProp.id !== '') originalFormFieldQuestionIds.push(formFieldProp.id);
+
+      // Remove the ids from the original question from the schema ids
+      const filteredSchemaIds = schemaIds.slice(); // Create a copy to modify
+      originalFormFieldQuestionIds.forEach((idToRemove) => {
+        const indexToRemove = filteredSchemaIds.indexOf(idToRemove);
+        if (indexToRemove !== -1) {
+          filteredSchemaIds.splice(indexToRemove, 1);
+        }
+      });
+
       // Combine both arrays, along with the parent question ID and count occurrences of the ID
-      const allIds = [...schemaIds, ...formFieldIds];
-      if (!formFieldProp || formFieldProp.id !== formField.id) {
-        allIds.push(formField.id);
-      }
+      const allIds = [...filteredSchemaIds, ...formFieldIds];
       const occurrences = allIds.filter((id) => id === idToTest).length;
+
+      // Return true if ID occurs more than once
       return occurrences > 1;
     },
-    [schema, getAllQuestionIds, formField, formFieldProp],
+    [schema, formField, formFieldProp],
   );
 
   const addObsGroupQuestion = useCallback(() => {