Add separate NotificationHelper to keep code together

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

lib/Cron/ScheduledNotifications.php

@@ -21,106 +21,35 @@
  *
  */
 
-/**
- * Created by PhpStorm.
- * User: jus
- * Date: 16.05.17
- * Time: 12:34
- */
-
 namespace OCA\Deck\Cron;
 
-use DateTime;
 use OC\BackgroundJob\Job;
-use OCA\Deck\Db\Acl;
-use OCA\Deck\Db\BoardMapper;
 use OCA\Deck\Db\Card;
 use OCA\Deck\Db\CardMapper;
-use OCP\IUser;
+use OCA\Deck\Notification\NotificationHelper;
-use OCP\Notification\IManager;
 
 class ScheduledNotifications extends Job {
 
 	/** @var CardMapper */
 	protected $cardMapper;
-	/** @var BoardMapper */
+	/** @var NotificationHelper */
-	protected $boardMapper;
+	protected $notificationHelper;
-	/** @var IManager */
-	protected $notificationManager;
-	/** @var array */
-	private $users = [];
-	/** @var array */
-	private $boards = [];
 
 	public function __construct(
 		CardMapper $cardMapper,
-		BoardMapper $boardMapper,
+		NotificationHelper $notificationHelper
-		IManager $notificationManager
 	) {
 		$this->cardMapper = $cardMapper;
-		$this->boardMapper = $boardMapper;
+		$this->notificationHelper = $notificationHelper;
-		$this->notificationManager = $notificationManager;
 	}
 
 	public function run($argument) {
 		// Notify board owner and card creator about overdue cards
-		// TODO: Once assigning users is possible, those should be notified instead of all users of the board
 		$cards = $this->cardMapper->findOverdue();
 		/** @var Card $card */
 		foreach ($cards as $card) {
-			// check if notification has already been sent
+			$this->notificationHelper->sendCardDuedate($card);
-			// ideally notifications should not be deleted once seen by the user so we can
-			// also deliver due date notifications for users who have been added later to a board
-			// this should maybe be addressed in nextcloud/server
-			if ($card->getNotified()) {
-				continue;
 		}
-			$boardId = $this->cardMapper->findBoardId($card->getId());
-			/** @var IUser $user */
-			foreach ($this->getUsers($boardId) as $user) {
-				$this->sendNotification($user, $card, $boardId);
-			}
-			$this->cardMapper->markNotified($card);
-		}
-	}
-
-	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 = \OC::$server->getGroupManager()->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];
-	}
-
-	private function sendNotification($user, $card, $boardId) {
-		$notification = $this->notificationManager->createNotification();
-		$notification
-			->setApp('deck')
-			->setUser($user)
-			->setObject('card', $card->getId())
-			->setSubject('card-overdue', [$card->getTitle(), $this->boards[(string)$boardId]->getTitle()]);
-		// this is only needed, if a notification exists for a user and the notified attribute is not set on the card
-		// if ($this->notificationManager->getCount($notification) > 0)
-		//	return;
-		$notification
-			->setDateTime(new DateTime($card->getDuedate()));
-		$this->notificationManager->notify($notification);
 	}
 
 }

lib/Notification/NotificationHelper.php

