refactor: extract email batch-time logic into UserSettings::getEmailBatchTimeSetting()

lib/Consumer.php

@@ -39,8 +39,7 @@ public function __construct(
 	public function receive(IEvent $event): void {
 		$selfAction = $event->getAffectedUser() === $event->getAuthor();
 		$notificationSetting = $this->userSettings->getUserSetting($event->getAffectedUser(), 'notification', $event->getType());
-		$emailSetting = $this->userSettings->getUserSetting($event->getAffectedUser(), 'email', $event->getType());
-		$emailSetting = ($emailSetting) ? $this->userSettings->getUserSetting($event->getAffectedUser(), 'setting', 'batchtime') : false;
+		$emailSetting = $this->userSettings->getEmailBatchTimeSetting($event->getAffectedUser(), $event->getType());
 
 		$activityId = $this->data->send($event);
 

lib/FilesHooks.php

@@ -711,7 +711,7 @@ protected function shareWithUser(string $shareWith, Node $fileSource, string $fi
 		$this->addNotificationsForUser(
 			$shareWith, 'shared_with_by', [[$fileSource->getId() => $fileTarget], $this->currentUser->getUserIdentifier()],
 			$fileSource->getId(), $fileTarget, $fileSource instanceof File,
-			$this->userSettings->getUserSetting($shareWith, 'email', Files_Sharing::TYPE_SHARED) ? $this->userSettings->getUserSetting($shareWith, 'setting', 'batchtime') : false,
+			$this->userSettings->getEmailBatchTimeSetting($shareWith, Files_Sharing::TYPE_SHARED),
 			(bool)$this->userSettings->getUserSetting($shareWith, 'notification', Files_Sharing::TYPE_SHARED),
 		);
 	}
@@ -760,7 +760,7 @@ protected function shareByLink(Node $fileSource, string $sharedBy) {
 		$this->addNotificationsForUser(
 			$sharedBy, 'shared_link_self', [[$fileSource->getId() => $relativePath]],
 			$fileSource->getId(), $relativePath, $fileSource instanceof File,
-			$this->userSettings->getUserSetting($sharedBy, 'email', Files_Sharing::TYPE_SHARED) ? $this->userSettings->getUserSetting($sharedBy, 'setting', 'batchtime') : false,
+			$this->userSettings->getEmailBatchTimeSetting($sharedBy, Files_Sharing::TYPE_SHARED),
 			(bool)$this->userSettings->getUserSetting($sharedBy, 'notification', Files_Sharing::TYPE_SHARED),
 		);
 	}
@@ -868,7 +868,7 @@ protected function unshareFromUser(IShare $share) {
 		$this->addNotificationsForUser(
 			$share->getSharedWith(), $actionUser, [[$share->getNodeId() => $share->getTarget()], $this->currentUser->getUserIdentifier()],
 			$share->getNodeId(), $share->getTarget(), $share->getNodeType() === 'file',
-			$this->userSettings->getUserSetting($share->getSharedWith(), 'email', Files_Sharing::TYPE_SHARED) ? $this->userSettings->getUserSetting($share->getSharedWith(), 'setting', 'batchtime') : false,
+			$this->userSettings->getEmailBatchTimeSetting($share->getSharedWith(), Files_Sharing::TYPE_SHARED),
 			(bool)$this->userSettings->getUserSetting($share->getSharedWith(), 'notification', Files_Sharing::TYPE_SHARED),
 		);
 	}
@@ -967,7 +967,7 @@ protected function unshareLink(IShare $share) {
 		$this->addNotificationsForUser(
 			$owner, $actionSharer, [[$share->getNodeId() => $share->getTarget()]],
 			$share->getNodeId(), $share->getTarget(), $share->getNodeType() === 'file',
-			$this->userSettings->getUserSetting($owner, 'email', Files_Sharing::TYPE_SHARED) ? $this->userSettings->getUserSetting($owner, 'setting', 'batchtime') : false,
+			$this->userSettings->getEmailBatchTimeSetting($owner, Files_Sharing::TYPE_SHARED),
 			(bool)$this->userSettings->getUserSetting($owner, 'notification', Files_Sharing::TYPE_SHARED),
 		);
 
@@ -976,7 +976,7 @@ protected function unshareLink(IShare $share) {
 			$this->addNotificationsForUser(
 				$owner, $actionOwner, [[$share->getNodeId() => $share->getTarget()], $share->getSharedBy()],
 				$share->getNodeId(), $share->getTarget(), $share->getNodeType() === 'file',
-				$this->userSettings->getUserSetting($owner, 'email', Files_Sharing::TYPE_SHARED) ? $this->userSettings->getUserSetting($owner, 'setting', 'batchtime') : false,
+				$this->userSettings->getEmailBatchTimeSetting($owner, Files_Sharing::TYPE_SHARED),
 				(bool)$this->userSettings->getUserSetting($owner, 'notification', Files_Sharing::TYPE_SHARED),
 			);
 		}
@@ -1070,7 +1070,7 @@ protected function shareNotificationForSharer(string $subject, string $shareWith
 		$this->addNotificationsForUser(
 			$sharer, $subject, [[$fileSource->getId() => $path], $shareWith],
 			$fileSource->getId(), $path, $fileSource instanceof File,
-			$this->userSettings->getUserSetting($sharer, 'email', Files_Sharing::TYPE_SHARED) ? $this->userSettings->getUserSetting($sharer, 'setting', 'batchtime') : false,
+			$this->userSettings->getEmailBatchTimeSetting($sharer, Files_Sharing::TYPE_SHARED),
 			(bool)$this->userSettings->getUserSetting($sharer, 'notification', Files_Sharing::TYPE_SHARED),
 		);
 	}
@@ -1082,7 +1082,7 @@ protected function reshareNotificationForSharer(string $owner, string $subject,
 		$this->addNotificationsForUser(
 			$owner, $subject, [[$sourceId => $sourcePath], $this->currentUser->getUserIdentifier(), $shareWith],
 			$sourceId, $sourcePath, $isFile,
-			$this->userSettings->getUserSetting($owner, 'email', Files_Sharing::TYPE_SHARED) ? $this->userSettings->getUserSetting($owner, 'setting', 'batchtime') : false,
+			$this->userSettings->getEmailBatchTimeSetting($owner, Files_Sharing::TYPE_SHARED),
 			(bool)$this->userSettings->getUserSetting($owner, 'notification', Files_Sharing::TYPE_SHARED),
 		);
 	}

lib/UserSettings.php

@@ -36,6 +36,17 @@ public function __construct(
 	) {
 	}
 
+	/**
+	 * Returns the batch time for email notifications if email is enabled for
+	 * the given user and activity type, or false if email is disabled.
+	 */
+	public function getEmailBatchTimeSetting(string $user, string $type): int|false {
+		if (!$this->getUserSetting($user, 'email', $type)) {
+			return false;
+		}
+		return (int)$this->getUserSetting($user, 'setting', 'batchtime');
+	}
+
 	/**
 	 * Get the user setting
 	 * Falling back to the admin default if not set for the user

tests/FilesHooksTest.php

@@ -582,15 +582,14 @@ public function testShareWithUser(string $itemType, string $fileTarget): void {
 			'shareNotificationForOriginalOwners',
 		]);
 
-		$this->settings->expects($this->exactly(3))
+		$this->settings->expects($this->once())
 			->method('getUserSetting')
-			->willReturnMap(
-				[
-					['recipient', 'notification', Files_Sharing::TYPE_SHARED, true],
-					['recipient', 'email', Files_Sharing::TYPE_SHARED, true],
-					['recipient', 'setting', 'batchtime', 42],
-				]
-			);
+			->with('recipient', 'notification', Files_Sharing::TYPE_SHARED)
+			->willReturn(true);
+		$this->settings->expects($this->once())
+			->method('getEmailBatchTimeSetting')
+			->with('recipient', Files_Sharing::TYPE_SHARED)
+			->willReturn(42);
 
 		$node = $this->getNodeMock(1337, 'path.txt', $isFile);
 		$filesHooks->expects($this->once())
@@ -767,13 +766,14 @@ public function testReshareNotificationForSharer(): void {
 			'addNotificationsForUser',
 		]);
 
-		$this->settings->expects($this->exactly(3))
+		$this->settings->expects($this->once())
 			->method('getUserSetting')
-			->willReturnMap([
-				['owner', 'notification', Files_Sharing::TYPE_SHARED, true],
-				['owner', 'email', Files_Sharing::TYPE_SHARED, true],
-				['owner', 'setting', 'batchtime', 21],
-			]);
+			->with('owner', 'notification', Files_Sharing::TYPE_SHARED)
+			->willReturn(true);
+		$this->settings->expects($this->once())
+			->method('getEmailBatchTimeSetting')
+			->with('owner', Files_Sharing::TYPE_SHARED)
+			->willReturn(21);
 
 		$filesHooks->expects($this->once())
 			->method('addNotificationsForUser')
@@ -799,13 +799,14 @@ public function testShare(): void {
 
 		$node = $this->getNodeMock(42, '/user/files/path');
 
-		$this->settings->expects($this->exactly(3))
+		$this->settings->expects($this->once())
 			->method('getUserSetting')
-			->willReturnMap([
-				['user', 'notification', Files_Sharing::TYPE_SHARED, true],
-				['user', 'email', Files_Sharing::TYPE_SHARED, true],
-				['user', 'setting', 'batchtime', 21],
-			]);
+			->with('user', 'notification', Files_Sharing::TYPE_SHARED)
+			->willReturn(true);
+		$this->settings->expects($this->once())
+			->method('getEmailBatchTimeSetting')
+			->with('user', Files_Sharing::TYPE_SHARED)
+			->willReturn(21);
 
 		$filesHooks->expects($this->once())
 			->method('addNotificationsForUser')
@@ -934,13 +935,14 @@ public function testShareNotificationForSharer(): void {
 
 		$node = $this->getNodeMock(42, '/user/files/path');
 
-		$this->settings->expects($this->exactly(3))
+		$this->settings->expects($this->once())
 			->method('getUserSetting')
-			->willReturnMap([
-				['user', 'notification', Files_Sharing::TYPE_SHARED, true],
-				['user', 'email', Files_Sharing::TYPE_SHARED, true],
-				['user', 'setting', 'batchtime', 21],
-			]);
+			->with('user', 'notification', Files_Sharing::TYPE_SHARED)
+			->willReturn(true);
+		$this->settings->expects($this->once())
+			->method('getEmailBatchTimeSetting')
+			->with('user', Files_Sharing::TYPE_SHARED)
+			->willReturn(21);
 
 		$filesHooks->expects($this->once())
 			->method('addNotificationsForUser')
@@ -1053,13 +1055,14 @@ public function testLeaveShare(): void {
 		$share->method('getShareType')
 			->willReturn(IShare::TYPE_USER);
 
-		$this->settings->expects($this->exactly(3))
+		$this->settings->expects($this->once())
 			->method('getUserSetting')
-			->willReturnMap([
-				['with', 'notification', Files_Sharing::TYPE_SHARED, true],
-				['with', 'email', Files_Sharing::TYPE_SHARED, true],
-				['with', 'setting', 'batchtime', 21],
-			]);
+			->with('with', 'notification', Files_Sharing::TYPE_SHARED)
+			->willReturn(true);
+		$this->settings->expects($this->once())
+			->method('getEmailBatchTimeSetting')
+			->with('with', Files_Sharing::TYPE_SHARED)
+			->willReturn(21);
 
 		$filesHooks->expects($this->once())
 			->method('addNotificationsForUser')

tests/UserSettingsTest.php

@@ -72,4 +72,31 @@ public function testGetDefaultSetting(string $method, string $type, $expected):
 		}
 		$this->assertEquals($expected, self::invokePrivate($this->userSettings, 'getDefaultSetting', [$method, $type]));
 	}
+
+	public function testGetEmailBatchTimeSettingEmailDisabled(): void {
+		$this->config->method('getAppValue')
+			->with('activity', 'enable_email', 'yes')
+			->willReturn('no');
+
+		$this->assertFalse($this->userSettings->getEmailBatchTimeSetting('user', 'some_type'));
+	}
+
+	public function testGetEmailBatchTimeSettingEmailEnabled(): void {
+		$this->config->method('getAppValue')
+			->willReturn('yes');
+		$this->config->method('getUserValue')
+			->willReturnCallback(function (string $user, string $app, string $key, $default) {
+				if (str_contains($key, 'batchtime')) {
+					return 7200;
+				}
+				return true;
+			});
+
+		$setting = $this->createMock(ActivitySettings::class);
+		$setting->method('isDefaultEnabledMail')->willReturn(true);
+		$setting->method('canChangeMail')->willReturn(true);
+		$this->activityManager->method('getSettingById')->willReturn($setting);
+
+		$this->assertSame(7200, $this->userSettings->getEmailBatchTimeSetting('user', 'some_type'));
+	}
 }