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

Calendar events that have exclusion dates may not be excluded #3250

Closed
jkriegshauser opened this issue Oct 26, 2023 · 5 comments
Closed

Calendar events that have exclusion dates may not be excluded #3250

jkriegshauser opened this issue Oct 26, 2023 · 5 comments

Comments

@jkriegshauser
Copy link
Contributor

I have an event that is as follows:

BEGIN:VEVENT
DTSTART;TZID=America/Los_Angeles:20230315T181000
DTEND;TZID=America/Los_Angeles:20230315T195000
RRULE:FREQ=WEEKLY;UNTIL=20231129T075959Z;BYDAY=WE
EXDATE;TZID=America/Los_Angeles:20231025T181000
DTSTAMP:20231025T233434Z
UID:[email protected]
CREATED:20230306T193128Z
LAST-MODIFIED:20231024T222515Z
SEQUENCE:0
STATUS:CONFIRMED
SUMMARY:My Event
TRANSP:OPAQUE
END:VEVENT

Note that there is an EXDATE for today. However, it was showing on the Magic Mirror.

jkriegshauser added a commit to jkriegshauser/MagicMirror that referenced this issue Oct 26, 2023
jkriegshauser added a commit to jkriegshauser/MagicMirror that referenced this issue Oct 26, 2023
@sdetweil sdetweil self-assigned this Oct 26, 2023
@sdetweil
Copy link
Collaborator

note that the dates from rrule.between are jacked cause of the tz vs local thing..

so it got TODAYS date and the timezone time which makes the even tomorrow..(tz in the future of UTC) oops..

this seems to be because of the rrule byweekday I have tried to recorrect those..

watch out for datekey being wrong, because date is damaged and not corrected

const dateKey = date.toISOString().substring(0, 10);

@jkriegshauser
Copy link
Contributor Author

@sdetweil it appears to be wrong even if the iCal event and local time are in the same timezone. In the event above, note BYDAY=WE, so it's on a Wednesday. When rule.between() is executed within the America/Los_Angeles timezone, the dates returned were exclusively on Tuesdays.

