Refactored calendarfetcherutils to fix many of the timezone and DST related issues and make debugging way easier #3806

plebcity · 2025-06-03T20:36:58Z

Refactored calendarfetcherutils to remove as many of the date conversions as possible and use moment tz when calculating recurring events, this will make debugging a lot easier and fixes problems from the past with offsets and DST not being handled properly. Also added some tests to test the behavior of the refactored methodes to make sure the correct event dates are returned.

Refactored calendar.js aswell to make sure the unix UTC start and end date of events are properly converted to a local timezone and displayed correctly for the user.

This PR relates to: #3797

modules/default/calendar/calendar.js

plebcity · 2025-06-04T20:54:05Z

There is something broken, I will try to fix it tomorrow. I now have an ics calendar to test all cases locally so that should make it easier to find the bug. I took a quick look and it looks like some events might not have a timezone in the start field (probably full day events)

sdetweil · 2025-06-04T21:02:29Z

correct, full day events are always considered to be local time

today, wherever you are

there can be local time events as well without timezone

all of the test cases use ics files from the mock folder with specific clock dates and times set in the testcase runner

…ions as possible and use moment tz when calculating recurring events, this will make debugging a lot easier and fixes problems from the past with offsets and DST not being handled properly. Also added some tests to test the behavior of the refactored methodes to make sure the correct event dates are returned

…ate of an event to the proper timezone. This should now display the correct times in the DOM

…ride handling

…ta should be loaded. Also fixed a bug when determining new recurring events with an original timezone

sdetweil · 2025-06-05T12:35:42Z

have you tested w modules that use the calendar broadcast

MMM-CalendarExt3 (there are multiple modules in the family)
MMM-MonthlyCalendar

i think there are more.
if the data changed, they are broken

im guessing they will be broken as you changed to calendar ui side too

plebcity · 2025-06-05T13:06:05Z

have you tested w modules that use the calendar broadcast

MMM-CalendarExt3 (there are multiple modules in the family) MMM-MonthlyCalendar

i think there are more. if the data changed, they are broken

im guessing they will be broken as you changed to calendar ui side too

No I haven't tested those yet. They could be broken because the timestamps of startDate and endDate are now always in UTC timezone and they use the proper unix timestamp instead of the format('x') which adds 3 extra 0's at the end for milliseconds. Locally everything works fine with my own calendar's now but can't seem te figure out why electron shows errors.

…ne is loaded correctly

…exdates and recurrences

… wasn't expecting the correct result since Sydney is +10 but the asserted time was only +2 from Berlin so +4 in total

…running events, now subtracted the event duration from the searchFromDate

plebcity · 2025-06-06T09:27:56Z

have you tested w modules that use the calendar broadcast

MMM-CalendarExt3 (there are multiple modules in the family) MMM-MonthlyCalendar

i think there are more. if the data changed, they are broken

im guessing they will be broken as you changed to calendar ui side too

Do you think it's important to stay backwards compatible with millisecond timestamps instead of using the second based unix timestamp? If so then I will change the return timestamps back to milliseconds and change the calendar.js to first convert it back to unix.

…at(x) so the timestamps returned will be in milliseconds again instead of the unix timestamp in seconds, changed calendar.js accordingly

plebcity · 2025-06-06T11:34:04Z

@sdetweil All tests have passed and I have made the necessary changes to make this backwards compatible with the old event broadcast, so timestamps are milliseconds and strings again. This will hopefully make it much easier to work with and more robust to edge cases in timezones.
Also after this PR is merged I will create an issue to migrate magicmirror to Luxon since moment and moment-timezone is in maintenance mode right now. I thought it was too much of change to introduce Luxon to magicmirror right now since moment and moment-timezone were already being used.

tests/unit/modules/default/calendar/calendar_fetcher_utils_spec.js

KristjanESPERANTO

Good work 👍

Also after this PR is merged I will create an issue to migrate magicmirror to Luxon since moment and moment-timezone is in maintenance mode right now.

