Teammates: Add support for daylight saving time

Created on 2 May 2014  路  32Comments  路  Source: TEAMMATES/teammates

_From [email protected] on June 30, 2011 16:11:15_

Does time zone support work correctly when day light saving is involved?

_Original issue: http://code.google.com/p/teammatespes/issues/detail?id=85_

committers only p.Medium

Most helpful comment

Sneak peek:
screen shot 2018-03-20 at 3 55 57 am

All 32 comments

_From [email protected] on December 05, 2011 23:53:36_

Labels: Priority-Medium

_From [email protected] on July 12, 2012 22:49:22_

Status: Accepted

_From [email protected] on February 20, 2013 04:15:26_

Labels: Type-Task Difficulty-High

_From [email protected] on February 21, 2013 09:17:45_

Hi Dr. Damith,

I am not sure how teammates is implementing the timezone change for daylight savings however, to make sure that it is working correctly, it will be good to use the class SimpleTimeZone http://docs.oracle.com/javase/6/docs/api/java/util/SimpleTimeZone.html

_From [email protected] on February 22, 2013 04:39:47_

Thanks for the pointer Allan. Whoever doing this issue should look into it. As of now, I don't think we are handling it all.

@ashrayjain assigning to you, as discussed.

@damithc Hey prof, it turns out we have two more fields (session visible from and responses visible from), apart from session start and session end. Our discussed solution of adding 2 new columns in the db for session start offset and session end offset will not be enough for this. We could solve this problem in the following 3 ways:

  1. We could simply add 2 more columns for the new fields. However, if we ever need to add any more date related info to the Session object, we will need to add new columns for DST as well.
  2. We could store the timezone ID string (like Asia/Singapore or Australia/Adelaide) in our db and remove all other offset related columns (like timeZone, timeZoneDouble as well as all the dst offset columns). Then we will need to update all code that uses the timeZone fields to use Java's standard library and apply this timezone ID (which includes DST info) when retrieving timestamps from the db.
  3. We could do away with storing timezone info separately and change all date fields in our db to have the correct timezone (eg. SGT) as part of the timestamp as opposed to the current value (UTC).

An important consideration would be on how we will handle transitioning for existing sessions. What do you think?

It has to be 2 or 3. What is the common wisdom on handling timezones and DST? Any articles on this?

There are a lot of different opinions on StackOverflow about this, and a lot of times they are for specific cases. The best article that I think we should follow is this one.
https://www.w3.org/TR/timezone/#guidelinessummary

On the same article, these sections are also of interest:
https://www.w3.org/TR/timezone/#guidelinesfuturerecurring
https://www.w3.org/TR/timezone/#negotiating

The general overview is that, especially when dates in the future are involved, storing the timezone ID is the best way. Other things can be used as fallbacks but timezone IDs should be used if available.

I'm ok with storing timezone ID. We also need to discuss where to store it. For example, may be we should store in in the Course object rather than the Session object? Explore these further and we can discuss on Wednesday.

You also need to figure out how to save DST lookup table and how to keep it updated.

Since we will be using the standard library for Java for DST conversions, we will need to update the standard library's data. The common method for updating this data is outlined here.
http://www.oracle.com/technetwork/java/javase/tzupdater-readme-136440.html

However, I will need to look into how it would work with GAE.

With GAE, the standard library is locked in and out of our control. This is the only issue about this I could find.

