(feat) O3-3049: adding evaluated historical expression support #226

arodidev · 2024-04-25T15:07:57Z

Requirements

This PR has a title that briefly describes the work done including the ticket number. If there is a ticket, make sure your PR title includes a conventional commit label. See existing PR titles for inspiration.
My work conforms to the OpenMRS 3.0 Styleguide and design documentation.
My work includes tests or is validated by existing tests.

Summary

This introduces the ability for the user to get the historical value (the field's previous value) based on an evaluated expression provided on the form's schema.

Screenshots

https://www.loom.com/share/5ee375006f1b4cf4b9fdf9fcbcf78bc4

Related Issue

Other

tt t

arodidev · 2024-04-25T15:11:37Z

Extends logic in this PR

samuelmale

@arodidev can we have some unit tests coverage around this?

samuelmale · 2024-04-29T08:27:58Z

src/components/section/form-section.component.tsx


 interface FieldComponentMap {
  fieldComponent: React.ComponentType<FormFieldProps>;
  fieldDescriptor: FormField;
  handler: SubmissionHandler;
 }

+const historicalValueTransformer = (field, obs) => {


I believe we support this elsewhere in the framework. With existing support, what you're trying to achieve here has been abstracted within the handlers. @arodidev Can we do away with the duplication?

Yes @samuelmale the logic exists within the handlers, although it exists as part of a greater process which we do not need much of, eg. pulling the obs from the encounter, while in our case we already have the obs to be evaluated. I think this approach would render the returned obs from the evaluated historical expression to be obsolete.

Would you suggest modifying the handler to accomodate instances where the obs is already present?

@samuelmale I'm requesting some guidance on this before progressing, thanks.

samuelmale · 2024-04-29T08:35:02Z

src/components/section/form-section.component.tsx

+            const transformedHistoricalValue = historicalValue
+              ? historicalValueTransformer(fieldDescriptor, historicalValue)
+              : null;
+


We shouldn't treat the historicalValue vs previousFieldValue as separate things, we should instead do something like:

const previousFieldValue = !isEmpty(fieldDescriptor.historicalExpression) ? evaluatePreviousValueFromHD(fieldDescriptor.historicalExpression) : pickPreviousValueFromEncounter(encounterContext.previousEncounter);

samuelmale · 2024-04-29T08:41:41Z

src/utils/common-expression-helpers.ts

@@ -444,6 +444,17 @@ export class CommonExpressionHelpers {
    return Promise.resolve(lazy);
  };
 }
+export class HistoricalDataSourceService {


Can we move this to its own file? I also feel like it's valid to move the expression runner components out of the utils to a new folder "expression-runner".

samuelmale · 2024-04-29T08:46:33Z

src/utils/expression-runner.ts

  // register dependencies
  findAndRegisterReferencedFields(node, parts, fields);
+  //adds values to the determinants


Do you need this comment?

Will resolve

…e-lib into hxp-test

…ngine-lib into hxp-test

CynthiaKamau · 2024-05-08T06:28:27Z

@arodidev is it possible to move this from draft?

samuelmale

@arodidev can you resolve the merge conflicts?

samuelmale · 2024-05-08T21:45:22Z

src/components/inputs/multi-select/multi-select.component.tsx

+    if (!isEmpty(previousValue)) {
+      if (Array.isArray(previousValue)) {
+        const valuesToSet = previousValue.map((eachItem) => eachItem.value);
+        setFieldValue(question.id, valuesToSet);
+        onChange(question.id, valuesToSet, setErrors, setWarnings);
+        question.value = handler?.handleFieldSubmission(question, valuesToSet, encounterContext);
+      } else {
+        const valueToSet = [previousValue.value];
+        setFieldValue(question.id, valueToSet);
+        onChange(question.id, valueToSet, setErrors, setWarnings);
+        question.value = handler?.handleFieldSubmission(question, valueToSet, encounterContext);
+      }


To prevent the code duplication here, I would simply:

if (!isEmpty(previousValue) { const previousValues = Array.isArray(previousValue) ? previousValue.map(item => item.value) : [previousValue.value]; // .... }

src/components/section/form-section.component.tsx

samuelmale · 2024-05-08T22:08:24Z

src/components/section/helpers.ts

    default:
      return value?.display;
  }
 };
+
+export const historicalValueTransformer = (field: FormField, obs: OpenmrsObs) => {


const refineHistoricalValue = (field: FormField, value: OpenmrsObs) => {

OR

const extractObsValueAndDisplay = (field: FormField, obs: OpenmrsObs) => {

samuelmale · 2024-05-08T22:18:51Z

src/components/section/helpers.ts

      return formatDate(new Date(value?.display)) || formatDate(value?.value);
    case 'checkbox':
-      return Array.isArray(value) ? previousValueDisplayForCheckbox(value) : null;
+      return Array.isArray(value) ? previousValueDisplayForCheckbox(value) : value.display;


Is it possible to receive a null or undefined argument for value?

No it is not. The following evaluations need to be truthy before the component is called.
encounterContext?.previousEncounter && (previousFieldValue || historicalValue) && (isTrue(fieldDescriptor.questionOptions.enablePreviousValue) || fieldDescriptor.historicalExpression)Ï

samuelmale · 2024-05-08T22:20:52Z

src/utils/historical-data-source.ts

@@ -0,0 +1,11 @@
+export class HistoricalDataSourceService {


It feels wrong having a service live under "utils", since it's Datasource related, I would rather move this to the "datasource" folder.

arodidev · 2024-05-09T18:12:19Z

@samuelmale All comments on this PR have been resolved, I beleive it is ready to be merged.

samuelmale

LGTM

* Initial commit(will remove the date bug fix) * cleanup * Getting the HD object to evaluate tt t * aligning Hxp with previous value * more cleanup * resolving logic * revamped transformer, migrated historical data source class * combine hxp and previous value logic * Added tests * PR resolutions * Fix CI build failure

arodidev added 5 commits April 25, 2024 10:47

Initial commit(will remove the date bug fix)

9f695bf

cleanup

0ddccb8

Getting the HD object to evaluate

7ba63f5

tt t

aligning Hxp with previous value

057eeb6

more cleanup

fefff75

resolving logic

275d38b

arodidev changed the title ~~(feat) adding evaluated historical expression support~~ (feat) O3-3049: adding evaluated historical expression support Apr 26, 2024

Merge branch 'main' into hxp-test

1ee3ec3

samuelmale reviewed Apr 29, 2024

View reviewed changes

arodidev added 5 commits April 29, 2024 20:05

Merge branch 'openmrs:main' into main

c828283

Merge branch 'main' of https://github.com/arodidev/openmrs-form-engin…

946c057

…e-lib into hxp-test

revamped transformer, migrated historical data source class

3002ee9

Merge branch 'openmrs:main' into hxp-test

aac7853

Merge branch 'hxp-test' of https://github.com/arodidev/openmrs-form-e…

7d2f202

…ngine-lib into hxp-test

arodidev marked this pull request as draft May 7, 2024 18:38

combine hxp and previous value logic

34775a0

Added tests

a9ca725

arodidev marked this pull request as ready for review May 8, 2024 19:07

arodidev requested review from samuelmale and CynthiaKamau May 8, 2024 19:08

samuelmale requested a review from denniskigen May 8, 2024 21:28

samuelmale reviewed May 8, 2024

View reviewed changes

arodidev added 3 commits May 9, 2024 09:18

Merge branch 'main' into hxp-test

4c35011

PR resolutions

adc92b7

Merge branch 'main' into hxp-test

8a023f5

Fix CI build failure

acee939

samuelmale approved these changes May 9, 2024

View reviewed changes

samuelmale merged commit 30cf1b1 into openmrs:main May 9, 2024
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(feat) O3-3049: adding evaluated historical expression support #226

(feat) O3-3049: adding evaluated historical expression support #226

arodidev commented Apr 25, 2024 •

edited

Loading

arodidev commented Apr 25, 2024 •

edited

Loading

samuelmale left a comment •

edited

Loading

samuelmale Apr 29, 2024

arodidev Apr 29, 2024

arodidev Apr 30, 2024

arodidev May 8, 2024

samuelmale Apr 29, 2024

samuelmale Apr 29, 2024

samuelmale Apr 29, 2024

arodidev Apr 29, 2024

CynthiaKamau commented May 8, 2024

samuelmale left a comment •

edited

Loading

samuelmale May 8, 2024

samuelmale May 8, 2024

samuelmale May 8, 2024

arodidev May 9, 2024

samuelmale May 8, 2024

arodidev commented May 9, 2024

samuelmale left a comment

(feat) O3-3049: adding evaluated historical expression support #226

(feat) O3-3049: adding evaluated historical expression support #226

Conversation

arodidev commented Apr 25, 2024 • edited Loading

Requirements

Summary

Screenshots

Related Issue

Other

arodidev commented Apr 25, 2024 • edited Loading

samuelmale left a comment • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CynthiaKamau commented May 8, 2024

samuelmale left a comment • 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

arodidev commented May 9, 2024

samuelmale left a comment

Choose a reason for hiding this comment

arodidev commented Apr 25, 2024 •

edited

Loading

arodidev commented Apr 25, 2024 •

edited

Loading

samuelmale left a comment •

edited

Loading

samuelmale left a comment •

edited

Loading