Made the modifications suggested. Also added the alter queries for modifying the existing datetime data types to timestamp.
Thanks & Regards
Placed a review on github(also has a couple questions there to clarify some things):
Made the modifications suggested.
Place a review on github.
Meant to say:
I placed a review on github. -brady
We have made the modifications suggested.
Just testing this out. Getting no errors, but changing the time-zone seems to have no effect on the php time (via the calendar widget current setting) or when the mysql time (inserted entries into the database that use the NOW() mysql command). I remember it working on your previous revisions. Just wondering if it is working for you?
We have tested it as a fresh installation and as an upgrade and both are working. We basically tested it with log and Vitals. By calendar widget which area do you mean ? is it Globals --> calendar.
Ok, starting to sort this out more.
I agree that the logs and vitals is correct.
1. Go to a patient summary and then click one of the links in the Clinical Reminders widget (such as Education: Weight) and then click in the date/time field. Note that this defaults to today, which is using the local server date/time.
2. In the vitals edit, if you click on the date field and click today, it will go to the date of the local server date/time (so if it should be the 30th according to the timezone setting but is 31th on local server, then it will go to the 31st).
3. As in 2 above, the same happens in the Main Calendar screen, when click Today button.
The today in calendar represents the current date of the user , not the server time.When viewing in a user browser it represents his local time. I think this behavior is correct. This is how today is defined in calendar script " var today=new Date()".
Can I get clarification on the globals timezone setting then. I assumed this was to set the timezone of the clinic. Is that an incorrect assumption? Doesn't it seem a bit inconsistent that the vitals form defaults to the globals server set timezone while the date widget defaults (the thing that is selected by default when I click on the date widget in the Clinical reminders links) to the current user browser timezone?? Perhaps an example use case of what the main point of this will help me understand the goal of this.
Couple quick questions to further help me clarify the goal:
Should the timezone be a per user option? Miscellaneous->Preferences
Should we also have an auto_calculate setting that uses a mechanism like this:
We have clients from different time zones which is hosted on same server. When clients trying interpret logs or the time at which the forms were entered or signed off they get confused, as all these are stored in the server time. By implementing this mechanism they can interpret the same without worrying about server time which is more clearer to them. We have implemented this mechanism in the calendar by modifying the dynarch calendar library. Please note that this will not affect the appointment time /date as this is stored as two independent fields. This feature will only affect the timestamp fields.
You can find this commit at: https://github.com/zhhealthcare/openemr/commit/7f21b5fcdd180ad0c7fd903485f737d9d04dad1b
Sorry to be coming late to this party. I'm concerned about the mass chages of datetime to timestamp. These are very different animals and careful testing and review are called for. For example timestamp will not store a value before 1970 or after 2037, and SQL queries that do arithmetic on timestamp values will not work (I'm pretty sure we have some of those). I think timestamp is primarily intended for recording when changes to data were made.
I'm very concerned about things breaking.
The arithmetic on timestamp field will behave correctly. For example try this query on a timestamp field you will get the correct result. Only limitation is you cannot store date before 1970 and date after year 2037. Fields which we are converting to timestamp do not expect any date before 1970 and the issue of year 2037 will be solved, we expect.
I may be wrong about arithmetic. While the MySQL 5.1 manual says:
DATE_ADD(date,INTERVAL expr unit), DATE_SUB(date,INTERVAL expr unit)
These functions perform date arithmetic. The date argument specifies the starting date or datetime value.
Something later mentions that timestamp works also.
However this really is a big change. Have you tested that all the reports with date filters are working correctly? Is everything the same if you back up and then restore to a server with a different time zone?
Another concern - many dates, encounter dates in particular, are very much associated with a location. I don't think it will be good for someone in Australia to see a visit dated April 2 when the visit occurred in New York on April 1. That person will be very much aware of where the encounter occurred, and will assume that the date is New York time.
One of the bigger risks with this change is what happens when upgrading existing instances.
What happens to the existing values in a date or datetime field when it's converted to timestamp?
This will not make any changes to date fields. The datetime fields will be converted to time stamp according to the server timezone at that time . Timestamp is basically a GMT time of that event, only difference is mysql will interpret and show these values according to the timezone setting.
The display of values is not determined on the basis of in which time zone the user is but to which timezone the the OEMR installation is set to.
I don't think you can say that just because a field is datetime it should be converted to the logged-in user's time zone. For example encounter date (form_encounter.date) is a datetime field, and as previously noted it has a strong implied association with the place of the visit. Every datetime field should be looked at separately in light of what it's used for. The brush seems too broad.
It is not converting to the logged in user's time zone. The change is brought in an option in Globals for setting the timezone. It is interpreting the datetime according to that. This is not doing any calculation according to the logged in user's time zone but the setting in Global --> locale-->timezone.
OK so it's the timezone of the site of the logged-in user, not the logged-in user's time zone. But there are similar issues, for example if a clinic moves to a new location that happens to be in a different time zone. Also an installation might have multiple locations (service facilities) in different time zones.
Your problem description says "When clients trying interpret logs or the time at which the forms were entered or signed off they get confused, as all these are stored in the server time." But you are changing a lot more stuff than just the times that forms were entered or signed off.
In theory, this code is migrating the set timezone options from the php/mysql/servers themselves into the OpenEMR instance. On testing so far, the timestamp seem to work identically to the datetime entries, although agree more testing is needed.
I do see Rod's point, though, if an encounter is billed for a certain day/time and then the facility moves, then if re-bill the encounters, there is potential for the days to move, which would probably be a bad thing. I didn't think of this; it seemed nice to store the timestamp for fields, which is stored in UTC time and can then be localized to the user. As Rod points out though, this can cause some real headaches if a clinic moves location or changes timezones. So, then would need to figure out which datetime entries should not change (such as encounter dates) and keep them datetime, although we then may be headed down a very slippery slope. What would happen if didn't modify any of the database entries, but modified the sql/php timezones? Then, at least all the entries after the proper timezone change would be right along with input of data and wouldn't get messed up if change timezones. Would that work?
Presently if a facility shifted from one time zone to other what will be happening is there will be datetime in two time zones which is not properly marked. Only way we can interpret the date is by noting down the date and time at which the change happens.
As rod suggested its better to keep encounter date in date time. In standard code the time portion is not at all used. Somebody may have used it for their customization.
I tried searching for "best practices" with datetime vs. timestamp, and didn't find anything that looked authoritative. So I guess we just have to apply some common sense. After the discussion here, my thoughts are:
1. Make the least disruptive change that is needed to solve the problem. Everything we do should have a good reason behind it.
2. Needlessly converting datetime to timestamp may have unintended consequences; for example an invalid value may have just one wrong digit and it can be guessed which one is incorrect, whereas if you convert you end up with a zero timestamp and no clue why.
3. Timestamp seems intended mostly for automatically generated times, so that the stored value is not dependent on the server's time zone setting. Datetime has no associated time zone. "What you store is what you get." It seems right to use datetime when time zone is not interesting, for example with encounters and their appointments.
4. Yes 2038 is a ways off, but still....
Log in to post a comment.