@@ -0,0 +1,153 @@
+<?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 DateTime;
+use OCA\Deck\Db\Acl;
+use OCA\Deck\Db\BoardMapper;
+use OCA\Deck\Db\CardMapper;
+use OCP\IGroupManager;
+use OCP\IUser;
+use OCP\Notification\IManager;
+
+class NotificationHelper {
+
+	/** @var CardMapper */
+	protected $cardMapper;
+	/** @var BoardMapper */
+	protected $boardMapper;
+	/** @var IManager */
+	protected $notificationManager;
+	/** @var IGroupManager */
+	protected $groupManager;
+	/** @var string */
+	protected $currentUser;
+	/** @var array */
+	private $users = [];
+	/** @var array */
+	private $boards = [];
+
+	public function __construct(
+		CardMapper $cardMapper,
+		BoardMapper $boardMapper,
+		IManager $notificationManager,
+		$userId
+	) {
+		$this->cardMapper = $cardMapper;
+		$this->boardMapper = $boardMapper;
+		$this->notificationManager = $notificationManager;
+		$this->currentUser = $userId;
+	}
+
+	public function sendCardDuedate($card) {
+		// check if notification has already been sent
+		// ideally notifications should not be deleted once seen by the user so we can
+		// also deliver due date notifications for users who have been added later to a board
+		// this should maybe be addressed in nextcloud/server
+		if ($card->getNotified()) {
+			return;
+		}
+
+		// TODO: Once assigning users is possible, those should be notified instead of all users of the board
+		$boardId = $this->cardMapper->findBoardId($card->getId());
+		/** @var IUser $user */
+		foreach ($this->getUsers($boardId) as $user) {
+			$notification = $this->notificationManager->createNotification();
+			$notification
+				->setApp('deck')
+				->setUser($user)
+				->setObject('card', $card->getId())
+				->setSubject('card-overdue', [
+					$card->getTitle(), $this->boards[(string)$boardId]->getTitle()
+				])
+				->setDateTime(new DateTime($card->getDuedate()));
+			$this->notificationManager->notify($notification);
+		}
+		$this->cardMapper->markNotified($card);
+	}
+
+	/**
+	 * Send notifications that a board was shared with a user/group
+	 *
+	 * @param $boardId
+	 * @param $acl
+	 */
+	public function sendBoardShared($boardId, $acl) {
+		$board = $this->boardMapper->find($boardId);
+		if ($acl->getType() === Acl::PERMISSION_TYPE_USER) {
+			$notification = $this->generateBoardShared($board, $acl->getParticipant());
+			$this->notificationManager->notify($notification);
+		}
+		if ($acl->getType() === Acl::PERMISSION_TYPE_GROUP) {
+			$this->groupManager = \OC::$server->getGroupManager();
+			$group = $this->groupManager->get($acl->getParticipant());
+			foreach ($group->getUsers() as $user) {
+				$notification = $this->generateBoardShared($board, $user->getUID());
+				$this->notificationManager->notify($notification);
+			}
+		}
+	}
+
+	private function generateBoardShared($board, $userId) {
+		$notification = $this->notificationManager->createNotification();
+		$notification
+			->setApp('deck')
+			->setUser($userId)
+			->setDateTime(new DateTime())
+			->setObject('board', $board->getId())
+			->setSubject('board-shared', [$board->getTitle(), $this->currentUser]);
+		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 = \OC::$server->getGroupManager()->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/BoardService.php

@@ -23,12 +23,11 @@
 
 namespace OCA\Deck\Service;
 
-use DateTime;
-use OCA\Deck\ArchivedItemException;
 use OCA\Deck\Db\Acl;
 use OCA\Deck\Db\AclMapper;
 use OCA\Deck\Db\IPermissionMapper;
 use OCA\Deck\Db\Label;
+use OCA\Deck\Notification\NotificationHelper;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\IL10N;
 use OCA\Deck\Db\Board;
@@ -43,15 +42,22 @@ class BoardService {
 	private $aclMapper;
 	private $l10n;
 	private $permissionService;
-	private $userId;
+	private $notificationHelper;
 
-	public function __construct(BoardMapper $boardMapper, IL10N $l10n, LabelMapper $labelMapper, AclMapper $aclMapper, PermissionService $permissionService, $userId) {
+	public function __construct(
+		BoardMapper $boardMapper,
+		IL10N $l10n,
+		LabelMapper $labelMapper,
+		AclMapper $aclMapper,
+		PermissionService $permissionService,
+		NotificationHelper $notificationHelper
+	) {
 		$this->boardMapper = $boardMapper;
 		$this->labelMapper = $labelMapper;
 		$this->aclMapper = $aclMapper;
 		$this->l10n = $l10n;
 		$this->permissionService = $permissionService;
-		$this->userId = $userId;
+		$this->notificationHelper = $notificationHelper;
 	}
 
 	public function findAll($userInfo) {
@@ -212,20 +218,10 @@ class BoardService {
 		$acl->setPermissionEdit($edit);
 		$acl->setPermissionShare($share);
 		$acl->setPermissionManage($manage);
+
 		/* Notify users about the shared board */
-		if($acl->getType() === Acl::PERMISSION_TYPE_USER) {
+		$this->notificationHelper->sendBoardShared($boardId, $acl);
-			$board = $this->boardMapper->find($boardId);
+
-			$initiator = \OC::$server->getUserManager()->get($this->userId);
-			$manager = \OC::$server->getNotificationManager();
-			$notification = $manager->createNotification();
-			$notification
-				->setApp('deck')
-				->setUser($participant['uid'])
-				->setDateTime(new DateTime())
-				->setObject('board', $boardId)// $type and $id
-				->setSubject('board-shared', [$board->getTitle(), $initiator->getUID()]);
-			$manager->notify($notification);
-		}
 		$newAcl = $this->aclMapper->insert($acl);
 		$this->boardMapper->mapAcl($newAcl);
 		return $newAcl;

tests/unit/Service/BoardServiceTest.php

@@ -29,13 +29,11 @@ use OCA\Deck\Db\AclMapper;
 use OCA\Deck\Db\Board;
 use OCA\Deck\Db\BoardMapper;
 use OCA\Deck\Db\LabelMapper;
-use OCA\Deck\Db\User;
+use OCA\Deck\Notification\NotificationHelper;
-use OCP\IGroupManager;
-use OCP\ILogger;
 use OCP\IUser;
-use OCP\IUserManager;
+use \Test\TestCase;
 
-class BoardServiceTest extends \Test\TestCase {
+class BoardServiceTest extends TestCase {
 
 	/** @var BoardService */
 	private $service;
@@ -49,6 +47,8 @@ class BoardServiceTest extends \Test\TestCase {
 	private $boardMapper;
 	/** @var PermissionService */
 	private $permissionService;
+	/** @var NotificationHelper */
+	private $notificationHelper;
 
 	private $userId = 'admin';
 
@@ -59,13 +59,15 @@ class BoardServiceTest extends \Test\TestCase {
 		$this->boardMapper = $this->createMock(BoardMapper::class);
 		$this->labelMapper = $this->createMock(LabelMapper::class);
 		$this->permissionService = $this->createMock(PermissionService::class);
+		$this->notificationHelper = $this->createMock(NotificationHelper::class);
 
 		$this->service = new BoardService(
 			$this->boardMapper,
 			$this->l10n,
 			$this->labelMapper,
 			$this->aclMapper,
-			$this->permissionService
+			$this->permissionService,
+			$this->notificationHelper
 		);
 
 		$user = $this->createMock(IUser::class);
@@ -165,6 +167,8 @@ class BoardServiceTest extends \Test\TestCase {
 		$acl->resolveRelation('participant', function($participant) use (&$user) {
 			return null;
 		});
+		$this->notificationHelper->expects($this->once())
+			->method('sendBoardShared');
 		$this->aclMapper->expects($this->once())
 			->method('insert')
 			->with($acl)