(fix) O3-3001: Form engine shouldn't send encounterDatetime when saving a new form

[vasharma05]

Requirements

• This PR has a title that briefly describes the work done, including the ticket number if there is a ticket.
• My work conforms to the OpenMRS 3.0 Styleguide and design documentation.
• My work includes tests or is validated by existing tests.

Summary

Since we are facing issues with users not being able to save forms, in case their systems aren't/can't sync with the NTP servers.

If their computer has their time ahead with even 1~2 sec, they get the error from the BE that the encounter date-time is in the future.

Fix: We will not send the encounter date and time from the client's system and the server will add the date and time itself.

Screenshots

Before

Screen.Recording.2024-03-27.at.14.56.05.mov

After

Screen.Recording.2024-03-27.at.14.58.38.mov

Related Issue

https://issues.openmrs.org/browse/O3-3001

Other

None

[donaldkibet]

Thanks @vasharma05, this is much needed fix. Looking at it, we might also want to expand this to not include encounterDatetime set through the UI, if the currentDate is same as the encounterDatetime IMO this is to enable RDE when picking dates in the past cc @ojwanganto

[ojwanganto]

Thanks @vasharma05 and @donaldkibet. We have, indeed, encountered this a number of times in our deployments and am happy to see a fix. Adding to @donaldkibet's point, how do we want to handle encounter date on the UI such that we can also support RDE? I wish there was an intelligent way of determining this in-prior to help add or exclude encounter datetime from the payload.

[mks-d]

@vasharma05 no unit tests? Given the critically of the issues that have arisen around this, it would definitely be good to state clearly through unit tests what the expected outcomes are in different situations... (when the encounter datetime is provided, when the encounter datetime isn't provided, etc).

Cc @rbuisson

[mks-d]

Surely this is a leftover?

[icrc-jofrancisco]

I just want to point out that there are modules where this value apparently may be necessary, such as fast-data-entry. I would ask that before approving/merging, you consider this scenario to make sure nothing breaks or comes up with an update for other modules.
CC: @vasharma05 , @mks-d , @donaldkibet
Thanks

[vasharma05]

Hi @icrc-jofrancisco!
What are the specific flows that require the encounter date time?
AFAIK, the forms where the users fill in the encounter date time in the form itself works as expected.
Please let me know.
Thanks!

[vasharma05]

Looking at it, we might also want to expand this to not include encounterDatetime set through the UI, if the currentDate is same as the encounterDatetime IMO this is to enable RDE when picking dates in the past cc @ojwanganto

Hi @donaldkibet @ojwanganto, yes RDE flows must be discussed thorougly..I believe the RDE flow can be discussed for sure, but since this is a critical bug, we can take that separately.

[icrc-jofrancisco]

Hi @icrc-jofrancisco! What are the specific flows that require the encounter date time? AFAIK, the forms where the users fill in the encounter date time in the form itself works as expected. Please let me know. Thanks!

For example, I was thinking about this scenario.
As I said above, I didn't do any "exhaustive" analysis, and it's probably not even related. If it's not, ignore it.
Thanks,

[ibacher]

Looking at it, we might also want to expand this to not include encounterDatetime set through the UI, if the currentDate is same as the encounterDatetime

Indeed. Although, for this, we need some reasonable definition of "same" since I think "===" is very likely to lead right back to the initial issue.

@icrc-jofrancisco In the case of that flow, payload.encounterDatetime (if not collected on the form) will be null and set to the current time anyway (handleEncounterCreate is called after the call to generateFormPayload, which is the thing changed here).

[vasharma05]

Hi @ibacher @denniskigen , requesting your review in this PR.
Thanks!

[denniskigen]

What are these changes to the lockfile owing to, @vasharma05?

[ibacher]

I think the other case we need to consider is editing existing forms and this makes something like the rule @donaldkibet suggested essential before we can merge this in.

[vasharma05]

Hi @ibacher !
I've added a video capture of editing the saved form with time being in future.
Please review.
Thanks!

Screen.Recording.2024-04-06.at.16.26.13.mov

[vasharma05]

Hi @ibacher @denniskigen , please re-review the changes.
Thanks!

[ibacher]

Thanks @vasharma05!