Update today XPath function to not return current time

[jkuester]

What feature do you want to improve?
Currently we have a custom implementation of the today functionality that is identical to what is returned by the now function (the current date and time in the current timezone).

Describe the improvement you'd like
As noted in this OpenRosa issue (enketo/enketo#901), this behavior does not seem to follow the ODK Spec. Unfortunately, the wording in the spec is a little confusing (to me at least) with regards to how it is actually supposed to behave. However, both openrosa and javarosa have coalesced on a single interpretation of the spec where now returns the current date and time while today returns the current date with the time set to midnight of the current timezone. I think we should remove our custom implementation of today and just use the one provided by OpenRosa.

As things stand today, we have a bunch of forms that are doing things like date(floor(decimal-date-time(today()))) to get the start of today. All this extra logic could be eliminated with the new implementation of today. Any logic that relies on the current time being included in today can just update to the spec-compliant now function.

As discussed in places like here, here, and here the Enketo uplift is already going to cause subtle behavior changes in the behavior of format-date and decimal-date (both frequently used along with today). Changing today at the same time as these other changes would allow projects to implement proper fixes to their date handling once and for all instead of breaking some things now with the Enketo uplift and again later if we decide to switch to a spec-compliant version of today.

Describe alternatives you've considered
One alternative would be to leave today alone and add a new custom function (e.g. today-date) that would return today's date without a time value. This would be more passive and backwards compatible, but less spec-compliant.

Additional context
This has been an ongoing saga in CHT code for years:

• Enketo: now() and today() give different results, specification says they should return identically-formatted results #1340
• XForm field with calculate="today()" gets wrong date in certain timezones #1878
• Incorrect XLSForm meta data values in Enketo #5768

#5831 was the PR that added the current custom functionality for today to cht-core.

The big difference now is that both openrosa and javarosa seem to have agreed on an implementation (that is different from what we have).

[garethbowen]

@abbyad I think what @jkuester has proposed is the right solution, but given this is such a long standing issue and that you were involved in the OG issue would you mind chiming in?

[abbyad]

I agree that we should align to the ODK XForm spec, and that this is likely the best time to do so. That said, this will need revision of nearly all forms that use dates, so we should do everything possible to help app builders convert and verify their forms.

[garethbowen]

@jkuester Thinking about 4.0.0 scheduling, are you planning to work on this? If so, when do you think you'll have time to get it ready?

[jkuester]

@garethbowen sorry for the late response here! My goal is to address this issue next week. Let me know if you think that will be too late on the 4.0.0 timeline.

[garethbowen]

@jkuester I think that'll be ok. archv3 AT will take some time. Please let me know if there's anything I can do to help make progress on this.

[jkuester]

@garethbowen as I go though our default/standard config and evaluate if anything needs changed, I just wanted to run my criteria by you and see if it sounds correct. Mostly it is pretty simple. For each place we are currently using today:

• If we do not actually care about the time value, keep using today
• If we do care about time, switch to use now

Where things become a bit complicated is when we have places where we are currently using today along with floor. (e.g. date-time(floor(decimal-date-time(today())) + 3) for setting a due date in 3 days.) With the new changes to the today function that remove the time data and only return today's date at midnight in the current timezone, I initially thought I could just replace the previous statement with date-time(decimal-date-time(today()) + 3) and the floor call would no longer be necessary. However, I realized that there is a subtle difference in behavior. (I will use now in the following example as a stand-in for the old behavior of today just to avoid confusion between the old today and the new one.) Since decimal-date-time returns the days since the Unix epoch (starting at midnight UTC) floor(decimal-date-time(now())) will actually return the days since the Unix epoch until today's date at midnight UTC. decimal-date-time(today()), on the other hand, will return the days since the Unix epoch until today's date at midnight in the current timezone.

It seems like the new behavior of decimal-date-time(today()) is what we would actually want in most cases. The above due date example would become date-time(decimal-date-time(today()) + 3) (or even better, with the new add-date function it would be add-date(today(), 0, 0, 3)) which would return the date, 3 days from now, at midnight in the current timezone. Let me know if this all sounds correct to you, or if you think we need to handle these combinations of floor and today in a different way!

[jkuester]

This is ready for AT on https://github.com/medic/cht-core/tree/7731_refactor_today

Basically the behavior of the today XPath function, used in the form config, has changed so that it only returns the date at midnight in the current timezone instead of also returning the current time. This is a non-passive change. To account for this change, I have updated our default/standard forms that use the today function.

So, the main thing to test here is that all of the forms listed below still behave exactly like they do in master. I have listed all the fields on each for that were touched and if it was the constraint or the calculate that was edited. Where the constraint was edited, it should be enough to validate that the range of values you can pick remains the same.

Edited forms:

Default config

• app:
  • death_report
    • date_of_death - constraint
  • delivery
    • t_danger_signs_referral_follow_up_date - calculate
    • woman_death_date - constraint
    • delivery_date - constraint
    • baby_death_date - constraint
    • t_danger_signs_referral_follow_up_date - calculate
    • pnc_visits/days - calculate
  • pnc_danger_sign_follow_up_baby
    • t_danger_signs_referral_follow_up_date - calculate
  • pnc_danger_sign_follow_up_mother
    • t_danger_signs_referral_follow_up_date - calculate
  • pregnancy
    • t_danger_signs_referral_follow_up_date - calculate
    • method_lmp/u_lmp_date - constraint
    • method_edd/u_edd - constraint
    • g_lmp_date_8601 - calculate
    • anc_visits_hf_past/visited_dates_group/visited_date_single - constraint
    • anc_visits_hf_past/visited_dates_group/visited_dates/visited_date - constraint
    • anc_visits_hf_next/anc_next_visit_date/appointment_date - constraint
    • next_visit_weeks - calculate
  • pregnancy_danger_sign
    • t_danger_signs_referral_follow_up_date - calculate
  • pregnancy_danger_sign_follow_up
    • t_danger_signs_referral_follow_up_date - calculate
  • pregnancy_home_visit
    • days_since_lmp - calculate
    • t_danger_signs_referral_follow_up_date - calculate
    • weeks_since_lmp_rounded_ctx - calculate
    • miscarriage_date - constraint
    • abortion_date - constraint
    • update_g_age/update_method/u_edd_new - constraint
    • update_g_age/lmp_date_8601_new - calculate
    • anc_visits_hf_past/visited_date_single - constraint
    • anc_visits_hf_past/visited_dates/visited_date - constraint
    • anc_next_visit_date/appointment_date - constraint
    • next_visit_weeks - calculate
• contact:
  • clinic-create
    • dob_calendar - constraint
  • distric_hospital-create
    • dob_calendar - constraint
  • health_center-create
    • dob_calendar - constraint
  • person-create
    • dob_calendar - constraint
  • person-edit
    • dob_calendar - constraint

Standard config

• app:
  • death_confirmation
    • date_of_death - constraint
  • delivery
    • g_birth_date - constraint
  • pregnancy
    • g_lmp_calendar - constraint
• collect:
  • child
    • birth_date - constraint
• contact:
  • person-create
    • date_of_birth - constraint
  • person-edit
    • date_of_birth - constraint

[tatilepizs]

Thank you very much @jkuester for all the details about the changes, this is really helpful because that way I know where to focus the testing.

Using cht core version 7731_refactor_today all the forms with changes worked as expected.

Tested forms for default config:

• district_hospital-create
• district_hospital-edit
• health_center-create
• health_center-edit
• clinic-create
• clinic-edit
• person-create
• person-edit
• pregnancy
  • with previous visits
  • with future visits
• pregnancy_home_visit
  • with miscarriage
  • with abortion
  • with still pregnant
• pregnancy_danger_sign
• delivery
  • with mother alive and well
  • with mother dead
  • with child alive and well
  • with child dead
• pnc_danger_sign_follow_up_mother
• t_danger_signs_referral_follow_up_date
• death_report

Tested forms for standard config:

• district_hospital-create
• district_hospital-edit
• health_center-create
• health_center-edit
• clinic-create
• clinic-edit
• person-create
• person-edit
• pregnancy
• delivery
• death_confirmation

[garethbowen]

@jkuester I know I'm late to the party but I agree with your solution WRT timezones. Thanks!