Notifications: Add more tests

Signed-off-by: Julius Härtl <jus@bitgrid.net>

lib/Notification/NotificationHelper.php

@@ -25,8 +25,10 @@ namespace OCA\Deck\Notification;
 
 use DateTime;
 use OCA\Deck\Db\Acl;
+use OCA\Deck\Db\Board;
 use OCA\Deck\Db\BoardMapper;
 use OCA\Deck\Db\CardMapper;
+use OCA\Deck\Service\PermissionService;
 use OCP\IGroupManager;
 use OCP\IUser;
 use OCP\Notification\IManager;
@@ -37,6 +39,8 @@ class NotificationHelper {
 	protected $cardMapper;
 	/** @var BoardMapper */
 	protected $boardMapper;
+	/** @var PermissionService */
+	protected $permissionService;
 	/** @var IManager */
 	protected $notificationManager;
 	/** @var IGroupManager */
@@ -44,19 +48,19 @@ class NotificationHelper {
 	/** @var string */
 	protected $currentUser;
 	/** @var array */
-	private $users = [];
-	/** @var array */
 	private $boards = [];
 
 	public function __construct(
 		CardMapper $cardMapper,
 		BoardMapper $boardMapper,
+		PermissionService $permissionService,
 		IManager $notificationManager,
 		IGroupManager $groupManager,
 		$userId
 	) {
 		$this->cardMapper = $cardMapper;
 		$this->boardMapper = $boardMapper;
+		$this->permissionService = $permissionService;
 		$this->notificationManager = $notificationManager;
 		$this->groupManager = $groupManager;
 		$this->currentUser = $userId;
@@ -73,15 +77,16 @@ class NotificationHelper {
 
 		// TODO: Once assigning users is possible, those should be notified instead of all users of the board
 		$boardId = $this->cardMapper->findBoardId($card->getId());
+		$board = $this->getBoard($boardId);
 		/** @var IUser $user */
-		foreach ($this->getUsers($boardId) as $user) {
+		foreach ($this->permissionService->findUsers($boardId) as $user) {
 			$notification = $this->notificationManager->createNotification();
 			$notification
 				->setApp('deck')
-				->setUser($user)
+				->setUser($user->getUID())
 				->setObject('card', $card->getId())
 				->setSubject('card-overdue', [
-					$card->getTitle(), $this->boards[(string)$boardId]->getTitle()
+					$card->getTitle(), $board->getTitle()
 				])
 				->setDateTime(new DateTime($card->getDuedate()));
 			$this->notificationManager->notify($notification);
@@ -96,7 +101,7 @@ class NotificationHelper {
 	 * @param $acl
 	 */
 	public function sendBoardShared($boardId, $acl) {
-		$board = $this->boardMapper->find($boardId);
+		$board = $this->getBoard($boardId);
 		if ($acl->getType() === Acl::PERMISSION_TYPE_USER) {
 			$notification = $this->generateBoardShared($board, $acl->getParticipant());
 			$this->notificationManager->notify($notification);
@@ -110,6 +115,17 @@ class NotificationHelper {
 		}
 	}
 
+	/**
+	 * @param $boardId
+	 * @return Board
+	 */
+	private function getBoard($boardId) {
+		if (!array_key_exists($boardId, $this->boards)) {
+			$this->boards[$boardId] = $this->boardMapper->find($boardId);
+		}
+		return $this->boards[$boardId];
+	}
+
 	private function generateBoardShared($board, $userId) {
 		$notification = $this->notificationManager->createNotification();
 		$notification
@@ -121,34 +137,4 @@ class NotificationHelper {
 		return $notification;
 	}
 
-	/**
-	 * Get users that have access to a board
-	 *
-	 * @param $boardId
-	 * @return mixed
-	 */
-	private function getUsers($boardId) {
-		// cache users of a board so we don't query them for every cards
-		if (array_key_exists((string)$boardId, $this->users)) {
-			return $this->users[(string)$boardId];
-		}
-		$this->boards[(string)$boardId] = $this->boardMapper->find($boardId, false, true);
-		$users = [$this->boards[(string)$boardId]->getOwner()];
-		/** @var Acl $acl */
-		foreach ($this->boards[(string)$boardId]->getAcl() as $acl) {
-			if ($acl->getType() === Acl::PERMISSION_TYPE_USER) {
-				$users[] = $acl->getParticipant();
-			}
-			if ($acl->getType() === Acl::PERMISSION_TYPE_GROUP) {
-				$group = $this->groupManager->get($acl->getParticipant());
-				/** @var IUser $user */
-				foreach ($group->getUsers() as $user) {
-					$users[] = $user->getUID();
-				}
-			}
-		}
-		$this->users[(string)$boardId] = array_unique($users);
-		return $this->users[(string)$boardId];
-	}
-
 }

lib/Service/PermissionService.php

@@ -28,24 +28,43 @@ use OCA\Deck\Db\AclMapper;
 use OCA\Deck\Db\Board;
 use OCA\Deck\Db\BoardMapper;
 use OCA\Deck\Db\IPermissionMapper;
+use OCA\Deck\Db\User;
 use OCA\Deck\NoPermissionException;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\IGroupManager;
 use OCP\ILogger;
-
+use OCP\IUserManager;
 
 
 class PermissionService {
 
+	/** @var BoardMapper */
 	private $boardMapper;
+	/** @var AclMapper */
 	private $aclMapper;
+	/** @var ILogger */
 	private $logger;
+	/** @var IUserManager */
+	private $userManager;
+	/** @var IGroupManager */
+	private $groupManager;
+	/** @var string */
 	private $userId;
+	/** @var array */
+	private $users;
 
-	public function __construct(ILogger $logger, AclMapper $aclMapper, BoardMapper $boardMapper, IGroupManager $groupManager, $userId) {
+	public function __construct(
+		ILogger $logger,
+		AclMapper $aclMapper,
+		BoardMapper $boardMapper,
+		IUserManager $userManager,
+		IGroupManager $groupManager,
+		$userId
+	) {
 		$this->aclMapper = $aclMapper;
 		$this->boardMapper = $boardMapper;
 		$this->logger = $logger;
+		$this->userManager = $userManager;
 		$this->groupManager = $groupManager;
 		$this->userId = $userId;
 	}
@@ -159,4 +178,38 @@ class PermissionService {
 		}
 		return $hasGroupPermission;
 	}
+
+	/**
+	 * Find a list of all users (including the ones from groups)
+	 * Required to allow assigning them to cards
+	 *
+	 * @param $boardId
+	 * @return array|null
+	 */
+	public function findUsers($boardId) {
+		// cache users of a board so we don't query them for every cards
+		if (array_key_exists((string)$boardId, $this->users)) {
+			return $this->users[(string)$boardId];
+		}
+		$board = $this->boardMapper->find($boardId);
+		$users = [
+			new User($this->userManager->get($board->getOwner()))
+		];
+		$acls = $this->aclMapper->findAll($boardId);
+		/** @var Acl $acl */
+		foreach ($acls as $acl) {
+			if ($acl->getType() === Acl::PERMISSION_TYPE_USER) {
+				$user = $this->userManager->get($acl->getParticipant());
+				$users[] = new User($user);
+			}
+			if($acl->getType() === Acl::PERMISSION_TYPE_GROUP) {
+				$group = $this->groupManager->get($acl->getParticipant());
+				foreach ($group->getUsers() as $user) {
+					$users[] = new User($user);
+				}
+			}
+		}
+		$this->users[(string)$boardId] = array_unique($users);
+		return $this->users;
+	}
 }

tests/unit/Cron/ScheduledNoificationsTest.php

@@ -0,0 +1,63 @@
+<?php
+/**
+ * @copyright Copyright (c) 2017 Julius Härtl <jus@bitgrid.net>
+ *
+ * @author Julius Härtl <jus@bitgrid.net>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ *  This program is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU Affero General Public License as
+ *  published by the Free Software Foundation, either version 3 of the
+ *  License, or (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU Affero General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Affero General Public License
+ *  along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *  
+ */
+
+namespace OCA\Deck\Cron;
+
+use OCA\Deck\Db\Board;
+use OCA\Deck\Db\BoardMapper;
+use OCA\Deck\Db\Card;
+use OCA\Deck\Db\CardMapper;
+use OCA\Deck\Notification\NotificationHelper;
+
+class ScheduledNoificationsTest extends \Test\TestCase {
+
+	/** @var CardMapper|\PHPUnit_Framework_MockObject_MockObject */
+	protected $cardMapper;
+	/** @var NotificationHelper|\PHPUnit_Framework_MockObject_MockObject */
+	protected $notificationHelper;
+	/** @var ScheduledNotifications */
+	protected $scheduledNotifications;
+
+	public function setUp() {
+		parent::setUp();
+		$this->cardMapper = $this->createMock(CardMapper::class);
+		$this->notificationHelper = $this->createMock(NotificationHelper::class);
+		$this->scheduledNotifications = new ScheduledNotifications($this->cardMapper, $this->notificationHelper);
+	}
+
+	public function testDeleteCron() {
+		$c1 = new Card();
+		$c2 = new Card();
+		$cards = [$c1, $c2];
+		$this->cardMapper->expects($this->once())
+			->method('findOverdue')
+			->willReturn($cards);
+		$this->notificationHelper->expects($this->at(0))
+			->method('sendCardDuedate')
+			->with($c1);
+		$this->notificationHelper->expects($this->at(1))
+			->method('sendCardDuedate')
+			->with($c1);
+		$this->scheduledNotifications->run(null);
+	}
+}

tests/unit/Notification/NotificationHelperTest.php

@@ -0,0 +1,187 @@
+<?php
+/**
+ * @copyright Copyright (c) 2017 Julius Härtl <jus@bitgrid.net>
+ *
+ * @author Julius Härtl <jus@bitgrid.net>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ *  This program is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU Affero General Public License as
+ *  published by the Free Software Foundation, either version 3 of the
+ *  License, or (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU Affero General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Affero General Public License
+ *  along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCA\Deck\Notification;
+
+
+use OCA\Deck\Db\Board;
+use OCA\Deck\Db\BoardMapper;
+use OCA\Deck\Db\Card;
+use OCA\Deck\Db\CardMapper;
+use OCA\Deck\Db\User;
+use OCA\Deck\Service\PermissionService;
+use OCP\IGroupManager;
+use OCP\Notification\IManager;
+use OCP\Notification\INotification;
+
+class NotificationHelperTest extends \Test\TestCase {
+
+	/** @var CardMapper */
+	protected $cardMapper;
+	/** @var BoardMapper */
+	protected $boardMapper;
+	/** @var PermissionService */
+	protected $permissionService;
+	/** @var IManager */
+	protected $notificationManager;
+	/** @var IGroupManager */
+	protected $groupManager;
+	/** @var string */
+	protected $currentUser;
+	/** @var NotificationHelper */
+	protected $notificationHelper;
+
+	public function setUp() {
+		parent::setUp();
+		$this->cardMapper = $this->createMock(CardMapper::class);
+		$this->boardMapper = $this->createMock(BoardMapper::class);
+		$this->permissionService = $this->createMock(PermissionService::class);
+		$this->notificationManager = $this->createMock(IManager::class);
+		$this->groupManager = $this->createMock(IGroupManager::class);
+		$this->currentUser = 'admin';
+		$this->notificationHelper = new NotificationHelper(
+			$this->cardMapper,
+			$this->boardMapper,
+			$this->permissionService,
+			$this->notificationManager,
+			$this->groupManager,
+			$this->currentUser
+		);
+
+	}
+
+	public function testSendCardDuedateAlreadyNotified() {
+		$card = $this->createMock(Card::class);
+		$card->expects($this->once())
+			->method('__call')
+			->with('getNotified', [])
+			->willReturn(true);
+		$this->notificationHelper->sendCardDuedate($card);
+	}
+
+	private function createUserMock($uid) {
+		$user = $this->createMock(User::class);
+		$user->expects($this->any())
+			->method('getUID')
+			->willReturn($uid);
+		return $user;
+	}
+
+	public function testSendCardDuedate() {
+		$card = $this->createMock(Card::class);
+		$card->expects($this->at(0))
+			->method('__call')
+			->with('getNotified', [])
+			->willReturn(false);
+		$card->expects($this->at(1))
+			->method('__call')
+			->with('getId', [])
+			->willReturn(123);
+		for($i=0; $i<3; $i++) {
+			$card->expects($this->at($i*3+2))
+				->method('__call')
+				->with('getId', [])
+				->willReturn(123);
+			$card->expects($this->at($i*3+3))
+				->method('__call', [])
+				->with('getTitle')
+				->willReturn('MyCardTitle');
+		}
+		$this->cardMapper->expects($this->once())
+			->method('findBoardId')
+			->with(123)
+			->willReturn(234);
+		$board = $this->createMock(Board::class);
+		$board->expects($this->any())
+			->method('__call')
+			->with('getTitle', [])
+			->willReturn('MyBoardTitle');
+		$this->boardMapper->expects($this->once())
+			->method('find')
+			->with(234)
+			->willReturn($board);
+
+		$users = [
+			$this->createUserMock('foo'),
+			$this->createUserMock('bar'),
+			$this->createUserMock('asd')
+
+		];
+		$this->permissionService->expects($this->once())
+			->method('findUsers')
+			->with(234)
+			->willReturn($users);
+
+
+		$n1 = $this->createMock(INotification::class);
+		$n2 = $this->createMock(INotification::class);
+		$n3 = $this->createMock(INotification::class);
+
+		$n1->expects($this->once())->method('setApp')->with('deck')->willReturn($n1);
+		$n1->expects($this->once())->method('setUser')->with('foo')->willReturn($n1);
+		$n1->expects($this->once())->method('setObject')->with('card', 123)->willReturn($n1);
+		$n1->expects($this->once())->method('setSubject')->with('card-overdue', ['MyCardTitle', 'MyBoardTitle'])->willReturn($n1);
+		$n1->expects($this->once())->method('setDateTime')->willReturn($n1);
+
+		$n2->expects($this->once())->method('setApp')->with('deck')->willReturn($n2);
+		$n2->expects($this->once())->method('setUser')->with('bar')->willReturn($n2);
+		$n2->expects($this->once())->method('setObject')->with('card', 123)->willReturn($n2);
+		$n2->expects($this->once())->method('setSubject')->with('card-overdue', ['MyCardTitle', 'MyBoardTitle'])->willReturn($n2);
+		$n2->expects($this->once())->method('setDateTime')->willReturn($n2);
+
+		$n3->expects($this->once())->method('setApp')->with('deck')->willReturn($n3);
+		$n3->expects($this->once())->method('setUser')->with('asd')->willReturn($n3);
+		$n3->expects($this->once())->method('setObject')->with('card', 123)->willReturn($n3);
+		$n3->expects($this->once())->method('setSubject')->with('card-overdue', ['MyCardTitle', 'MyBoardTitle'])->willReturn($n3);
+		$n3->expects($this->once())->method('setDateTime')->willReturn($n3);
+
+		$this->notificationManager->expects($this->at(0))
+			->method('createNotification')
+			->willReturn($n1);
+		$this->notificationManager->expects($this->at(1))
+			->method('notify')
+			->with($n1);
+		$this->notificationManager->expects($this->at(2))
+			->method('createNotification')
+			->willReturn($n2);
+		$this->notificationManager->expects($this->at(3))
+			->method('notify')
+			->with($n2);
+		$this->notificationManager->expects($this->at(4))
+			->method('createNotification')
+			->willReturn($n3);
+		$this->notificationManager->expects($this->at(5))
+			->method('notify')
+			->with($n3);
+
+		$this->cardMapper->expects($this->once())
+			->method('markNotified')
+			->with($card);
+
+		$this->notificationHelper->sendCardDuedate($card);
+
+	}
+
+
+
+}

tests/unit/Service/PermissionServiceTest.php

@@ -32,17 +32,23 @@ use OCA\Deck\NoPermissionException;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\IGroupManager;
 use OCP\ILogger;
+use OCP\IUserManager;
 
 class PermissionServiceTest extends \PHPUnit_Framework_TestCase {
 
-    /** @var \PHPUnit_Framework_MockObject_MockObject|PermissionService */
+	/** @var PermissionService*/
 	private $service;
-    /** @var \PHPUnit_Framework_MockObject_MockObject|ILogger */
+    /** @var ILogger */
 	private $logger;
+	/** @var AclMapper */
 	private $aclMapper;
-    /** @var \PHPUnit_Framework_MockObject_MockObject|BoardMapper */
+    /** @var BoardMapper */
     private $boardMapper;
+    /** @var IUserManager */
+    private $userManager;
+    /** @var IGroupManager */
 	private $groupManager;
+	/** @var string */
 	private $userId = 'admin';
 
 	public function setUp() {
@@ -54,6 +60,7 @@ class PermissionServiceTest extends \PHPUnit_Framework_TestCase {
 			->disableOriginalConstructor()->getMock();
 		$this->boardMapper = $this->getMockBuilder(BoardMapper::class)
 			->disableOriginalConstructor()->getMock();
+		$this->userManager = $this->createMock(IUserManager::class);
 		$this->groupManager = $this->getMockBuilder(IGroupManager::class)
 			->disableOriginalConstructor()->getMock();
 
@@ -61,6 +68,7 @@ class PermissionServiceTest extends \PHPUnit_Framework_TestCase {
 			$this->logger,
 			$this->aclMapper,
 			$this->boardMapper,
+			$this->userManager,
 			$this->groupManager,
 			'admin'
 		);