diff --git a/lib/Consumer.php b/lib/Consumer.php index 0aed62e67..71f9af5ad 100644 --- a/lib/Consumer.php +++ b/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); diff --git a/lib/FilesHooks.php b/lib/FilesHooks.php index dbcb85ae5..c36a62ba9 100644 --- a/lib/FilesHooks.php +++ b/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), ); } diff --git a/lib/UserSettings.php b/lib/UserSettings.php index 7eed94bf4..dfdd412d3 100644 --- a/lib/UserSettings.php +++ b/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 diff --git a/tests/FilesHooksTest.php b/tests/FilesHooksTest.php index a79eecd3e..d5193b90b 100644 --- a/tests/FilesHooksTest.php +++ b/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') diff --git a/tests/UserSettingsTest.php b/tests/UserSettingsTest.php index 4c0d35310..d95d24d26 100644 --- a/tests/UserSettingsTest.php +++ b/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')); + } }