feat:use dhis2 expression parser #69

9sneha-n · 2024-09-25T15:54:26Z

📌 References

Issue: Closes https://app.clickup.com/t/8695pt0av, #8695pt0av

📝 Implementation

Use dhis2 expression parser for parsing program rule conditions
Recursively update all survey questions (form field) that need to be updated as a side effect of the current question update

📹 Screenshots/Screen capture

testing videos : https://drive.google.com/drive/folders/1BQkuP6o0fi0GxY-cqbTJzlCAj9Ivk7bu?usp=sharing

🔥 Notes to the tester

tokland

Some inline comments:

tokland · 2024-11-11T09:11:17Z

src/data/entities/D2ExpressionParser.ts

+    boolean: xp.ValueType.BOOLEAN,
+    date: xp.ValueType.DATE,
+    number: xp.ValueType.NUMBER,
+};


Let's move auxiliar/helper methods down, so the first we see when opening the file is the class and its public methods.

tokland · 2024-11-11T09:11:28Z

src/data/entities/D2ExpressionParser.ts

+
+export class D2ExpressionParser {
+    public evaluateRuleEngineCondition(
+        ruleCondtion: string,


typo: ruleCondtion

tokland · 2024-11-11T09:15:24Z

src/data/entities/D2ExpressionParser.ts

+            );
+
+            const programVariables = expressionParser.collectProgramVariablesNames();
+            programVariables.forEach(programVariable => {


[functional programming / immutability] let's refactor this block to avoid the forEach. We can for example build some from programVariables and concat to the previous ones to build the final variablesMap in one go.

tokland · 2024-11-11T09:16:09Z

src/data/entities/D2ExpressionParser.ts

+                                "date",
+                                new Date().toISOString().split("T")[0]
+                            )
+                        );


Multi-line expressions like this one are hard to read. Create local variables for intermediate values.

tokland · 2024-11-11T09:16:51Z

src/data/entities/D2ExpressionParser.ts

+                undefined,
+                undefined,
+                undefined
+            );


I think we can remove the undefined args, is not adding too much value.

tokland · 2024-11-11T09:33:26Z

src/domain/entities/Questionnaire/QuestionnaireQuestion.ts

+                ...question,
+                isVisible: updatedIsVisible,
+                errors: updatedErrors,
+            };


[subjective] The return value is the same except for the value: undefined. This does not seem accidental repetition, to keep it DRY, move the condition inside.

I have faced this issue before as well, typescript cannot resolve the type unless the condition is an if else. Is there any other way to resolve this?

I have faced this issue before as well, typescript cannot resolve the type unless the condition is an if else. Is there any other way to resolve this?

I see, things like this happen for union types, not TS's fault, it's normal. Try this approach:

...(question.isVisible !== updatedIsVisible ? { value: undefined } : {})

wow, nice one! It works 👍

tokland · 2024-11-11T09:36:06Z

src/domain/entities/Questionnaire/QuestionnaireQuestion.ts