I recommend opening a general issue for replacing 'moment' because Luxon is not the only option. We should talk about all options before we decide.

plebcity · 2025-06-06T21:11:16Z

Good work 👍
I recommend opening a general issue for replacing 'moment' because Luxon is not the only option. We should talk about all options before we decide.

Thanks! I created a new issue for it with a broader scope: #3808

tests/electron/modules/calendar_spec.js

rejas · 2025-06-07T08:55:33Z

modules/default/calendar/calendarfetcherutils.js

-				let d1;
-				let d2;

+				// TODO This should be a seperate function.


so why not do it in this PR?

I felt like this was too much of a change right now so I left a TODO for somebody else to fix in the future or whenever I have some more time to fix this myself. What I meant with this TODO is change the entire flow of the createEventList method to seperate recurring events from single events. Then you probably also want to extract the push to newEvents aswell.

modules/default/calendar/calendarfetcherutils.js

In PR #3806 I noticed that ESLint did not complain about the use of `console`. Then I realised that the rule `no-console` was not activated. I have now changed this and replaced a few places where `console` was used. We can't apply the rule to all .js files because not all of them use our logger. Therefore I had to add an extra config block for it.

@bughaver

## [2.32.0] - 2025-07-01 Thanks to: @bughaver, @bugsounet, @khassel, @KristjanESPERANTO, @plebcity, @rejas, @sdetweil. > ⚠️ This release needs nodejs version `v22.14.0 or higher` ### Added - [config] Allow to change module order for final renderer (or dynamically with CSS): Feature `order` in config (#3762) - [clock] Added option 'disableNextEvent' to hide next sun event (#3769) - [clock] Implement short syntax for clock week (#3775) ### Changed - [refactor] Simplify module loading process (#3766) - Use `node --run` instead of `npm run` (#3764) and adapt `start:dev` script (#3773) - [workflow] Run linter and spellcheck with LTS node version (#3767) - [workflow] Split "Run test" step into two steps for more clarity (#3767) - [linter] Review linter setup (#3783) - Fix command to lint markdown in `CONTRIBUTING.md` - Re-activate JSDoc linting and fix linting issues - Refactor ESLint config to use `defineConfig` and `globalIgnores` - Replace `eslint-plugin-import` with `eslint-plugin-import-x` - Switch Stylelint config to flat format and simplify Stylelint scripts - [workflow] Replace Node.js version v23 with v24 (#3770) - [refactor] Replace deprecated constants `fs.F_OK` and `fs.R_OK` (#3789) - [refactor] Replace `ansis` with built-in function `util.styleText` (#3793) - [core] Integrate stuff from `vendor` and `fonts` folders into main `package.json`, simplifies install and maintaining dependencies (#3795, #3805) - [l10n] Complete translations (with the help of translation tools) (#3794) - [refactor] Refactored `calendarfetcherutils` in Calendar module to handle timezones better (#3806) - Removed as many of the date conversions as possible - Use `moment-timezone` when calculating recurring events, this will fix problems from the past with offsets and DST not being handled properly - Added some tests to test the behavior of the refactored methods to make sure the correct event dates are returned - [linter] Enable ESLint rule `no-console` and replace `console` with `Log` in some files (#3810) - [tests] Review and refactor translation tests (#3792) ### Fixed - [fix] Handle spellcheck issues (#3783) - [calendar] fix fullday event rrule until with timezone offset (#3781) - [feat] Add rule `no-undef` in config file validation to fix #3785 (#3786) - [fonts] Fix `roboto.css` to avoid error message `Unknown descriptor 'var(' in @font-face rule.` in firefox console (#3787) - [tests] Fix and refactor e2e test `Same keys` in `translations_spec.js` (#3809) - [tests] Fix e2e tests newsfeed and calendar to exit without open handles (#3817) ### Updated - [core] Update dependencies including electron to v36 (#3774, #3788, #3811, #3804, #3815, #3823) - [core] Update package type to `commonjs` - [logger] Review factory code part: use `switch/case` instead of `if/else if` (#3812) --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Michael Teeuw <michael@xonaymedia.nl> Co-authored-by: Kristjan ESPERANTO <35647502+KristjanESPERANTO@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Ross Younger <crazyscot@gmail.com> Co-authored-by: Veeck <github@veeck.de> Co-authored-by: Bugsounet - Cédric <github@bugsounet.fr> Co-authored-by: jkriegshauser <joshuakr@nvidia.com> Co-authored-by: illimarkangur <116028111+illimarkangur@users.noreply.github.com> Co-authored-by: sam detweiler <sdetweil@gmail.com> Co-authored-by: vppencilsharpener <tim.pray@gmail.com> Co-authored-by: veeck <michael.veeck@nebenan.de> Co-authored-by: Paranoid93 <6515818+Paranoid93@users.noreply.github.com> Co-authored-by: Brian O'Connor <btoconnor@users.noreply.github.com> Co-authored-by: WallysWellies <59727507+WallysWellies@users.noreply.github.com> Co-authored-by: Jason Stieber <jrstieber@gmail.com> Co-authored-by: jargordon <50050429+jargordon@users.noreply.github.com> Co-authored-by: Daniel <32464403+dkallen78@users.noreply.github.com> Co-authored-by: Ryan Williams <65094007+ryan-d-williams@users.noreply.github.com> Co-authored-by: Panagiotis Skias <panagiotis.skias@gmail.com> Co-authored-by: Marc Landis <dirk.rettschlag@gmail.com> Co-authored-by: HeikoGr <20295490+HeikoGr@users.noreply.github.com> Co-authored-by: Pedro Lamas <pedrolamas@gmail.com> Co-authored-by: veeck <gitkraken@veeck.de> Co-authored-by: Magnus <34011212+MagMar94@users.noreply.github.com> Co-authored-by: Ikko Eltociear Ashimine <eltociear@gmail.com> Co-authored-by: DevIncomin <56730075+Developer-Incoming@users.noreply.github.com> Co-authored-by: Nathan <n8nyoung@gmail.com> Co-authored-by: mixasgr <mixasgr@users.noreply.github.com> Co-authored-by: Savvas Adamtziloglou <savvas-gr@greeklug.gr> Co-authored-by: Konstantinos <geraki@gmail.com> Co-authored-by: OWL4C <124401812+OWL4C@users.noreply.github.com> Co-authored-by: BugHaver <43462320+bughaver@users.noreply.github.com> Co-authored-by: BugHaver <43462320+lsaadeh@users.noreply.github.com> Co-authored-by: Koen Konst <koenspero@gmail.com> Co-authored-by: Koen Konst <c.h.konst@avisi.nl>

