From 3d8bfef71a1e1e8656f8a0335d5b9da99be308f0 Mon Sep 17 00:00:00 2001 From: Nico Donath Date: Fri, 22 May 2026 12:08:11 +0000 Subject: [PATCH] fix(dav): keep cancelled occurrence in iTip REQUEST so attendees keep it cancelled When an organizer cancels a single occurrence of a recurring event, the broker emits a per-instance CANCEL plus a REQUEST for the attendee's remaining instances. The REQUEST omitted the cancelled instance, but processMessageRequest replaces all components of the attendee's stored object, so it dropped the CANCELLED override the CANCEL had just added and the occurrence reappeared as a normal event on the attendee's calendar. Keep the cancelled instance in the REQUEST so the override survives the component replace. Two adjustments keep the pipeline coherent with the kept override: - IMipPlugin no longer builds an email from a newly cancelled override, which sent a second "Invitation:" email (with response buttons) for the occurrence the CANCEL email had just cancelled. Cancelling a previously modified occurrence produces no CANCEL message, so that override keeps its REQUEST email. - TipBroker derives the attendee-side event status from the master instance; the aggregate status (last component wins) would treat the attendee copy as a cancelled event once it carries the override, muting all attendee replies. Assisted-by: ClaudeCode:claude-opus-4-7 Assisted-by: ClaudeCode:claude-fable-5 Signed-off-by: Nico Donath --- apps/dav/lib/CalDAV/Schedule/IMipPlugin.php | 40 +++ apps/dav/lib/CalDAV/TipBroker.php | 29 +- .../unit/CalDAV/Schedule/IMipPluginTest.php | 267 ++++++++++++++++++ apps/dav/tests/unit/CalDAV/TipBrokerTest.php | 30 ++ 4 files changed, 359 insertions(+), 7 deletions(-) diff --git a/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php b/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php index 18d7559f1a702..37eb9b952068c 100644 --- a/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php +++ b/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php @@ -147,6 +147,19 @@ public function schedule(Message $iTipMessage) { $oldEvents = $this->getVCalendar(); $modified = $this->eventComparisonService->findModified($newEvents, $oldEvents); + // A newly cancelled occurrence is announced through the separate CANCEL + // message; it rides along in the REQUEST only so the attendee's stored copy + // stays cancelled and must not produce an invitation email of its own. + if (strcasecmp($iTipMessage->method, self::METHOD_REQUEST) === 0 && !empty($modified['new'])) { + $modified['new'] = array_values(array_filter( + $modified['new'], + fn (VEvent $component): bool => !$this->isNewlyCancelledOccurrence($component, $oldEvents), + )); + if (empty($modified['new'])) { + $iTipMessage->scheduleStatus = '1.0;We got the message, but it\'s not significant enough to warrant an email'; + return; + } + } /** @var VEvent $vEvent */ $vEvent = array_pop($modified['new']); /** @var VEvent $oldVevent */ @@ -338,6 +351,33 @@ public function schedule(Message $iTipMessage) { } } + /** + * Whether this component is an occurrence override that got cancelled in the + * same write that created it - the only case the broker announces through a + * per-instance CANCEL message. Cancelling a previously modified occurrence + * produces no CANCEL message, so that override must keep its REQUEST email. + * Mirrors the broker's instance tracking, which only registers components + * carrying an ATTENDEE. + */ + private function isNewlyCancelledOccurrence(VEvent $component, ?VCalendar $oldEvents): bool { + if (!isset($component->STATUS) || $component->STATUS->getValue() !== 'CANCELLED' + || !isset($component->{'RECURRENCE-ID'})) { + return false; + } + if ($oldEvents === null) { + return true; + } + $recurrenceId = $component->{'RECURRENCE-ID'}->getValue(); + foreach ($oldEvents->getComponents() as $oldComponent) { + if ($oldComponent instanceof VEvent + && isset($oldComponent->ATTENDEE, $oldComponent->{'RECURRENCE-ID'}) + && $oldComponent->{'RECURRENCE-ID'}->getValue() === $recurrenceId) { + return false; + } + } + return true; + } + /** * @return ?VCalendar */ diff --git a/apps/dav/lib/CalDAV/TipBroker.php b/apps/dav/lib/CalDAV/TipBroker.php index e83111330328b..3816f7fcd5de5 100644 --- a/apps/dav/lib/CalDAV/TipBroker.php +++ b/apps/dav/lib/CalDAV/TipBroker.php @@ -262,6 +262,23 @@ protected function allowInvitationForwarding(VEvent $vevent): bool { return true; } + /** + * parseEventInfo() aggregates STATUS with the last parsed component winning, + * so a kept cancelled override would mark the whole event as cancelled and + * suppress the attendee's replies for the remaining occurrences; judge the + * event status by the master instance instead. + * + * @return array + */ + #[\Override] + protected function parseEventForAttendee(VCalendar $calendar, array $eventInfo, array $oldEventInfo, $attendee) { + if (isset($eventInfo['instances']['master'])) { + $status = $eventInfo['instances']['master']->STATUS?->getValue(); + $eventInfo['status'] = $status === null ? null : strtoupper($status); + } + return parent::parseEventForAttendee($calendar, $eventInfo, $oldEventInfo, $attendee); + } + /** * This method is used in cases where an event got updated, and we * potentially need to send emails to attendees to let them know of updates @@ -319,13 +336,11 @@ protected function parseEventForOrganizer(VCalendar $calendar, array $eventInfo, } return $messages; } - // detect if a new cancelled instance was created - $cancelledNewInstances = []; + // detect if a new cancelled instance was created and send a CANCEL for it if (isset($oldEventInfo['instances'])) { $instancesDelta = array_diff_key($eventInfo['instances'], $oldEventInfo['instances']); foreach ($instancesDelta as $id => $instance) { if ($instance->STATUS?->getValue() === 'CANCELLED') { - $cancelledNewInstances[] = $id; foreach ($eventInfo['attendees'] as $attendee) { $messages[] = $this->generateMessage( [$id => $instance], $organizerHref, $organizerName, $attendee, $objectId, $objectType, $objectSequence, 'CANCEL', $template @@ -366,10 +381,10 @@ protected function parseEventForOrganizer(VCalendar $calendar, array $eventInfo, // otherwise any created or modified instances will be sent as REQUEST $instances = array_intersect_key($eventInfo['instances'], array_flip(array_keys($eventInfo['attendees'][$attendee]['instances']))); - // Remove already-cancelled new instances from REQUEST - if (!empty($cancelledNewInstances)) { - $instances = array_diff_key($instances, array_flip($cancelledNewInstances)); - } + // Keep newly cancelled instances IN the REQUEST. processMessageRequest replaces all + // components of the attendee's stored object with the ones from the message, so a + // REQUEST that omitted the cancelled instance would drop the CANCELLED override that + // the accompanying CANCEL added and the occurrence would reappear as a normal event. // Skip if no instances left to send if (empty($instances)) { diff --git a/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php b/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php index 048dcd13b67e3..1ab44d90cc835 100644 --- a/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php +++ b/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php @@ -1237,4 +1237,271 @@ public function testExternalAttendeesDisabledForSystemUser(): void { $this->plugin->schedule($message); $this->assertEquals('1.1', $message->getScheduleStatus()); } + + public function testRequestWithOnlyCancelledOccurrenceSendsNoEmail(): void { + $message = new Message(); + $message->method = 'REQUEST'; + $newVCalendar = new VCalendar(); + $cancelledOverride = new VEvent($newVCalendar, 'one', [ + 'UID' => 'uid-1234', + 'RECURRENCE-ID' => new \DateTime('2016-01-08 00:00:00'), + 'SEQUENCE' => 1, + 'STATUS' => 'CANCELLED', + 'SUMMARY' => 'Fellowship meeting', + 'DTSTART' => new \DateTime('2016-01-08 00:00:00'), + ]); + $cancelledOverride->add('ORGANIZER', 'mailto:gandalf@wiz.ard'); + $cancelledOverride->add('ATTENDEE', 'mailto:' . 'frodo@hobb.it', ['RSVP' => 'TRUE']); + $message->message = $newVCalendar; + $message->sender = 'mailto:gandalf@wiz.ard'; + $message->senderName = 'Mr. Wizard'; + $message->recipient = 'mailto:' . 'frodo@hobb.it'; + $message->significantChange = true; + // stored old copy: the series without the override, so the cancelled + // override is newly created in this write + $oldVCalendar = new VCalendar(); + $oldVCalendar->add(new VEvent($oldVCalendar, 'one', [ + 'UID' => 'uid-1234', + 'SEQUENCE' => 0, + 'SUMMARY' => 'Fellowship meeting', + 'DTSTART' => new \DateTime('2016-01-01 00:00:00'), + ])); + $this->plugin->setVCalendar($oldVCalendar); + + $this->service->expects(self::once()) + ->method('getLastOccurrence') + ->willReturn(1496912700); + $this->config->expects(self::once()) + ->method('getValueBool') + ->with('dav', 'caldav_external_attendees_disabled', false) + ->willReturn(false); + // The REQUEST carries only the cancelled override, kept so the attendee's + // stored copy stays cancelled through Sabre's full component replace. The + // occurrence cancellation is announced by the accompanying CANCEL message, + // so this REQUEST must not additionally send an invitation email. + $this->eventComparisonService->expects(self::once()) + ->method('findModified') + ->willReturn(['new' => [$cancelledOverride], 'old' => []]); + $this->service->expects(self::never()) + ->method('getCurrentAttendee'); + $this->service->expects(self::never()) + ->method('buildBodyData'); + $this->mailer->expects(self::never()) + ->method('send'); + // deliberate suppression, not the "significant but nothing changed" anomaly + $this->logger->expects(self::never()) + ->method('warning'); + + $this->plugin->schedule($message); + $this->assertEquals('1.0', $message->getScheduleStatus()); + } + + public function testRequestForCancelledExistingOccurrenceStillSendsEmail(): void { + $message = new Message(); + $message->method = 'REQUEST'; + $newVCalendar = new VCalendar(); + $cancelledOverride = new VEvent($newVCalendar, 'one', [ + 'UID' => 'uid-1234', + 'RECURRENCE-ID' => new \DateTime('2016-01-08 00:00:00'), + 'SEQUENCE' => 2, + 'STATUS' => 'CANCELLED', + 'SUMMARY' => 'Fellowship meeting', + 'DTSTART' => new \DateTime('2016-01-08 00:00:00'), + ]); + $cancelledOverride->add('ORGANIZER', 'mailto:gandalf@wiz.ard'); + $cancelledOverride->add('ATTENDEE', 'mailto:' . 'frodo@hobb.it', ['RSVP' => 'TRUE']); + $message->message = $newVCalendar; + $message->sender = 'mailto:gandalf@wiz.ard'; + $message->senderName = 'Mr. Wizard'; + $message->recipient = 'mailto:' . 'frodo@hobb.it'; + $message->significantChange = true; + // stored old copy already contains a live override for this occurrence: + // no CANCEL message is generated for it, this REQUEST is the only notice + $oldVCalendar = new VCalendar(); + $oldVCalendar->add(new VEvent($oldVCalendar, 'one', [ + 'UID' => 'uid-1234', + 'SEQUENCE' => 0, + 'SUMMARY' => 'Fellowship meeting', + 'DTSTART' => new \DateTime('2016-01-01 00:00:00'), + ])); + $oldOverride = new VEvent($oldVCalendar, 'one', [ + 'UID' => 'uid-1234', + 'RECURRENCE-ID' => new \DateTime('2016-01-08 00:00:00'), + 'SEQUENCE' => 1, + 'SUMMARY' => 'Fellowship meeting', + 'DTSTART' => new \DateTime('2016-01-08 00:00:00'), + ]); + $oldOverride->add('ORGANIZER', 'mailto:gandalf@wiz.ard'); + $oldOverride->add('ATTENDEE', 'mailto:' . 'frodo@hobb.it', ['RSVP' => 'TRUE']); + $oldVCalendar->add($oldOverride); + $this->plugin->setVCalendar($oldVCalendar); + $data = [ + 'invitee_name' => 'Mr. Wizard', + 'meeting_title' => 'Fellowship meeting', + 'attendee_name' => 'frodo@hobb.it', + ]; + $attendees = $cancelledOverride->select('ATTENDEE'); + $atnd = ''; + foreach ($attendees as $attendee) { + if (strcasecmp($attendee->getValue(), $message->recipient) === 0) { + $atnd = $attendee; + } + } + $this->service->expects(self::once()) + ->method('getLastOccurrence') + ->willReturn(1496912700); + $this->config->expects(self::exactly(2)) + ->method('getValueBool') + ->willReturnMap([ + ['dav', 'caldav_external_attendees_disabled', false, false], + ['core', 'mail_providers_enabled', true, false], + ]); + $this->eventComparisonService->expects(self::once()) + ->method('findModified') + ->willReturn(['new' => [$cancelledOverride], 'old' => [$oldOverride]]); + $this->service->expects(self::once()) + ->method('getCurrentAttendee') + ->with($message) + ->willReturn($atnd); + $this->service->expects(self::once()) + ->method('isRoomOrResource') + ->with($atnd) + ->willReturn(false); + $this->service->expects(self::once()) + ->method('isCircle') + ->with($atnd) + ->willReturn(false); + $this->service->expects(self::once()) + ->method('buildBodyData') + ->with($cancelledOverride, $oldOverride) + ->willReturn($data); + $this->service->expects(self::once()) + ->method('getFrom'); + $this->service->expects(self::once()) + ->method('addSubjectAndHeading') + ->with($this->emailTemplate, 'request', 'Mr. Wizard', 'Fellowship meeting', true); + $this->service->expects(self::once()) + ->method('addBulletList') + ->with($this->emailTemplate, $cancelledOverride, $data); + $this->service->expects(self::once()) + ->method('getAttendeeRsvpOrReqForParticipant') + ->willReturn(false); + $this->mailer->expects(self::once()) + ->method('send') + ->willReturn([]); + $this->plugin->schedule($message); + $this->assertEquals('1.1', $message->getScheduleStatus()); + } + + public function testRequestKeepsChangeWhenCancelledOccurrenceIsAlsoPresent(): void { + $message = new Message(); + $message->method = 'REQUEST'; + $newVCalendar = new VCalendar(); + $newVevent = new VEvent($newVCalendar, 'one', [ + 'UID' => 'uid-1234', + 'SEQUENCE' => 1, + 'SUMMARY' => 'Fellowship meeting without (!) Boromir', + 'DTSTART' => new \DateTime('2016-01-01 00:00:00'), + ]); + $newVevent->add('ORGANIZER', 'mailto:gandalf@wiz.ard'); + $newVevent->add('ATTENDEE', 'mailto:' . 'frodo@hobb.it', ['RSVP' => 'TRUE', 'CN' => 'Frodo']); + $cancelledOverride = new VEvent($newVCalendar, 'one', [ + 'UID' => 'uid-1234', + 'RECURRENCE-ID' => new \DateTime('2016-01-08 00:00:00'), + 'SEQUENCE' => 1, + 'STATUS' => 'CANCELLED', + 'SUMMARY' => 'Fellowship meeting without (!) Boromir', + 'DTSTART' => new \DateTime('2016-01-08 00:00:00'), + ]); + $cancelledOverride->add('ORGANIZER', 'mailto:gandalf@wiz.ard'); + $cancelledOverride->add('ATTENDEE', 'mailto:' . 'frodo@hobb.it', ['RSVP' => 'TRUE']); + $message->message = $newVCalendar; + $message->sender = 'mailto:gandalf@wiz.ard'; + $message->senderName = 'Mr. Wizard'; + $message->recipient = 'mailto:' . 'frodo@hobb.it'; + $message->significantChange = true; + $oldVCalendar = new VCalendar(); + $oldVEvent = new VEvent($oldVCalendar, 'one', [ + 'UID' => 'uid-1234', + 'SEQUENCE' => 0, + 'SUMMARY' => 'Fellowship meeting', + 'DTSTART' => new \DateTime('2016-01-01 00:00:00'), + ]); + $oldVEvent->add('ORGANIZER', 'mailto:gandalf@wiz.ard'); + $oldVEvent->add('ATTENDEE', 'mailto:' . 'frodo@hobb.it', ['RSVP' => 'TRUE', 'CN' => 'Frodo']); + $oldVCalendar->add($oldVEvent); + $data = [ + 'invitee_name' => 'Mr. Wizard', + 'meeting_title' => 'Fellowship meeting without (!) Boromir', + 'attendee_name' => 'frodo@hobb.it', + ]; + $attendees = $newVevent->select('ATTENDEE'); + $atnd = ''; + foreach ($attendees as $attendee) { + if (strcasecmp($attendee->getValue(), $message->recipient) === 0) { + $atnd = $attendee; + } + } + $this->plugin->setVCalendar($oldVCalendar); + $this->service->expects(self::once()) + ->method('getLastOccurrence') + ->willReturn(1496912700); + $this->config->expects(self::exactly(2)) + ->method('getValueBool') + ->willReturnMap([ + ['dav', 'caldav_external_attendees_disabled', false, false], + ['core', 'mail_providers_enabled', true, false], + ]); + // The cancelled override rides along in the REQUEST but must not become the + // event the email describes: it is dropped, leaving the real change. + $this->eventComparisonService->expects(self::once()) + ->method('findModified') + ->willReturn(['new' => [$newVevent, $cancelledOverride], 'old' => [$oldVEvent]]); + $this->service->expects(self::once()) + ->method('getCurrentAttendee') + ->with($message) + ->willReturn($atnd); + $this->service->expects(self::once()) + ->method('isRoomOrResource') + ->with($atnd) + ->willReturn(false); + $this->service->expects(self::once()) + ->method('isCircle') + ->with($atnd) + ->willReturn(false); + $this->service->expects(self::once()) + ->method('buildBodyData') + ->with($newVevent, $oldVEvent) + ->willReturn($data); + $this->service->expects(self::once()) + ->method('getFrom'); + $this->service->expects(self::once()) + ->method('addSubjectAndHeading') + ->with($this->emailTemplate, 'request', 'Mr. Wizard', 'Fellowship meeting without (!) Boromir', true); + $this->service->expects(self::once()) + ->method('addBulletList') + ->with($this->emailTemplate, $newVevent, $data); + $this->service->expects(self::once()) + ->method('getAttendeeRsvpOrReqForParticipant') + ->willReturn(true); + $this->config->expects(self::once()) + ->method('getValueString') + ->with('dav', 'invitation_link_recipients', 'yes') + ->willReturn('yes'); + $this->service->expects(self::once()) + ->method('createInvitationToken') + ->with($message, $newVevent, 1496912700) + ->willReturn('token'); + $this->service->expects(self::once()) + ->method('addResponseButtons') + ->with($this->emailTemplate, 'token'); + $this->service->expects(self::once()) + ->method('addMoreOptionsButton') + ->with($this->emailTemplate, 'token'); + $this->mailer->expects(self::once()) + ->method('send') + ->willReturn([]); + $this->plugin->schedule($message); + $this->assertEquals('1.1', $message->getScheduleStatus()); + } } diff --git a/apps/dav/tests/unit/CalDAV/TipBrokerTest.php b/apps/dav/tests/unit/CalDAV/TipBrokerTest.php index 0af957823787b..2ed59bc55136e 100644 --- a/apps/dav/tests/unit/CalDAV/TipBrokerTest.php +++ b/apps/dav/tests/unit/CalDAV/TipBrokerTest.php @@ -303,7 +303,37 @@ public function testParseEventForOrganizerCreatedInstanceCancelled(): void { $this->assertEquals($mutatedCalendar->VEVENT[1]->ATTENDEE[0]->getValue(), $messages[0]->recipient); $this->assertCount(1, $messages[0]->message->VEVENT); $this->assertEquals('20240715T080000', $messages[0]->message->VEVENT->{'RECURRENCE-ID'}->getValue()); + // the REQUEST keeps the cancelled instance, otherwise processMessageRequest (which replaces + // all components) would drop the CANCELLED override on the attendee's copy (issue #6655) + $this->assertEquals('REQUEST', $messages[1]->method); + $this->assertCount(2, $messages[1]->message->VEVENT); + $this->assertEquals('20240715T080000', $messages[1]->message->VEVENT[1]->{'RECURRENCE-ID'}->getValue()); + $this->assertEquals('CANCELLED', $messages[1]->message->VEVENT[1]->STATUS->getValue()); + + } + /** + * Tests attendee responding to the remaining occurrences while their copy + * carries a cancelled single instance + */ + public function testParseEventForAttendeeReplyWithCancelledInstance(): void { + // attendee copy: recurring event plus a cancelled single instance + $originalCalendar = clone $this->vCalendar2a; + $cancelledInstance = clone $originalCalendar->VEVENT; + $cancelledInstance->add('RECURRENCE-ID', '20240715T080000', ['TZID' => 'America/Toronto']); + $cancelledInstance->STATUS->setValue('CANCELLED'); + $originalCalendar->add($cancelledInstance); + $originalEventInfo = $this->invokePrivate($this->broker, 'parseEventInfo', [$originalCalendar]); + // attendee accepts the series + $mutatedCalendar = clone $originalCalendar; + $mutatedCalendar->VEVENT->ATTENDEE['PARTSTAT'] = 'ACCEPTED'; + $mutatedEventInfo = $this->invokePrivate($this->broker, 'parseEventInfo', [$mutatedCalendar]); + // test iTip generation - the cancelled instance must not mute the reply + $messages = $this->invokePrivate($this->broker, 'parseEventForAttendee', [$mutatedCalendar, $mutatedEventInfo, $originalEventInfo, 'mailto:attendee1@example.org']); + $this->assertCount(1, $messages); + $this->assertEquals('REPLY', $messages[0]->method); + $this->assertEquals($mutatedCalendar->VEVENT->ATTENDEE->getValue(), $messages[0]->sender); + $this->assertEquals($mutatedCalendar->VEVENT->ORGANIZER->getValue(), $messages[0]->recipient); } /**