Broken, unmaintained upstream "rrule" fails parsing calendar EXDATEs and UNTIL correctly

[berlincount]

MagicMitch² develop, Electron, Node 6.9.1, Ubuntu

When having a recurring Google Calendar event, e.g. 3 times daily repeat:

BEGIN:VEVENT
DTSTART;TZID=Europe/Berlin:20161213T060000
DTEND;TZID=Europe/Berlin:20161213T070000
RRULE:FREQ=DAILY;COUNT=3
DTSTAMP:20161212T231711Z
UID:[email protected]
CREATED:20161212T231602Z
DESCRIPTION:
LAST-MODIFIED:20161212T231624Z
LOCATION:
SEQUENCE:1
STATUS:CONFIRMED
SUMMARY:Testrepeated
TRANSP:OPAQUE
END:VEVENT

Everything works as expected, i.e. three events show up correctly.

When deleting only middle occurance:

BEGIN:VEVENT
DTSTART;TZID=Europe/Berlin:20161213T060000
DTEND;TZID=Europe/Berlin:20161213T070000
RRULE:FREQ=DAILY;COUNT=3
EXDATE;TZID=Europe/Berlin:20161214T060000
DTSTAMP:20161212T231911Z
UID:[email protected]
CREATED:20161212T231602Z
DESCRIPTION:
LAST-MODIFIED:20161212T231624Z
LOCATION:
SEQUENCE:1
STATUS:CONFIRMED
SUMMARY:Testrepeated
TRANSP:OPAQUE
END:VEVENT

The removed event described by the EXDATE doesn't disappear correctly.

There is an additional issue that "UNTIL" statements in the RRULE doesn't seem to be honored correctly either, i.e. if there's a recurring calendar item that was defined three years ago and I skip an occurance next week it'll still be shown by the calendar module.

The functionality seems to be provided by https://github.com/jkbrzt/rrule, which is passed through by the vendored ical.js, and used by calling rrule.between().

The rrule package doesn't seem to be maintained anymore jkbrzt/rrule#160, and the bug is present in both the latest release and the master.

Not sure whether we should fork & fix, or correct the data we're getting from rrule.between instead.

Ideas?

[berlincount]

This issue might be specific to Google Calendar. Hmmm. Still, the format gets parsed wrong, it seems.

[MichMich]

Somebody willing to send a PR? :)

[berlincount]

Willing, yes, able anytime soon, no ;)

https://www.npmjs.com/package/rrule-alt is the replacement package for rrule, and basically rrule with a few open PRs merged and someone maintaining it. It should be a drop-in replacement that already fixes some issues, but not all of the mentioned ones.

Recommended course of action is writing test cases against rrule-alt (or otherhow ensure the mentioned issues are addressed) and fix things there properly. Once that's done, the dependencies of MMM (indirect via ical.js) should be adjusted to use rrule-alt instead of rrule and we're good.

I suggest someone picks this up when feeling the itch, otherwise I'll pick it up in a few weeks time and make note here when I start working it. Probably not before end of this quarter.

HTH.

[MichMich]

I switched to rrule-alt in the develop branch. Please let me know if this works for you. Make sure you run npm install after pulling the latest commits.

[MichMich]

Did anyone manage to test the develop branch? @berlincount? @Jopyth?

[Jopyth]

@MichMich I will finally be able to upgrade to the new development version (including 2.1.0 stuff) over the weekend, so I can test if it works.

[berlincount]

Just tested, UNTIL works correctly now, EXDATEs (e.g. individual events moved for an hour or cancelled) still don't. I'd still consider it a worthwhile improvement, albeit not closing the issue completely - yet. Still want & need to find time to address this upstream, probably end of this quarter.

[Jopyth]

Yup, definitely not working correctly for me either.

Edit: Ok I was confused by the fact, that you changed the rrult-alt inside the ical stuff, but did not update the package.json in there. Does not work for me either way, I will see if I can build a test case in the rrule-alt package which fails, or maybe something between our end and their end breaks it.

[Jopyth]

I think the problem lies within ical.js not rrule. It parses the feed, and only passes selected values to rrule, EXDATE are missing.

[Jopyth]

@berlincount Can you verify, that #636 works for you, too?

Edit: By the way, problem was indeed not within rrule, it was the ical.js package.

[roramirez]

Hi everyone,

Maybe a good idea will be a test case for this issue. I don't to much about this issue and I'm not figured out about the real problem here. Apparently is not a trivial problem.

[Jopyth]

Test case is not a bad idea, however I think this is not the right project for it. Such test cases are implemented in the rrule-alt package (https://github.com/arolson101/rrule-alt/blob/master/test/rrulestr.js#L189). The real prolem was a supoptimal use of this package within (the old version of) ical.js. They kind of changed it there (https://github.com/peterbraden/ical.js/blob/master/ical.js#L186), but still do not remove the EXDATE entries, I think. So it would be better to improve this stuff in the ical.js project, and switch over to a npm version of it.

[berlincount]

I've been testing with a shared Google calendar, which I consider common enough to be relevant.

Testcase, without PR:

• create event for today 13h, repeating forever, restart MM for immediate reload -> works, shows correctly
• limit events to 5 repeats, restart, works, 5 appointments show
• remove limit, add end-date in two days, restart, works, 3 appointments show
• change middle event to take place at 13:30, restart, doesn't work, keeps showing at 13h :(
• remove middle event, restart, doesn't work, keeps showing

Testcase, with PR:

• create event for today 18h, repeating forever, restart MM for immediate reload -> works, shows correctly
• limit events to 3 repeats, restart, works, 3 appointments show
• remove limit, add end-date in three days, restart, works, 4 appointments show
• change second event to take place at 18:30, restart, doesn't work, keeps showing at 18h :(
• remove third event, restart, works, doesn't show anymore

... sooo, #636 is a notable improvement (thanks!), but doesn't solve everything ...

[MichMich]

Closed due to inactivity. Feel free to reopen.

[retroflex]

Still having this problem with only first deleted event in a series not being shown in MM. Second deleted event is shown. The exdate parameter in ical.js has been changed to an array:

https://github.com/peterbraden/ical.js/blob/master/ical.js

That sounds like a fix to this problem. I tried copying peterbraden/ical.js to my MM and changed to rrule-alt, but that did not solve the problem. Maybe there are more changes in MM/ical.js? Is there someone more knowledgable who can have a look at this?

Anyway, it might be a good idea to update to the latest ical.js.

[pinsdorf]

I'm currently reworking the calendar module to address issue #1105. I'd assume my fix also covers this issue as I've integrated the ical.js library from Mozilla project to have more robust parsing. However, due to time constraints I need some more days. Stay tuned.

@retroflex, do you have a test case for me to validate your issue? Thanks.

[retroflex]

@pinsdorf I created an event that reoccured 5 times daily, then removed event 2 and 3. In the current master it showed events 1, 3, 4, 5 when it should show 1, 4, 5.

[retroflex]

@pinsdorf Any update on your calendar rework?