diff --git a/lib/Notification/NotificationHelper.php b/lib/Notification/NotificationHelper.php index 723923925..97b900c73 100644 --- a/lib/Notification/NotificationHelper.php +++ b/lib/Notification/NotificationHelper.php @@ -1,4 +1,7 @@ * @@ -24,19 +27,24 @@ namespace OCA\Deck\Notification; use DateTime; +use Exception; use OCA\Deck\AppInfo\Application; use OCA\Deck\Db\Acl; use OCA\Deck\Db\AssignmentMapper; 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\ConfigService; use OCA\Deck\Service\PermissionService; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\Comments\IComment; use OCP\IConfig; use OCP\IGroupManager; use OCP\Notification\IManager; +use OCP\Notification\INotification; class NotificationHelper { @@ -80,10 +88,10 @@ class NotificationHelper { } /** - * @param $card - * @throws \OCP\AppFramework\Db\DoesNotExistException + * @throws DoesNotExistException + * @throws Exception thrown on invalid due date */ - public function sendCardDuedate($card) { + public function sendCardDuedate(Card $card): void { // 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 @@ -117,7 +125,7 @@ class NotificationHelper { $notification ->setApp('deck') ->setUser((string)$user->getUID()) - ->setObject('card', $card->getId()) + ->setObject('card', (string)$card->getId()) ->setSubject('card-overdue', [ $card->getTitle(), $board->getTitle() ]) @@ -128,25 +136,29 @@ class NotificationHelper { $this->cardMapper->markNotified($card); } - public function markDuedateAsRead($card) { + public function markDuedateAsRead(Card $card): void { $notification = $this->notificationManager->createNotification(); $notification ->setApp('deck') - ->setObject('card', $card->getId()) + ->setObject('card', (string)$card->getId()) ->setSubject('card-overdue', []); $this->notificationManager->markProcessed($notification); } - public function sendCardAssigned($card, $userId) { + public function sendCardAssigned(Card $card, string $userId): void { $boardId = $this->cardMapper->findBoardId($card->getId()); - $board = $this->getBoard($boardId); + try { + $board = $this->getBoard($boardId); + } catch (Exception $e) { + return; + } $notification = $this->notificationManager->createNotification(); $notification ->setApp('deck') - ->setUser((string) $userId) + ->setUser($userId) ->setDateTime(new DateTime()) - ->setObject('card', $card->getId()) + ->setObject('card', (string)$card->getId()) ->setSubject('card-assigned', [ $card->getTitle(), $board->getTitle(), @@ -155,29 +167,56 @@ class NotificationHelper { $this->notificationManager->notify($notification); } + public function markCardAssignedAsRead(Card $card, string $userId): void { + $notification = $this->notificationManager->createNotification(); + $notification + ->setApp('deck') + ->setUser($userId) + ->setObject('card', (string)$card->getId()) + ->setSubject('card-assigned', []); + $this->notificationManager->markProcessed($notification); + } + /** * Send notifications that a board was shared with a user/group - * - * @param $boardId - * @param Acl $acl - * @throws \InvalidArgumentException */ - public function sendBoardShared($boardId, $acl) { - $board = $this->getBoard($boardId); + public function sendBoardShared(int $boardId, Acl $acl, bool $markAsRead = false): void { + try { + $board = $this->getBoard($boardId); + } catch (Exception $e) { + return; + } + if ($acl->getType() === Acl::PERMISSION_TYPE_USER) { $notification = $this->generateBoardShared($board, $acl->getParticipant()); - $this->notificationManager->notify($notification); + if ($markAsRead) { + $this->notificationManager->markProcessed($notification); + } else { + $notification->setDateTime(new DateTime()); + $this->notificationManager->notify($notification); + } } if ($acl->getType() === Acl::PERMISSION_TYPE_GROUP) { $group = $this->groupManager->get($acl->getParticipant()); + if ($group === null) { + return; + } foreach ($group->getUsers() as $user) { + if ($user->getUID() === $this->currentUser) { + continue; + } $notification = $this->generateBoardShared($board, $user->getUID()); - $this->notificationManager->notify($notification); + if ($markAsRead) { + $this->notificationManager->markProcessed($notification); + } else { + $notification->setDateTime(new DateTime()); + $this->notificationManager->notify($notification); + } } } } - public function sendMention(IComment $comment) { + public function sendMention(IComment $comment): void { foreach ($comment->getMentions() as $mention) { $card = $this->cardMapper->find($comment->getObjectId()); $boardId = $this->cardMapper->findBoardId($card->getId()); @@ -194,27 +233,22 @@ class NotificationHelper { } /** - * @param $boardId - * @return Board - * @throws \OCP\AppFramework\Db\DoesNotExistException + * @throws DoesNotExistException + * @throws MultipleObjectsReturnedException */ - private function getBoard($boardId, bool $withLabels = false, bool $withAcl = false) { + private function getBoard(int $boardId, bool $withLabels = false, bool $withAcl = false): Board { if (!array_key_exists($boardId, $this->boards)) { $this->boards[$boardId] = $this->boardMapper->find($boardId, $withLabels, $withAcl); } return $this->boards[$boardId]; } - - /** - * @param Board $board - */ - private function generateBoardShared($board, $userId) { + + private function generateBoardShared(Board $board, string $userId): INotification { $notification = $this->notificationManager->createNotification(); $notification ->setApp('deck') - ->setUser((string) $userId) - ->setDateTime(new DateTime()) - ->setObject('board', $board->getId()) + ->setUser($userId) + ->setObject('board', (string)$board->getId()) ->setSubject('board-shared', [$board->getTitle(), $this->currentUser]); return $notification; } diff --git a/lib/Service/AssignmentService.php b/lib/Service/AssignmentService.php index f78e383a9..96965f01b 100644 --- a/lib/Service/AssignmentService.php +++ b/lib/Service/AssignmentService.php @@ -74,6 +74,8 @@ class AssignmentService { * @var IEventDispatcher */ private $eventDispatcher; + /** @var string|null */ + private $currentUser; public function __construct( PermissionService $permissionService, @@ -138,8 +140,7 @@ class AssignmentService { } - if ($userId !== $this->currentUser) { - /* Notifyuser about the card assignment */ + if ($type === Assignment::TYPE_USER && $userId !== $this->currentUser) { $this->notificationHelper->sendCardAssigned($card, $userId); } @@ -183,8 +184,12 @@ class AssignmentService { $assignment = $this->assignedUsersMapper->delete($assignment); $card = $this->cardMapper->find($cardId); $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $card, ActivityManager::SUBJECT_CARD_USER_UNASSIGN, ['assigneduser' => $userId]); + if ($type === Assignment::TYPE_USER && $userId !== $this->currentUser) { + $this->notificationHelper->markCardAssignedAsRead($card, $userId); + } $this->changeHelper->cardChanged($cardId); + $this->eventDispatcher->dispatchTyped(new CardUpdatedEvent($card)); return $assignment; diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index 20bb259d9..afd45721c 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -510,11 +510,10 @@ class BoardService { $acl->setPermissionEdit($edit); $acl->setPermissionShare($share); $acl->setPermissionManage($manage); - - $this->notificationHelper->sendBoardShared($boardId, $acl); - $newAcl = $this->aclMapper->insert($acl); + $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_BOARD, $newAcl, ActivityManager::SUBJECT_BOARD_SHARE); + $this->notificationHelper->sendBoardShared((int)$boardId, $acl); $this->boardMapper->mapAcl($newAcl); $this->changeHelper->boardChanged($boardId); @@ -599,9 +598,8 @@ class BoardService { } } - $this->notificationHelper->sendBoardShared($acl->getBoardId(), $acl); - $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_BOARD, $acl, ActivityManager::SUBJECT_BOARD_UNSHARE); + $this->notificationHelper->sendBoardShared($acl->getBoardId(), $acl, true); $this->changeHelper->boardChanged($acl->getBoardId()); $version = \OCP\Util::getVersion()[0]; diff --git a/lib/Service/CardService.php b/lib/Service/CardService.php index abc6ba554..629185e21 100644 --- a/lib/Service/CardService.php +++ b/lib/Service/CardService.php @@ -243,6 +243,7 @@ class CardService { $this->cardMapper->update($card); $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $card, ActivityManager::SUBJECT_CARD_DELETE); + $this->notificationHelper->markDuedateAsRead($card); $this->changeHelper->cardChanged($card->getId(), false); $this->eventDispatcher->dispatchTyped(new CardDeletedEvent($card)); @@ -322,6 +323,15 @@ class CardService { $card->setOrder($order); $card->setOwner($owner); $card->setDuedate($duedate); + $resetDuedateNotification = false; + if ( + $card->getDuedate() === null || + (new \DateTime($card->getDuedate())) != (new \DateTime($changes->getBefore()->getDuedate())) + ) { + $card->setNotified(false); + $resetDuedateNotification = true; + } + if ($deletedAt !== null) { $card->setDeletedAt($deletedAt); } @@ -341,6 +351,9 @@ class CardService { $card = $this->cardMapper->update($card); + if ($resetDuedateNotification) { + $this->notificationHelper->markDuedateAsRead($card); + } $this->changeHelper->cardChanged($card->getId(), true); $this->eventDispatcher->dispatchTyped(new CardUpdatedEvent($card)); diff --git a/tests/unit/Notification/NotificationHelperTest.php b/tests/unit/Notification/NotificationHelperTest.php index e5b8951d0..0d5b2527c 100644 --- a/tests/unit/Notification/NotificationHelperTest.php +++ b/tests/unit/Notification/NotificationHelperTest.php @@ -130,7 +130,8 @@ class NotificationHelperTest extends \Test\TestCase { $card = Card::fromParams([ 'notified' => false, 'id' => 123, - 'title' => 'MyCardTitle' + 'title' => 'MyCardTitle', + 'duedate' => '2020-12-24' ]); $this->cardMapper->expects($this->once()) ->method('findBoardId') @@ -225,7 +226,8 @@ class NotificationHelperTest extends \Test\TestCase { $card = Card::fromParams([ 'notified' => false, 'id' => 123, - 'title' => 'MyCardTitle' + 'title' => 'MyCardTitle', + 'duedate' => '2020-12-24' ]); $card->setAssignedUsers([ new User($users[0]) @@ -323,7 +325,8 @@ class NotificationHelperTest extends \Test\TestCase { $card = Card::fromParams([ 'notified' => false, 'id' => 123, - 'title' => 'MyCardTitle' + 'title' => 'MyCardTitle', + 'duedate' => '2020-12-24' ]); $card->setAssignedUsers([ new User($users[0]) @@ -470,7 +473,7 @@ class NotificationHelperTest extends \Test\TestCase { ->with(123) ->willReturn($board); $user = $this->createMock(IUser::class); - $user->expects($this->once()) + $user->expects($this->any()) ->method('getUID') ->willReturn('userA'); $group = $this->createMock(IGroup::class);