search

medic / cht-core

The CHT Core Framework makes it faster to build responsive, offline-first digital health apps that equip health workers to provide better care in their communities. It is a central resource of the Community Health Toolkit.
https://communityhealthtoolkit.org
GNU Affero General Public License v3.0
439 stars 207 forks source link

Empty integer values in Enketo forms evaluate to 0 #7222

Closed jkuester closed 2 years ago

jkuester commented 3 years ago

Describe the bug

The ODK Documentation specifies:

Unanswered number questions are nil... The empty value will not be converted to zero. The result of a calculation including NaN will also be NaN.

When a form in cht-core is saved with an unanswered number question, the value for that question is properly recorded as ''. However, if a calculation is performed in the form that references the unanswered number question, the value for the question will be treated as 0 instead of NaN. So, if you have some other form element that is based on a calculation involving the number question, it will behave unexpectedly.

To Reproduce

type name label relevant
integer my_number My Number  
note my_note This is shown if number < 10 ${my_number} < 10

Steps to reproduce the behavior:

  1. Deploy the test_form.xlsx to a CHT instance
  2. Open the form
  3. The label This is shown if number < 10 is displayed even though there is no value for My Number.

Expected behavior

The calculation behavior should match what is documented for ODK (unanswered number questions are nil). Notably, if I upload that same test_form to https://getodk.org/xlsform/ it behaves as expected and the note is not shown unless a value < 10 is actually entered for My Number.

Additional context

Originally posted to the CHT forum.

It seems possible that this is caused by cht-core using an older version of enketo-core and this issue may be fixed by https://github.com/medic/cht-core/issues/6345. However, I was not able to find any issues/documentation in the Enketo project around this issue to indicate it was something that they fixed.

jkuester commented 2 years ago

This issue is fixed in version 2.0.0 of https://github.com/enketo/openrosa-xpath-evaluator by this commit https://github.com/enketo/openrosa-xpath-evaluator/commit/02bb3c301e1c584db87ffdf736a65ea3d630f8e3.

Basically, in older versions of this code when performing a mathematical operation in a calculate the expression was evaluated by the JavaScript engine using the exact values from the nodes. This mean that unanswered questions (whose value is just an empty string) had their value directly included in the calculation and the JS engine just massaged everything so there was no error (e.g. 1 + '' just evaluates to 1).

However, in the newer versions of this code the values from the nodes (including unanswered questions) are first processed through a new asNumber function before the expression is evaluated by the JavaScript engine. That function handles converting unanswered questions (empty strings) to NaN).

jkuester commented 2 years ago

Just wanted to note that the ODK docs do describe how to properly deal with empty values in Math functions.

The TLDR is that anywhere that we are performing a calculation that involves the value from an integer or decimal question that is not required, and we want to assume 0 for the calculation, we will need to update the calculate to use the coalesce function. (e.g. ${potentially_empty_value} > 0 becomes coalesce(${potentially_empty_value}, 0) > 0

jkuester commented 2 years ago

This also applies to relevant logic!

One thing to note is that does not affect the behavior of statements like ${potentially_empty_value} != ''. That check will still properly return true anytime there is a value and false when the question has not been answered.

jkuester commented 2 years ago

@medic/quality-assurance this is another issue that should be resolved by the Enketo uplift: https://github.com/medic/cht-core/pull/7256

tatilepizs commented 2 years ago

Config: Default Environment: Local with docker helper script Platform: WebApp Browser: Chrome

File .xlsx used to test -> test_form.xlsx

Reproducible on Master

Using the test_form.xlsx file the message was displayed even when the field was empty.

Test video. [video](https://user-images.githubusercontent.com/94494491/184237509-d14c72de-dec9-431d-9dd0-cf8a541db959.mov)

Fixed on 6345-enketo-uplift

Using the test_form.xlsx file, the message was not displayed if the field was empty.

Test video. [video](https://user-images.githubusercontent.com/94494491/184237754-5b8a9f65-f8a8-450a-b401-1de661c9e3c3.mov)