Teammates: InstructorCoursesPage: course creation time is displayed in UTC instead of local time

Created on 9 Feb 2018  路  7Comments  路  Source: TEAMMATES/teammates

CourseAttributes.createdAt is stored in UTC. However, when it is displayed, it uses TimeHelper#formatDateTimeForInstructorCoursesPage and does not make use of the course's timezone information. Fixing it is not so trivial because the course timezone is stored as a String instead of double like in other places, and currently we don't have a mapping between timezone string and timezone double yet.

a-UIX c.Bug

Most helpful comment

Minor Point: I think timezone should be stored in database a a String but should be convertedto ZoneId upon load so the type of timezone fields are ZoneId.

All 7 comments

Isn't storing it as string ideal? It was always the plan to store it as string instead of double, since the double value changes depending on whether DST is active in that zone. We can use ZoneId.of(string) and apply it to the instant to get a ZonedDateTime right?

Displaying this createdAt field can be a practice on how to display dates in timezones affected by DST. This will be how we display all dates in future when DST is fully implemented.

I see. I thought that the string is in the format specified in our TimeHelper class which would not be recognized by ZoneId.of(String). Since it can be easily recognized by ZoneId, I will just include the fix in my PR.

Minor Point: I think timezone should be stored in database a a String but should be convertedto ZoneId upon load so the type of timezone fields are ZoneId.

Minor Point: I think timezone should be stored in database a a String but should be convertedto ZoneId upon load so the type of timezone fields are ZoneId.

I believe just changing the type of timeZone from String to ZoneId should work out of the box, as Objectify would handle the conversion (this needs to be verified though). I have a feeling Objectify saves ZoneIds as strings by default, using their ID (since that is the way ZoneIds are serialised I believe).

@tran-tien-dat what's the status of this issue? Do we have a fix in mind? Is it being handled? On a separate note, best not to cite issue numbers in commit message subjects (if you do, it can spam the issue thread, as you can see above)

@damithc it's time related so we have fixed it when rewriting time to java.time. Probably won't be merged to master soon though, until we have finished migrating.

Regarding issue number in commit message, I will take note of that.

Noted. Will put on hold to prevent others submitting PRs.

Was this page helpful?
0 / 5 - 0 ratings