plebcity force-pushed the fix-calendar-fetcher-util-for-dst branch from d7ce842 to d4cd5a1 Compare June 3, 2025 20:45

KristjanESPERANTO reviewed Jun 4, 2025

View reviewed changes

modules/default/calendar/calendar.js Outdated Show resolved Hide resolved

plebcity force-pushed the fix-calendar-fetcher-util-for-dst branch from 6b2d17d to 4cc7cab Compare June 4, 2025 20:10

plebcity force-pushed the fix-calendar-fetcher-util-for-dst branch from 4cc7cab to 93e1bbc Compare June 4, 2025 20:59

plebcity force-pushed the fix-calendar-fetcher-util-for-dst branch from 611b7b5 to ed08eb0 Compare June 5, 2025 11:31

plebcity and others added 4 commits June 5, 2025 13:47

Refactored calendar.js aswell to convert the unix UTC start and end d…

bb168ee

…ate of an event to the proper timezone. This should now display the correct times in the DOM

Removed unused functions and fixed a small issue with recurrence over…

93c833b

…ride handling

Fixed script issue in calendar.js, both moment and moment-timezone da…

7f22e9a

…ta should be loaded. Also fixed a bug when determining new recurring events with an original timezone

plebcity force-pushed the fix-calendar-fetcher-util-for-dst branch from ed08eb0 to 7f22e9a Compare June 5, 2025 11:47

