"All day" nodes are not detected as such because of the granularity of the Date field
| Project: | Calendar |
| Version: | 6.x-2.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Hello,
I found an annoying bug, it is located in includes/calendar.inc, near line 519 in calendar_build_nodes() :
// Make date objects
$node->calendar_start_date = date_create($node->calendar_start, timezone_open($to_zone));
$node->calendar_end_date = date_create($node->calendar_end, timezone_open($to_zone));
if (date_format($node->calendar_start_date, 'H:i:s') == '00:00:00' &&
date_format($node->calendar_end_date, 'H:i:s') == '23:59:59') {
$node->calendar_all_day = TRUE;
}
else {
$node->calendar_all_day = FALSE;
}In this code, it is defined if a node is considered as a "calendar_all_day" node. When it is true, those nodes will be displayed as such in the calendar view. The problem is that my CCK Date field does not include "seconds" in its "Granularity". So when I create a node with start date as YYYY-MM-DD 00:00 and end date as YYYY-MM-DD 23:59 it is not considered as a "calendar_all_day" because the end date is set as YYYY-MM-DD 23:59:00 in the database (no matter if it's a TIMESTAMP or a DATETIME of course). The seconds are defaulted at "00" because the form node does not set them (since they are not available due to the "Granularity" of the Date field.)
If I use a Date field with seconds in the Granularity and I set the seconds to "59", all works as expected, all my "All day" nodes are displayed in the calendar views correctly. I don't see what would be the best approach to check if a day is complete or not, but this is definitely a bug and should be done differently (or at very least, documented -> Only use Date fields with complete time). I suppose a way to handle this would be to check the "Granularity" of the field should and then determine the exact range that would compose a complete day for each Date field.
Something related, but which might have to be splited into another issue, in Week view, the nodes are not displayed unless they are "All day". They can be displayed if you enable grouping (by hour, for example) for this view, why aren't the nodes which last only one hour or are just a "start date" (without end date) displayed on Week view ? They are displayed on Month and Day views, so why not on Week view ? I discovered it because my nodes with the previously mentioned granularity weren't displayed, since they aren't "All day" according to the current code.
P.S. : All this was somewhat hard to figure out, because in the node page, the date field was displaying "(All day)" although "internally" it was not considered as a "calendar_all_day".
#1
Here is a patch that fixes this issue.
It works out what granularity and increment your field uses and calculates all_day accordingly.
The patch was created for 6.x-2.x-dev (2009-May-12) but will also apply to 6.x-2.1 (with an offset of 1 line).
Please try it and let me know if you have any issues with it.
#2
This isn't critical
#3
Ok, sorry for the "critical".
I tested the patch, it works as expected. Thank you.
There is one thing left, on the node with a date field, the way that days are "detected" as "All day" includes another case (when themed), when both start and end dates are the same and their time is 00:00:00 it displays "(All day)" after the date. I do not think that this is the best approach, because it is not possible to have an event that happens exactly at midnight on a certain day (but it is possible to have one that happens exactly at 01:00:00 for example). Anyways it is how it is done in the Date module. But in the Calendar Week and Day views, such node displays in the "All times" group (which is not really logical because it could be grouped by the hour too... like it is happening with nodes starting and ending at 01:00:00, they get grouped in "01:00"... why isn't there a "00:00" group ?). So either we stay consistent with Date module and display them in "All day" or we should fill an issue in their queue to ensure only event starting at 00:00:00 and ending 23:59:59 (without forgetting granualrity, as you just fixed are marked as "All day". (**EDIT** Seems like I am not the only one thinking the date field way to detect "All day" is not logical : #449274: Impossible to set time to midnight?)
The thing is, in any case, grouping by the time seems to put all event starting at 00:00:00 in the "All times", which does not make more sense, or maybe I didn't understand the meaning of this group. Problem, if there is one, might be located in theme/theme.incm near line 646 :
if ($start_time == '00:00:00' && $next_start_time == '23:59:59') {$hour = t('All times');
}
#4
This still is a problem in 6.x-2.2! #444766: Calendar Week view shows All Times and All Day seems to address a similar problem!
#5
Marked #450174: All Day events dont show in all dat slot on day view as duplicate of this issue.
#6
Patch also applies cleanly to 6.x-2.2 and 6.x-2.x-dev (2009-Jul-29).
#7
I am still seeing this strange behavior:
http://img.skitch.com/20090811-xmi35m3p8b86pxw3nxnuk8jqbj.jpg
#8
@agerson
What to/from date/time do you have set for the labor day node?
[edit] Also, what is the granularity of that date field.
#9
The from date was september 7th 2009, the time field was blank, which I believe the module stores as 12am. The to date was left blank which I believe the module sets the to date to be the same as the from date. How do I find the "granularity" for the field?
#10
This doesn't seem to work for my imported iCal feed. iCal has this information for an (All Day) event.
DTSTART;VALUE=DATE:20090204
DTEND;VALUE=DATE:20090205
So I figure since there isn't any time info, your patch doesn't apply to this instance. I added this extra argument on line 558 and it achieves the desired affect.
- date_format($node->calendar_end_date, $max_format) {+ date_format($node->calendar_end_date, $max_format == $max_time || date_format($node->calendar_start_date, 'H:i:s') == 0) {
of course this second way works as well but maybe the logic is not as sound as it should be for either?
date_format($node->calendar_end_date, $max_format == $max_time || date_format($node->calendar_start_date, 'H:i:s') == date_format($node->calendar_end_date, 'H:i:s') {#11
I am seeing the same problem as @agerson, i.e.: a node shows "(All day)", yet appears in the "Before 9:00 AM" row.
The field's Time Increment is 1, and its granularity has Year, Month, Day, Hour and Minute selected. Changing the Time Increment doesn't seem to have an effect on the problem.
[Edit: Silly me. I did not apply the patch correctly. After having done so, this problem goes away. I hope KarenS incorporates this soon.]
#12
when I try to apply your patch on my multilingual site (french and arabic site)
I have a white screen of death ...
#13
Justin W Freeman wrote in #6 on August 10, 2009 - 02:44:
> Patch also applies cleanly to 6.x-2.2 and 6.x-2.x-dev (2009-Jul-29).
Confirming the issue for Calendar 6.x-2.2 with Date 6.x-2.4; also confirming that the patch seems to apply cleanly:
> patch < calendar-447728-1.patchHmm... Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|diff -rupN calendar_old//includes/calendar.inc calendar/includes/calendar.inc
|--- calendar_old//includes/calendar.inc 2009-05-12 08:24:27.000000000 +1000
|+++ includes/calendar.inc 2009-05-26 12:31:43.000000000 +1000
--------------------------
Patching file includes/calendar.inc using Plan A...
Hunk #1 succeeded at 516.
done
However, the bug is not fixed for me by this patch (PHP 5.2.10 with Suhosin-Patch 0.9.7; Granularity of "Date" field: Year, Month, Day, Hour; localized version for German, some (minor) modifications to the "calendar" view).
Thanks & greetings, -asb
#14
Made a few changes to the patch file to use the date_seconds and date_minutes functions provided by the Date module.
There was a bug when dealing with multi-day events that required a tertiary check of the max_time.
#15
Parden my previous posting, I had a bug in the format generation code.
This one works.
#16
The patch (#15) was rejected.
I have problem not only with events that have hour (and more) granularity, but also with events that have max. day granularity. As mentioned above (original issue), they are not visible at all in the week (and also in the day) view. It's quite a big problem.
Suprisingly, the only "solution" (I have found so far) is to set the Date zone completely off (settings for the whole site... but it is not possible on many websites). I don't see any relationship, but without timezone the events are at least visible as supposed (in the right days).
#17
+1 to follow the request. We used the patch #15 for a website.