enable when expression can access variable

[FikriMilano]

IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).

Fixes #2096 #2094

Description
Provide proper contextMap when evaluating the following:

• enableWhenExpression can access variablesMap and launchContextMap
• variableExpression can access launchContextMap

Alternative(s) considered
N/A

Type
Feature

Video

Demo.webm

Checklist

• I have read and acknowledged the Code of conduct.
• I have read the Contributing page.
• I have signed the Google Individual CLA, or I am covered by my company's Corporate CLA.
• I have discussed my proposed solution with code owners in the linked issue(s) and we have agreed upon the general approach.
• I have run ./gradlew spotlessApply and ./gradlew spotlessCheck to check my code follows the style guide of this project.
• I have run ./gradlew check and ./gradlew connectedCheck to test my changes locally.
• I have built and run the demo app(s) to verify my change fixes the issue and/or does not break the demo app(s).

[FikriMilano]

hello this is ready for review

[FikriMilano]

@omarismail94 @santosh-pingle @jingtang10

[MJ1998]

I think we have too many parameter dependencies in functions. Lot of parameters are constant for a specific questionnaire session, for example, questionnaire, questionnaireItemParentMap, questionnaireLaunchContextMap.

What we can do is create a constructor based EnablementEvaluator & ExpressionEvaluator which takes in the above parameters. And maintain an object of these in the QuestionnaireViewModel. Then for each APIs in EnablementEvaluator & ExpressionEvaluator we won't need to pass those parameters. Think about this.

[FikriMilano]

@MJ1998 I like the idea, makes sense 👍

[jingtang10]

is this ready for review fikri?

[FikriMilano]

@jingtang10 updating the kotlin doc is the only thing left

[jingtang10]

Some small changes I've just applied - and ran spotless. Thanks again @FikriMilano for this great PR!

[jingtang10]

Thanks @FikriMilano expressionEvaluator.API() or enablementEvaluator.API() invocations look really good. ExpressionEvaluator having questionnaireLaunchContextMap makes perfect sense because then EnablementEvaluator can use expressionEvaluator without having the knowledge of questionnaireLaunchContextMap. Great work with refactoring.

But I think all these APIs should have questionnaireResponse and it should not be a part of ExpressionEvaluator constructor parameters. This is how I think: In a Questionnaire session whenever an answer is changed and we calculate an expression we need to provide it the expression (obviously), the questionnaireItem it's part of, the questionnaireResponse (as it changes after every user input) and all the other static variables you have rightfully put as constructor parameters (questionnaire, launchContext, itemParentMap). Hence questionnaireResponse should go in API invocations for example evaluateExpression(expression, questionnaireItem, questionnaireResponse)

these have been addressed.

addressed

[jingtang10]

Jajoo - I'm satisfied that your review comments have been addressed and I've approved this PR. So I've "dismissed" your review to unblock merge. But if you have any further comments please add them and/or create another issue.