(fix) O3-2252: Calculated values shouldn't be overwritten by the Encounter's values #318
Conversation
|
Size Change: -245 kB (-19.5%) 🎉 Total Size: 1.01 MB
ℹ️ View Unchanged
|
@CynthiaKamau Please strive for greater clarity in PR summaries. "This PR addresses the issue of null/undefined values on calculated fields in edit mode" what is the issue in question? It is not obvious what "null/undefined values on calculated fields in edit mode" means or why it would be a problem. And the phrase "The encounter value and calculated value are the same, if different" doesn't make sense. Thanks for your work on this; I hope this is useful feedback. |
|
@CynthiaKamau Are the next steps clear to you? Any blockers? |
I will update the description.Thanks for the feedback |
|
@samuelmale do you still have any reservations on this ? |
There was a problem hiding this comment.
Nice work @CynthiaKamau, just a couple of requests:
- For cases where we have an encounter value but the value from evaluating the calculate expression is null/undefined, can we fallback to the encounter value?
- Can we have a test case scenario asserting the above?
- Can we have a test case scenario with an async calculate expressions?
CynthiaKamau
force-pushed
the
O3-2252
branch
2 times, most recently
from
297600c
to
49bd386
Compare
| const encounterValue = formFieldHandlers[field.type]?.getInitialValue( | ||
| encounter, | ||
| field, | ||
| formFields, | ||
| encounterContext, | ||
| ); | ||
| formFieldHandlers[field.type].handleFieldSubmission(field, encounterValue, encounterContext); |
There was a problem hiding this comment.
So we are only falling back to the encounter's value when we run into an error?
src/hooks/useInitialValues.test.ts
Outdated
| }, | ||
| id: 'latest_mother_hiv_status', | ||
| }; | ||
| allFormFields.push(fieldWithCalculateExpression); |
There was a problem hiding this comment.
This mutates the object at the top level of this test. This will lead to behavior that varies depending on which tests are run, or the order in which they are run. Objects at the top level of a test file should be considered immutable. Use the spread syntax to define new form fields within specific tests.
|
|
||
| it('should fall back to encounter value if the calculated expression result is null or undefined', async () => { | ||
| let hook = null; | ||
| let formFields: Array<FormField> = [ |
There was a problem hiding this comment.
This looks very similar to the other formFields variable. It would be easier to understand what the difference is if you defined this at the top level of the test file and then used it in each test file where it is needed, building new modified copies using the object spread syntax as necessary.
brandones
changed the title
… values
Requirements
Summary
This PR resolves the issue of null or undefined initial values for calculated fields when entering edit mode. Previously, the assumption was that all calculated fields would be retrieved through asynchronous functions from the expression runner. This assumption led to problems when synchronous functions were used, resulting in null or undefined initial values.
Screenshots
https://www.loom.com/share/866edea9d4c34b91ab4ef3ef93eba49c?sid=7ffa7c0d-58cd-42e2-a30f-64d4448462d7
Related Issue
Other