fix tests

Signed-off-by: Robin Appelman <robin@icewind.nl>

lib/Consumer.php

@@ -74,7 +74,7 @@ class Consumer implements IConsumer {
 		if ($createStream) {
 			$activityId = $this->data->send($event);
 
-			if (!$selfAction && $notificationSetting) {
+			if (!$selfAction && $notificationSetting && $activityId) {
 				$this->notificationGenerator->sendNotificationForEvent($event, $activityId);
 			}
 		}

lib/Data.php

@@ -61,7 +61,7 @@ class Data {
 	 */
 	public function send(IEvent $event): int {
 		if ($event->getAffectedUser() === '' || $event->getAffectedUser() === null) {
-			return false;
+			return 0;
 		}
 
 		// store in DB

lib/FilesHooks.php

@@ -189,7 +189,7 @@ class FilesHooks {
 			return;
 		}
 
-		list($filePath, $uidOwner, $fileId) = $this->getSourcePathAndOwner($filePath);
+		[$filePath, $uidOwner, $fileId] = $this->getSourcePathAndOwner($filePath);
 		if ($fileId === 0) {
 			// Could not find the file for the owner ...
 			return;
@@ -324,7 +324,7 @@ class FilesHooks {
 			$this->moveCase = 'moveCross';
 		}
 
-		list($this->oldParentPath, $this->oldParentOwner, $this->oldParentId) = $this->getSourcePathAndOwner($oldDir);
+		[$this->oldParentPath, $this->oldParentOwner, $this->oldParentId] = $this->getSourcePathAndOwner($oldDir);
 		if ($this->oldParentId === 0) {
 			// Could not find the file for the owner ...
 			$this->moveCase = false;
@@ -372,8 +372,8 @@ class FilesHooks {
 		$fileName = basename($newPath);
 		$oldFileName = basename($oldPath);
 
-		list(, , $fileId) = $this->getSourcePathAndOwner($newPath);
-		list($parentPath, $parentOwner, $parentId) = $this->getSourcePathAndOwner($dirName);
+		[, , $fileId] = $this->getSourcePathAndOwner($newPath);
+		[$parentPath, $parentOwner, $parentId] = $this->getSourcePathAndOwner($dirName);
 		if ($fileId === 0 || $parentId === 0) {
 			// Could not find the file for the owner ...
 			return;
@@ -435,8 +435,8 @@ class FilesHooks {
 		$fileName = basename($newPath);
 		$oldFileName = basename($oldPath);
 
-		list(, , $fileId) = $this->getSourcePathAndOwner($newPath);
-		list($parentPath, $parentOwner, $parentId) = $this->getSourcePathAndOwner($dirName);
+		[, , $fileId] = $this->getSourcePathAndOwner($newPath);
+		[$parentPath, $parentOwner, $parentId] = $this->getSourcePathAndOwner($dirName);
 		if ($fileId === 0 || $parentId === 0) {
 			// Could not find the file for the owner ...
 			return;
@@ -665,7 +665,7 @@ class FilesHooks {
 
 		if ($owner === null || $owner !== $currentUser) {
 			/** @var \OCP\Files\Storage\IStorage $storage */
-			list($storage,) = $view->resolvePath($path);
+			[$storage,] = $view->resolvePath($path);
 
 			if ($owner !== null && !$storage->instanceOfStorage('OCA\Files_Sharing\External\Storage')) {
 				Filesystem::initMountPoints($owner);
@@ -673,7 +673,7 @@ class FilesHooks {
 				// Probably a remote user, let's try to at least generate activities
 				// for the current user
 				if ($currentUser === null) {
-					list(,$owner,) = explode('/', $view->getAbsolutePath($path), 3);
+					[,$owner,] = explode('/', $view->getAbsolutePath($path), 3);
 				} else {
 					$owner = $currentUser;
 				}
@@ -1182,7 +1182,10 @@ class FilesHooks {
 		// Add activity to stream
 		if ($streamSetting && (!$selfAction || $this->userSettings->getUserSetting($this->currentUser->getUID(), 'setting', 'self'))) {
 			$activityId = $this->activityData->send($event);
-			$this->notificationGenerator->sendNotificationForEvent($event, $activityId);
+
+			if ($activityId) {
+				$this->notificationGenerator->sendNotificationForEvent($event, $activityId);
+			}
 		}
 
 		// Add activity to mail queue

tests/ConsumerTest.php

@@ -24,6 +24,7 @@ declare(strict_types=1);
 namespace OCA\Activity\Tests;
 
 use OCA\Activity\Consumer;
+use OCA\Activity\NotificationGenerator;
 use OCP\Activity\IManager;
 use PHPUnit\Framework\MockObject\MockObject;
 use OCA\Activity\Data;
@@ -52,12 +53,17 @@ class ConsumerTest extends TestCase {
 	/** @var \OCA\Activity\UserSettings */
 	protected $userSettings;
 
+	/** @var NotificationGenerator|MockObject */
+	protected $notificationGenerator;
+
 	protected function setUp(): void {
 		parent::setUp();
 		$this->deleteTestActivities();
 
 		$this->activityManager = $this->createMock(IManager::class);
 		$this->data = $this->createMock(Data::class);
+		$this->data->method('send')
+			->willReturn(1);
 
 		$this->userSettings = $this->getMockBuilder(UserSettings::class)
 			->onlyMethods(array('getUserSetting'))
@@ -66,6 +72,8 @@ class ConsumerTest extends TestCase {
 
 		$l10n = $this->createMock(IL10N::class);
 
+		$this->notificationGenerator = $this->createMock(NotificationGenerator::class);
+
 		$this->l10nFactory = $this->createMock(IFactory::class);
 		$this->l10nFactory->expects($this->any())
 			->method('get')
@@ -76,8 +84,8 @@ class ConsumerTest extends TestCase {
 			->method('getUserSetting')
 			->with($this->stringContains('affectedUser'), $this->anything(), $this->anything())
 			->willReturnMap([
-				['affectedUser', 'stream', 'type', true],
-				['affectedUser2', 'stream', 'type', true],
+				['affectedUser', 'notification', 'type', true],
+				['affectedUser2', 'notification', 'type', true],
 				['affectedUser', 'setting', 'self', true],
 				['affectedUser2', 'setting', 'self', false],
 				['affectedUser', 'email', 'type', true],
@@ -137,7 +145,7 @@ class ConsumerTest extends TestCase {
 	 * @param array|false $expected
 	 */
 	public function testReceiveStream(string $type, string $author, string $affectedUser, string $subject, $expected): void {
-		$consumer = new Consumer($this->data, $this->activityManager, $this->userSettings, $this->l10nFactory);
+		$consumer = new Consumer($this->data, $this->activityManager, $this->userSettings, $this->notificationGenerator);
 		$event = \OC::$server->getActivityManager()->generateEvent();
 		$event->setApp('test')
 			->setType($type)
@@ -150,7 +158,7 @@ class ConsumerTest extends TestCase {
 			->setLink('link');
 		$this->deleteTestActivities();
 
-		if ($expected === false) {
+		if ($author === $affectedUser && $this->userSettings->getUserSetting($affectedUser, 'setting', 'self') === false) {
 			$this->data->expects($this->never())
 				->method('send');
 		} else {
@@ -172,7 +180,7 @@ class ConsumerTest extends TestCase {
 	 */
 	public function testReceiveEmail(string $type, string $author, string $affectedUser, string $subject, $expected): void {
 		$time = time();
-		$consumer = new Consumer($this->data, $this->activityManager, $this->userSettings, $this->l10nFactory);
+		$consumer = new Consumer($this->data, $this->activityManager, $this->userSettings, $this->notificationGenerator);
 		$event = \OC::$server->getActivityManager()->generateEvent();
 		$event->setApp('test')
 			->setType($type)
@@ -195,4 +203,38 @@ class ConsumerTest extends TestCase {
 
 		$consumer->receive($event);
 	}
+
+	/**
+	 * @dataProvider receiveData
+	 *
+	 * @param string $type
+	 * @param string $author
+	 * @param string $affectedUser
+	 * @param string $subject
+	 * @param array|false $expected
+	 */
+	public function testReceiveNotification(string $type, string $author, string $affectedUser, string $subject, $expected): void {
+		$consumer = new Consumer($this->data, $this->activityManager, $this->userSettings, $this->notificationGenerator);
+		$event = \OC::$server->getActivityManager()->generateEvent();
+		$event->setApp('test')
+			->setType($type)
+			->setAffectedUser($affectedUser)
+			->setAuthor($author)
+			->setTimestamp(time())
+			->setSubject($subject, ['subjectParam1', 'subjectParam2'])
+			->setMessage('message', ['messageParam1', 'messageParam2'])
+			->setObject('', 0 , 'file')
+			->setLink('link');
+		$this->deleteTestActivities();
+
+		if ($expected === false || $author === $affectedUser) {
+			$this->notificationGenerator->expects($this->never())
+				->method('sendNotificationForEvent');
+		} else {
+			$this->notificationGenerator->expects($this->once())
+				->method('sendNotificationForEvent');
+		}
+
+		$consumer->receive($event);
+	}
 }

tests/DataTest.php

@@ -56,12 +56,10 @@ class DataTest extends TestCase {
 
 		$this->activityLanguage = Util::getL10N('activity', 'en');
 		$this->activityManager = $this->createMock(IManager::class);
-		$this->session = $this->createMock(IUserSession::class);
 
 		$this->data = new Data(
 			$this->activityManager,
-			\OC::$server->getDatabaseConnection(),
-			$this->session
+			\OC::$server->getDatabaseConnection()
 		);
 	}
 
@@ -111,7 +109,7 @@ class DataTest extends TestCase {
 			$event->setAuthor($actionUser);
 		}
 
-		$this->assertSame($expectedActivity, $this->data->send($event) !== false);
+		$this->assertSame($expectedActivity, $this->data->send($event) !== 0);
 
 		$connection = \OC::$server->getDatabaseConnection();
 		$query = $connection->prepare('SELECT `user`, `affecteduser` FROM `*PREFIX*activity` WHERE `app` = ? ORDER BY `activity_id` DESC');

tests/FilesHooksTest.php

@@ -75,6 +75,8 @@ class FilesHooksTest extends TestCase {
 	protected $userMountCache;
 	/** @var IConfig|MockObject */
 	protected $config;
+	/** @var NotificationGenerator|MockObject */
+	protected $notificationGenerator;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -89,6 +91,7 @@ class FilesHooksTest extends TestCase {
 		$this->urlGenerator = $this->createMock(IURLGenerator::class);
 		$this->userMountCache = $this->createMock(IUserMountCache::class);
 		$this->config = $this->createMock(IConfig::class);
+		$this->notificationGenerator = $this->createMock(NotificationGenerator::class);
 
 		$this->filesHooks = $this->getFilesHooks();
 	}
@@ -125,6 +128,7 @@ class FilesHooksTest extends TestCase {
 					$currentUser,
 					$this->userMountCache,
 					$this->config,
+					$this->notificationGenerator
 				])
 				->onlyMethods($mockedMethods)
 				->getMock();
@@ -143,7 +147,8 @@ class FilesHooksTest extends TestCase {
 			$logger,
 			$currentUser,
 			$this->userMountCache,
-			$this->config
+			$this->config,
+			$this->notificationGenerator
 		);
 	}
 

tests/UserSettingsTest.php

@@ -23,6 +23,7 @@
 namespace OCA\Activity\Tests;
 
 use OCA\Activity\UserSettings;
+use OCP\Activity\ActivitySettings;
 use OCP\Activity\IManager;
 use OCP\Activity\ISetting;
 use OCP\IConfig;
@@ -46,7 +47,6 @@ class UserSettingsTest extends TestCase {
 
 	public function getDefaultSettingData(): array {
 		return [
-			['stream', 'type1', true],
 			['email', 'type1', false],
 			['setting', 'self', true],
 			['setting', 'selfemail', false],
@@ -70,10 +70,7 @@ class UserSettingsTest extends TestCase {
 					->with($type)
 					->willThrowException(new \InvalidArgumentException());
 			} else {
-				$s = $this->createMock(ISetting::class);
-				$s->expects($method === 'stream' ? $this->once() : $this->never())
-					->method('isDefaultEnabledStream')
-					->willReturn($expected);
+				$s = $this->createMock(ActivitySettings::class);
 				$s->expects($method === 'email' ? $this->once() : $this->never())
 					->method('isDefaultEnabledMail')
 					->willReturn($expected);