Tickets :: Comment on [#8176] isAllDay() logic incorrect?

* Your Email Address
* Spam protection	Enter the letters below: .___. __..__ __ [__ \| (__ \| \/ ` \| \|___.__)\|__/\__.
Comment	> I'm running into what I believe is a bug in the display of > non-recurring all day events. I have the following event: > > > > SELECT event_title, event_recurtype, event_start, event_end > > FROM kronolith_events WHERE event_id = 'eed64d52188a4a4732607fa1f12f812a'\G > > ************************* 1. row ************************* > > event_title: all day > > event_recurtype: 0 > > event_start: 2009-04-09 00:00:00 > > event_end: 2009-04-10 00:00:00 > > > > According to the logic in isAllDay() in Driver.php lines 1703-1711, > that should be an all day event: > > > > function isAllDay() > > { > > return ($this->start->hour == 0 && $this->start->min == 0 && > $this->start->sec == 0 && > > (($this->end->hour == 0 && $this->end->min == 0 && > $this->end->sec == 0) \|\| > > ($this->end->hour == 23 && $this->end->min == 59)) && > > ($this->end->mday > $this->start->mday \|\| > > $this->end->month > $this->start->month \|\| > > $this->end->year > $this->start->year)); > > } > > > > The start and end hour, minute, and second are all 0; and the end > mday is greater than the start mday. > > > > Unfortunately, by the time isAllDay() is called on this event, the > end mday has been changed by code in Kronolith.php: > > > > /* If the event doesn't end at 12am set the end date to the > > * current end date. If it ends at 12am and does not end at > > * the same time that it starts (0 duration), set the end date > > * to the previous day's end date. / > > if ($event->end->hour != 0 \|\| > > $event->end->min != 0 \|\| > > $event->end->sec != 0 \|\| > > $event->start->compareDateTime($event->end) == 0 \|\| > > $event->isAllDay()) { > > $eventEnd = Util::cloneObject($event->end); > > } else { > > $eventEnd = new Horde_Date( > > array('hour' => 23, > > 'min' => 59, > > 'sec' => 59, > > 'month' => $event->end->month, > > 'mday' => $event->end->mday - 1, > > 'year' => $event->end->year)); > > } > > > > Consequently, when isAllDay() is called, the end mday is no longer > greater than the start mday, and this event is _not_ displayed as an > all-day event, but rather as an event that lasts pretty much all of > the day. > > > > I think* that the fix will be to change the logic in inAllDay() to this: > > > > return ($this->start->hour == 0 && $this->start->min == 0 && > $this->start->sec == 0 && > > (($this->end->hour == 0 && $this->end->min == 0 && > $this->end->sec == 0 && > > ($this->end->mday > $this->start->mday \|\| > > $this->end->month > $this->start->month \|\| > > $this->end->year > $this->start->year)) \|\| > > ($this->end->hour == 23 && $this->end->min == 59 && > $this->end->sec == 59))); > > > > The old logic tested for (start is 00:00:00) and (end is 00:00:00 or > 23:59) and (end day is after start day). This revised logic tests > for (start is 00:00:00) and (end is 23:59:59 or (end is 00:00:00 and > end day is after start day)). > > > > That appears to work in my tests, but I'm not entirely sure if there > are other edge cases I'm not considering. I've attached a patch with > that change anyway.
Attachment
Watch this ticket

Saved Queries