Merge pull request #2993 from nextcloud/backport/2950/stable1.2

lib/Notification/NotificationHelper.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @copyright Copyright (c) 2017 Julius Härtl <jus@bitgrid.net>
  *
@@ -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;
 	}

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);
 		}
 
@@ -185,6 +186,9 @@ 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->dispatch(

lib/Service/BoardService.php

@@ -532,11 +532,10 @@ class BoardService {
 		$acl->setPermissionShare($share);
 		$acl->setPermissionManage($manage);
 
-		/* Notify users about the shared board */
-		$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, $newAcl);
 		$this->boardMapper->mapAcl($newAcl);
 		$this->changeHelper->boardChanged($boardId);
 
@@ -628,6 +627,7 @@ class BoardService {
 			}
 		}
 		$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];

lib/Service/CardService.php

@@ -240,6 +240,7 @@ class CardService {
 		$card->setDeletedAt(time());
 		$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->dispatch(
@@ -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->dispatch(

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);