@@ -152,100 +152,102 @@ export class QuestionnaireQuestion {
    }

    static updateQuestions(


Only if we have time (probably not...). This is the kind of logic that really benefits from having some tests, as it's not trivial at all and extremely easy to introduce a bug. No need to cover all cases, just a subset that are used in the app and we know to be tricky.

Noted, will do this if i have time remaining after creation of ppt for meeting.

tokland · 2024-11-11T09:37:05Z

src/domain/entities/Questionnaire/QuestionnaireRules.ts

@@ -45,11 +42,12 @@ export interface QuestionnaireRuleAction {
 }
 export interface QuestionnaireRule {
    id: Id;
-    condition: string; //condition is parsed with dataelementId e.g: #{dataElementId} == 'Yes'
+    condition: string;
    dataElementIds: Id[]; // all dataElements in condition (there could be mutiple conditions)
    teAttributeIds: Id[]; // all trackedEntityAttributes in condition (there could be mutiple conditions)


typo: mutiple

tokland · 2024-11-11T09:38:21Z

src/domain/entities/Questionnaire/QuestionnaireRules.ts

@@ -45,11 +42,12 @@ export interface QuestionnaireRuleAction {
 }
 export interface QuestionnaireRule {
    id: Id;
-    condition: string; //condition is parsed with dataelementId e.g: #{dataElementId} == 'Yes'
+    condition: string;


Here we need some kind of documentation to know that kind of things we can do in a condition.

tokland · 2024-11-11T09:41:52Z

src/domain/entities/Questionnaire/QuestionnaireSection.ts

-                    updatedSection.isVisible === false && section.isVisible === true
-                ),
-            };
+            if (section.isVisible === false && updatedSection.isVisible === true) {


When working with boolean, it's more declarative just a / !a

tokland · 2024-11-11T09:49:13Z

src/data/entities/D2ExpressionParser.ts

+
+export class D2ExpressionParser {
+    public evaluateRuleEngineCondition(
+        ruleCondtion: string,


Add some docs somewhere (probably the easiest is a link to DHIS2 documentation) so we know what kind of conditions we can do.

9sneha-n · 2024-11-14T17:03:07Z

@tokland Ready for re-review.

tokland

Code-wise, all good by me.

9sneha-n added 10 commits September 24, 2024 22:54

feat:use dhis2 expression parser

92247a3

fix: section hiding

93ea9a5

feat: D2ExpressionParserWrapper

0d1e895

fix: remove custom rule parser code

8a065b8

fix: some review comments

0c0d032

fix: code review comments

222fed1

feat: partially working recursion

c127aa8

feat: working recursive question updates

7e0dd8f

fix: return an either

39474f9

fix: handle generic variable current_date

8bbb990

9sneha-n requested a review from tokland November 10, 2024 10:06

9sneha-n marked this pull request as ready for review November 10, 2024 10:06

9sneha-n added 3 commits November 10, 2024 19:19

feat: reset questions which are hidden, reset question on hidden section

9159682

chore: comments

c68fb01

chore: remove unwanted code

844e44e

tokland reviewed Nov 11, 2024

View reviewed changes

9sneha-n added 2 commits November 14, 2024 16:30

fix: refactor for better coding standards

5c31f06

refactor: better coding std

72dd7f2

9sneha-n requested a review from tokland November 14, 2024 17:02

tokland approved these changes Nov 18, 2024

View reviewed changes

MiquelAdell merged commit b892c5a into development Nov 19, 2024
1 check passed

MiquelAdell deleted the feature/coding-dojo branch November 19, 2024 08:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat:use dhis2 expression parser #69

feat:use dhis2 expression parser #69

9sneha-n commented Sep 25, 2024 •

edited

Loading

tokland left a comment

tokland Nov 11, 2024

tokland Nov 11, 2024

tokland Nov 11, 2024 •

edited

Loading

tokland Nov 11, 2024

tokland Nov 11, 2024

tokland Nov 11, 2024 •

edited

Loading

9sneha-n Nov 14, 2024

tokland Nov 14, 2024 •

edited

Loading

9sneha-n Nov 14, 2024

tokland Nov 11, 2024 •

edited

Loading

9sneha-n Nov 14, 2024

tokland Nov 11, 2024

tokland Nov 11, 2024

tokland Nov 11, 2024

tokland Nov 11, 2024

9sneha-n commented Nov 14, 2024

tokland left a comment

		@@ -152,100 +152,102 @@ export class QuestionnaireQuestion {
		}

		static updateQuestions(

feat:use dhis2 expression parser #69

feat:use dhis2 expression parser #69

Conversation

9sneha-n commented Sep 25, 2024 • edited Loading

📌 References

📝 Implementation

📹 Screenshots/Screen capture

🔥 Notes to the tester

tokland left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tokland Nov 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tokland Nov 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tokland Nov 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tokland Nov 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

9sneha-n commented Nov 14, 2024

tokland left a comment

Choose a reason for hiding this comment

9sneha-n commented Sep 25, 2024 •

edited

Loading

tokland Nov 11, 2024 •

edited

Loading

tokland Nov 11, 2024 •

edited

Loading

tokland Nov 14, 2024 •

edited

Loading

tokland Nov 11, 2024 •

edited

Loading