Skip to content

fix(bulkactivity): bulk query user settings #2237

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

fix(bulkactivity): bulk query user settings #2237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
458e4af
fix(bulkactivity): bulk query user settings
miaulalala Nov 18, 2025
8c75fe9
fixup! fix(bulkactivity): bulk query user settings
miaulalala Nov 19, 2025
4bb1632
fixup! fix(bulkactivity): bulk query user settings
miaulalala Nov 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 25 additions & 18 deletions lib/Consumer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
use OCP\Activity\IEvent;
use OCP\Activity\IManager;
use OCP\Activity\ISetting;
use Throwable;
use OCP\Config\IUserConfig;
use OCP\Config\ValueType;
use OCP\DB\Exception;

class Consumer implements IConsumer, IBulkConsumer {

Expand All @@ -22,6 +24,7 @@
protected IManager $manager,
protected UserSettings $userSettings,
protected NotificationGenerator $notificationGenerator,
protected IUserConfig $userConfig,
) {
}

Expand Down Expand Up @@ -53,56 +56,60 @@
}

/**
* 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)) {
return;
}

$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', 'notify_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());
}

if ($canChangeMail === true) {
$emailSetting = $this->userSettings->getUserSetting($event->getAffectedUser(), 'email', $event->getType());
$emailSetting = ($emailSetting) ? $this->userSettings->getUserSetting($event->getAffectedUser(), 'setting', 'batchtime') : false;
}
$notificationSetting = $userPushSettings[$affectedUser] ?? false;
$emailSetting = $userEmailSettings[$affectedUser] ?? false;
$$batchTimeSettings[$affectedUser] ?? false;

if (isset($notificationSetting) && $notificationSetting === true) {
if ($notificationSetting !== false) {
$this->notificationGenerator->sendNotificationForEvent($event, $activityId, $notificationSetting);

Check failure on line 103 in lib/Consumer.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

InvalidArgument 

lib/Consumer.php:103:81: InvalidArgument: Argument 3 of OCA\Activity\NotificationGenerator::sendNotificationForEvent expects bool|null, but array<array-key, mixed>|scalar provided (see https://psalm.dev/004)
}

if (isset($emailSetting) && $emailSetting !== false) {
$latestSend = $event->getTimestamp() + $emailSetting;
$this->data->storeMail($event, $latestSend);

Check failure on line 108 in lib/Consumer.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

InvalidScalarArgument 

lib/Consumer.php:108:36: InvalidScalarArgument: Argument 2 of OCA\Activity\Data::storeMail expects int, but float|int provided (see https://psalm.dev/012)
}
}
if ($shouldFlush === true) {
$this->notificationGenerator->flushNotifications();
}
}
}
115 changes: 102 additions & 13 deletions tests/ConsumerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@
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;
use OCP\L10N\IFactory;
Expand All @@ -48,6 +51,9 @@ class ConsumerTest extends TestCase {
protected IManager&MockObject $activityManager;
protected NotificationGenerator&MockObject $notificationGenerator;
protected UserSettings $userSettings;
private IUserConfig&MockObject $userConfig;
private IEvent $event;
private Consumer $consumer;

protected function setUp(): void {
parent::setUp();
Expand All @@ -62,6 +68,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);
Expand All @@ -80,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 {
Expand Down Expand Up @@ -120,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)
Expand All @@ -136,15 +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);
$event = Server::get(IManager::class)->generateEvent();
$event->setApp('test')
$this->event->setApp('test')
->setType($type)
->setAffectedUser($affectedUser)
->setAuthor($author)
Expand All @@ -160,17 +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);
$event = Server::get(IManager::class)->generateEvent();
$event->setApp('test')
$this->event->setApp('test')
->setType($type)
->setAffectedUser($affectedUser)
->setAuthor($author)
Expand All @@ -189,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, );
}

}
Loading