Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Empty integer values in Enketo forms evaluate to 0 #7222

Closed
jkuester opened this issue Jul 29, 2021 · 5 comments · Fixed by #7256
Closed

Empty integer values in Enketo forms evaluate to 0 #7222

jkuester opened this issue Jul 29, 2021 · 5 comments · Fixed by #7256
Assignees
Labels
Enketo Affects Enketo forms Type: Bug Fix something that isn't working as intended
Milestone

Comments

@jkuester
Copy link
Contributor

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 #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 jkuester added the Type: Bug Fix something that isn't working as intended label Jul 29, 2021
@jkuester jkuester added the Enketo Affects Enketo forms label Jan 14, 2022
@jkuester
Copy link
Contributor Author

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

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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 jkuester self-assigned this May 31, 2022
@jkuester jkuester added this to the 4.0.0 milestone May 31, 2022
@jkuester
Copy link
Contributor Author

@medic/quality-assurance this is another issue that should be resolved by the Enketo uplift: #7256

@garethbowen garethbowen moved this to Ready for AT in Product Team Activities Jul 31, 2022
@tatilepizs tatilepizs self-assigned this Aug 11, 2022
@tatilepizs tatilepizs moved this from Ready for AT to AT in Progress in Product Team Activities Aug 11, 2022
@tatilepizs
Copy link
Contributor

tatilepizs commented Aug 11, 2022

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.
Screen.Recording.2022-08-11.at.2.25.10.PM.mov

Fixed on 6345-enketo-uplift

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

Test video.
Screen.Recording.2022-08-11.at.2.31.26.PM.mov

@tatilepizs tatilepizs moved this from AT in Progress to Ready to Merge in Product Team Activities Aug 11, 2022
Repository owner moved this from Ready to Merge to Done in Product Team Activities Aug 16, 2022
@garethbowen garethbowen changed the title ODK Form Calculations: empty integer evaluates to 0 Empty integer values in Enketo forms evaluate to 0 Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enketo Affects Enketo forms Type: Bug Fix something that isn't working as intended
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants