rossta / montrose

Recurring events library for Ruby. Enumerable recurrence objects and convenient chainable interface.
https://rossta.net/montrose/
MIT License
843 stars 53 forks source link

Bug: improper calculation of "Friday 13th each year" #88

Closed garmoshka-mo closed 7 years ago

garmoshka-mo commented 7 years ago

Programming rule "let's have a party 🎉 each year on some Friday 13th"

  schedule = Montrose.yearly(mday: 13, day: :friday)
  puts schedule.take(5)

"yearly" - in english language means "once a year" - right?

gives improper result (few events in some years):

2017-10-13 19:39:35 UTC 2018-04-13 19:39:35 UTC 2018-07-13 19:39:35 UTC 2019-09-13 19:39:35 UTC 2019-12-13 19:39:35 UTC

rossta commented 7 years ago

@garmoshka-mo I'm not sure this is a bug, but I understand the term yearly may be ambiguous in this case.

I believe this behavior is consistent with the RFC spec for ICAL recurrence rules in which, depending on the structure of the rule, there can be multiple occurrences within a given year, even though the frequency is specified as "yearly".

Note in the example below, excerpted from https://tools.ietf.org/html/rfc5545#section-3.8.5.3, the recurrence yields multiple date occurrences within a given year:

      Yearly in June and July for 10 occurrences:

       DTSTART;TZID=America/New_York:19970610T090000
       RRULE:FREQ=YEARLY;COUNT=10;BYMONTH=6,7

       ==> (1997 9:00 AM EDT) June 10;July 10
           (1998 9:00 AM EDT) June 10;July 10
           (1999 9:00 AM EDT) June 10;July 10
           (2000 9:00 AM EDT) June 10;July 10
           (2001 9:00 AM EDT) June 10;July 10

         Note: Since none of the BYDAY, BYMONTHDAY, or BYYEARDAY
         components are specified, the day is gotten from "DTSTART".

In the example you've provided, BYMONTH is implicitly any month in the year. This explains why you may end up with multiple occurrences per year and the behavior is identical to using Montrose.monthly as the frequency.

So, while I understand it may be surprising behavior, I'd prefer to leave as is.

garmoshka-mo commented 7 years ago

@rossta definitely thank you for deep analysis of question and detailed explanations!

With regards to RFC example - it looks natural to trigger two dates for person's request "remind me yearly in July AND June on day 10".

I'm not very familiar with RFCs principles, do I correctly understand, that it usually describes general requirements, but not strict algorithm? If so, is it possible that particular implementation of algorithm for handling frequency may contain issues? Why I'm asking, is because there is cases which described by unambiguous statements, but give unexpected results. For example:

Yearly on Sunday review family album

irb(main):011:0> puts Montrose.yearly(day: :sunday).take 5
2017-08-13 06:41:05 +0000
2017-08-20 06:41:05 +0000
2017-08-27 06:41:05 +0000
2017-09-03 06:41:05 +0000
2017-09-10 06:41:05 +0000

Monthly on Monday make report for previous month

irb(main):012:0> puts Montrose.monthly(day: :monday).take 5
2017-08-14 06:41:37 +0000
2017-08-21 06:41:37 +0000
2017-08-28 06:41:37 +0000
2017-09-04 06:41:37 +0000
2017-09-11 06:41:37 +0000

This result would be expected by stating weekly(day: :monday)

If mentioned commands would give expected by human results - would it conflict with RFC?

rossta commented 7 years ago

I think the results of both of your new examples still make sense if you accept my interpretation of the RFC.

When you say "yearly on Sunday", no month or week is specified; this is still ambiguous IMHO. My interpretation is that, then, all possible months and all weeks would match. Montrose does provide an explicit mechanism for specifying the nth day of the year by weekday:

// first Sunday of the year
pry(main)> puts Montrose.yearly(day: { sunday: [1] }, starts: Date.today.beginning_of_year).take(5)
2017-01-01 00:00:00 -0500
2018-01-07 00:00:00 -0500
2019-01-06 00:00:00 -0500
2020-01-05 00:00:00 -0500
2021-01-03 00:00:00 -0500

For the second example, no arguments are specified that would pin the results to a given week, so again, all possible weeks are matched. Similarly, you can ask for the first Monday of each month as follows:

pry(main)> puts Montrose.monthly(day: { monday: [1] }, starts: Date.today.beginning_of_month).take(5)
2017-08-07 00:00:00 -0400
2017-09-04 00:00:00 -0400
2017-10-02 00:00:00 -0400
2017-11-06 00:00:00 -0500
2017-12-04 00:00:00 -0500

Montrose does not assume you want only the first occurrence within a given interval unless the rules dictate that should be the behavior. Hope that's helpful.