Koen Konst added 3 commits June 5, 2025 19:54

Import moment first before moment-timezone to make sure moment timezo…

4357221

…ne is loaded correctly

Fixed an issue with slicedMultiDay events and datekey generation for …

68a80ee

…exdates and recurrences

Fixed a bug in full day recurring events and fixed a broken test that…

0fb0be0

… wasn't expecting the correct result since Sydney is +10 but the asserted time was only +2 from Berlin so +4 in total

plebcity force-pushed the fix-calendar-fetcher-util-for-dst branch from c1f6b22 to 0fb0be0 Compare June 6, 2025 08:53

Fixed a small issue where we didn't search back enough for currently …

51550a4

…running events, now subtracted the event duration from the searchFromDate

Made the calendarfetcherutil backwards compatible again by using form…

7e2690f

…at(x) so the timestamps returned will be in milliseconds again instead of the unix timestamp in seconds, changed calendar.js accordingly

plebcity force-pushed the fix-calendar-fetcher-util-for-dst branch from 505632b to 7e2690f Compare June 6, 2025 10:01

Fix typo and add PR id

4901174

KristjanESPERANTO reviewed Jun 6, 2025

View reviewed changes

tests/unit/modules/default/calendar/calendar_fetcher_utils_spec.js Outdated Show resolved Hide resolved

KristjanESPERANTO and others added 2 commits June 6, 2025 20:22

Update cspell.config.json

25e02b4

remove unused unit test for calendar module

f52a8c7

KristjanESPERANTO approved these changes Jun 6, 2025

View reviewed changes

KristjanESPERANTO requested a review from sdetweil June 7, 2025 05:59

rejas reviewed Jun 7, 2025

View reviewed changes

removed console.log statements which were used for debugging

e1bbbea

rejas approved these changes Jun 7, 2025

View reviewed changes

rejas merged commit faf15ad into MagicMirrorOrg:develop Jun 7, 2025
12 of 15 checks passed

plebcity deleted the fix-calendar-fetcher-util-for-dst branch June 7, 2025 13:20

This was referenced Jun 9, 2025

[Bug] Calendar Hour not showing correct when converting from lower timezone to higher one (UTC+2 -> UTC+3) #3784

Closed

[Bug] calendar client sets fullday repeating until with a timezone offset, rrule excludes the exact day #3781

Closed

KristjanESPERANTO mentioned this pull request Jun 9, 2025

[linter] Enable ESLint rule no-console #3810

Merged

khassel mentioned this pull request Jun 30, 2025

Release 2.32.0 #3826

Merged

khassel mentioned this pull request Jul 1, 2025

[Bug] one calendar unit test fails since today #3830

Closed

4 tasks

This was referenced Jul 11, 2025

[Bug] CalendarFetcherUtils.js, excludedEvents:["string"] not working #3841

Closed

[Bug] Calendar.js limitDays config option not working correctly #3840

Closed

Uh oh!

Refactored calendarfetcherutils to fix many of the timezone and DST related issues and make debugging way easier #3806

Refactored calendarfetcherutils to fix many of the timezone and DST related issues and make debugging way easier #3806

Uh oh!

Conversation

plebcity commented Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

plebcity commented Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sdetweil commented Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sdetweil commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

plebcity commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

plebcity commented Jun 6, 2025

Uh oh!

plebcity commented Jun 6, 2025

Uh oh!

Uh oh!

KristjanESPERANTO left a comment

Choose a reason for hiding this comment

Uh oh!

plebcity commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

rejas Jun 7, 2025

Choose a reason for hiding this comment

Uh oh!

plebcity Jun 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

plebcity commented Jun 3, 2025 •

edited

Loading

plebcity commented Jun 4, 2025 •

edited

Loading

sdetweil commented Jun 4, 2025 •

edited

Loading

sdetweil commented Jun 5, 2025 •

edited

Loading

plebcity commented Jun 5, 2025 •

edited

Loading

plebcity commented Jun 6, 2025 •

edited

Loading