fix: refactor for better coding standards

src/data/entities/D2ExpressionParser.ts

@@ -2,65 +2,22 @@ import * as xp from "@dhis2/expression-parser";
 import _c from "../../domain/entities/generic/Collection";
 import { Either } from "../../domain/entities/generic/Either";
 
-export type ProgramRuleVariableType = "text" | "number" | "date" | "boolean";
-
-export type ProgramRuleVariableName = string;
-export type ProgramRuleVariableValue = {
-    type: ProgramRuleVariableType;
-    value: string;
-};
-
-const VariableValueTypeMap: Record<ProgramRuleVariableType, xp.ValueType> = {
-    text: xp.ValueType.STRING,
-    boolean: xp.ValueType.BOOLEAN,
-    date: xp.ValueType.DATE,
-    number: xp.ValueType.NUMBER,
-};
-
 export class D2ExpressionParser {
     public evaluateRuleEngineCondition(
-        ruleCondtion: string,
-        ruleVariables: Map<ProgramRuleVariableName, ProgramRuleVariableValue>
+        ruleCondition: string,
+        variableValues: Map<ProgramRuleVariableName, ProgramRuleVariableValue>
     ): Either<Error, boolean> {
         try {
             const expressionParser = new xp.ExpressionJs(
-                ruleCondtion,
+                ruleCondition,
                 xp.ExpressionMode.RULE_ENGINE_CONDITION
             );
 
-            const variables = expressionParser.collectProgramRuleVariableNames();
-            const variablesValueMap = this.mapProgramVariables(variables, ruleVariables);
-            const variablesMap = new Map(
-                variablesValueMap.map(variable => [variable.programRuleVariable, variable.value])
-            );
+            const ruleVariables = this.mapProgramRuleVariables(expressionParser, variableValues);
+            const genericVariables = this.mapProgramVariables(expressionParser);
+            const variables = new Map([...ruleVariables, ...genericVariables]);
 
-            const programVariables = expressionParser.collectProgramVariablesNames();
-            programVariables.forEach(programVariable => {
-                switch (programVariable) {
-                    case "current_date": {
-                        variablesMap.set(
-                            programVariable,
-                            this.getVariableValueByType(
-                                "date",
-                                new Date().toISOString().split("T")[0]
-                            )
-                        );
-                        break;
-                    }
-                    default:
-                        throw new Error(
-                            `Unhandled Program variable of type : ${programVariable}. Please contact developer`
-                        );
-                }
-            });
-
-            const expressionData = new xp.ExpressionDataJs(
-                variablesMap,
-                undefined,
-                undefined,
-                undefined,
-                undefined
-            );
+            const expressionData = new xp.ExpressionDataJs(variables);
 
             const parsedResult: boolean = expressionParser.evaluate(
                 () => console.debug(""),
@@ -69,11 +26,7 @@ export class D2ExpressionParser {
 
             return Either.success(parsedResult);
         } catch (error) {
-            return Either.error(
-                new Error(
-                    `An error occurred while evaluating the rule in D2ExpressionParser::evaluateRuleEngineCondition: ${error}`
-                )
-            );
+            return Either.error(error as Error);
         }
     }
 
@@ -85,11 +38,12 @@ export class D2ExpressionParser {
         return new xp.VariableValueJs(valueType, stringValue, [], null);
     };
 
-    private mapProgramVariables(
-        programRuleVariables: string[],
+    private mapProgramRuleVariables(
+        expressionParser: xp.ExpressionJs,
         ruleVariables: Map<string, ProgramRuleVariableValue>
     ) {
-        return programRuleVariables.map(programRuleVariable => {
+        const programRuleVariables = expressionParser.collectProgramRuleVariableNames();
+        const variablesValueMap = programRuleVariables.map(programRuleVariable => {
             const currentProgramRuleVariableValue = ruleVariables.get(programRuleVariable);
 
             if (!currentProgramRuleVariableValue)
@@ -110,5 +64,45 @@ export class D2ExpressionParser {
                 value: variableValue,
             };
         });
+
+        return new Map(
+            variablesValueMap.map(variable => [variable.programRuleVariable, variable.value])
+        );
+    }
+
+    private mapProgramVariables(expressionParser: xp.ExpressionJs) {
+        const programVariables = expressionParser.collectProgramVariablesNames();
+        const programVariableValues = programVariables.map(programVariable => {
+            switch (programVariable) {
+                case "current_date": {
+                    const currentISODate = new Date().toISOString().split("T")[0];
+                    const currentDate = this.getVariableValueByType("date", currentISODate);
+                    return { programVariable: programVariable, value: currentDate };
+                }
+                default:
+                    throw new Error(
+                        `Unhandled Program variable of type : ${programVariable}. Please contact developer`
+                    );
+            }
+        });
+
+        const programVariablesMap = new Map(
+            programVariableValues.map(variable => [variable.programVariable, variable.value])
+        );
+
+        return programVariablesMap;
     }
 }
+export type ProgramRuleVariableType = "text" | "number" | "date" | "boolean";
+export type ProgramRuleVariableName = string;
+export type ProgramRuleVariableValue = {
+    type: ProgramRuleVariableType;
+    value: string;
+};
+
+const VariableValueTypeMap: Record<ProgramRuleVariableType, xp.ValueType> = {
+    text: xp.ValueType.STRING,
+    boolean: xp.ValueType.BOOLEAN,
+    date: xp.ValueType.DATE,
+    number: xp.ValueType.NUMBER,
+};

src/domain/entities/Questionnaire/Questionnaire.ts

@@ -272,10 +272,9 @@ export class Questionnaire {
             ...allQsInQuestionnaireStages,
         ];
 
-        const allQsInQuestionnaireWithUpdatedQ = allQsInQuestionnaire.map(question => {
-            if (question.id === updatedQuestion.id) return updatedQuestion;
-            else return question;
-        });
+        const allQsInQuestionnaireWithUpdatedQ = allQsInQuestionnaire.map(question =>
+            question.id === updatedQuestion.id ? updatedQuestion : question
+        );
 
         const applicableRules = getApplicableRules(
             updatedQuestion,
@@ -374,13 +373,13 @@ export class Questionnaire {
         questionnaire: Questionnaire,
         rules: QuestionnaireRule[]
     ): QuestionnaireEntity | undefined {
-        const updatedEntityQuestions = QuestionnaireQuestion.updateQuestions(
-            [],
-            questionnaireEntity.questions,
-            updatedQuestion,
-            rules,
-            questionnaire
-        );
+        const updatedEntityQuestions = QuestionnaireQuestion.updateQuestions({
+            processedQuestions: [],
+            questions: questionnaireEntity.questions,
+            updatedQuestion: updatedQuestion,
+            rules: rules,
+            questionnaire: questionnaire,
+        });
 
         return {
             ...questionnaireEntity,

src/domain/entities/Questionnaire/QuestionnaireQuestion.ts

@@ -125,6 +125,14 @@ export interface QuestionOption extends NamedRef {
     code?: string;
 }
 
+export type UpdateQuestionOptions = {
+    processedQuestions: Question[];
+    questions: Question[];
+    updatedQuestion: Question;
+    rules: QuestionnaireRule[];
+    questionnaire: Questionnaire;
+};
+
 export class QuestionnaireQuestion {
     static isValidNumberValue(s: string, numberType: NumberQuestion["numberType"]): boolean {
         if (!s) return true;
@@ -151,14 +159,29 @@ export class QuestionnaireQuestion {
         return { ...question, value };
     }
 
-    static updateQuestions(
-        processedQuestions: Question[],
-        questions: Question[],
-        updatedQuestion: Question,
-        rules: QuestionnaireRule[],
-        questionnaire: Questionnaire
+    static updateQuestions(updateQuestionOptions: UpdateQuestionOptions): Question[] {
+        const sortedUpdatedQuestions = this.getSortedUpdatedQuestions(updateQuestionOptions);
+
+        updateQuestionOptions.processedQuestions.push(updateQuestionOptions.updatedQuestion);
+
+        const allQuestionsRequiringUpdate = this.getQuestionsToUpdateAsSideEffect(
+            updateQuestionOptions,
+            sortedUpdatedQuestions
+        );
+
+        return allQuestionsRequiringUpdate.length === 0
+            ? sortedUpdatedQuestions
+            : this.recursivelyApplySideEffects(
+                  updateQuestionOptions,
+                  allQuestionsRequiringUpdate,
+                  sortedUpdatedQuestions
+              );
+    }
+
+    private static getSortedUpdatedQuestions(
+        updateQuestionOptions: UpdateQuestionOptions
     ): Question[] {
-        //1. Update the question value before anything else, the updated value needs to be used to parse rule conditions
+        const { questions, updatedQuestion, rules } = updateQuestionOptions;
         const updatedQuestions = questions.map(question => {
             if (question.id === updatedQuestion.id) {
                 return updatedQuestion;
@@ -178,9 +201,15 @@ export class QuestionnaireQuestion {
         const sortedUpdatedQuestions = _(parsedAndUpdatedQuestions)
             .sortBy(question => question.sortOrder)
             .value();
-        processedQuestions.push(updatedQuestion);
 
-        //2. Get all questions that need to be updated as a side effect of the current question update
+        return sortedUpdatedQuestions;
+    }
+
+    private static getQuestionsToUpdateAsSideEffect(
+        updateQuestionOptions: UpdateQuestionOptions,
+        sortedUpdatedQuestions: Question[]
+    ): Question[] {
+        const { questionnaire, updatedQuestion, processedQuestions } = updateQuestionOptions;
         const allQuestionIdsRequiringUpdate = _(
             questionnaire.rules.flatMap(rule => {
                 if (
@@ -203,26 +232,34 @@ export class QuestionnaireQuestion {
         const allQuestionsRequiringUpdate = sortedUpdatedQuestions.filter(question =>
             allQuestionIdsRequiringUpdate.includes(question.id)
         );
-        if (allQuestionsRequiringUpdate.length === 0) return sortedUpdatedQuestions;
 
-        //3. Recursively update all questions that need to be updated as a side effect of the current question update
+        return allQuestionsRequiringUpdate;
+    }
+
+    private static recursivelyApplySideEffects(
+        updateQuestionOptions: UpdateQuestionOptions,
+        allQuestionsRequiringUpdate: Question[],
+        sortedUpdatedQuestions: Question[]
+    ) {
+        const { questionnaire, processedQuestions } = updateQuestionOptions;
         const finalUpdatesWithSideEffects = allQuestionsRequiringUpdate.reduce(
             (acc, questionRequiringUpdate) => {
                 const currentApplicableRules = getApplicableRules(
                     questionRequiringUpdate,
                     questionnaire.rules,
                     acc
                 );
-                //4. Maintain a dependency graph to avoid infinite recursive calls,
-                // once a question has been processed, it should not be processed again
+                //Maintain a dependency graph to avoid infinite recursive calls,
+                //once a question has been processed, it should not be processed again
                 processedQuestions.push(questionRequiringUpdate);
-                const updates = this.updateQuestions(
+                const updates = this.updateQuestions({
                     processedQuestions,
-                    acc,
-                    questionRequiringUpdate,
-                    currentApplicableRules,
-                    questionnaire
-                );
+                    questions: acc,
+                    updatedQuestion: questionRequiringUpdate,
+                    rules: currentApplicableRules,
+                    questionnaire,
+                });
+
                 return updates;
             },
             sortedUpdatedQuestions

src/domain/entities/Questionnaire/QuestionnaireRules.ts

@@ -42,9 +42,9 @@ export interface QuestionnaireRuleAction {
 }
 export interface QuestionnaireRule {
     id: Id;
-    condition: string;
-    dataElementIds: Id[]; // all dataElements in condition (there could be mutiple conditions)
-    teAttributeIds: Id[]; // all trackedEntityAttributes in condition (there could be mutiple conditions)
+    condition: string; //dhis2 program rule condition. https://docs.dhis2.org/en/full/develop/dhis-core-version-master/developer-manual.html#webapi_program_rules
+    dataElementIds: Id[]; // all dataElements in condition (there could be multiple conditions)
+    teAttributeIds: Id[]; // all trackedEntityAttributes in condition (there could be multiple conditions)
     actions: QuestionnaireRuleAction[];
     parsedResult?: boolean; //calculate the condition and store the result
     programRuleVariables: D2ProgramRuleVariable[] | undefined;

src/domain/entities/Questionnaire/QuestionnaireSection.ts

@@ -56,16 +56,16 @@ export class QuestionnaireSectionM {
                 ? this.updateSection(section, rules)
                 : section;
 
-            if (section.isVisible === false && updatedSection.isVisible === true) {
+            if (!section.isVisible && updatedSection.isVisible) {
                 //reset all questions in the section that was hidden
                 const resetQuestions = section.questions.reduce((acc, hiddenQuestion) => {
-                    return QuestionnaireQuestion.updateQuestions(
-                        section.questions,
-                        acc,
-                        { ...hiddenQuestion, value: undefined },
-                        rules,
-                        questionnaire
-                    );
+                    return QuestionnaireQuestion.updateQuestions({
+                        processedQuestions: section.questions,
+                        questions: acc,
+                        updatedQuestion: { ...hiddenQuestion, value: undefined },
+                        rules: rules,
+                        questionnaire: questionnaire,
+                    });
                 }, section.questions);
 
                 return {
@@ -75,13 +75,13 @@ export class QuestionnaireSectionM {
             } else
                 return {
                     ...updatedSection,
-                    questions: QuestionnaireQuestion.updateQuestions(
-                        [],
-                        updatedSection.questions,
-                        updatedQuestion,
-                        rules,
-                        questionnaire
-                    ),
+                    questions: QuestionnaireQuestion.updateQuestions({
+                        processedQuestions: [],
+                        questions: updatedSection.questions,
+                        updatedQuestion: updatedQuestion,
+                        rules: rules,
+                        questionnaire: questionnaire,
+                    }),
                 };
         });