The node-ical library changes all EXDATE entries to ISO, so it is fairly easy to find those, assuming that the recurrence dates from rrule are correct (which they're not).

@sdetweil
Copy link
Collaborator

assuming.. duh..

yes, you have to look at the dates returned to see that trhe date and time are mangled... date from one, tz from another..
when BYDAY/BYWEEKDAY.. if you remove that clause u get different date results..

you can see it in the debug output

BUT as I said before WE cannot TELL its mangled with certainty, as rrule is processing from the rrule object and not from the event object.

I corrrect the non-excluded date/times when BYDAY is set

// if the rrule byweekday WAS explicitly set , correct it
									// reduce the time by the offset
									if (curEvent.rrule.origOptions.byweekday !== undefined) {
										// Apply the correction to the date/time to get it UTC relative
										date = new Date(date.getTime() - Math.abs(24 * 60) * 60000);
									}

the 'problem' is that the tz adjustment sets the date to utc time, 1am UTC is yesterday 20:00 in ny. and today 11pm UTC is tomorrow 1am in german tz. and rrule expects local time.

khassel pushed a commit to khassel/MagicMirror that referenced this issue Oct 31, 2023
@jkriegshauser
Copy link
Contributor Author

Do you know a better rrule js library to use? We could see about forking node-ical to use something else.

@sdetweil
Copy link
Collaborator

I do not know of anything better

jkriegshauser added a commit to jkriegshauser/MagicMirror that referenced this issue Oct 31, 2023
jkriegshauser added a commit to jkriegshauser/MagicMirror that referenced this issue Oct 31, 2023
rejas pushed a commit that referenced this issue Nov 1, 2023
Hello and thank you for wanting to contribute to the MagicMirror²
project

**Please make sure that you have followed these 4 rules before
submitting your Pull Request:**

> 1. Base your pull requests against the `develop` branch.
> 2. Include these infos in the description:
>
> - Does the pull request solve a **related** issue?
> - If so, can you reference the issue like this `Fixes
#<issue_number>`?
> - What does the pull request accomplish? Use a list if needed.
> - If it includes major visual changes please add screenshots.
>
> 3. Please run `npm run lint:prettier` before submitting so that
>    style issues are fixed.
> 4. Don't forget to add an entry about your changes to
>    the CHANGELOG.md file.

**Note**: Sometimes the development moves very fast. It is highly
recommended that you update your branch of `develop` before creating a
pull request to send us your changes. This makes everyone's lives
easier (including yours) and helps us out on the development team.

Thanks again and have a nice day!
MichMich added a commit that referenced this issue Jan 1, 2024
## [2.26.0] - 01-01-2024

Thanks to: @bnitkin, @bugsounet, @dependabot, @jkriegshauser,
@kaennchenstruggle, @KristjanESPERANTO and @Ybbet.

Special thanks to @khassel, @rejas and @sdetweil for taking over most
(if not all) of the work on this release as project collaborators. This
version would not be there without their effort. Thank you guys! You are
awesome!

This release also marks the latest release by Michael Teeuw. For more
info, please read the following post: [A New Chapter for MagicMirror:
The Community Takes the
Lead](https://forum.magicmirror.builders/topic/18329/a-new-chapter-for-magicmirror-the-community-takes-the-lead).

### Added

- Added update notification updater (for 3rd party modules)
- Added node 21 to the test matrix
- Added transform object to calendar:customEvents
- Added ESLint rules for jest (including jest/expect-expect and
jest/no-done-callback)

### Removed

- Removed Codecov workflow (not working anymore, other workflow
required) (#3107)
- Removed titleReplace from calendar, replaced + extended by
customEvents (backward compatibility included) (#3249)
- Removed failing unit test (#3254)
- Removed some unused variables

### Updated

- Update electron to v27 and update other dependencies as well as github
actions
- Update newsfeed: Use `html-to-text` instead of regex for transform
description
- Review ESLint config (#3269)
- Updated dependencies
- Clock module: optionally display current moon phase in addition to
rise/set times
- electron is now per default started without gpu, if needed it must be
enabled with new env var `ELECTRON_ENABLE_GPU=1` on startup (#3226)
- Replace prettier by stylistic in ESLint config to lint JavaScript (and
disable some rules for `config/config.js*` files)
- Update node-ical to v0.17.1 and fix tests

### Fixed

- Avoid fade out/in on updateDom when many calendars are used
- Fix the option eventClass on customEvents.
- Fix yr API version in locationforecast and sunrise call (#3227)
- Fix cloneObject() function to respect RegExp (#3237)
- Fix newsfeed module for feeds using "a10:updated" tag (#3238)
- Fix issue template (#3167)
- Fix #3256 filter out bad results from rrule.between
- Fix calendar events sometimes not respecting deleted events (#3250)
- Fix electron loadurl locally on Windows when address "0.0.0.0" (#2550)
- Fix updatanotification (update_helper.js): catch error if reponse is
not an JSON format (check PM2)
- Fix missing typeof in calendar module
- Fix style issues after prettier update
- Fix calendar test (#3291) by moving "Exdate check" from e2e to
electron to run on a Thursday
- Fix calendar config params `fetchInterval` and `excludedEvents` were
never used from single calendar config (#3297)
- Fix MM_PORT variable not used in electron and allow full path for
MM_CONFIG_FILE variable (#3302)

---------

Signed-off-by: naveen <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Karsten Hassel <[email protected]>
Co-authored-by: Malte Hallström <[email protected]>
Co-authored-by: Veeck <[email protected]>
Co-authored-by: veeck <[email protected]>
Co-authored-by: dWoolridge <[email protected]>
Co-authored-by: Johan <[email protected]>
Co-authored-by: Dario Mratovich <[email protected]>
Co-authored-by: Dario Mratovich <[email protected]>
Co-authored-by: Magnus <[email protected]>
Co-authored-by: Naveen <[email protected]>
Co-authored-by: buxxi <[email protected]>
Co-authored-by: Thomas Hirschberger <[email protected]>
Co-authored-by: Kristjan ESPERANTO <[email protected]>
Co-authored-by: Andrés Vanegas Jiménez <[email protected]>
Co-authored-by: Dave Child <[email protected]>
Co-authored-by: grenagit <[email protected]>
Co-authored-by: Grena <[email protected]>
Co-authored-by: Magnus Marthinsen <[email protected]>
Co-authored-by: Patrick <[email protected]>
Co-authored-by: Piotr Rajnisz <[email protected]>
Co-authored-by: Suthep Yonphimai <[email protected]>
Co-authored-by: CarJem Generations (Carter Wallace) <[email protected]>
Co-authored-by: Nicholas Fogal <[email protected]>
Co-authored-by: JakeBinney <[email protected]>
Co-authored-by: OWL4C <[email protected]>
Co-authored-by: Oscar Björkman <[email protected]>
Co-authored-by: Ismar Slomic <[email protected]>
Co-authored-by: Jørgen Veum-Wahlberg <[email protected]>
Co-authored-by: Eddie Hung <[email protected]>
Co-authored-by: Bugsounet - Cédric <[email protected]>
Co-authored-by: bugsounet <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Knapoc <[email protected]>
Co-authored-by: sam detweiler <[email protected]>
Co-authored-by: veeck <[email protected]>
Co-authored-by: Paranoid93 <[email protected]>
Co-authored-by: NolanKingdon <[email protected]>
Co-authored-by: J. Kenzal Hunter <[email protected]>
Co-authored-by: Teddy <[email protected]>
Co-authored-by: TeddyStarinvest <[email protected]>
Co-authored-by: martingron <[email protected]>
Co-authored-by: dgoth <[email protected]>
Co-authored-by: kaennchenstruggle <[email protected]>
Co-authored-by: jkriegshauser <[email protected]>
Co-authored-by: Ben Nitkin <[email protected]>
@khassel khassel closed this as completed Jan 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants