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

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>

[skip ci]

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

@@ -32,6 +32,7 @@
 use OCP\Activity\IEvent;
 use OCP\Activity\IManager;
 use OCP\Config\IUserConfig;
+use OCP\IAppConfig;
 use OCP\IDBConnection;
 use OCP\IL10N;
 use OCP\L10N\IFactory;
@@ -52,6 +53,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;
 
@@ -69,7 +71,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
@@ -94,6 +99,7 @@ protected function setUp(): void {
 			$this->userSettings,
 			$this->notificationGenerator,
 			$this->userConfig,
+			$this->appConfig,
 		);
 
 		$this->event = Server::get(IManager::class)->generateEvent();
@@ -142,6 +148,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')