JonasWanke / rrule

🔁 Recurrence rule parsing & calculation as defined in the iCalendar RFC
https://pub.dev/packages/rrule
Apache License 2.0
52 stars 22 forks source link

byWeekDays with Weekly is wrongly unsupported #42

Closed jdumont0201 closed 1 year ago

jdumont0201 commented 2 years ago

Describe the bug Combining byWeekDays with weekly throws an assert error, however it is a legit case.

The code

RecurrenceRule(
            frequency: Frequency.weekly,
            interval: 1,
            byWeekDays: {ByWeekDayEntry(DateTime.wednesday, DateTime.friday)},
            until: DateTime(2022, 10, 1, 0, 0, 0).toUtc(),
);

throws

assert([Frequency.monthly, Frequency.yearly].contains(frequency) || byWeekDays.noneHasOccurrence,)
The BYDAY rule part MUST NOT be specified with a numeric value when the FREQ rule part is not set to MONTHLY or YEARLY.```

but I believe it is a wrong interpretation of the specification.

Indeed, it must not be specified by a numeric value, but it can be with an array value to describe a usecase like "Repeat every week on Wednesdays and Fridays" as in

FREQ=WEEKLY;BYDAY=WE,FR;INTERVAL=1;UNTIL=20221001T000000Z

Same for count:

FREQ=WEEKLY;BYDAY=WE,FR;INTERVAL=1;COUNT=10

Expected result Using another library https://github.com/teambition/rrule-go

r, _ := rrule.NewRRule(rrule.ROption{
    Freq:      rrule.WEEKLY,
    Interval:  1,
    Byweekday: []rrule.Weekday{rrule.WE, rrule.FR},
    Until:     time.Date(2022, 10, 1, 0, 0, 0, 0, time.UTC),
})

yields

"Wed,2022-09-21",
"Fri,2022-09-23",
"Wed,2022-09-28",
"Fri,2022-09-30"

which is cover this case correctly.

Environment:

JonasWanke commented 1 year ago

Hi and sorry for the late response! The combination that you want to use is actually supported, but there's a small mistake in your code: You create one instance of ByWeekDayEntry and pass your two week days. However, the constructor only takes one weekday – the second parameter is the occurrence (defaults to null). E.g., you would write ByWeekDayEntry(DateTime.wednesday, 5) to express 5WE (“only on the fifth Wednesday”).

To specify multiple weekdays, write {ByWeekDayEntry(DateTime.wednesday), ByWeekDayEntry(DateTime.friday)}. Applying this change to your code above yields:

final recurrenceRule = RecurrenceRule(
  frequency: Frequency.weekly,
  interval: 1,
  byWeekDays: {
    ByWeekDayEntry(DateTime.wednesday),
    ByWeekDayEntry(DateTime.friday),
  },
  until: DateTime(2022, 10, 1, 0, 0, 0).toUtc(),
);
print(
  recurrenceRule.getAllInstances(
    start: DateTime(2022, 09, 19, 0, 0, 0).toUtc(),
  ),
);

// Output: [2022-09-21 22:00:00.000Z, 2022-09-23 22:00:00.000Z, 2022-09-28 22:00:00.000Z, 2022-09-30 22:00:00.000Z]

Also, you should create DateTimes for this package using DateTime.utc(…) instead of DateTime(…).toUtc(), since the latter performs a conversion that is usually not wanted. That's why the output DateTimes above are not at midnight (since my local timezone is not UTC). When this is fixed, the output looks like Go's output from above (just with different formatting):

final recurrenceRule = RecurrenceRule(
  frequency: Frequency.weekly,
  interval: 1,
  byWeekDays: {
    ByWeekDayEntry(DateTime.wednesday),
    ByWeekDayEntry(DateTime.friday),
  },
  until: DateTime.utc(2022, 10, 1, 0, 0, 0),
);
print(
  recurrenceRule.getAllInstances(start: DateTime.utc(2022, 09, 19, 0, 0, 0)),
);

// Output: [2022-09-21 00:00:00.000Z, 2022-09-23 00:00:00.000Z, 2022-09-28 00:00:00.000Z, 2022-09-30 00:00:00.000Z]