cht-core icon indicating copy to clipboard operation
cht-core copied to clipboard

Update to latest enketo-core

Open garethbowen opened this issue 4 years ago • 111 comments

The next time we bump our dependency on enketo-core we will have to switch api to depend on enketo-transformer instead of the archived enketo-xslt. The transformer depends on the version of libxslt which works on node 12 so we could consider swapping to use that instead of the native xsltproc process. Additionally we may be able to use the transformer for more of the work that we do ourselves in the generate-xform service in API.

Summary of AT Issues:

  • [x] 1. Default config. Form validation error needs to be specific when a phone number has been taken/used
    • Existing issue, not caused by Enketo uplift (comment)
    • Mentioned in a comment on https://github.com/medic/cht-core/issues/6390
  • [x] 2. Default config. Client-side phone validation does not 'complain' where some characters are added to a 'valid' phone number
    • Existing behavior, not caused by Enketo uplift (comment)
    • Seems like this might actually be the desired behavior
  • [x] 3. Default config. Custom place name label not showing on create/edit place (this is also the same on master)
    • Existing issue, not caused by Enketo uplift
    • Logged https://github.com/medic/cht-core/issues/7478 and this should be fixed in https://github.com/medic/cht-core/pull/7479
  • [x] 4. Default config. When adding/editing a person's DOB, if you check Date of birth unknown and then do not enter a value for Months then the value stored for the person's date_of_birth is an "Invalid Date"
    • Behavior change triggered by upgrading our version of openrosa-xpath-evaluator (comment)
    • We inadvertently fixed https://github.com/medic/cht-core/issues/7222
    • ~~Proposal is to use a patch to revert behavior change for now (and wait for a major version bump)~~
    • Decided to keep behavior. Have updated the configs (including the person add/edit forms) to properly handle the behavior change. (comment)
  • [x] 5. Default config. Date picker pop up does not close when clicking outside of it. Only when pressing tab key.
    • Behavior changed. Now clicking anywhere within the date question's div will open the widget (comment). Need to decide if this is acceptable behavior...
  • [x] 6. Default config. Also, date picker is already open when entering the Pregnancy registration form, when selecting Last menstrual period (LMP) or Expected date of delivery (EDD)
    • More generally, questions that are not part of a field-list group are not being automatically selected when the page is opened
    • Caused by a code change in enketo-core to deliberately focus on the first element on the page (comment)
    • I have updated the android-widget to prevent this from triggering text input (and on-screen keyboard) when a date question is focused on (the widget, itself, is not automatically opened by the focus).
  • [x] 7. Default config. In the Pregnancy registration form, ANC visits at Health Facility (past) input, if you type a big number, the page completely freeze)
    • Existing behavior, not caused by Enketo uplift
    • The big number triggers a bunch of entries in the repeat so technically this is expected behavior that we probably do not need to worry about (comment)
  • [x] 8. Default config. In the Gestational Age section and and ANC Visit at Health Facility (Scheduled) section, from the Pregnancy Registration, all the information presented is in bold.
    • Layout changed due to a change in the css. The css has been updated so that notes are not bold. (comment)
  • [x] 9. Default config. When completing a delivery, the child registered during birth is not created.
    • Caused by behavior chance in Enketo form validation that caused inputs to be cleared. Fixed by updating our logic for making inputs always relevant. (comment)
  • [x] 10. Default config. Date label not showing on ANC Visits at Health Facility (Scheduled) / Pregnancy registration
    • Existing behavior, not caused by Enketo uplift
    • This "works as expected" since the date field on the form in marked with NO_LABEL (comment)
  • [x] 11. Default config. When a Pregnancy registration is submitted, and there are more than 4 ANC Visits registered, the report is is only showing the correct labels for the first four visits
    • Existing behavior, not caused by Enketo uplift
    • This "works as expected" since there are only translations for the first 4 past ANC visits (comment)
  • [x] 12. Default config. On enketo branch with default config pregnancy_home_visit form throwing an error when trying to open
    • Initial error loading the form caused by config code in the form that is no longer supported. I updated the form to properly pass one parameter to the decimal-date-time function.
    • Additionally there were issues loading the external contact-summary data into the form which required code changes (comment)
  • [x] 13. Default config. When a Health facility ANC reminder is created, the report is showing an Invalid Date value for the Pregnancy visit task date label.
    • This appears to be fixed now as I cannot recreate it. I expect the issue was caused by the problems loading the contact-summary that were fixed in #12 (comment)
  • [x] 14. Default config. When completing a delivery, the child death registered during birth is not created.(related to #9)
    • This appears to be fixed now as I cannot recreate it. I expect the issue was fixed by changes related to #9.
  • [x] 15. Standard config. After completing the child_health_registration form, on the Area detail view, the report generated is displayed showing an id instead of the child's name.
    • Caused by an issue in the code for making the inputs group relevant. Fixed now. (comment)
  • [x] 16. Standard config. Adding to #15, the report generated for the Child Health Registration is not showing the correct information .
    • Caused by same issue with inputs group as #15. Fixed by same changes.
  • [x] 17. Standard config. After creating an Immunization Visit the report say that Baby did not attend their immunizations visit, and the Vaccines that I chosen (Hepatitis A and B) are marked as No.
    • Caused by change in Enketo behavior for non-relevant fields with default behaviors. Refactored form logic to fix behavior. (comment)
  • [x] 18. Standard config. Nutrition Treatment Exit report is showing long labels for the collected information. For example, report.nutrition_exit.patient_name instead of Name. This is also the same on master.
    • Existing behavior not caused by Enketo uplift. Similar to #11, the "long labels" are shown for this form because there is no provided translation.
  • [x] 19 Standard config. Child Nutrition Screening is presenting #15, #16, and #18. The difference is that this is happening also in master.
    • Existing issue, not caused by Enketo uplift. (comment)
    • I have included this in https://github.com/medic/cht-core/issues/7478 and fixed it in https://github.com/medic/cht-core/pull/7479
  • [x] 20 Standard config. Pregnancy Form displays a user icon and warning icon.
    • Caused by a change somewhere in the processing of embedded HTML where invalid whitespace is no longer supported. (comment)
    • I have updated the logic for parsing the embedded HTML to clear out any invalid whitespace
  • [x] 21 Standard config. Pregnancy Form doesn't display summary information on submit step.
    • Caused by same issue as #21. Fixed by same solution.
  • [x] 22 Standard config. The report generated by the pregnancy form do not match the correct person.
    • This seems to be working now since. I expect it was caused by the same issue as #9 and/or #15.
  • [x] 23 Standard config. Adding to #22. The report generated by the delivery form do not match the correct person.
    • This seems to be working now since. I expect it was caused by the same issue as #9 and/or #15.
  • [x] 24 Standard config. The analytics related to the delivery are not updated.
    • This seems to be working now since. I expect it was caused by the same issue as #15.
  • [x] 25 Standard config. After submitting nutrition_followup form, several things break.
    • Caused by same issue as #17 where invalid data was being saved for forms. Fixed by updating the nutrition_followup form. (contact)
  • [x] 26 Standard config. Postnatal Care Visit and Pregnancy Visit reports are not showing Danger Signs.
    • Caused by same issue as #17 where invalid data was being saved for forms. Fixed by updating the forms.
  • [x] 27 Standard config. First report in the list has to be deleted twice to actually be deleted. This is also the same on master.
    • Not a real issue, it was a sync problem
  • [x] 28 Muso config. Unable to install config-muso on an instance running 6345-enketo-uplift.
    • I am not able to recreate this issue. It may have been fixed by one of the previous changes, or we may need to see if there is some difference in our setup.
  • [x] 29 Brac config. Creating a CHP Area or a Family, the error message for mobile number format is not displayed. This is present in master and 6345-enketo-uplift.
    • Existing issue, not caused by Enketo uplift. Some form configuration is missing and I have logged a separate issue on the config-brac repo to address it. (comment)
  • [x] 30 Brac config. Unable to edit a person's information.
    • Caused by the custom phone-widget being too broadly applied now. Fixed by making the selector for that widget more precise. (comment)
  • [x] 31 Brac config. Unable to load pregnancy follow up form from task tab.
    • Caused by behavior change in the new Enketo/Openrosa xPath logic to be more stringent with regards to mal-formed expressions.
    • The pregnancy_visit form will need to be updated. I have logged: https://github.com/medic/config-brac/issues/436
  • [x] 32 Brac config. Target tab is not showing any information (one pregnancy was registered).
    • Caused (mostly?) by a change to the format-date function behavior. When the pregnancy form submits with valid data, this problem does not present. (comment)
    • ~~We still need to land on a long-term fix for this. Waiting on https://github.com/enketo/openrosa-xpath-evaluator/issues/154~~
      • In the meantime I have added a custom format-date implementation to the branch that reverts to the old behavior. Going forwards we can consider keeping these changes or reverting them in favor of something else.
    • Our change to openrosa was accepted! I have pulled in the latest version of that library so that we should have the original behavior for format-date
  • [x] 33 Brac config. After submitting the pregnancy_referral_follow_up the task was not deleted and it throws an error.
    • I believe this was caused by invalid data created as a result of #32. Once I addressed those issues, this problem no longer presents.
  • [x] 34 Brac config. Household Equity survey task is not being generated when a new family is created.
    • This problem is no longer presenting for me. I am honestly not sure which changes may have fixed it (maybe related to #15?), but it seems to be working fine now.
  • [x] 35 Brac config. After creating a FP Visit, in master, the total number of visits was increment to 1, but in enketo branch the total number remained in 0.
    • As with #34,this problem is no longer presenting, but I am not sure exactly why or what was causing it in the first place (also maybe #15?).

garethbowen avatar Apr 07 '20 04:04 garethbowen

Moved to 3.12.0 to free up engineers to work on 3.11.0.

garethbowen avatar May 26 '20 00:05 garethbowen

This is blocked on updating to 5.9.2 first: https://github.com/medic/cht-core/issues/4386

garethbowen avatar Jul 23 '20 20:07 garethbowen

@garethbowen, I wanted to know if CHT has the capability to track a CHV's visit length in XLSForm. I checked at the documentation and I found that the the user_edit_time in this link mentions Enketo, and the XLSForm spec has astart/endproperty (http://xlsform.org/en/#metadata), but due to an Enketo or CHT bug/issue that was not usable in CHT XLSForms.

Would like to know when it will be fixed.

iesmail-znz avatar Sep 30 '20 09:09 iesmail-znz

Hi @iesmail-znz, the start and end meta fields should be available since 3.7. What problems are you currently seeing trying to use them? If they aren't working as expected it would be worth opening a bug report with details as a new issue.

On a related note, I see documentation on those fields in our old (and now archived) docs site, but can't find it in the new CHT docs site. I will port that over now.

abbyad avatar Oct 05 '20 15:10 abbyad

bumping to release following completion of https://github.com/medic/cht-core/issues/4386

MaxDiz avatar Oct 15 '20 13:10 MaxDiz

Adding to the App Builders backlog too since this is needed to use supported ODK XPath functions in CHT apps.

Edit: added link to Forum post.

abbyad avatar Jan 06 '21 20:01 abbyad

@garethbowen a couple of follow-up questions from the roadmap planning meeting:

  • Is there a QA plan to test forms?
  • Based on the QA plan, would it make more sense to upgrade to enketo latest in a single release (ie combine with improvements scheduled in 3.12 - https://github.com/medic/cht-core/issues/4386 - the original plan, I believe)?

MaxDiz avatar Jan 21 '21 20:01 MaxDiz

Many forms are tested as part of the standard release test plan. It would be very useful to test a wider range of real world forms as well and that plan should be developed when this issue is being QAed. If time permits this test plan should be automated because we usually update enketo-core every release and we need to catch regressions early.

The reason there are two very similar looking issues is we've made good progress towards the other one, so it should be relatively easy from a development perspective to finish that off ready for AT. This issue would then be a separate body of work which may be trivial or highly difficult depending on what breaking changes exist in the later versions of enketo-core. Obviously the two issues can only be developed one at a time but they could be ATed together to save time. In short I'm happy for this issue to be scheduled in 3.12 to reduce testing effort, but we should keep the issues separate so we can ship the other one, and postpone this one if it turns out to be difficult.

garethbowen avatar Jan 24 '21 22:01 garethbowen

ok, moving to v3.12 to align with #4386

MaxDiz avatar Jan 25 '21 13:01 MaxDiz

Maybe this can be done after this update: https://github.com/medic/angular10-migration/issues/22 while working on new Enketo widgets I see the opportunity of extracting common logic into a utils lib or parent class, example: getting, setting fields, constructors, etc.

latin-panda avatar Mar 30 '21 05:03 latin-panda

Possible duplicate: https://github.com/medic/cht-core/issues/4386 ?

latin-panda avatar Jun 30 '21 05:06 latin-panda

Not exactly a duplicate, but there's a lot of history here. That issue had a lot of work done on it, even having a PR submitted. Unfortunately the PR was a long way away from being able to be merged, so it got delayed. Now that master has moved on the PR is even further away from being able to be merged, so perhaps we're now at the point of starting from scratch, so I'll close the other issue and we can just move forward with this one.

When picking this issue up refer to this PR for reference, and choose whether to revive it or start a new PR from scratch.

garethbowen avatar Jun 30 '21 15:06 garethbowen

For the record, I think we are going to have to resolve this issue https://github.com/medic/cht-core/issues/4517 as a part of the enketo uplift.

jkuester avatar Aug 17 '21 16:08 jkuester

@medic/quality-assurance Ready for AT on https://github.com/medic/cht-core/tree/6345-enketo-uplift

jkuester avatar Jan 10 '22 14:01 jkuester

@medic/quality-assurance just wanted to add a few notes regarding what to look out for on this issue. Basically the code changes for this issue affected every part of filling out forms (both contact and app forms). So everything from the form loading/saving to how the questions are displayed to the actual processing of form logic (as well as all the different widgets) is updated. But, the goal is for everything to work exactly the same as before. The notable features added in these changes are support for more Enketo widgets and xPath functions (complete lists of these, along with more info on the actual code changes, can be found in the PR description: https://github.com/medic/cht-core/pull/7256).

There is also a new test form available that contains sample questions for all the various Enketo widget that we support now.

And finally, it may be simpler to think about these changes in terms of what is not affected... There changes should not affect anything having to do with targets or tasks, syncing Pouch/CouchDB, user management or system administration of any kind.

jkuester avatar Jan 12 '22 23:01 jkuester

Thanks @jkuester . Just wondering... in cht-conf, we have a validate-form library, which also probably points back to validateXml in api. Do you think these two have been affected and might need a retouch as well?

ngaruko avatar Jan 13 '22 02:01 ngaruko

Good call @ngaruko! The answer is yes, this workflow has also been affected by the upgrade. Once again, we do not intend for there to be any behavior changes here, but the underlying code has been modified to use newer versions of enketo libraries.

jkuester avatar Jan 13 '22 17:01 jkuester

We will dump all issues/observations here - and raise separate issues later, if necessary. (some observations might be unrelated to this PR)

  1. Form validation error needs to be specific when a phone number has been taken/used image 2 . Client-side phone validation does not 'complain' where some characters are added to a 'valid' phone number image

ngaruko avatar Jan 17 '22 01:01 ngaruko

  1. Custom place name label not showing on create/edit place (this is also the same on master) image

ngaruko avatar Jan 17 '22 01:01 ngaruko

  1. On enketo branch with default config, when adding/editing a person's DOB, if you check Date of birth unknown and then do not enter a value for Months: Captura de Pantalla 2022-01-17 a la(s) 18 17 26

Then the value stored for the person's date_of_birth is an "Invalid Date": Captura de Pantalla 2022-01-17 a la(s) 18 21 05

lorerod avatar Jan 17 '22 21:01 lorerod

  1. Date picker pop up does not close when clicking outside of it. Only when pressing tab key. (Tested on Create/edit contact, Pregnancy registration, Delivery registration)
  • On branch: https://user-images.githubusercontent.com/21312057/149853096-d13d0af1-64d1-4424-b69c-091bc8424785.mov

  • On master: https://user-images.githubusercontent.com/21312057/149853117-11f8f208-4ac2-49a6-bfad-554f4c7c25b4.mov

  1. Also, date picker is already open when entering the Pregnancy registration form, when selecting Last menstrual period (LMP) or Expected date of delivery (EDD) (not happening on master): https://user-images.githubusercontent.com/21312057/149978840-b016d419-d955-4d32-835c-4fa847976557.mov

lorerod avatar Jan 18 '22 01:01 lorerod

  1. When adding/editing a person's DOB, if you check Date of birth unknown and then do not enter a value for Months: Captura de Pantalla 2022-01-17 a la(s) 18 17 26

Then the value stored for the person's date_of_birth is an "Invalid Date": Captura de Pantalla 2022-01-17 a la(s) 18 21 05

Did you wanna have a look at this @jkuester ?

ngaruko avatar Jan 18 '22 03:01 ngaruko

It turns out the birth date issue (when the months are not filled out) is being causes by the fact that we inadvertently "fixed" this issue: https://github.com/medic/cht-core/issues/7222 (and now an empty value for months is not properly handled by the form logic...)

This could be a show stopper for getting these changes into a minor release, but I am currently investigating the possibility of recreating the old behavior in the new Enketo logic.

jkuester avatar Jan 18 '22 15:01 jkuester

  1. In the Pregnancy registration form, ANC visits at Health Facility (past) input, if you type a big number, the page completely freeze, there is not error in console, it is just freeze. I had to refresh the browser. This is happening in master and in 6345-enketo-uplift
  • https://user-images.githubusercontent.com/94494491/149972491-d1394ab5-694a-423f-909e-fd665696753a.mov

Note: This also happens in the Delivery registration form.

tatilepizs avatar Jan 18 '22 15:01 tatilepizs

  1. In the Gestational Age section and and ANC Visit at Health Facility (Scheduled) section, from the Pregnancy Registration, all the information presented is in bold. This is not an impediment, but I am mentioning it because in master the dates that need to be verified are highlighted, and in 6345-enketo-uplift everything look exactly the same.
  • Master image Captura de Pantalla 2022-01-18 a la(s) 17 07 18

  • 6345-enketo-uplift image Captura de Pantalla 2022-01-18 a la(s) 17 06 50

tatilepizs avatar Jan 18 '22 16:01 tatilepizs

  1. On 6345-enketo-uplift branch using default config. When completing a delivery:
  • The child registered during birth is not created

Test Steps: Using a woman of child bearing age that is pregnant and has a delivery date in the next few days. Process a delivery with a live child and facility birth.

Expected Result:

  • [X] The report shows associated to the person.
  • [X] The past pregnancy card shows.
  • [ ] The child registered during birth is created and displays the proper information.
  • [X] The targets page for active pregnancies, live births is updated, and in-facility deliveries

https://user-images.githubusercontent.com/21312057/150019380-9b31050b-4582-455d-ba5c-69fcb9421048.mov

lorerod avatar Jan 18 '22 21:01 lorerod

  1. Date label not showing on ANC Visits at Health Facility (Scheduled) / Pregnancy registration (This is happening in master and in 6345-enketo-uplift)

image

tatilepizs avatar Jan 19 '22 14:01 tatilepizs

  1. When a Pregnancy registration is submitted, and there are more than 4 ANC Visits registered, the report is is only showing the correct labels for the first four visits. (This is happening in master and in 6345-enketo-uplift)
  • Pregnancy with 4 or less visits: image

  • Pregnancy with more than 4 visits image

tatilepizs avatar Jan 19 '22 15:01 tatilepizs

  1. On enketo branch with default config pregnancy_home_visit form throwing an error when trying to open:
Error Log
Error loading form Error: ["FormLogicError: Could not evaluate: if(selected(../pregnancy_summary/g_age_correct, 'no'), ../update_g_age/edd_8601_new, format-date-time(date-time(decimal-date-time(../context_vars/lmp_date_8601_ctx, 280)), \"%Y-%m-%d\")), message: too many args"]
    at :8444/main.js?_sw-precache=f1346a5eea83770e18b0ec7215d70d8d:1:3157214
    at x.value (:8444/polyfills.js?_sw-precache=3248789045bd0746fd35f1328ccaeccc:1:22878)
    at x.value (:8444/polyfills.js?_sw-precache=3248789045bd0746fd35f1328ccaeccc:1:16935)
    at :8444/polyfills.js?_sw-precache=3248789045bd0746fd35f1328ccaeccc:1:33762
    at x.value (:8444/polyfills.js?_sw-precache=3248789045bd0746fd35f1328ccaeccc:1:23570)
    at x.value (:8444/polyfills.js?_sw-precache=3248789045bd0746fd35f1328ccaeccc:1:17742)
    at b (:8444/polyfills.js?_sw-precache=3248789045bd0746fd35f1328ccaeccc:1:25889)
    at x.value [as invoke] (:8444/polyfills.js?_sw-precache=3248789045bd0746fd35f1328ccaeccc:1:25558)
    at M (:8444/polyfills.js?_sw-precache=3248789045bd0746fd35f1328ccaeccc:1:37752)
    at IDBTransaction.L (:8444/polyfills.js?_sw-precache=3248789045bd0746fd35f1328ccaeccc:1:37998)
(anonymous) @ main.js?_sw-precache=f1346a5eea83770e18b0ec7215d70d8d:1

lorerod avatar Jan 19 '22 16:01 lorerod

  1. When a Health facility ANC reminder is created, the report is showing an Invalid Date value for the Pregnancy visit task date label.
  • 6345-enketo-uplift image

  • master image

tatilepizs avatar Jan 19 '22 17:01 tatilepizs