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

Fix comma-separated EXDATES, change ISO string keys #84

Merged
merged 7 commits into from
Nov 26, 2018

Conversation

mbalfour
Copy link
Contributor

Added support for comma-separated EXDATES, since those are allowed by the RFC. While testing this, I discovered some flaws with using the timestamp for EXDATE and RECURRENCE lookups. As a result, the lookup key has been changed to be a pure date string, instead of the date+time ISO string. Added tests, example code, and documentation showing how to use this.

Added support for comma-separated EXDATES, since those are allowed by the RFC.  While testing this, I discovered some flaws with using the timestamp for EXDATE and RECURRENCE lookups.  As a result, the lookup key has been changed to be a pure date string, instead of the date+time ISO string.  Added tests, example code, and documentation showing how to use this.
Removed extra exdate function, fixed some whitespace.
@mbalfour
Copy link
Contributor Author

This pull request fixes #76 . Note that there's an API change as well - the key for recurrence ID and exdate lookups has changed from a full timestamp ("2017-04-28T00:10:55.964Z") to just a date ("2017-04-28"). This change was needed because the time portion was unreliable for lookups.

Return error message if HTTP returns anything other than 200 OK, instead
of trying to parse the results.
@pinsdorf
Copy link

pinsdorf commented Feb 1, 2018

@peterbraden, is there any plan to merge the PR by @mbalfour ? There is just a merge conflict on the readme file. Thanks!

@jhofker
Copy link

jhofker commented Nov 25, 2018

Just wanted to say thanks for this fix! The MagicMirror calendar module uses this project and is suffering from this issue.

@mbalfour
Copy link
Contributor Author

No problem, I actually made this fix specifically for the MagicMirror calendar module. :) While it didn't make it into that, it has been submitted back into the MMM-CalendarExt extension: https://github.com/eouia/MMM-CalendarExt . The PR back into default calendar got rejected since it also included a monthly view that the maintainer didn't want to take. I was working on a monthly view, and fixed the recurrences while trying to figure out why the monthly view didn't match my "real" calendars. You can find that PR here if you're interested in reviving it: MagicMirrorOrg/MagicMirror#375 . I switched my focus to getting MMM-CalendarExt to work with recurrences since I wanted a monthly view, but if you examine that previous PR (or the equivalent code that made it into MMM-CalendarExt) it shouldn't be too much effort to pull out the relevant parts and create a new PR that fixes recurrences in the default calendar module. I just haven't had the time to do it myself.

@peterbraden
Copy link
Owner

Hi, sorry for the lack of update. I resolved the merge (hopefully correctly, as I don't have the tests on this machine :s )

Merging now. Thanks for your patience!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants