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

RA-1988 - Allow DatetimePicker to support different timezones #102

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

icrc-toliveira
Copy link
Contributor

@icrc-toliveira icrc-toliveira commented May 6, 2022

Issue: https://issues.openmrs.org/browse/RA-1988

Description:
When using a date time picker (with time), it will not store the date with time-zone information.
This will result in date being stored always as if was UTC, even when is not.

Changes done:

  • Change initial date string format from "dd MMM yyyy HH:mm" to "dd MMM yyyy HH:mm:ss" (now show's seconds, to be in order with the remaining formats)
  • Change date Picker format from "dd M yyyy HH:mm" to "dd M yyyy hh:ii:ss" (this is time used after picking with the date time picker)
  • Updating format every time a change happens (this is required in order to send time zone)

Related PR's:

…to "dd MMM yyyy HH:mm:ss"

- Change datePicker format from "dd M yyyy HH:mm" to "dd M yyyy hh:ii:ss" (this is time used after picking with the datetimepicker)
- Updating format every time a change happens (this is required in order to send timezone)
@icrc-toliveira icrc-toliveira changed the title RA-1988 - Change initial date string format from "dd MMM yyyy HH:mm" … RA-1988 - Allow DatetimePicker to support different timezones May 6, 2022
@icrc-toliveira
Copy link
Contributor Author

Hi @dkayiwa,
Could you take a look at this

@sherrif10
Copy link
Member

@icrc-toliveira Thanks for working on this, Do you mind sharing the screenshot .

- Added new parse format for start/end date to prevent known error
- Changed datePickerFormat format to "dd M yyyy hh:ii:ss" (prevents parse error for french)
- Dates are now always saved as "YYYY-MM-DDTHH:mm:ss.sssZ", and timezone "cleaning"is done by htmlformentryui
…commons into RA-1988

� Conflicts:
�	omod/src/main/webapp/fragments/field/datetimepicker.gsp
…ut requesting time, while keeping timezone

- To accomplish that, date is parse into "DD MMM YYYY", while keeping the timezone in the input format
@icrc-toliveira
Copy link
Contributor Author

icrc-toliveira commented Oct 21, 2022

Hi @sherrif10

Sorry for my late response,
In the past month I did take a deeper look into this and found many issues.

When developed, datetimepicker didn´t seem to take time zones into account, and in order for this to work a lot of stuff had to be changed.

  • This PR, start with a refactor of the code, to make it easier to read. (use of ternary operators instead of relaying of multiple if's)
  • After that I added a new shortDateStringFormat to handle some of the start / end dates that were causing troubles (occurring when the start / end given were dates without time)
  • Other issue was the inability for the datetimepicker to handle different languages, the format used was not working for French language and completely breaking if a prior date was saved in English and the edited in French. (datePickerFormat as 'dd M yyyy hh:ii:ss', seems to work for every language)
  • Dates can now be saved in both "YYYY-MM-DD HH:mm:ss" (when no time / timezone is needed) and "YYYY-MM-DDTHH:mm:ss.sssZ". The use of both format, is done in order to keep compatibility with older versions, since there is no good way of having this work for both (htmlformentryui) encounters and the edit visit page.
  • Sometimes date would only save in a 12 hour format instead of 24

Example of 12h format bug:
image

@gracepotma
Copy link

@dkayiwa could you kindly review this for merging? Apparently it's one of several blockers for ICRC.

@dkayiwa
Copy link
Member

dkayiwa commented Nov 8, 2022

@icrc-toliveira remember to always hit the Request Code Review button on the JIRA ticket, whenever you are ready for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants