diff --git a/lib/Consumer.php b/lib/Consumer.php index 0aed62e67..36b1aa257 100644 --- a/lib/Consumer.php +++ b/lib/Consumer.php @@ -16,6 +16,7 @@ use OCP\Config\IUserConfig; use OCP\Config\ValueType; use OCP\DB\Exception; +use OCP\IConfig; class Consumer implements IConsumer, IBulkConsumer { @@ -25,6 +26,7 @@ public function __construct( protected UserSettings $userSettings, protected NotificationGenerator $notificationGenerator, protected IUserConfig $userConfig, + protected IConfig $config, ) { } @@ -78,13 +80,16 @@ public function bulkReceive(IEvent $event, array $affectedUserIds, ISetting $set $canChangeMail = $setting->canChangeMail(); $canChangePush = $setting instanceof ActivitySettings && $setting->canChangeNotification() === true; + $defaultPushEnabled = $setting instanceof ActivitySettings && $setting->isDefaultEnabledNotification(); - $userPushSettings = $userEmailSettings = $batchTimeSettings = null; + $userPushSettings = null; if ($canChangePush === true) { $userPushSettings = $this->userConfig->getValuesByUsers('activity', 'notify_notification_' . $event->getType(), ValueType::BOOL, $affectedUserIds); } - if ($canChangeMail === true || $setting->isDefaultEnabledMail() === true) { + $userEmailSettings = $batchTimeSettings = null; + $emailEnabled = $this->config->getAppValue('activity', 'enable_email', 'yes') !== 'no'; + if ($emailEnabled && ($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); } @@ -95,14 +100,17 @@ public function bulkReceive(IEvent $event, array $affectedUserIds, ISetting $set continue; } $event->setAffectedUser($affectedUser); - $notificationSetting = $userPushSettings[$affectedUser] ?? null; - if ($notificationSetting !== null) { - $notificationSetting = (bool)$notificationSetting; + + if ($canChangePush === true) { + $notificationSetting = isset($userPushSettings[$affectedUser]) ? (bool)$userPushSettings[$affectedUser] : $defaultPushEnabled; + } else { + $notificationSetting = $defaultPushEnabled; } + $emailSetting = $userEmailSettings[$affectedUser] ?? false; $emailSetting = ($emailSetting) ? ($batchTimeSettings[$affectedUser] ?? false) : false; - if ($notificationSetting !== false) { + if ($notificationSetting === true) { $this->notificationGenerator->sendNotificationForEvent($event, $activityId, $notificationSetting); } diff --git a/tests/ConsumerTest.php b/tests/ConsumerTest.php index 629ba4aae..14d0660ca 100644 --- a/tests/ConsumerTest.php +++ b/tests/ConsumerTest.php @@ -33,6 +33,7 @@ use OCP\Activity\IManager; use OCP\Activity\ISetting; use OCP\Config\IUserConfig; +use OCP\IConfig; use OCP\IDBConnection; use OCP\IL10N; use OCP\L10N\IFactory; @@ -53,6 +54,7 @@ class ConsumerTest extends TestCase { protected NotificationGenerator&MockObject $notificationGenerator; protected UserSettings $userSettings; private IUserConfig&MockObject $userConfig; + private IConfig&MockObject $config; private IEvent $event; private Consumer $consumer; @@ -70,6 +72,8 @@ protected function setUp(): void { $this->notificationGenerator = $this->createMock(NotificationGenerator::class); $this->l10nFactory = $this->createMock(IFactory::class); $this->userConfig = $this->createMock(IUserConfig::class); + $this->config = $this->createMock(IConfig::class); + $this->config->method('getAppValue')->willReturn('yes'); $this->data->method('send') ->willReturn(1); @@ -95,6 +99,7 @@ protected function setUp(): void { $this->userSettings, $this->notificationGenerator, $this->userConfig, + $this->config, ); $this->event = Server::get(IManager::class)->generateEvent(); @@ -143,6 +148,7 @@ public function testReceiveStream(string $type, string $author, string $affected $this->userSettings, $this->notificationGenerator, $this->userConfig, + $this->config, ); $event = Server::get(IManager::class)->generateEvent(); $event->setApp('test') @@ -322,9 +328,7 @@ public function testBulkReceiveNoMailWhenSettingDisabled(): void { $this->data->expects($this->never()) ->method('storeMail'); - // Notification is still sent because $notificationSetting defaults to null - // and null !== false, so the default is to send notifications - $this->notificationGenerator->expects($this->once()) + $this->notificationGenerator->expects($this->never()) ->method('sendNotificationForEvent'); $this->consumer->bulkReceive($this->event, ['affectedUser'], $settings); @@ -434,8 +438,8 @@ public function testBulkReceiveMultipleUsersWithMixedSettings(): void { return []; }); - // user1 and user2 get notifications (user2 has null setting which defaults to send), author is skipped - $this->notificationGenerator->expects($this->exactly(2)) + // only user1 has an explicit notification=true in DB; user2 has no entry so falls back to isDefaultEnabledNotification()=false + $this->notificationGenerator->expects($this->once()) ->method('sendNotificationForEvent'); // user2 gets email, author is skipped $this->data->expects($this->once()) @@ -445,6 +449,48 @@ public function testBulkReceiveMultipleUsersWithMixedSettings(): void { $this->consumer->bulkReceive($this->event, ['user1', 'user2', 'author'], $settings); } + public function testBulkReceiveNoMailWhenAdminEmailDisabled(): void { + $time = time(); + $this->event->setApp('activity') + ->setType('type') + ->setAuthor('author') + ->setTimestamp($time) + ->setSubject('subject', ['subjectParam1']) + ->setMessage('message', ['messageParam1']) + ->setObject('', 0, 'file') + ->setLink('link'); + + $config = $this->createMock(IConfig::class); + $config->method('getAppValue')->willReturn('no'); + + $consumer = new Consumer( + $this->data, + $this->activityManager, + $this->userSettings, + $this->notificationGenerator, + $this->userConfig, + $config, + ); + + $settings = $this->createMock(ActivitySettings::class); + $settings->method('canChangeMail')->willReturn(true); + $settings->method('isDefaultEnabledMail')->willReturn(true); + $settings->method('canChangeNotification')->willReturn(false); + $settings->method('isDefaultEnabledNotification')->willReturn(false); + + $this->data->expects($this->once()) + ->method('bulkSend') + ->willReturn([1 => 'affectedUser']); + + $this->userConfig->expects($this->never()) + ->method('getValuesByUsers'); + + $this->data->expects($this->never()) + ->method('storeMail'); + + $consumer->bulkReceive($this->event, ['affectedUser'], $settings); + } + public function testBulkReceiveWithISetting(): void { $this->event->setApp('activity') ->setType('type') @@ -463,9 +509,8 @@ public function testBulkReceiveWithISetting(): void { ->method('bulkSend') ->willReturn([1 => 'affectedUser']); - // Notification is still sent because $notificationSetting defaults to null (not false) - // when ISetting is used (canChangeNotification not available), and null !== false - $this->notificationGenerator->expects($this->once()) + // ISetting is not ActivitySettings so $defaultPushEnabled is false — no notification sent + $this->notificationGenerator->expects($this->never()) ->method('sendNotificationForEvent'); $this->data->expects($this->never()) ->method('storeMail');