occurrences for one time event schedules and using add_recurrence_time

[thoughtafter]

The initial start_time should be an occurrence, at least assuming that occurrences should mirror the to_ical display of occurrences. This is important for 1 time events (which have no occurrences) and when using add_recurrence_rule (which drop the initial occurrence),

irb(main):014:0> s = IceCube::Schedule.new
=> 
irb(main):015:0> s.start_time = Time.local(2013, 1, 1, 16)
=> 2013-01-01 16:00:00 -0800
irb(main):016:0> s.end_time = Time.local(2013, 1, 1, 20)
=> 2013-01-01 20:00:00 -0800
irb(main):017:0> s.to_ical
=> "DTSTART;TZID=PST:20130101T160000\nDTEND;TZID=PST:20130101T200000"
irb(main):018:0> s.all_occurrences
=> []

[avit]

An empty schedule is somewhat pathological, but yes, that seems like the right thing to do... at least keep the representation consistent between:

• to_ical
• to_s
• all_occurrences

[thoughtafter]

My point is that it is not consistent because to_ical clearly shows an event while to_s and all_occurrences do not. According to the RFC setting start time and end time is enough to define an event so, if you want RFC compliance, to_ical is correct but the tracking of occurrences is not.

[thoughtafter]

From the RFC, which is very clear on this point:

The "DTSTART" property for a "VEVENT" specifies the inclusive start of the event. For recurring events, it also specifies the very first instance in the recurrence set.

And:

The recurrence set is generated by considering the initial "DTSTART" property along with the "RRULE", "RDATE", "EXDATE" and "EXRULE" properties contained within the iCalendar object. The "DTSTART" property defines the first instance in the recurrence set.

And maybe this library isn't trying to be RFC 2445 compliant but if that's the case it should be stated clearly so people who are looking for that compliance do not waste their time.

[avit]

Yes, we should try to be consistent with the spec. I think this change should not affect too many people, the following are currently pathological cases:

Empty schedule: it's useless without an rtime. We should include the start occurrence.

A schedule with one rtime that differs from start_time: who would even do this? Currently, this schedule has one occurrence, but the ical has DTSTART and RTIME, good for two occurrences. We should include the start occurrence to be consistent.

A schedule with one rtime that matches the start_time: Currently that's what we do for a single-event occurrence. We should preserve this, but it needs fixing for ical. (This would become functionally equal to an empty schedule.)

1. IceCube::Schedule all_occurrences should have one occurrence for start_time when empty

      expected: [2012-12-12 12:12:12 -0800]
   
           got: [] (using ==)
   

2. IceCube::Schedule all_occurrences should have two occurrences with a recurrence time after start_time

      expected: [2012-12-12 12:12:12 -0800, 2013-01-13 01:13:01 -0800]
   
           got: [2013-01-13 01:13:01 -0800] (using ==)
   

3. IceCube::Schedule add_recurrence_time raises ArgumentError if the time is before start_time
   expected ArgumentError but nothing was raised

4. IceCube to_ical should not have an rtime that duplicates start time

      expected: "DTSTART:20121212T120000Z"
   
           got: "DTSTART:20121212T120000Z\nRDATE:20121212T120000Z" (using ==)
   

5. IceCube::Schedule to_s should show start time for an empty schedule

      expected: "March 20, 2010"
   
           got: "" (using ==)
   

The only question I have is item 3: I'm wondering if we should raise an error since it doesn't make sense to add invisible times then strip them out (currently, they show up for ical and to_s but not occurrences). Feedback welcome: is it valid in icalendar to have an RTIME earlier than DTSTART? Possibilities for adding early rtimes (before start_time):

• Accept them, show them consistently in all_occurrences, to_s, to_ical
• Silently drop them
• Raise an error

Thanks for looking into this with me @thoughtafter, can I trouble you to check the RFC for this last question?

[avit]

Actually, I think I'll reconsider item 3 in its own issue, it has its own set of concerns... but from all I could find on RDATE:

The "DTSTART" property defines the first instance in the recurrence set.

My interpretation is that nothing should exist before the start time, so we should ignore any earlier times in all representations, whether or not adding them to the schedule is allowed.

[thoughtafter]

I believe you are correct in your interpretation: any occurrences before DTSTART are invalid. The trickier question is what to do about it. I've given it some thought and haven't come to a conclusion. It's hard to imagine how someone could make this happen. However, I wanted to add that there is another solution:

if RDATE is added that is less than DTSTART change DTSTART to RDATE and add RDATE with previous DTSTART.

This is the only solution that allows arbitrary RDATES to be added and not get an exception or dates dropped. I'm not saying this is the best solution, part of me thinks an exception is the way to go and leave it up to the user to make sure they get what they want. But it's something to consider.