fix(bulkReceive): honour admin email toggle and ISetting notification defaults

The admin Enable notification emails toggle was not checked in
bulkReceive(), so emails could be queued even when the toggle was
disabled. Also, ISetting-based settings that do not implement
ActivitySettings had canChangeNotification() called on them, which
caused a fatal error; the default push-enabled state was also ignored.

Injects IAppConfig and uses getValueString() to check the toggle,
consistent with the approach used elsewhere. ISetting instances that are
not ActivitySettings now fall back to the setting default via
isDefaultEnabledNotification() without calling canChangeNotification().

Signed-off-by: Anna Larch <anna@nextcloud.com>

Author: miaulalala

lib/Consumer.php

@@ -16,6 +16,7 @@
 use OCP\Config\IUserConfig;
 use OCP\Config\ValueType;
 use OCP\DB\Exception;
+use OCP\IAppConfig;
 
 class Consumer implements IConsumer, IBulkConsumer {
 
@@ -25,6 +26,7 @@ public function __construct(
 		protected UserSettings $userSettings,
 		protected NotificationGenerator $notificationGenerator,
 		protected IUserConfig $userConfig,
+		protected IAppConfig $appConfig,
 	) {
 	}
 
@@ -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->appConfig->getValueString('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);
 			}
 

tests/ConsumerTest.php

@@ -33,6 +33,7 @@
 use OCP\Activity\IManager;
 use OCP\Activity\ISetting;
 use OCP\Config\IUserConfig;
+use OCP\IAppConfig;
 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 IAppConfig&MockObject $appConfig;
 	private IEvent $event;
 	private Consumer $consumer;
 
@@ -70,7 +72,10 @@ protected function setUp(): void {
 		$this->notificationGenerator = $this->createMock(NotificationGenerator::class);
 		$this->l10nFactory = $this->createMock(IFactory::class);
 		$this->userConfig = $this->createMock(IUserConfig::class);
-
+		$this->appConfig = $this->createMock(IAppConfig::class);
+		$this->appConfig->method('getValueString')
+			->with('activity', 'enable_email', 'yes')
+			->willReturn('yes');
 		$this->data->method('send')
 			->willReturn(1);
 		$this->l10nFactory
@@ -95,6 +100,7 @@ protected function setUp(): void {
 			$this->userSettings,
 			$this->notificationGenerator,
 			$this->userConfig,
+			$this->appConfig,
 		);
 
 		$this->event = Server::get(IManager::class)->generateEvent();
@@ -143,6 +149,7 @@ public function testReceiveStream(string $type, string $author, string $affected
 			$this->userSettings,
 			$this->notificationGenerator,
 			$this->userConfig,
+			$this->appConfig,
 		);
 		$event = Server::get(IManager::class)->generateEvent();
 		$event->setApp('test')
@@ -322,9 +329,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 +439,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 +450,50 @@ 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');
+
+		$appConfig = $this->createMock(IAppConfig::class);
+		$appConfig->method('getValueString')
+			->with('activity', 'enable_email', 'yes')
+			->willReturn('no');
+
+		$consumer = new Consumer(
+			$this->data,
+			$this->activityManager,
+			$this->userSettings,
+			$this->notificationGenerator,
+			$this->userConfig,
+			$appConfig,
+		);
+
+		$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 +512,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');