jkbrzt / rrule

JavaScript library for working with recurrence rules for calendar dates as defined in the iCalendar RFC and more.
https://jkbrzt.github.io/rrule
Other
3.27k stars 506 forks source link

toText() doesn't describe rrule with unimplemented features as approximate #540

Open salty-horse opened 2 years ago

salty-horse commented 2 years ago

Using rrule 2.7.1.

Due to a wrong test in isFullyConvertible, most rules are treated as "fully convertible" even though they are not.

The following example uses the unsupported BYHOUR. However, any rule where the FREQ field appears before the unsupported field, will automatically be considered as supported:

> let rrule = require('rrule')
> rrule.rrulestr('FREQ=DAILY;BYHOUR=9,10,11,12,13,14,15,16;BYMINUTE=0,20,40').toText()
'every day at 9, 10, 11, 12, 13, 14, 15 and 16' // WRONG
> rrule.rrulestr('BYHOUR=9,10,11,12,13,14,15,16;BYMINUTE=0,20,40;FREQ=DAILY').toText()
'every day at 9, 10, 11, 12, 13, 14, 15 and 16 (~ approximate)' // CORRECT

This is due to this buggy check in isFullyConvertible:

for (const key in rrule.origOptions) {
  if (contains(['dtstart', 'wkst', 'freq'], key)) return true  // BUG: First encounter of these fields will 
                                                               // cause the whole rule to be convertible
  if (!contains(ToText.IMPLEMENTED[rrule.options.freq], key)) return false
}

Is this return true supposed to be a continue?

The function should only return a positive result after all fields have been checked.