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

Deal with summer time changes #38

Open
fabiodrg opened this issue Sep 17, 2018 · 7 comments
Open

Deal with summer time changes #38

fabiodrg opened this issue Sep 17, 2018 · 7 comments
Labels
extractor Extractor related issues help wanted Extra attention is needed

Comments

@fabiodrg
Copy link
Collaborator

fabiodrg commented Sep 17, 2018

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*(7243600E3) 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 fabiodrg added the extractor Extractor related issues label Sep 17, 2018
@fabiodrg
Copy link
Collaborator Author

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
Copy link
Owner

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
Copy link
Owner

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 fabiodrg self-assigned this Oct 10, 2018
@fabiodrg
Copy link
Collaborator Author

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

@msramalho
Copy link
Owner

No problem 😄

@msramalho msramalho changed the title [Moodle] Deal with summer time changes Deal with summer time changes Feb 19, 2019
@msramalho
Copy link
Owner

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

@msramalho
Copy link
Owner

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?

@fabiodrg fabiodrg removed their assignment Nov 1, 2021
@fabiodrg fabiodrg added the help wanted Extra attention is needed label Nov 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extractor Extractor related issues help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants