From 07132126a9f21e33673981af8e743630a42e31f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Mon, 18 Sep 2017 20:59:16 +0200 Subject: [PATCH] Add separate NotificationHelper to keep code together MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Cron/ScheduledNotifications.php | 83 +------------ lib/Notification/NotificationHelper.php | 153 ++++++++++++++++++++++++ lib/Service/BoardService.php | 32 +++-- tests/unit/Service/BoardServiceTest.php | 16 ++- 4 files changed, 183 insertions(+), 101 deletions(-) create mode 100644 lib/Notification/NotificationHelper.php diff --git a/lib/Cron/ScheduledNotifications.php b/lib/Cron/ScheduledNotifications.php index 048cb3baf..faa7660e4 100644 --- a/lib/Cron/ScheduledNotifications.php +++ b/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 OCP\Notification\IManager; +use OCA\Deck\Notification\NotificationHelper; class ScheduledNotifications extends Job { /** @var CardMapper */ protected $cardMapper; - /** @var BoardMapper */ - protected $boardMapper; - /** @var IManager */ - protected $notificationManager; - /** @var array */ - private $users = []; - /** @var array */ - private $boards = []; + /** @var NotificationHelper */ + protected $notificationHelper; public function __construct( CardMapper $cardMapper, - BoardMapper $boardMapper, - IManager $notificationManager + NotificationHelper $notificationHelper ) { $this->cardMapper = $cardMapper; - $this->boardMapper = $boardMapper; - $this->notificationManager = $notificationManager; + $this->notificationHelper = $notificationHelper; } 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 - // 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); + $this->notificationHelper->sendCardDuedate($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); - } - } \ No newline at end of file diff --git a/lib/Notification/NotificationHelper.php b/lib/Notification/NotificationHelper.php new file mode 100644 index 000000000..ea2cb0c72 --- /dev/null +++ b/lib/Notification/NotificationHelper.php @@ -0,0 +1,153 @@ + + * + * @author Julius Härtl + * + * @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 . + * + */ + +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]; + } + +} \ No newline at end of file diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index 8d5efdbf4..83786fba1 100644 --- a/lib/Service/BoardService.php +++ b/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) { - $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); - } + $this->notificationHelper->sendBoardShared($boardId, $acl); + $newAcl = $this->aclMapper->insert($acl); $this->boardMapper->mapAcl($newAcl); return $newAcl; diff --git a/tests/unit/Service/BoardServiceTest.php b/tests/unit/Service/BoardServiceTest.php index d8fb6080e..77ab541e3 100644 --- a/tests/unit/Service/BoardServiceTest.php +++ b/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 OCP\IGroupManager; -use OCP\ILogger; +use OCA\Deck\Notification\NotificationHelper; 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)