Event Repeater dosen't work properly with multi-day events
| Project: | Event Repeat |
| Version: | 5.x-2.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
I tried to add two-day event two times but it doesn't work.
I filled form with settings:
Repeat type: Weekly
Count: 2
Interval: 1
Days: Monday, Tuesday
Event has time: 15 October 2007 10:00
Event has end date: 16 October 2007 16:00
My timezone: Europe/London
After that I got 3 days assigned: 15 Oct (All Day), 16 Oct (All Day), 16 Oct
(10-16)
instead of 4 days: 15 Oct, 16 Oct, 22 Oct, 23 Oct
The second event seems to be 1 day instead of 2 day event.
I've tried to use all possible versions.
First I tested on official release of the Event and Event Repeat modules,
but it didn't work at all, so I download latest version Event (HEAD) and Event
Repeater (DRUPAL-5--2) from CVS repository, but I experience the same
problem.
I've also tested on fresh copy of Drupal 5.2.
See attachment for details.
Thanks,
Rafal
| Attachment | Size |
|---|---|
| Clipboard02.png | 33.98 KB |
#1
You're probably right that this is happening across all version. I think it's always been a problem with this module.
#2
Patch for multi-day supporting (for eventrepeat.module, 1.58.2.8 2007/09/26 version from CVS).
* Multi-day events now working with repeating patterns
* Fixed the 'Tuesday' bug as a default day when a user hasn't selected any day of a week
* Fixed 1970 year in 'admin/settings/eventrepeat/debug' page with a new version of the Event module.
* Optimized some notices with Undefined variables (especially in loops when Drupal execute hundred times of error_handler())
* TODO: on multi-day events if user select some specified Day(s) of Week there is no correction for next days of this event
#3
I'll spend some time reviewing this later today.
#4
I've spent some time looking through this, and for the most part it looks good. There are several issues being addressed at once here, though, and that is making it a bit harder to wade through this and the 5.x-1x version at the same time.
A lot of it seems to just be fixing the TRUE checks to !empty(), which is easy to follow.
For the purposes of understanding how I'll have to patch the 5.x-1.0 version to incorporate these fixes, can you provide a bit more explanation of why you had to makes the changes that I've called out below? Just a quick: it was doing this so I found that doing this make this work" type of description would help.
@@ -1573,7 +1573,7 @@
//update the event start and end time TODO: event times are
//getting adjusted here (possible cause of double shift)
db_query("UPDATE {event} SET event_start = '%s', event_end = '%s' WHERE nid = %d", $start_time, $end_time, $old_date->nid);
- $last_rendered = $new_date->date;
+ $last_rendered = $end_time;
}
//update the last rendered value for the updated sequence
@@ -1591,8 +1591,8 @@
//calculate the repeat_last_rendered value from the event start date
$last_rendered = event_explode_date($last_rendered->event_start);
- $last_rendered['hour'] = '23';
- $last_rendered['minute'] = $last_rendered['second'] = '59';
+ //$last_rendered['hour'] = '23';
+ //$last_rendered['minute'] = $last_rendered['second'] = '59';
$last_rendered = event_implode_date($last_rendered);
}
//no new dates means that the sequence is completely rendered,
@@ -1621,9 +1621,9 @@
else {
// calculate the repeat_last_rendered value from the event start
// date and sets it to 23:59:59 on that day
- $lastrendered = event_explode_date($node->event['start_orig']);
- $lastrendered['hour'] = '23';
- $lastrendered['minute'] = $lastrendered['second'] = '59';
+ $lastrendered = event_explode_date($node->event['end_orig']);
+ //$lastrendered['hour'] = '23';
+ //$lastrendered['minute'] = $lastrendered['second'] = '59';
$lastrendered = event_implode_date($lastrendered);
And further down this chunk:
@@ -1998,10 +1998,12 @@
$event_start_time = event_explode_date($repeat_data->event_start);
$event_end_time = event_explode_date($repeat_data->event_end);
$event_date = event_explode_date($date_to_render->date);
+ $event_duration = event_duration($event_start_time, $event_end_time);
+ $event_date_end = event_date_later($event_date, $event_duration);
$repeat_data->event_start = event_implode_date(array_merge($event_date, array('hour' => $event_start_time['hour'], 'minute' => $event_start_time['minute'], 'second' => $event_start_time['second'])));
- $repeat_data->event_end = event_implode_date(array_merge($event_date, array('hour' => $event_end_time['hour'], 'minute' => $event_end_time['minute'], 'second' => $event_end_time['second'])));
+ $repeat_data->event_end = event_implode_date(array_merge($event_date_end, array('hour' => $event_end_time['hour'], 'minute' => $event_end_time['minute'], 'second' => $event_end_time['second'])));
$repeat_data->start_in_dst = event_is_dst($repeat_data->timezone, $repeat_data->event_start);
$repeat_data->end_in_dst = event_is_dst($repeat_data->timezone, $repeat_data->event_end);
Mostly I just want to make sure I understand how flipping start to end and end to start affects everything.
Thanks!
#5
Sean B Fuller:
In followed code the last rendered variable store information about which date was last rendered and from which date you can start creating a next events. If you set it as start date of this event ($new_date->date), then multi-day events will superimpose on a previous one. So we need to set it as the last day of last added event.
- $last_rendered = $new_date->date;+ $last_rendered = $end_time;
Followed code I think is unnecessary, because you don't need to change time of last rendered date to 23:59, because it's not interpreted when you calculate next date of events using last rendered variable. And you don't need to thinking why on admin/settings/eventrepeat/debug page (Repeat End and Last Rendered) there is 23:59 time instead the correct time of your event.
- $last_rendered['hour'] = '23';- $last_rendered['minute'] = $last_rendered['second'] = '59';
+ //$last_rendered['hour'] = '23';
+ //$last_rendered['minute'] = $last_rendered['second'] = '59';
We need to also calculate duration of our event (in days) to set correct end of date of our event.
+ $event_duration = event_duration($event_start_time, $event_end_time);+ $event_date_end = event_date_later($event_date, $event_duration);
Before the end date of our repeated event was just the same like start date of event (you can see below), so it not makes sense (all repeated events were only one-day). So we need to change the end date of our event to correct date (date of event_start + event_duration() of this event).
$repeat_data->event_start = event_implode_date(array_merge($event_date, array('hour' => $event_start_time['hour'], 'minute' => $event_start_time['minute'], 'second' => $event_start_time['second'])));- $repeat_data->event_end = event_implode_date(array_merge($event_date, array('hour' => $event_end_time['hour'], 'minute' => $event_end_time['minute'], 'second' => $event_end_time['second'])));
+ $repeat_data->event_end = event_implode_date(array_merge($event_date_end, array('hour' => $event_end_time['hour'], 'minute' => $event_end_time['minute'], 'second' => $event_end_time['second'])));
And I think that it is good idea to change converting function from is_string() to !is_numeric() from this patch, it is more suitable. Because if you have number but in string format it will try to convert it to unixtime even if it's already in unixtime format. But if you use !is_numeric() it will check if this variable is not a number and also if it's a not numeric string.
- array("data" => format_date($projection->repeat_start, 'small', '', 0)),- array("data" => ($projection->repeat_end > 0) ? format_date($projection->repeat_end, 'small', '', 0) : check_plain($projection->repeat_end)),
- array("data" => format_date($projection->repeat_last_rendered, 'small', '', 0)),
+ array("data" => format_date(is_string($projection->repeat_start) ? strtotime($projection->repeat_start) : $projection->repeat_start, 'small', '', 0)),
+ array("data" => format_date(is_string($projection->repeat_end) ? strtotime($projection->repeat_end) : $projection->repeat_end, 'small', '', 0)),
+ array("data" => format_date(is_string($projection->repeat_last_rendered) ? strtotime($projection->repeat_last_rendered) : $projection->repeat_last_rendered, 'small', '', 0)),
++ array("data" => format_date(!is_numeric($projection->repeat_start) ? strtotime($projection->repeat_start) : $projection->repeat_start, 'small', '', 0)),
++ array("data" => format_date(!is_numeric($projection->repeat_end) ? strtotime($projection->repeat_end) : $projection->repeat_end, 'small', '', 0)),
++ array("data" => format_date(!is_numeric($projection->repeat_last_rendered) ? strtotime($projection->repeat_last_rendered) : $projection->repeat_last_rendered, 'small', '', 0)),
#6
This is great. Thanks. I'm having some issue getting in to cvs right now. Once I get over this, I'll do a final test and incorporate it.
#7
The all day and multi-day events work great, but I think I found an issue with the patch. Events that do not span more than one day end up with two events on the first day. To see this happen, create an event that only lasts an hour, and make it repeat daily or weekly for a week or more. You'll see two events on the first day. Screen shot attached.
kenorb, can you confirm this?
#8
From what I have read there are going to be lots of little things like this.
Thanks
Robert