-
Notifications
You must be signed in to change notification settings - Fork 125
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
RRule Yearly with BYMONTH and BYDAY parts not calculating correctly #626
Comments
@phil-davis we have seen another similar issue. Can you advise on this one before we take the next step? Thanks! |
https://datatracker.ietf.org/doc/html/rfc5545
So the RFC expects that an event has a DTSTART specified that actually is a "valid" event date-time that the RRULE filter would generate. The existing sabre-io/vobject code is more flexible than that. It allows DTSTART to be "whatever date" and usually finds and schedules the first occurrence of the event on the first date on or after that, which the RRULE filter will allow. So this example is a bug - sometimes there may be no occurrences of an event from the DTSTART through to the end of the start year. The code should not return some date in the DTSTART year that does not match the RRULE. The code should be returning the first occurrence as the first date in the next year that matches the RRULE. If someone has a proposed fix, please make a PR with unit test cases and code to make them pass. |
PR ready for your review. |
Fixed by #656 - thanks |
The following recurring event:
Starts Sep 1 and should recur every year last Friday of June but first recurrence falls on Fri, Sep 29 and then next ones are in June next years.
It seems the code at https://github.com/sabre-io/vobject/blob/master/lib/Recur/RRuleIterator.php#L612 bypasses BYMONTH checks for next yearly recurrence if the current day of month is less than the recurring one. Shouldn't we always check if the next recurrences happen on the specified BYMONTH no matter what or am I misunderstanding RFC5545?
The text was updated successfully, but these errors were encountered: