From a0c564f946fa0f4ccd8aa591a89d9d4f2e8316b3 Mon Sep 17 00:00:00 2001 From: Aleksander Machniak Date: Tue, 20 Dec 2022 14:15:39 +0100 Subject: [PATCH] Fix various calendar recurrence issues --- plugins/calendar/calendar.php | 9 +- .../drivers/caldav/caldav_calendar.php | 8 +- .../calendar/drivers/kolab/kolab_driver.php | 73 ++++++------ .../lib/libcalendaring_recurrence.php | 21 +++- plugins/libcalendaring/libcalendaring.php | 21 ++++ .../libcalendaring/tests/RecurrenceTest.php | 108 +++++++++++++++++- 6 files changed, 189 insertions(+), 51 deletions(-) diff --git a/plugins/calendar/calendar.php b/plugins/calendar/calendar.php index 42942e43..727a97d9 100644 --- a/plugins/calendar/calendar.php +++ b/plugins/calendar/calendar.php @@ -2457,11 +2457,10 @@ $("#rcmfd_new_category").keypress(function(event) { */ private function notify_attendees($event, $old, $action = 'edit', $comment = null, $rsvp = null) { - $is_cancelled = false; - if ($action == 'remove' || ($event['status'] == 'CANCELLED' && ($old['status'] ?? '') != $event['status'])) { - $event['cancelled'] = true; - $is_cancelled = true; - } + $is_cancelled = $action == 'remove' + || (!empty($event['status']) && $event['status'] == 'CANCELLED' && ($old['status'] ?? '') != $event['status']); + + $event['cancelled'] = $is_cancelled; if ($rsvp === null) { $rsvp = !$old || ($event['sequence'] ?? 0) > ($old['sequence'] ?? 0); diff --git a/plugins/calendar/drivers/caldav/caldav_calendar.php b/plugins/calendar/drivers/caldav/caldav_calendar.php index 85b8bb5c..873e1f4e 100644 --- a/plugins/calendar/drivers/caldav/caldav_calendar.php +++ b/plugins/calendar/drivers/caldav/caldav_calendar.php @@ -298,7 +298,7 @@ class caldav_calendar extends kolab_storage_dav_folder // find and merge exception for the first instance if ($virtual && !empty($event['recurrence']) && !empty($event['recurrence']['EXCEPTIONS'])) { foreach ($event['recurrence']['EXCEPTIONS'] as $exception) { - if ($event['_instance'] == $exception['_instance']) { + if (libcalendaring::is_recurrence_exception($event, $exception)) { unset($exception['calendar'], $exception['className'], $exception['_folder_id']); // clone date objects from main event before adjusting them with exception data if (is_object($event['start'])) { @@ -307,6 +307,7 @@ class caldav_calendar extends kolab_storage_dav_folder if (is_object($event['end'])) { $event['end'] = clone $record['end']; } + kolab_driver::merge_exception_data($event, $exception); } } @@ -365,9 +366,6 @@ class caldav_calendar extends kolab_storage_dav_folder // $config = kolab_storage_config::get_instance(); // $config->apply_links($events); - // Avoid session race conditions that will loose temporary subscriptions - // $this->cal->rc->session->nowrite = true; - return $events; } @@ -649,7 +647,7 @@ class caldav_calendar extends kolab_storage_dav_folder return $events; } - // use libkolab to compute recurring events + // use libcalendaring to compute recurring events $recurrence = libcalendaring::get_recurrence($event); $i = 0; diff --git a/plugins/calendar/drivers/kolab/kolab_driver.php b/plugins/calendar/drivers/kolab/kolab_driver.php index c326238a..84bfddb0 100644 --- a/plugins/calendar/drivers/kolab/kolab_driver.php +++ b/plugins/calendar/drivers/kolab/kolab_driver.php @@ -829,7 +829,7 @@ class kolab_driver extends calendar_driver { $ret = true; $success = false; - $savemode = isset($event['_savemode']) ? $event['_savemode'] : null; + $savemode = $event['_savemode'] ?? null; if (!$force) { unset($event['attendees']); @@ -864,7 +864,7 @@ class kolab_driver extends calendar_driver // removing an exception instance if ((!empty($event['recurrence_id']) || !empty($event['isexception'])) && !empty($master['exceptions'])) { foreach ($master['exceptions'] as $i => $exception) { - if ($exception['_instance'] == $event['_instance']) { + if (libcalendaring::is_recurrence_exception($event, $exception)) { unset($master['exceptions'][$i]); // set event date back to the actual occurrence if (!empty($exception['recurrence_date'])) { @@ -1071,12 +1071,14 @@ class kolab_driver extends calendar_driver } // Stick to the master timezone for all occurrences (Bifrost#T104637) - $master_tz = $master['start']->getTimezone(); - $event_tz = $event['start']->getTimezone(); + if (empty($master['allday']) || !empty($event['allday'])) { + $master_tz = $master['start']->getTimezone(); + $event_tz = $event['start']->getTimezone(); - if ($master_tz->getName() != $event_tz->getName()) { - $event['start']->setTimezone($master_tz); - $event['end']->setTimezone($master_tz); + if ($master_tz->getName() != $event_tz->getName()) { + $event['start']->setTimezone($master_tz); + $event['end']->setTimezone($master_tz); + } } } @@ -1138,14 +1140,20 @@ class kolab_driver extends calendar_driver if ($reschedule) { unset($event['recurrence']['EXCEPTIONS'], $event['exceptions'], $master['recurrence']['EXDATE']); } - else if (isset($event['recurrence']['EXCEPTIONS']) && is_array($event['recurrence']['EXCEPTIONS'])) { - // only keep relevant exceptions - $event['recurrence']['EXCEPTIONS'] = array_filter( - $event['recurrence']['EXCEPTIONS'], - function($exception) use ($event) { - return $exception['start'] > $event['start']; - } - ); + else { + if (isset($event['recurrence']['EXCEPTIONS']) && is_array($event['recurrence']['EXCEPTIONS'])) { + // only keep relevant exceptions + $event['recurrence']['EXCEPTIONS'] = array_filter( + $event['recurrence']['EXCEPTIONS'], + function($exception) use ($event) { + return $exception['start'] > $event['start']; + } + ); + + // set link to top-level exceptions + $event['exceptions'] = &$event['recurrence']['EXCEPTIONS']; + } + if (isset($event['recurrence']['EXDATE']) && is_array($event['recurrence']['EXDATE'])) { $event['recurrence']['EXDATE'] = array_filter( $event['recurrence']['EXDATE'], @@ -1154,8 +1162,6 @@ class kolab_driver extends calendar_driver } ); } - // set link to top-level exceptions - $event['exceptions'] = &$event['recurrence']['EXCEPTIONS']; } // compute remaining occurrences @@ -1223,7 +1229,7 @@ class kolab_driver extends calendar_driver $event['sequence'] = max($old['sequence'] ?? 0, $master['sequence'] ?? 0) + 1; } else if (!isset($event['sequence'])) { - $event['sequence'] = !empty($old['sequence']) ? $old['sequence'] : $master['sequence']; + $event['sequence'] = !empty($old['sequence']) ? $old['sequence'] : $master['sequence'] ?? 1; } // save properties to a recurrence exception instance @@ -1306,7 +1312,7 @@ class kolab_driver extends calendar_driver } // TODO: forward changes to exceptions (which do not yet have differing values stored) - if (!empty($event['recurrence']) && !empty($event['recurrence']['EXCEPTIONS']) && !$with_exceptions) { + if (!empty($event['recurrence']) && !empty($event['recurrence']['EXCEPTIONS']) && empty($with_exceptions)) { // determine added and removed attendees $old_attendees = $current_attendees = $added_attendees = []; @@ -1464,7 +1470,7 @@ class kolab_driver extends calendar_driver foreach ($master['recurrence']['EXCEPTIONS'] as $i => $exception) { // update a specific instance - if ($exception['_instance'] == $old['_instance']) { + if (libcalendaring::is_recurrence_exception($old, $exception)) { $existing = $i; // check savemode against existing exception mode. @@ -1472,7 +1478,7 @@ class kolab_driver extends calendar_driver $thisandfuture = !empty($exception['thisandfuture']); if ($thisandfuture === ($savemode == 'future')) { $event['_instance'] = $old['_instance']; - $event['thisandfuture'] = $old['thisandfuture']; + $event['thisandfuture'] = !empty($old['thisandfuture']); $event['recurrence_date'] = $old['recurrence_date']; $master['recurrence']['EXCEPTIONS'][$i] = $event; $saved = true; @@ -1480,12 +1486,17 @@ class kolab_driver extends calendar_driver } // merge the new event properties onto future exceptions - if ($savemode == 'future' && $exception['_instance'] >= $old['_instance']) { - unset($event['thisandfuture']); - self::merge_exception_data($master['recurrence']['EXCEPTIONS'][$i], $event, ['attendees']); + if ($savemode == 'future') { + $exception_instance = libcalendaring::recurrence_instance_identifier($exception, true); + $old_instance = libcalendaring::recurrence_instance_identifier($old, true); - if (!empty($added_attendees) || !empty($removed_attendees)) { - calendar::merge_attendee_data($master['recurrence']['EXCEPTIONS'][$i], $added_attendees, $removed_attendees); + if ($exception_instance >= $old_instance) { + unset($event['thisandfuture']); + self::merge_exception_data($master['recurrence']['EXCEPTIONS'][$i], $event, ['attendees']); + + if (!empty($added_attendees) || !empty($removed_attendees)) { + calendar::merge_attendee_data($master['recurrence']['EXCEPTIONS'][$i], $added_attendees, $removed_attendees); + } } } } @@ -1525,7 +1536,7 @@ class kolab_driver extends calendar_driver public static function add_exception(&$master, $event, $old = null) { if ($old) { - $event['_instance'] = $old['_instance']; + $event['_instance'] = $old['_instance'] ?? null; if (empty($event['recurrence_date'])) { $event['recurrence_date'] = !empty($old['recurrence_date']) ? $old['recurrence_date'] : $old['start']; } @@ -1534,10 +1545,6 @@ class kolab_driver extends calendar_driver $event['recurrence_date'] = $event['start']; } - if (empty($event['_instance']) && $event['recurrence_date'] instanceof DateTimeInterface) { - $event['_instance'] = libcalendaring::recurrence_instance_identifier($event, !empty($master['allday'])); - } - if (!isset($master['exceptions'])) { if (isset($master['recurrence']['EXCEPTIONS'])) { $master['exceptions'] = &$master['recurrence']['EXCEPTIONS']; @@ -1549,7 +1556,7 @@ class kolab_driver extends calendar_driver $existing = false; foreach ($master['exceptions'] as $i => $exception) { - if ($exception['_instance'] == $event['_instance']) { + if (libcalendaring::is_recurrence_exception($event, $exception)) { $master['exceptions'][$i] = $event; $existing = true; } @@ -2139,7 +2146,7 @@ class kolab_driver extends calendar_driver return $record; } - $record['id'] = $record['uid']; + $record['id'] = $record['uid'] ?? null; if (!empty($record['_instance'])) { $record['id'] .= '-' . $record['_instance']; diff --git a/plugins/libcalendaring/lib/libcalendaring_recurrence.php b/plugins/libcalendaring/lib/libcalendaring_recurrence.php index df802b7b..99e05679 100644 --- a/plugins/libcalendaring/lib/libcalendaring_recurrence.php +++ b/plugins/libcalendaring/lib/libcalendaring_recurrence.php @@ -33,6 +33,7 @@ class libcalendaring_recurrence protected $dateonly = false; protected $event; protected $duration; + protected $isStart = true; /** * Default constructor @@ -109,7 +110,23 @@ class libcalendaring_recurrence */ public function next_instance() { - if ($next_start = $this->next_start()) { + // Here's the workaround for an issue for an event with its start date excluded + // E.g. A daily event starting on 10th which is one of EXDATE dates + // should return 11th as next_instance() when called for the first time. + // Looks like Sabre is setting internal "current date" to 11th on such an object + // initialization, therefore calling next() would move it to 12th. + if ($this->isStart && ($next_start = $this->engine->getDtStart()) + && $next_start->format('Ymd') != $this->start->format('Ymd') + ) { + $next_start = $this->toDateTime($next_start); + } + else { + $next_start = $this->next_start(); + } + + $this->isStart = false; + + if ($next_start) { $next = $this->event; $next['start'] = $next_start; @@ -119,7 +136,7 @@ class libcalendaring_recurrence } $next['recurrence_date'] = clone $next_start; - $next['_instance'] = libcalendaring::recurrence_instance_identifier($next, !empty($this->event['allday'])); + $next['_instance'] = libcalendaring::recurrence_instance_identifier($next); unset($next['_formatobj']); diff --git a/plugins/libcalendaring/libcalendaring.php b/plugins/libcalendaring/libcalendaring.php index a7f63d1d..dbb0066c 100644 --- a/plugins/libcalendaring/libcalendaring.php +++ b/plugins/libcalendaring/libcalendaring.php @@ -1348,6 +1348,27 @@ class libcalendaring extends rcube_plugin } } + /** + * Check if a specified event is "identical" to the specified recurrence exception + * + * @param array Hash array with occurrence properties + * @param array Hash array with exception properties + * + * @return bool + */ + public static function is_recurrence_exception($event, $exception) + { + $instance_date = !empty($event['recurrence_date']) ? $event['recurrence_date'] : $event['start']; + $exception_date = !empty($exception['recurrence_date']) ? $exception['recurrence_date'] : $exception['start']; + + if ($instance_date instanceof DateTimeInterface && $exception_date instanceof DateTimeInterface) { + // Timezone??? + return $instance_date->format('Ymd') === $exception_date->format('Ymd'); + } + + return false; + } + /********* Attendee handling functions *********/ diff --git a/plugins/libcalendaring/tests/RecurrenceTest.php b/plugins/libcalendaring/tests/RecurrenceTest.php index d17257a8..39c2edd8 100644 --- a/plugins/libcalendaring/tests/RecurrenceTest.php +++ b/plugins/libcalendaring/tests/RecurrenceTest.php @@ -158,13 +158,11 @@ class RecurrenceTest extends PHPUnit\Framework\TestCase '2017-09-08 11:00:00', '2017-09-08 11:00:00', ), -/* array( array('FREQ' => 'MONTHLY', 'INTERVAL' => '1', 'BYMONTHDAY' => '8,9'), '2017-08-31 11:00:00', '2017-09-08 11:00:00', ), -*/ array( array('FREQ' => 'MONTHLY', 'INTERVAL' => '1', 'BYMONTHDAY' => '8,9'), '2017-09-08 11:00:00', @@ -185,13 +183,11 @@ class RecurrenceTest extends PHPUnit\Framework\TestCase '2017-09-08 11:00:00', '2017-09-08 11:00:00', ), -/* array( array('FREQ' => 'MONTHLY', 'INTERVAL' => '2', 'BYMONTHDAY' => '8'), '2017-08-31 11:00:00', '2017-09-08 11:00:00', // ?????? ), -*/ // yearly array( array('FREQ' => 'YEARLY', 'INTERVAL' => '1'), @@ -204,6 +200,7 @@ class RecurrenceTest extends PHPUnit\Framework\TestCase '2017-08-16 12:00:00', ), /* + // Not supported by Sabre (requires BYMONTH too) array( array('FREQ' => 'YEARLY', 'INTERVAL' => '1', 'BYDAY' => '-1MO'), '2017-08-16 11:00:00', @@ -215,7 +212,6 @@ class RecurrenceTest extends PHPUnit\Framework\TestCase '2017-08-16 11:00:00', '2017-08-28 11:00:00', ), -/* array( array('FREQ' => 'YEARLY', 'INTERVAL' => '1', 'BYMONTH' => '1', 'BYDAY' => '1MO'), '2017-08-16 11:00:00', @@ -226,7 +222,6 @@ class RecurrenceTest extends PHPUnit\Framework\TestCase '2017-08-16 11:00:00', '2017-09-04 11:00:00', ), -*/ array( array('FREQ' => 'YEARLY', 'INTERVAL' => '2'), '2017-08-16 11:00:00', @@ -238,6 +233,7 @@ class RecurrenceTest extends PHPUnit\Framework\TestCase '2017-08-16 11:00:00', ), /* + // Not supported by Sabre (requires BYMONTH too) array( array('FREQ' => 'YEARLY', 'INTERVAL' => '2', 'BYDAY' => '-1MO'), '2017-08-16 11:00:00', @@ -334,4 +330,104 @@ class RecurrenceTest extends PHPUnit\Framework\TestCase $this->assertEquals($start->getTimezone()->getName(), $next['start']->getTimezone()->getName(), 'Same timezone'); $this->assertTrue($next['start']->_dateonly, '_dateonly flag'); } + + /** + * Test for libcalendaring_recurrence::next_instance() + */ + function test_next_instance_exdate() + { + date_default_timezone_set('America/New_York'); + + $start = new libcalendaring_datetime('2023-01-18 10:00:00', new DateTimeZone('Europe/Berlin')); + $end = new libcalendaring_datetime('2023-01-18 10:30:00', new DateTimeZone('Europe/Berlin')); + $event = [ + 'start' => $start, + 'end' => $end, + 'recurrence' => [ + 'FREQ' => 'DAILY', + 'INTERVAL' => '1', + 'EXDATE' => [ + // Exclude the start date + new libcalendaring_datetime('2023-01-18 10:00:00', new DateTimeZone('Europe/Berlin')), + ], + ], + ]; + + $recurrence = new libcalendaring_recurrence($this->plugin, $event); + + $next = $recurrence->next_instance(); + + $this->assertEquals('2023-01-19 10:00:00', $next['start']->format('Y-m-d H:i:s')); + $this->assertEquals('Europe/Berlin', $next['start']->getTimezone()->getName()); + $this->assertFalse($next['start']->_dateonly); + + $next = $recurrence->next_instance(); + + $this->assertEquals('2023-01-20 10:00:00', $next['start']->format('Y-m-d H:i:s')); + $this->assertEquals('Europe/Berlin', $next['start']->getTimezone()->getName()); + $this->assertFalse($next['start']->_dateonly); + } + + /** + * Test for libcalendaring_recurrence::next_instance() + */ + function test_next_instance_dst() + { + date_default_timezone_set('America/New_York'); + + $start = new libcalendaring_datetime('2021-03-10 10:00:00', new DateTimeZone('Europe/Berlin')); + $end = new libcalendaring_datetime('2021-03-10 10:30:00', new DateTimeZone('Europe/Berlin')); + $event = [ + 'start' => $start, + 'end' => $end, + 'recurrence' => [ + 'FREQ' => 'MONTHLY', + 'INTERVAL' => '1', + ], + ]; + + $recurrence = new libcalendaring_recurrence($this->plugin, $event); + + $next = $recurrence->next_instance(); + + $this->assertEquals('2021-04-10 10:00:00', $next['start']->format('Y-m-d H:i:s')); + $this->assertEquals('Europe/Berlin', $next['start']->getTimezone()->getName()); + + $next = $recurrence->next_instance(); + + $this->assertEquals('2021-05-10 10:00:00', $next['start']->format('Y-m-d H:i:s')); + $this->assertEquals('Europe/Berlin', $next['start']->getTimezone()->getName()); + + $start = new libcalendaring_datetime('2021-10-10 10:00:00', new DateTimeZone('Europe/Berlin')); + $end = new libcalendaring_datetime('2021-10-10 10:30:00', new DateTimeZone('Europe/Berlin')); + $event = [ + 'start' => $start, + 'end' => $end, + 'recurrence' => [ + 'FREQ' => 'MONTHLY', + 'INTERVAL' => '1', + ], + ]; + + $recurrence = new libcalendaring_recurrence($this->plugin, $event); + + $next = $recurrence->next_instance(); + + $this->assertEquals('2021-11-10 10:00:00', $next['start']->format('Y-m-d H:i:s')); + $this->assertEquals('Europe/Berlin', $next['start']->getTimezone()->getName()); + + $next = $recurrence->next_instance(); + + $this->assertEquals('2021-12-10 10:00:00', $next['start']->format('Y-m-d H:i:s')); + $this->assertEquals('Europe/Berlin', $next['start']->getTimezone()->getName()); + + $next = $recurrence->next_instance(); + $next = $recurrence->next_instance(); + $next = $recurrence->next_instance(); + $next = $recurrence->next_instance(); + + $this->assertEquals('2022-04-10 10:00:00', $next['start']->format('Y-m-d H:i:s')); + $this->assertEquals('Europe/Berlin', $next['start']->getTimezone()->getName()); + + } }