search

msramalho / SigTools

📆 Sigarra Tools | An extension that makes the information system of the University of Porto slightly better.
https://chrome.google.com/webstore/detail/sigarra-to-calendar/piefgbacnljenipiifjopkfifeljjkme
Apache License 2.0
37 stars 0 forks source link

Deal with summer time changes #38

Open fabiodrg opened 5 years ago

fabiodrg commented 5 years ago

There's an issue in the Moodle extractor when doing recurrence calculations.

Let's say there's an event that starts at 10 October, 23h50. The event ends at 7th November, 23h50. When I click in the November calendar button, the Extractor is able to find the original event (10th October) and create the dropdown for it (instead of creating a single event for 7th November, it creates an event at 10 October, 23h, with some duration, that occurs weekly till 7th November).

That is possible with some calculations with timestamps in UTC. Based on the the server response for the 7th November event, it knows there's a 4 week difference to the original event, thus it subtracts from 7th November event timestamp the value 4(724*3600E3) in milliseconds. This is enough for finding the original event start timestamp.

At 28th October the hour changes, and that messes with the results. The generated Date object is 11th October, 00h50, and that's the value used for event start time.

let end = new Date('7/Nov/2018 23:50');

new Date(end.getTime() - 4*(\7\*24\*3600E3));

The result in Chrome is Thu Oct 11 2018 00:50:00 GMT+0100 (Western European Summer Time), and the ISO string is 2018-10-10T23:50:00.000Z.

I guess that the Date constructor somehow considers the locale timezone. I am not sure how to solve this. The server responses are Unix timestamps.

fabiodrg commented 5 years ago

This might help: https://momentjs.com/

It allows to create objects with unix timestamps, and by default it uses local settings, instead of UTC

// 1541634600 = 11/07/2018 @ 11:51pm (UTC)
let a = moment.unix(1541634600).subtract(4, 'weeks')
// 2018-10-10T22:50:00.000Z"
msramalho commented 5 years ago

This is actually an important topic, I had considered using moment.js before, as it is quite good, but avoided doing so due to the size of it, and for being yet another dependency, I suppose it can be useful in the future, and if it provides a useful solution for this problem, then I believe it makes sense to include it (and maybe we will use it in the future/ refactor some old stuff).

msramalho commented 5 years ago

As a matter of fact, since it is such a solid library for handling time, I am considering adopting it full-on (maybe even removing some of our date functions and replacing them with moment's ones, in the future)... what do you think? (1. of it being a solution to this particular problem, 2. future use and added value vs added size and dimension)

fabiodrg commented 5 years ago

I forgot to answer... sorry 😞 I agree, I think it makes sense to adopt the library.

msramalho commented 5 years ago

No problem 😄

msramalho commented 5 years ago

@iamdiogo also helped identify this bug outside moodle, on sigarra schedule page, on the modal when using the toISOString() function, see this line

msramalho commented 5 years ago

After some tests, maybe it is easier to simply "hardcode" the Date by using each individual getter (day, month, year) instead of toISOString or toUTCString, but not really sure what would be the best option.

What do you think?