Repeating calendar event shifted by 1 day when RRule is processed #3467
Comments
|
yes the rrule processor has trouble with some date combinations. not our code. and we can't tell as we don't process the ics file in MagicMirror code |
|
I did a little bit more digging and the extra day is added in // RRule can generate dates with an incorrect recurrence date. Process the array here and apply date correction.
if (hasByWeekdayRule) {
Log.debug("Rule has byweekday, checking for correction");
dates.forEach((date, index, arr) => {
// NOTE: getTimezoneOffset() is negative of the expected value. For America/Los_Angeles under DST (GMT-7),
// this value is +420. For Australia/Sydney under DST (GMT+11), this value is -660.
const tzOffset = date.getTimezoneOffset() / 60;
const hour = date.getHours();
if ((tzOffset < 0) && (hour < -tzOffset)) { // east of GMT
Log.debug(`East of GMT (tzOffset: ${tzOffset}) and hour=${hour} < ${-tzOffset}, Subtracting 1 day from ${date}`);
arr[index] = new Date(date.valueOf() - oneDayInMs);
} else if ((tzOffset > 0) && (hour >= (24 - tzOffset))) { // west of GMT
Log.debug(`West of GMT (tzOffset: ${tzOffset}) and hour=${hour} >= 24-${tzOffset}, Adding 1 day to ${date}`);
arr[index] = new Date(date.valueOf() + oneDayInMs);
}
});
if I’m not mistaken the time zone is set to null just before that code is executed. I think if calendar/event is matching the local time zone the adjustment shouldn’t be made. I tried to comment out the code above and it displays the date correctly, but of course it breaks some of the existing tests. I think that it needs additional checks before execution. Maybe checking that event timezone doesn’t match local timezone (although I didn’t dig into the code much yet) thanks |
|
that's my code trying to deal with this problem. no good fix yet |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
Bump, too keep it open as it’s still an issue. Hopefully @sdetweil has some time to look at it and |
|
yea ive just updated mine and it is still displaying full day events as 2 days. |
|
see this.. waiting on user test for submitting PR here and node-ical here is a test version of the fixes for all kinds of date problems. best to make a new folder and git clone there copy your config.js and custom.css from the prior folder this ONLY changes the default calendar if you need to fall back, just rename the folders around again so that all the testcases for node-ical and MagicMirror execute successfully. the 'BIG' change here is to get the local NON-TZ dates for the all the checking and conversion code is commented out or not used one fix in calendar.js for checking if a past date was too far back, and one change in calendarfetcher.js to put out a better diagnostic message of the parsed data.. (exdate was excluded cause JSON stringify couldn't convert the complex structure) |
|
@Pollard-J can u try my new fork? and post a calendar event I can test with.. thx |
|
@sdetweil i just tried this but i'm not that good when i come into problems. I cloned your file of MagicMirror, but it wouldn't execute the next 2 lines properly. ill just wait till its released. thank you for trying to help me. |
|
sorry i left out a critical step fixed above too |
|
@sdetweil thanks for the fix. I just tested the issue and Before: After: |
|
cool, thx for the feedback. we wouldn't release til january 1. so keep using and report anything else you see i need to fix |
|
fixed a bunch more testcases.. updated tests both mm and node-ical changed |
|
try again, note ne branch name here is an updated test version of the fixes for all kinds of calendar date problems. NOTE: the changed branch name best to make a new folder and git clone there git clone https://github.com/sdetweil/MagicMirror this ONLY changes the default calendar if you need to fall back, just rename the folders around again so that all the testcases for node-ical and MagicMirror execute successfully. the ‘BIG’ change here is to get the local NON-TZ dates for the all the checking and conversion code is commented out or not used one fix in calendar.js for checking if a past date was too far back, and one change in calendarfetcher.js to put out a better diagnostic message of the parsed data… (exdate was excluded cause JSON stringify couldn’t convert the complex structure) I added the tests you all have documented please re-pull and checkout the new branch (I deleted the old branch) |
I found a bug in MagicMirror
Please make sure to only submit reproducible issues. You can safely remove everything above the dividing line.
When submitting a new issue, please supply the following information:
Platform: Raspberry Pi 2 - Raspbian GNU/Linux 12 (bookworm). Also reproduced and debugged on OS X - Sonoma 14.5 with Chrome 125.0.6422.142
Node Version: v20.14.0
MagicMirror² Version: 2.27.0
Description: Repeating weekly calendar event is offest by 1 day when processed by Magic Mirror. For example when event is scheduled on Thursday in the calendar, it appears to be scheduled for Friday when displayed on MM. I ran through the
debug.jsand it seems that RRules are affecting it. Full logs and steps to reproduce are below. Possibly related to #3342 and #3291 fixes. @jkriegshauser Not sure if this is an edge case that wasn't tested. My MM installations are set toAmerica/Los_Angelestimezone.The calendar is set in the same timezone, event is set in the same timezone.
modules/default/calendar/calendarfetcherutils.jsaround line 300 seems to be adding an extra day.Steps to Reproduce:
1 . Add following ics file to the calendar config or as
urlvariable indebug.jsGist for basic.ics2. Launch MM and observe the
Test Entriesto show up on Fridays at 17:30 or runmodules/default/calendar/debug.jsand observe the output.Expected Results: Repeating calendar events to be displayed to occur on Thursday at 17:30
Actual Results: Events are displayed to occur on Fridays at 17:30
Configuration: Plain vanilla
config.jsbased onconfig.js.samplewith only addition of the sample calendar:Additional Notes:
Debug Output is a bit too long, here is a link to a gist of the debug output: Full Debug Output
Small relevant part
Calendar file (copy of the gist linked above)
The text was updated successfully, but these errors were encountered: