From 5c31f06e60a13bf5a08f6c372affdb97ec3210c0 Mon Sep 17 00:00:00 2001 From: 9sneha-n <9sneha.n@gmail.com> Date: Thu, 14 Nov 2024 16:30:09 +0530 Subject: [PATCH] fix: refactor for better coding standards --- src/data/entities/D2ExpressionParser.ts | 110 +++++++++--------- .../entities/Questionnaire/Questionnaire.ts | 21 ++-- .../Questionnaire/QuestionnaireQuestion.ts | 75 +++++++++--- .../Questionnaire/QuestionnaireRules.ts | 6 +- .../Questionnaire/QuestionnaireSection.ts | 30 ++--- 5 files changed, 136 insertions(+), 106 deletions(-) diff --git a/src/data/entities/D2ExpressionParser.ts b/src/data/entities/D2ExpressionParser.ts index 878c70c..6c788b4 100644 --- a/src/data/entities/D2ExpressionParser.ts +++ b/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 = { - 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 + ruleCondition: string, + variableValues: Map ): Either { 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 ) { - 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 = { + text: xp.ValueType.STRING, + boolean: xp.ValueType.BOOLEAN, + date: xp.ValueType.DATE, + number: xp.ValueType.NUMBER, +}; diff --git a/src/domain/entities/Questionnaire/Questionnaire.ts b/src/domain/entities/Questionnaire/Questionnaire.ts index c661b24..2cf838e 100644 --- a/src/domain/entities/Questionnaire/Questionnaire.ts +++ b/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, diff --git a/src/domain/entities/Questionnaire/QuestionnaireQuestion.ts b/src/domain/entities/Questionnaire/QuestionnaireQuestion.ts index fbf09ab..d503b35 100644 --- a/src/domain/entities/Questionnaire/QuestionnaireQuestion.ts +++ b/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,9 +232,16 @@ 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( @@ -213,16 +249,17 @@ export class QuestionnaireQuestion { 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 diff --git a/src/domain/entities/Questionnaire/QuestionnaireRules.ts b/src/domain/entities/Questionnaire/QuestionnaireRules.ts index e23a758..c9d5cd8 100644 --- a/src/domain/entities/Questionnaire/QuestionnaireRules.ts +++ b/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; diff --git a/src/domain/entities/Questionnaire/QuestionnaireSection.ts b/src/domain/entities/Questionnaire/QuestionnaireSection.ts index 151be36..a688190 100644 --- a/src/domain/entities/Questionnaire/QuestionnaireSection.ts +++ b/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, + }), }; });