search

pbogut / recurring_events

Elixir library for dealing with recurring events
MIT License
26 stars 8 forks source link

The lib does not respect RRULE RFC in case of invalid dates #19

Closed kelvinst closed 3 years ago

kelvinst commented 3 years ago

The RRULE FREQ=MONTHLY;INTERVAL=1;DTSTART=20201031T000000 should actually skip november according to RRULE RFC:

Recurrence rules may generate recurrence instances with an invalid date (e.g., February 30) or nonexistent local time (e.g., 1:30 AM on a day where the local time is moved forward by an hour at 1:00 AM). Such recurrence instances MUST be ignored and MUST NOT be counted as part of the recurrence set.

So basically, this test should pass:

assert RecurringEvents.take(~D[2020-10-31], %{freq: :monthly}, 2) == [~D[2020-10-31], ~D[2020-12-31]]
pbogut commented 3 years ago

It does not respect RRULE RFC at all, as per documentation "It loosely follows iCal Recurrence rule specification RFC 2445.". I've put loosely there because I had no intention to make it strict RFC compliance, in fact in the project I needed it for I needed it to round up to the last day of the month in this specific case.

As for changing it, If I ever do that it would have to be some kind of option like mode: :strict or something like that. If you fancy doing it yourself, I'm happy to accept PR with condition that it can work both ways.

kelvinst commented 3 years ago

Alright, I do have the change in my fork already, I just need to add a flag for it. Not sure if I'm going to be able to do that soon though. Let's see 👍

kelvinst commented 3 years ago

As for changing it, If I ever do that it would have to be some kind of option like mode: :strict or something like that. If you fancy doing it yourself, I'm happy to accept PR with condition that it can work both ways.

@pbogut

So this option you mention, do you prefer RecurringEvents.unfold(~D[2020-10-31], %{freq: :monthly, mode: :strict}) or RecurringEvents.unfold(~D[2020-10-31], %{freq: :monthly}, mode: :strict)?

PS.: the latter would make some functions conflict on the public API, probably possible to do not breaking compatibility of the main API, but will have to do some juggling with the functions params.

pbogut commented 3 years ago

I would say former, and it looks like that's the way you did it in PR, so that's fine.