Google does seem to update its timezone data but not very frequently as it turns out. I tested the standard library on a GAE test app using known timezone/dst updates from the past year (eg. North Korea's timezone update from August 2015). From what I tested, updates until June last year are in. I would guess that Google follows a yearly update cycle for this. So the question is, if this frequency is ok for us?

The other alternative is to use an external library instead of the standard library so that we can update the data as we please. The most popular alternative seems to be Joda Time.

Yes, I'm ok to use an external library as Java built-in date library is known to be weak. Is Joda time updated more regularly?

Yup sir. Joda seems to follow the updates to the database pretty closely.
Their latest version already includes the first updates for 2016 released
in late January. So should I go ahead with it?

Yes, go ahead with Joda.

@damithc Since we are adding timezone info to courses, I wanted to clarify some semantics. It looks like once a course is created, we don't allow the instructor to change the Course ID or Course Name (am i right?). As we will be adding the timezone option there, do we also disallow timezone updates after the course has been created? Is there any use case for timezone updates after course creation?

Note: We will still allow the user to change the detected value on the course create page, BEFORE the course is created.

@damithc bump.

No reason not to allow it right?
The other two fields should be allowed to change too. It's just that we didn't find time to do it.

Hmm. I don't see any reason to disallow changing course name and timezone. But maybe ID is more sensitive to change? @thyageshm your thoughts?

In any case, I think it will be better to add this functionality as a separate issue rather than with this one?

Yes, editability of each field can be separate issues. Yes, editing course ID is the trickiest and may never be allowed.

sir, I believe if we change the courseid, we would have to change it in every student/instructor in that course. We should consider if that is an acceptable overhead

@ashrayjain if this is being fixed by another pr, specify it in that pr.
If not, reminder to work on this. It has slipped a release

From what I understand, we have to:

  • Migrate FeedbackSession timeZone field from double to string containing time zone ID.

    • We can use the Etc/GMT-X time zones to maintain expected behaviour, as these time zones do not have DST. It is up to instructors to change the time zone to their correct local time zone if they want to enable DST.

  • Migrate timestamp data within FeedbackSession such that timestamps refer to the actual time in UTC rather than local time.

    • Fields include startTime, endTime, sessionVisibleFromTime, resultsVisibleFromTime.

    • Currently they are interpreted as local time. There is a dependency on session timeZone to interpret the time correctly.

  • Update all views of the time fields to interpret the timestamps as actual time in UTC, and pretty print them in the time zone specified by the timeZone field using a date library that handles DST.

Is there anything I'm missing?

@wkurniawan07 can weigh in. He was the last person to handle this hot potato 馃槤

The plan was to ditch timezone fields in feedback sessions completely and relying fully on the one in course, which already is in Olson timezone ID. The timezone field in the courses was actually added for this purpose.
I tried and met many roadblocks along the way, and realized one part of the puzzle is #6300, although evidently I did not manage to find the time to actually work on it 馃槢
After cleaning up all the APIs, the last part of the puzzle should be removing the timezone fields completely from the feedback session and relying on the course's.

For the reason that it has not been actually tried out, I cannot give a 100%-guarantee that the previous plan would _actually_ work, although it sounds plausible on paper. Migrating the timezone field in feedback session would (edit: might) also work, but will require another migration process. Since I was not actually involved in the discussion as for why use timezone in course vs use timezone in sessions, I'd open this topic back to square one again.

Well I guess since our users have gotten used to a session time zone, we shouldn't remove it immediately. We can first migrate the session time zone field, announce support for DST, then deprecate the use of a session specific time zone before finally removing it once we're sure no one uses it anymore?

From what I remember, we planned to show the time zone in the session (but readonly) even if it is stored in the course. That way, users will not notice a big difference UI-wise.

@whipermr5 I forgot to link this issue as well: #5928

Linking to this issue which contains some discussion: #8362

As discussed with @tran-tien-dat, we should not worry about local date times that are ambiguous or non-existent but leave it to the library to resolve. The javadoc for LocalDateTime#atZone explains clearly how these are resolved.

However I think we should still show a message to the user if the local date time they input is ambiguous or non-existent, stating clearly which time we resolved it to and, in the ambiguous case, allow a way to select the later offset time. Useful methods: ZonedDateTime.ofStrict(LocalDateTime, ZoneOffset, ZoneId), ZonedDateTime.withLaterOffsetAtOverlap().

Sneak peek:
screen shot 2018-03-20 at 3 55 57 am

Was this page helpful?
0 / 5 - 0 ratings