Skip to content

Add recurring events support#46

Merged
rickychilcott merged 12 commits intomainfrom
feature/recurring-events
Jul 20, 2024
Merged

Add recurring events support#46
rickychilcott merged 12 commits intomainfrom
feature/recurring-events

Conversation

@rickychilcott
Copy link
Copy Markdown
Member

Address #45

@rickychilcott
Copy link
Copy Markdown
Member Author

We use the calendar-recurrence gem to build up the occurrences, but instead of working with them directly, we duplicate the base event and change the dtstart and dtend to coincide with that occurrence.

I'm a little concerned about duplicate events, but my tests say it's not an issue. Needs checking...

Wondering if the documentation and the implementation feel right.

rickychilcott and others added 6 commits July 18, 2024 16:52
# Conflicts:
#	Gemfile
#	lib/jekyll-ical-tag/calendar_parser.rb
#	spec/calendar_feed_coordinator_spec.rb
# Conflicts:
#	lib/jekyll-ical-tag/calendar_parser.rb
@rickychilcott
Copy link
Copy Markdown
Member Author

@whatnotery - I was able to get your commit in (and updated the specs to reflect dropping the original event) in 1fe0464.

Can you try this branch once more and make sure it works for your needs? BTW, do you think the documentation is clear?

@rickychilcott
Copy link
Copy Markdown
Member Author

Actually @whatnotery, now that I think about it dropping the first occurrence might be wrong.

Imagine a recurring event defined 2 years ago. Asking for the occurrences between today and next year would give you all of those occurrence, none of which should be dropped.

I’ll need to work out this I tests tomorrow/this weekend.

@whatnotery
Copy link
Copy Markdown
Contributor

This commit resolves the issue as far as I can tell from the testing I did in my dummy applications. It doesn't do the drop(1) so events down the line aren't dropped but it utilizes the first occurrence from .occurrences_between instead from the initial parse

2526d40

@rickychilcott rickychilcott force-pushed the feature/recurring-events branch from 311acf2 to ca653df Compare July 19, 2024 13:37
@rickychilcott
Copy link
Copy Markdown
Member Author

Hmm I'm not sure about 2526d40. My read of that code is that it won't even include ANY recurring events.

Either way, I think ca653df and 6c066e1 will do it, and I think my tests prove it properly.

@rickychilcott
Copy link
Copy Markdown
Member Author

OK. Maybe i see how 2526d40 would do it. Every event (even if it isn't a recurring event) will have "occurrences"

I think that could would negatively interact with other filters though because of how @recurring_start_date and @recurring_end_date would limit. I think I like my solution, but I'm not 100% sure it's right.

Thanks for working through this with me.

@whatnotery
Copy link
Copy Markdown
Contributor

Yeah of course! Thanks for being such an awesome maintainer! It's crazy to me to have made a feature request and to have the feature built by the maintainer less than two days later. Thank you so much. I'll test it out later this evening or tomorrow

@rickychilcott rickychilcott merged commit f8a8189 into main Jul 20, 2024
@rickychilcott
Copy link
Copy Markdown
Member Author

Available in version 1.6.0. Thanks @whatnotery!

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.

2 participants