mirror of
https://github.com/nextcloud/server.git
synced 2026-02-23 01:40:59 -05:00
Merge pull request #35020 from nextcloud/backport/34909/stable23
[stable23] Fix duplicate event email notifications
This commit is contained in:
commit
81dbe70834
11 changed files with 219 additions and 19 deletions
|
|
@ -8,6 +8,7 @@ declare(strict_types=1);
|
|||
* @author Christoph Wurst <christoph@winzerhof-wurst.at>
|
||||
* @author Georg Ehrke <oc.list@georgehrke.com>
|
||||
* @author Roeland Jago Douma <roeland@famdouma.nl>
|
||||
* @author Richard Steinmetz <richard@steinmetz.cloud>
|
||||
*
|
||||
* @license GNU AGPL version 3 or any later version
|
||||
*
|
||||
|
|
@ -42,10 +43,12 @@ interface INotificationProvider {
|
|||
*
|
||||
* @param VEvent $vevent
|
||||
* @param string $calendarDisplayName
|
||||
* @param string[] $principalEmailAddresses All email addresses associated to the principal owning the calendar object
|
||||
* @param IUser[] $users
|
||||
* @return void
|
||||
*/
|
||||
public function send(VEvent $vevent,
|
||||
string $calendarDisplayName,
|
||||
array $principalEmailAddresses,
|
||||
array $users = []): void;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ declare(strict_types=1);
|
|||
* @author Georg Ehrke <oc.list@georgehrke.com>
|
||||
* @author Joas Schilling <coding@schilljs.com>
|
||||
* @author Roeland Jago Douma <roeland@famdouma.nl>
|
||||
* @author Richard Steinmetz <richard@steinmetz.cloud>
|
||||
*
|
||||
* @license GNU AGPL version 3 or any later version
|
||||
*
|
||||
|
|
@ -89,11 +90,13 @@ abstract class AbstractProvider implements INotificationProvider {
|
|||
*
|
||||
* @param VEvent $vevent
|
||||
* @param string $calendarDisplayName
|
||||
* @param string[] $principalEmailAddresses
|
||||
* @param IUser[] $users
|
||||
* @return void
|
||||
*/
|
||||
abstract public function send(VEvent $vevent,
|
||||
string $calendarDisplayName,
|
||||
array $principalEmailAddresses,
|
||||
array $users = []): void;
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -78,16 +78,28 @@ class EmailProvider extends AbstractProvider {
|
|||
*
|
||||
* @param VEvent $vevent
|
||||
* @param string $calendarDisplayName
|
||||
* @param string[] $principalEmailAddresses
|
||||
* @param array $users
|
||||
* @throws \Exception
|
||||
*/
|
||||
public function send(VEvent $vevent,
|
||||
string $calendarDisplayName,
|
||||
array $principalEmailAddresses,
|
||||
array $users = []):void {
|
||||
$fallbackLanguage = $this->getFallbackLanguage();
|
||||
|
||||
$organizerEmailAddress = null;
|
||||
if (isset($vevent->ORGANIZER)) {
|
||||
$organizerEmailAddress = $this->getEMailAddressOfAttendee($vevent->ORGANIZER);
|
||||
}
|
||||
|
||||
$emailAddressesOfSharees = $this->getEMailAddressesOfAllUsersWithWriteAccessToCalendar($users);
|
||||
$emailAddressesOfAttendees = $this->getAllEMailAddressesFromEvent($vevent);
|
||||
$emailAddressesOfAttendees = [];
|
||||
if (count($principalEmailAddresses) === 0
|
||||
|| ($organizerEmailAddress && in_array($organizerEmailAddress, $principalEmailAddresses, true))
|
||||
) {
|
||||
$emailAddressesOfAttendees = $this->getAllEMailAddressesFromEvent($vevent);
|
||||
}
|
||||
|
||||
// Quote from php.net:
|
||||
// If the input arrays have the same string keys, then the later value for that key will overwrite the previous one.
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ declare(strict_types=1);
|
|||
* @author Georg Ehrke <oc.list@georgehrke.com>
|
||||
* @author Roeland Jago Douma <roeland@famdouma.nl>
|
||||
* @author Thomas Citharel <nextcloud@tcit.fr>
|
||||
* @author Richard Steinmetz <richard@steinmetz.cloud>
|
||||
*
|
||||
* @license GNU AGPL version 3 or any later version
|
||||
*
|
||||
|
|
@ -81,11 +82,13 @@ class PushProvider extends AbstractProvider {
|
|||
*
|
||||
* @param VEvent $vevent
|
||||
* @param string $calendarDisplayName
|
||||
* @param string[] $principalEmailAddresses
|
||||
* @param IUser[] $users
|
||||
* @throws \Exception
|
||||
*/
|
||||
public function send(VEvent $vevent,
|
||||
string $calendarDisplayName = null,
|
||||
string $calendarDisplayName,
|
||||
array $principalEmailAddresses,
|
||||
array $users = []):void {
|
||||
if ($this->config->getAppValue('dav', 'sendEventRemindersPush', 'no') !== 'yes') {
|
||||
return;
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@ declare(strict_types=1);
|
|||
* @author Joas Schilling <coding@schilljs.com>
|
||||
* @author Roeland Jago Douma <roeland@famdouma.nl>
|
||||
* @author Thomas Citharel <nextcloud@tcit.fr>
|
||||
* @author Richard Steinmetz <richard@steinmetz.cloud>
|
||||
*
|
||||
* @license GNU AGPL version 3 or any later version
|
||||
*
|
||||
|
|
@ -32,6 +33,7 @@ namespace OCA\DAV\CalDAV\Reminder;
|
|||
|
||||
use DateTimeImmutable;
|
||||
use OCA\DAV\CalDAV\CalDavBackend;
|
||||
use OCA\DAV\Connector\Sabre\Principal;
|
||||
use OCP\AppFramework\Utility\ITimeFactory;
|
||||
use OCP\IGroup;
|
||||
use OCP\IGroupManager;
|
||||
|
|
@ -66,6 +68,9 @@ class ReminderService {
|
|||
/** @var ITimeFactory */
|
||||
private $timeFactory;
|
||||
|
||||
/** @var Principal */
|
||||
private $principalConnector;
|
||||
|
||||
public const REMINDER_TYPE_EMAIL = 'EMAIL';
|
||||
public const REMINDER_TYPE_DISPLAY = 'DISPLAY';
|
||||
public const REMINDER_TYPE_AUDIO = 'AUDIO';
|
||||
|
|
@ -90,19 +95,22 @@ class ReminderService {
|
|||
* @param IGroupManager $groupManager
|
||||
* @param CalDavBackend $caldavBackend
|
||||
* @param ITimeFactory $timeFactory
|
||||
* @param Principal $principalConnector
|
||||
*/
|
||||
public function __construct(Backend $backend,
|
||||
NotificationProviderManager $notificationProviderManager,
|
||||
IUserManager $userManager,
|
||||
IGroupManager $groupManager,
|
||||
CalDavBackend $caldavBackend,
|
||||
ITimeFactory $timeFactory) {
|
||||
ITimeFactory $timeFactory,
|
||||
Principal $principalConnector) {
|
||||
$this->backend = $backend;
|
||||
$this->notificationProviderManager = $notificationProviderManager;
|
||||
$this->userManager = $userManager;
|
||||
$this->groupManager = $groupManager;
|
||||
$this->caldavBackend = $caldavBackend;
|
||||
$this->timeFactory = $timeFactory;
|
||||
$this->principalConnector = $principalConnector;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -151,8 +159,14 @@ class ReminderService {
|
|||
$users[] = $user;
|
||||
}
|
||||
|
||||
$userPrincipalEmailAddresses = [];
|
||||
$userPrincipal = $this->principalConnector->getPrincipalByPath($reminder['principaluri']);
|
||||
if ($userPrincipal) {
|
||||
$userPrincipalEmailAddresses = $this->principalConnector->getEmailAddressesOfPrincipal($userPrincipal);
|
||||
}
|
||||
|
||||
$notificationProvider = $this->notificationProviderManager->getProvider($reminder['type']);
|
||||
$notificationProvider->send($vevent, $reminder['displayname'], $users);
|
||||
$notificationProvider->send($vevent, $reminder['displayname'], $userPrincipalEmailAddresses, $users);
|
||||
|
||||
$this->deleteOrProcessNext($reminder, $vevent);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@
|
|||
* @author Joas Schilling <coding@schilljs.com>
|
||||
* @author Roeland Jago Douma <roeland@famdouma.nl>
|
||||
* @author Thomas Citharel <nextcloud@tcit.fr>
|
||||
* @author Richard Steinmetz <richard@steinmetz.cloud>
|
||||
*
|
||||
* @license GNU AGPL version 3 or any later version
|
||||
*
|
||||
|
|
@ -45,6 +46,7 @@ use Sabre\VObject\Component;
|
|||
use Sabre\VObject\Component\VCalendar;
|
||||
use Sabre\VObject\Component\VEvent;
|
||||
use Sabre\VObject\DateTimeParser;
|
||||
use Sabre\VObject\Document;
|
||||
use Sabre\VObject\FreeBusyGenerator;
|
||||
use Sabre\VObject\ITip;
|
||||
use Sabre\VObject\Parameter;
|
||||
|
|
@ -163,6 +165,14 @@ class Plugin extends \Sabre\CalDAV\Schedule\Plugin {
|
|||
* @inheritDoc
|
||||
*/
|
||||
public function scheduleLocalDelivery(ITip\Message $iTipMessage):void {
|
||||
/** @var Component|null $vevent */
|
||||
$vevent = $iTipMessage->message->VEVENT ?? null;
|
||||
|
||||
// Strip VALARMs from incoming VEVENT
|
||||
if ($vevent && isset($vevent->VALARM)) {
|
||||
$vevent->remove('VALARM');
|
||||
}
|
||||
|
||||
parent::scheduleLocalDelivery($iTipMessage);
|
||||
|
||||
// We only care when the message was successfully delivered locally
|
||||
|
|
@ -199,18 +209,10 @@ class Plugin extends \Sabre\CalDAV\Schedule\Plugin {
|
|||
return;
|
||||
}
|
||||
|
||||
if (!isset($iTipMessage->message)) {
|
||||
if (!$vevent) {
|
||||
return;
|
||||
}
|
||||
|
||||
$vcalendar = $iTipMessage->message;
|
||||
if (!isset($vcalendar->VEVENT)) {
|
||||
return;
|
||||
}
|
||||
|
||||
/** @var Component $vevent */
|
||||
$vevent = $vcalendar->VEVENT;
|
||||
|
||||
// We don't support autoresponses for recurrencing events for now
|
||||
if (isset($vevent->RRULE) || isset($vevent->RDATE)) {
|
||||
return;
|
||||
|
|
|
|||
|
|
@ -591,4 +591,44 @@ class Principal implements BackendInterface {
|
|||
|
||||
return [];
|
||||
}
|
||||
|
||||
/**
|
||||
* Get all email addresses associated to a principal.
|
||||
*
|
||||
* @param array $principal Data from getPrincipal*()
|
||||
* @return string[] All email addresses without the mailto: prefix
|
||||
*/
|
||||
public function getEmailAddressesOfPrincipal(array $principal): array {
|
||||
$emailAddresses = [];
|
||||
|
||||
if (($primaryAddress = $principal['{http://sabredav.org/ns}email-address'])) {
|
||||
$emailAddresses[] = $primaryAddress;
|
||||
}
|
||||
|
||||
if (isset($principal['{DAV:}alternate-URI-set'])) {
|
||||
foreach ($principal['{DAV:}alternate-URI-set'] as $address) {
|
||||
if (str_starts_with($address, 'mailto:')) {
|
||||
$emailAddresses[] = substr($address, 7);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (isset($principal['{urn:ietf:params:xml:ns:caldav}calendar-user-address-set'])) {
|
||||
foreach ($principal['{urn:ietf:params:xml:ns:caldav}calendar-user-address-set'] as $address) {
|
||||
if (str_starts_with($address, 'mailto:')) {
|
||||
$emailAddresses[] = substr($address, 7);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (isset($principal['{http://calendarserver.org/ns/}email-address-set'])) {
|
||||
foreach ($principal['{http://calendarserver.org/ns/}email-address-set'] as $address) {
|
||||
if (str_starts_with($address, 'mailto:')) {
|
||||
$emailAddresses[] = substr($address, 7);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return array_values(array_unique($emailAddresses));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -81,6 +81,7 @@ class EmailProviderTest extends AbstractNotificationProviderTest {
|
|||
|
||||
public function testSendWithoutAttendees():void {
|
||||
[$user1, $user2, $user3, , $user5] = $users = $this->getUsers();
|
||||
$principalEmailAddresses = [$user1->getEmailAddress()];
|
||||
|
||||
$enL10N = $this->createMock(IL10N::class);
|
||||
$enL10N->method('t')
|
||||
|
|
@ -186,11 +187,12 @@ class EmailProviderTest extends AbstractNotificationProviderTest {
|
|||
$this->setupURLGeneratorMock(2);
|
||||
|
||||
$vcalendar = $this->getNoAttendeeVCalendar();
|
||||
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $users);
|
||||
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $principalEmailAddresses, $users);
|
||||
}
|
||||
|
||||
public function testSendWithAttendees(): void {
|
||||
public function testSendWithAttendeesWhenOwnerIsOrganizer(): void {
|
||||
[$user1, $user2, $user3, , $user5] = $users = $this->getUsers();
|
||||
$principalEmailAddresses = [$user1->getEmailAddress()];
|
||||
|
||||
$enL10N = $this->createMock(IL10N::class);
|
||||
$enL10N->method('t')
|
||||
|
|
@ -282,7 +284,81 @@ class EmailProviderTest extends AbstractNotificationProviderTest {
|
|||
$this->setupURLGeneratorMock(2);
|
||||
|
||||
$vcalendar = $this->getAttendeeVCalendar();
|
||||
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $users);
|
||||
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $principalEmailAddresses, $users);
|
||||
}
|
||||
|
||||
public function testSendWithAttendeesWhenOwnerIsAttendee(): void {
|
||||
[$user1, $user2, $user3] = $this->getUsers();
|
||||
$users = [$user2, $user3];
|
||||
$principalEmailAddresses = [$user2->getEmailAddress()];
|
||||
|
||||
$deL10N = $this->createMock(IL10N::class);
|
||||
$deL10N->method('t')
|
||||
->willReturnArgument(0);
|
||||
$deL10N->method('l')
|
||||
->willReturnArgument(0);
|
||||
|
||||
$this->l10nFactory
|
||||
->method('getUserLanguage')
|
||||
->willReturnMap([
|
||||
[$user2, 'de'],
|
||||
[$user3, 'de'],
|
||||
]);
|
||||
|
||||
$this->l10nFactory
|
||||
->method('findGenericLanguage')
|
||||
->willReturn('en');
|
||||
|
||||
$this->l10nFactory
|
||||
->method('languageExists')
|
||||
->willReturnMap([
|
||||
['dav', 'de', true],
|
||||
]);
|
||||
|
||||
$this->l10nFactory
|
||||
->method('get')
|
||||
->willReturnMap([
|
||||
['dav', 'de', null, $deL10N],
|
||||
]);
|
||||
|
||||
$template1 = $this->getTemplateMock();
|
||||
$message12 = $this->getMessageMock('uid2@example.com', $template1);
|
||||
$message13 = $this->getMessageMock('uid3@example.com', $template1);
|
||||
|
||||
$this->mailer->expects(self::once())
|
||||
->method('createEMailTemplate')
|
||||
->with('dav.calendarReminder')
|
||||
->willReturnOnConsecutiveCalls(
|
||||
$template1,
|
||||
);
|
||||
$this->mailer->expects($this->atLeastOnce())
|
||||
->method('validateMailAddress')
|
||||
->willReturnMap([
|
||||
['foo1@example.org', true],
|
||||
['foo3@example.org', true],
|
||||
['foo4@example.org', true],
|
||||
['uid1@example.com', true],
|
||||
['uid2@example.com', true],
|
||||
['uid3@example.com', true],
|
||||
['invalid', false],
|
||||
]);
|
||||
$this->mailer->expects($this->exactly(2))
|
||||
->method('createMessage')
|
||||
->with()
|
||||
->willReturnOnConsecutiveCalls(
|
||||
$message12,
|
||||
$message13,
|
||||
);
|
||||
$this->mailer->expects($this->exactly(2))
|
||||
->method('send')
|
||||
->withConsecutive(
|
||||
[$message12],
|
||||
[$message13],
|
||||
)->willReturn([]);
|
||||
$this->setupURLGeneratorMock(1);
|
||||
|
||||
$vcalendar = $this->getAttendeeVCalendar();
|
||||
$this->provider->send($vcalendar->VEVENT, $this->calendarDisplayName, $principalEmailAddresses, $users);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -392,6 +468,14 @@ class EmailProviderTest extends AbstractNotificationProviderTest {
|
|||
'DESCRIPTION' => 'DESCRIPTION 456',
|
||||
]);
|
||||
|
||||
$vcalendar->VEVENT->add(
|
||||
'ORGANIZER',
|
||||
'mailto:uid1@example.com',
|
||||
[
|
||||
'LANG' => 'en'
|
||||
]
|
||||
);
|
||||
|
||||
$vcalendar->VEVENT->add(
|
||||
'ATTENDEE',
|
||||
'mailto:foo1@example.org',
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ declare(strict_types=1);
|
|||
* @author Georg Ehrke <oc.list@georgehrke.com>
|
||||
* @author Roeland Jago Douma <roeland@famdouma.nl>
|
||||
* @author Thomas Citharel <nextcloud@tcit.fr>
|
||||
* @author Richard Steinmetz <richard@steinmetz.cloud>
|
||||
*
|
||||
* @license GNU AGPL version 3 or any later version
|
||||
*
|
||||
|
|
@ -107,7 +108,7 @@ class PushProviderTest extends AbstractNotificationProviderTest {
|
|||
|
||||
$users = [$user1, $user2, $user3];
|
||||
|
||||
$this->provider->send($this->vcalendar->VEVENT, $this->calendarDisplayName, $users);
|
||||
$this->provider->send($this->vcalendar->VEVENT, $this->calendarDisplayName, [], $users);
|
||||
}
|
||||
|
||||
public function testSend(): void {
|
||||
|
|
@ -160,7 +161,7 @@ class PushProviderTest extends AbstractNotificationProviderTest {
|
|||
->method('notify')
|
||||
->with($notification3);
|
||||
|
||||
$this->provider->send($this->vcalendar->VEVENT, $this->calendarDisplayName, $users);
|
||||
$this->provider->send($this->vcalendar->VEVENT, $this->calendarDisplayName, [], $users);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ declare(strict_types=1);
|
|||
* @author Georg Ehrke <oc.list@georgehrke.com>
|
||||
* @author Roeland Jago Douma <roeland@famdouma.nl>
|
||||
* @author Thomas Citharel <nextcloud@tcit.fr>
|
||||
* @author Richard Steinmetz <richard@steinmetz.cloud>
|
||||
*
|
||||
* @license GNU AGPL version 3 or any later version
|
||||
*
|
||||
|
|
@ -33,6 +34,7 @@ use OCA\DAV\CalDAV\Reminder\Backend;
|
|||
use OCA\DAV\CalDAV\Reminder\INotificationProvider;
|
||||
use OCA\DAV\CalDAV\Reminder\NotificationProviderManager;
|
||||
use OCA\DAV\CalDAV\Reminder\ReminderService;
|
||||
use OCA\DAV\Connector\Sabre\Principal;
|
||||
use OCP\AppFramework\Utility\ITimeFactory;
|
||||
use OCP\IGroupManager;
|
||||
use OCP\IUser;
|
||||
|
|
@ -66,6 +68,9 @@ class ReminderServiceTest extends TestCase {
|
|||
/** @var ReminderService */
|
||||
private $reminderService;
|
||||
|
||||
/** @var Principal|\PHPUnit\Framework\MockObject\MockObject */
|
||||
private $principalConnector;
|
||||
|
||||
public const CALENDAR_DATA = <<<EOD
|
||||
BEGIN:VCALENDAR
|
||||
PRODID:-//Nextcloud calendar v1.6.4
|
||||
|
|
@ -194,6 +199,7 @@ EOD;
|
|||
$this->groupManager = $this->createMock(IGroupManager::class);
|
||||
$this->caldavBackend = $this->createMock(CalDavBackend::class);
|
||||
$this->timeFactory = $this->createMock(ITimeFactory::class);
|
||||
$this->principalConnector = $this->createMock(Principal::class);
|
||||
|
||||
$this->caldavBackend->method('getShares')->willReturn([]);
|
||||
|
||||
|
|
@ -202,7 +208,8 @@ EOD;
|
|||
$this->userManager,
|
||||
$this->groupManager,
|
||||
$this->caldavBackend,
|
||||
$this->timeFactory);
|
||||
$this->timeFactory,
|
||||
$this->principalConnector);
|
||||
}
|
||||
|
||||
public function testOnCalendarObjectDelete():void {
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@
|
|||
* @author Morris Jobke <hey@morrisjobke.de>
|
||||
* @author Roeland Jago Douma <roeland@famdouma.nl>
|
||||
* @author Thomas Müller <thomas.mueller@tmit.eu>
|
||||
* @author Richard Steinmetz <richard@steinmetz.cloud>
|
||||
*
|
||||
* @license AGPL-3.0
|
||||
*
|
||||
|
|
@ -925,4 +926,34 @@ class PrincipalTest extends TestCase {
|
|||
['mailto:user3@foo.bar', 'user3@foo.bar', 'principals/users/user3'],
|
||||
];
|
||||
}
|
||||
|
||||
public function testGetEmailAddressesOfPrincipal(): void {
|
||||
$principal = [
|
||||
'{http://sabredav.org/ns}email-address' => 'bar@company.org',
|
||||
'{DAV:}alternate-URI-set' => [
|
||||
'/some/url',
|
||||
'mailto:foo@bar.com',
|
||||
'mailto:duplicate@example.com',
|
||||
],
|
||||
'{urn:ietf:params:xml:ns:caldav}calendar-user-address-set' => [
|
||||
'mailto:bernard@example.com',
|
||||
'mailto:bernard.desruisseaux@example.com',
|
||||
],
|
||||
'{http://calendarserver.org/ns/}email-address-set' => [
|
||||
'mailto:duplicate@example.com',
|
||||
'mailto:user@some.org',
|
||||
],
|
||||
];
|
||||
|
||||
$expected = [
|
||||
'bar@company.org',
|
||||
'foo@bar.com',
|
||||
'duplicate@example.com',
|
||||
'bernard@example.com',
|
||||
'bernard.desruisseaux@example.com',
|
||||
'user@some.org',
|
||||
];
|
||||
$actual = $this->connector->getEmailAddressesOfPrincipal($principal);
|
||||
$this->assertEquals($expected, $actual);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue