From 458e4af301f5f5512c3154efffa83a36d09a93ea Mon Sep 17 00:00:00 2001 From: Anna Larch Date: Tue, 18 Nov 2025 16:58:55 +0100 Subject: [PATCH 1/3] fix(bulkactivity): bulk query user settings Signed-off-by: Anna Larch --- lib/Consumer.php | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/lib/Consumer.php b/lib/Consumer.php index 801d2cd1a..8b50ebfd2 100644 --- a/lib/Consumer.php +++ b/lib/Consumer.php @@ -13,6 +13,8 @@ use OCP\Activity\IEvent; use OCP\Activity\IManager; use OCP\Activity\ISetting; +use OCP\Config\IUserConfig; +use OCP\Config\ValueType; use Throwable; class Consumer implements IConsumer, IBulkConsumer { @@ -22,6 +24,7 @@ public function __construct( protected IManager $manager, protected UserSettings $userSettings, protected NotificationGenerator $notificationGenerator, + protected IUserConfig $userConfig, ) { } @@ -72,30 +75,29 @@ public function bulkReceive(IEvent $event, array $affectedUserIds, ISetting $set } $canChangeMail = $setting->canChangeMail(); - $canChangePush = false; - if ($setting instanceof ActivitySettings && $setting->canChangeNotification() === true) { - $canChangePush = true; + $canChangePush = $setting instanceof ActivitySettings && $setting->canChangeNotification() === true; + + $userPushSettings = $userEmailSettings = $batchTimeSettings = null; + if ($canChangePush === true) { + $userPushSettings = $this->userConfig->getValuesByUsers('activity', 'notify_notification_ ' . $event->getType(), ValueType::BOOL, $affectedUserIds); } - if ($canChangePush === false && $canChangeMail === false) { - return; + if ($canChangeMail === true || $setting->isDefaultEnabledMail() === true) { + $userEmailSettings = $this->userConfig->getValuesByUsers('activity', 'notify_email_ ' . $event->getType(), ValueType::BOOL, $affectedUserIds); + $batchTimeSettings = $this->userConfig->getValuesByUsers('activity', 'setting_batchtime', ValueType::INT, $affectedUserIds); } + + $shouldFlush = $this->notificationGenerator->deferNotifications(); foreach ($activityIds as $activityId => $affectedUser) { if ($event->getAuthor() === $affectedUser) { continue; } $event->setAffectedUser($affectedUser); - if ($canChangePush === true) { - $notificationSetting = $this->userSettings->getUserSetting($affectedUser, 'notification', $event->getType()); - } + $notificationSetting = $userPushSettings[$affectedUser] ?? false; + $emailSetting = $userEmailSettings[$affectedUser] ?? $batchTimeSettings[$affectedUser] ?? false; - if ($canChangeMail === true) { - $emailSetting = $this->userSettings->getUserSetting($event->getAffectedUser(), 'email', $event->getType()); - $emailSetting = ($emailSetting) ? $this->userSettings->getUserSetting($event->getAffectedUser(), 'setting', 'batchtime') : false; - } - - if (isset($notificationSetting) && $notificationSetting === true) { + if ($notificationSetting !== false) { $this->notificationGenerator->sendNotificationForEvent($event, $activityId, $notificationSetting); } @@ -104,5 +106,8 @@ public function bulkReceive(IEvent $event, array $affectedUserIds, ISetting $set $this->data->storeMail($event, $latestSend); } } + if ($shouldFlush === true) { + $this->notificationGenerator->flushNotifications(); + } } } From 8c75fe9e5a56af7e63630362f5ca0e9183c2a88c Mon Sep 17 00:00:00 2001 From: Anna Larch Date: Wed, 19 Nov 2025 09:57:05 +0100 Subject: [PATCH 2/3] fixup! fix(bulkactivity): bulk query user settings --- lib/Consumer.php | 11 ++++++----- tests/ConsumerTest.php | 19 +++++++++++++++++-- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/lib/Consumer.php b/lib/Consumer.php index 8b50ebfd2..7d8bc8225 100644 --- a/lib/Consumer.php +++ b/lib/Consumer.php @@ -15,7 +15,7 @@ use OCP\Activity\ISetting; use OCP\Config\IUserConfig; use OCP\Config\ValueType; -use Throwable; +use OCP\DB\Exception; class Consumer implements IConsumer, IBulkConsumer { @@ -56,18 +56,20 @@ public function receive(IEvent $event): void { } /** - * Send an event to the notifications of a user + * Send an event to the notifications of a bulk of users * * @param IEvent $event - * @throws Throwable - * + * @param array $affectedUserIds + * @param ISetting $setting * @return void + * @throws Exception */ #[\Override] public function bulkReceive(IEvent $event, array $affectedUserIds, ISetting $setting): void { if (empty($affectedUserIds)) { return; } + $activityIds = $this->data->bulkSend($event, $affectedUserIds); if (empty($activityIds)) { @@ -87,7 +89,6 @@ public function bulkReceive(IEvent $event, array $affectedUserIds, ISetting $set $batchTimeSettings = $this->userConfig->getValuesByUsers('activity', 'setting_batchtime', ValueType::INT, $affectedUserIds); } - $shouldFlush = $this->notificationGenerator->deferNotifications(); foreach ($activityIds as $activityId => $affectedUser) { if ($event->getAuthor() === $affectedUser) { diff --git a/tests/ConsumerTest.php b/tests/ConsumerTest.php index d77849ca7..cd95b67b9 100644 --- a/tests/ConsumerTest.php +++ b/tests/ConsumerTest.php @@ -29,6 +29,7 @@ use OCA\Activity\NotificationGenerator; use OCA\Activity\UserSettings; use OCP\Activity\IManager; +use OCP\Config\IUserConfig; use OCP\IDBConnection; use OCP\IL10N; use OCP\L10N\IFactory; @@ -48,6 +49,7 @@ class ConsumerTest extends TestCase { protected IManager&MockObject $activityManager; protected NotificationGenerator&MockObject $notificationGenerator; protected UserSettings $userSettings; + private IUserConfig&MockObject $userConfig; protected function setUp(): void { parent::setUp(); @@ -62,6 +64,7 @@ protected function setUp(): void { $l10n = $this->createMock(IL10N::class); $this->notificationGenerator = $this->createMock(NotificationGenerator::class); $this->l10nFactory = $this->createMock(IFactory::class); + $this->userConfig = $this->createMock(IUserConfig::class); $this->data->method('send') ->willReturn(1); @@ -142,7 +145,13 @@ public function testReceiveStream(string $type, string $author, string $affected #[DataProvider('receiveData')] public function testReceiveEmail(string $type, string $author, string $affectedUser, string $subject, $expected): void { $time = time(); - $consumer = new Consumer($this->data, $this->activityManager, $this->userSettings, $this->notificationGenerator); + $consumer = new Consumer( + $this->data, + $this->activityManager, + $this->userSettings, + $this->notificationGenerator, + $this->userConfig, + ); $event = Server::get(IManager::class)->generateEvent(); $event->setApp('test') ->setType($type) @@ -168,7 +177,13 @@ public function testReceiveEmail(string $type, string $author, string $affectedU #[DataProvider('receiveData')] public function testReceiveNotification(string $type, string $author, string $affectedUser, string $subject, $expected): void { - $consumer = new Consumer($this->data, $this->activityManager, $this->userSettings, $this->notificationGenerator); + $consumer = new Consumer( + $this->data, + $this->activityManager, + $this->userSettings, + $this->notificationGenerator, + $this->userConfig, + ); $event = Server::get(IManager::class)->generateEvent(); $event->setApp('test') ->setType($type) From 4bb16322b6d2d2557f42e001a15da1c938bf7235 Mon Sep 17 00:00:00 2001 From: Anna Larch Date: Wed, 19 Nov 2025 15:52:05 +0100 Subject: [PATCH 3/3] fixup! fix(bulkactivity): bulk query user settings --- lib/Consumer.php | 5 +- tests/ConsumerTest.php | 124 ++++++++++++++++++++++++++++++++--------- 2 files changed, 102 insertions(+), 27 deletions(-) diff --git a/lib/Consumer.php b/lib/Consumer.php index 7d8bc8225..d98398d17 100644 --- a/lib/Consumer.php +++ b/lib/Consumer.php @@ -86,7 +86,7 @@ public function bulkReceive(IEvent $event, array $affectedUserIds, ISetting $set if ($canChangeMail === true || $setting->isDefaultEnabledMail() === true) { $userEmailSettings = $this->userConfig->getValuesByUsers('activity', 'notify_email_ ' . $event->getType(), ValueType::BOOL, $affectedUserIds); - $batchTimeSettings = $this->userConfig->getValuesByUsers('activity', 'setting_batchtime', ValueType::INT, $affectedUserIds); + $batchTimeSettings = $this->userConfig->getValuesByUsers('activity', 'notify_setting_batchtime', ValueType::INT, $affectedUserIds); } $shouldFlush = $this->notificationGenerator->deferNotifications(); @@ -96,7 +96,8 @@ public function bulkReceive(IEvent $event, array $affectedUserIds, ISetting $set } $event->setAffectedUser($affectedUser); $notificationSetting = $userPushSettings[$affectedUser] ?? false; - $emailSetting = $userEmailSettings[$affectedUser] ?? $batchTimeSettings[$affectedUser] ?? false; + $emailSetting = $userEmailSettings[$affectedUser] ?? false; + $$batchTimeSettings[$affectedUser] ?? false; if ($notificationSetting !== false) { $this->notificationGenerator->sendNotificationForEvent($event, $activityId, $notificationSetting); diff --git a/tests/ConsumerTest.php b/tests/ConsumerTest.php index cd95b67b9..af275f126 100644 --- a/tests/ConsumerTest.php +++ b/tests/ConsumerTest.php @@ -28,7 +28,9 @@ use OCA\Activity\Data; use OCA\Activity\NotificationGenerator; use OCA\Activity\UserSettings; +use OCP\Activity\IEvent; use OCP\Activity\IManager; +use OCP\Activity\ISetting; use OCP\Config\IUserConfig; use OCP\IDBConnection; use OCP\IL10N; @@ -50,6 +52,8 @@ class ConsumerTest extends TestCase { protected NotificationGenerator&MockObject $notificationGenerator; protected UserSettings $userSettings; private IUserConfig&MockObject $userConfig; + private IEvent $event; + private Consumer $consumer; protected function setUp(): void { parent::setUp(); @@ -83,6 +87,16 @@ protected function setUp(): void { ['affectedUser', 'setting', 'batchtime', 10], ['affectedUser2', 'setting', 'batchtime', 10], ]); + + $this->consumer = new Consumer( + $this->data, + $this->activityManager, + $this->userSettings, + $this->notificationGenerator, + $this->userConfig, + ); + + $this->event = Server::get(IManager::class)->generateEvent(); } protected function tearDown(): void { @@ -123,9 +137,7 @@ public static function receiveData(): array { #[DataProvider('receiveData')] public function testReceiveStream(string $type, string $author, string $affectedUser, string $subject): void { - $consumer = new Consumer($this->data, $this->activityManager, $this->userSettings, $this->notificationGenerator); - $event = Server::get(IManager::class)->generateEvent(); - $event->setApp('test') + $this->event->setApp('test') ->setType($type) ->setAffectedUser($affectedUser) ->setAuthor($author) @@ -139,21 +151,13 @@ public function testReceiveStream(string $type, string $author, string $affected $this->data->expects($this->once()) ->method('send'); - $consumer->receive($event); + $this->consumer->receive($this->event); } #[DataProvider('receiveData')] public function testReceiveEmail(string $type, string $author, string $affectedUser, string $subject, $expected): void { $time = time(); - $consumer = new Consumer( - $this->data, - $this->activityManager, - $this->userSettings, - $this->notificationGenerator, - $this->userConfig, - ); - $event = Server::get(IManager::class)->generateEvent(); - $event->setApp('test') + $this->event->setApp('test') ->setType($type) ->setAffectedUser($affectedUser) ->setAuthor($author) @@ -169,23 +173,15 @@ public function testReceiveEmail(string $type, string $author, string $affectedU } else { $this->data->expects($this->once()) ->method('storeMail') - ->with($event, $time + 10); + ->with($this->event, $time + 10); } - $consumer->receive($event); + $this->consumer->receive($this->event); } #[DataProvider('receiveData')] public function testReceiveNotification(string $type, string $author, string $affectedUser, string $subject, $expected): void { - $consumer = new Consumer( - $this->data, - $this->activityManager, - $this->userSettings, - $this->notificationGenerator, - $this->userConfig, - ); - $event = Server::get(IManager::class)->generateEvent(); - $event->setApp('test') + $this->event->setApp('test') ->setType($type) ->setAffectedUser($affectedUser) ->setAuthor($author) @@ -204,6 +200,84 @@ public function testReceiveNotification(string $type, string $author, string $af ->method('sendNotificationForEvent'); } - $consumer->receive($event); + $this->consumer->receive($this->event); + } + + public static function receiveBulkData(): array { + /** + * type + * author + * subject + * affectedUsers + * activityIds + * ISettings canChangeEmail, canChangePush + * IUserConfig notify_notification_type notify_email_type notify_setting_batchtime + * + */ + + return [ + // Empty affected users + ['type', 'author','subject', + [], + [], + [], + [] + ], + // Empty activity IDs + ['type', 'author','subject', + ['affectedUser', 'affectedUser1'], + [], + [], + [] + ], + ['type', 'author','subject', ['affectedUser', 'affectedUser2', 'affectedUser3'], [true, true], [false, false, false]], + ['type2', 'author', 'subject', false, ['affectedUser', 'affectedUser2', 'affectedUser3']], + ['type', 'author','subject_self', 'affectedUser', ['affectedUser', 'affectedUser2', 'affectedUser3']], + ['type', 'author', 'subject_self', 'affectedUser2', ['affectedUser', 'affectedUser2', 'affectedUser3']], + ['type', 'author', 'subject2', 'affectedUser', ['affectedUser', 'affectedUser2', 'affectedUser3']], + ['type', 'author', 'subject2', 'affectedUser2', ['affectedUser', 'affectedUser2', 'affectedUser3']], + ['type', 'affectedUser', 'subject_self', 'affectedUser', ['affectedUser', 'affectedUser2', 'affectedUser3']], + ['type', 'affectedUser2', 'subject_self', false, ['affectedUser', 'affectedUser2', 'affectedUser3']], + ['type', 'affectedUser', 'subject2', 'affectedUser', ['affectedUser', 'affectedUser2', 'affectedUser3']], + ['type', 'affectedUser2','subject2', false, ['affectedUser', 'affectedUser2', 'affectedUser3']], + ]; + } + + #[DataProvider('receiveBulkData')] + public function testBulkReceiveNotification(string $type, string $author, string $subject, $expected, array $affectedUsers): void { + $this->event->setApp('activity') + ->setType($type) + ->setAuthor($author) + ->setTimestamp(time()) + ->setSubject($subject, ['subjectParam1', 'subjectParam2']) + ->setMessage('message', ['messageParam1', 'messageParam2']) + ->setObject('', 0, 'file') + ->setLink('link'); + $this->deleteTestActivities(); + + $settings = $this->createMock(ISetting::class); + $settings->expects($this->any()) + ->method('getValuesByUsers') + ->with($affectedUsers) + ->willReturn($affectedUsers); + if (empty($affectedUsers)) { + $this->data->expects($this->never()) + ->method('bulkSend'); + $this->data->expects($this->never()) + ->method('storeMail'); + $this->notificationGenerator->expects($this->never()) + ->method('sendNotificationForEvent'); + } + + if ($expected === false || $author === $affectedUser) { + $this->notificationGenerator->expects($this->never()) + ->method('sendNotificationForEvent'); + } else { + $this->notificationGenerator->expects($this->once()) + ->method('sendNotificationForEvent'); + } + + $this->consumer->bulkReceive($this->event, $affectedUsers, ); } + }