Summary | isAllDay() logic incorrect? |
Queue | Kronolith |
Queue Version | 2.3.1 |
Type | Bug |
State | Resolved |
Priority | 3. High |
Owners | jan (at) horde (dot) org |
Requester | stpierre (at) nebrwesleyan (dot) edu |
Created | 04/09/2009 (5943 days ago) |
Due | |
Updated | 04/26/2009 (5926 days ago) |
Assigned | 04/16/2009 (5936 days ago) |
Resolved | 04/26/2009 (5926 days ago) |
Github Issue Link | |
Github Pull Request | |
Milestone | 2.3.2 |
Patch | Yes |
State ⇒ Resolved
http://cvs.horde.org/diff.php/kronolith/lib/Driver.php?rt=horde&r1=1.116.2.81&r2=1.116.2.82&ty=u
New Attachment: allday.patch
Milestone ⇒ 2.3.2
Priority ⇒ 3. High
State ⇒ Feedback
Assigned to Jan Schneider
Priority ⇒ 1. Low
Type ⇒ Bug
Summary ⇒ isAllDay() logic incorrect?
Queue ⇒ Kronolith
Milestone ⇒
Patch ⇒ Yes
State ⇒ Unconfirmed
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.