Bump version to 1.4.4

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

.github/workflows/integration.yml

@@ -34,7 +34,7 @@ jobs:
           POSTGRES_DB: nextcloud
         options: --health-cmd pg_isready --health-interval 5s --health-timeout 2s --health-retries 5
       mysql:
-        image: mariadb
+        image: mariadb:10.5
         ports:
           - 4444:3306/tcp
         env:

.github/workflows/phpunit.yml

@@ -35,7 +35,7 @@ jobs:
           POSTGRES_DB: nextcloud
         options: --health-cmd pg_isready --health-interval 5s --health-timeout 2s --health-retries 5
       mysql:
-        image: mariadb
+        image: mariadb:10.5
         ports:
           - 4444:3306/tcp
         env:

CHANGELOG.md

@@ -1,6 +1,34 @@
 # Changelog
 All notable changes to this project will be documented in this file.
 
+## 1.4.4
+
+### Fixed
+
+- #3301 Fix print style issues
+- #3307 Return false instead of throwing when getting calendar setting
+- #3227 Additional circle level check
+- #3304 Delete file shares through attachments API
+
+## 1.4.3 - 2021-07-09
+
+### Fixed
+
+* [#3143](https://github.com/nextcloud/deck/pull/3143) Always pass user id in share provider
+* [#3153](https://github.com/nextcloud/deck/pull/3153) Only offer stack creation in emptycontent with proper permissions
+* [#3164](https://github.com/nextcloud/deck/pull/3164) Always log generic exceptions
+* [#3169](https://github.com/nextcloud/deck/pull/3169) Reduce duplicate queries when fetching user boards an permissions
+
+
+## 1.4.2 - 2021-05-03
+
+### Fixed
+
+* [#3030](https://github.com/nextcloud/deck/pull/3030) Proper error handling when fetching comments fails
+* [#3031](https://github.com/nextcloud/deck/pull/3031) Allow searching for filters without a query to match all that have a given filter set
+* [#3039](https://github.com/nextcloud/deck/pull/3039) Catch any error during circle detail fetching
+* [#3040](https://github.com/nextcloud/deck/pull/3040) Get attachment from the user node instead of the share source
+
 ## 1.4.1 - 2021-04-20
 
 ### Fixed

appinfo/info.xml

@@ -1,6 +1,5 @@
-<?xml version="1.0"?>
-<info xmlns:xsi= "http://www.w3.org/2001/XMLSchema-instance"
-      xsi:noNamespaceSchemaLocation="https://apps.nextcloud.com/schema/apps/info.xsd">
+<?xml version="1.0" encoding="utf-8"?>
+<info xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://apps.nextcloud.com/schema/apps/info.xsd">
     <id>deck</id>
     <name>Deck</name>
     <summary>Personal planning and team project organization</summary>
@@ -17,12 +16,12 @@
 - 🚀 Get your project organized
 
 </description>
-    <version>1.4.1</version>
+    <version>1.4.4</version>
     <licence>agpl</licence>
     <author>Julius Härtl</author>
     <namespace>Deck</namespace>
     <types>
-        <dav />
+        <dav/>
     </types>
     <category>organization</category>
     <category>office</category>
@@ -36,7 +35,7 @@
         <database min-version="9.4">pgsql</database>
         <database>sqlite</database>
         <database min-version="5.5">mysql</database>
-        <nextcloud min-version="21" max-version="22" />
+        <nextcloud min-version="21" max-version="21"/>
     </dependencies>
     <background-jobs>
         <job>OCA\Deck\Cron\DeleteCron</job>

css/icons.scss

@@ -22,7 +22,9 @@
 .icon-activity {
 	@include icon-color('activity-dark', 'activity', $color-black);
 }
-
+.icon-comment--unread {
+	@include icon-color('comment', 'actions', $color-primary, 1, true);
+}
 
 .avatardiv.circles {
 	background: var(--color-primary);

css/print.scss

@@ -2,21 +2,26 @@
 	/* hide stuff */
 	#body-user {
 		#header,
-		div#app-navigation,
-		div.board-header-controls,
+		.app-navigation,
+		.app-sidebar,
+		.board-header-controls,
+		.board-actions,
 		#app-navigation-toggle,
 		#app-navigation-toggle-custom,
 		div#controls.ng-scope div.crumb:not(.title),
 		div#controls.ng-scope div.crumb a.bullet,
 		a.ng-binding + a,
 		div.card.create,
+		.stack__header .action-item,
 		button.card-options {
 			display: none !important;
 		}
+
 		#content {
 			margin: 0;
 			padding: 0;
 		}
+
 		#app-content {
 			margin: 0 !important;
 		}
@@ -75,6 +80,11 @@
 		margin: 2cm;
 	}
 
+	.board {
+		max-height: none !important;
+		overflow: visible !important;
+	}
+
 	div#innerBoard {
 		display:flex;
 		flex-wrap: wrap;

docs/API.md

@@ -959,6 +959,7 @@ For now only `deck_file` is supported as an attachment type.
 
 ### DELETE /boards/{boardId}/stacks/{stackId}/cards/{cardId}/attachments/{attachmentId} - Delete an attachment
 
+    
 #### Request parameters
 
 | Parameter    | Type    | Description                                   |

lib/Db/BoardMapper.php

@@ -23,11 +23,12 @@
 
 namespace OCA\Deck\Db;
 
+use OC\Cache\CappedMemoryCache;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\IDBConnection;
-use OCP\ILogger;
 use OCP\IUserManager;
 use OCP\IGroupManager;
+use Psr\Log\LoggerInterface;
 
 class BoardMapper extends DeckMapper implements IPermissionMapper {
 	private $labelMapper;
@@ -35,16 +36,20 @@ class BoardMapper extends DeckMapper implements IPermissionMapper {
 	private $stackMapper;
 	private $userManager;
 	private $groupManager;
+	private $logger;
 
 	private $circlesEnabled;
 
+	private $userBoardCache;
+
 	public function __construct(
 		IDBConnection $db,
 		LabelMapper $labelMapper,
 		AclMapper $aclMapper,
 		StackMapper $stackMapper,
 		IUserManager $userManager,
-		IGroupManager $groupManager
+		IGroupManager $groupManager,
+		LoggerInterface $logger
 	) {
 		parent::__construct($db, 'deck_boards', Board::class);
 		$this->labelMapper = $labelMapper;
@@ -52,6 +57,10 @@ class BoardMapper extends DeckMapper implements IPermissionMapper {
 		$this->stackMapper = $stackMapper;
 		$this->userManager = $userManager;
 		$this->groupManager = $groupManager;
+		$this->logger = $logger;
+
+		$this->userBoardCache = new CappedMemoryCache();
+
 
 		$this->circlesEnabled = \OC::$server->getAppManager()->isEnabledForUser('circles');
 	}
@@ -86,13 +95,21 @@ class BoardMapper extends DeckMapper implements IPermissionMapper {
 	}
 
 	public function findAllForUser(string $userId, int $since = -1, $includeArchived = true): array {
-		$groups = $this->groupManager->getUserGroupIds(
-			$this->userManager->get($userId)
-		);
-		$userBoards = $this->findAllByUser($userId, null, null, $since, $includeArchived);
-		$groupBoards = $this->findAllByGroups($userId, $groups,null, null, $since, $includeArchived);
-		$circleBoards = $this->findAllByCircles($userId, null, null,  $since, $includeArchived);
-		return array_unique(array_merge($userBoards, $groupBoards, $circleBoards));
+		$useCache = ($since === -1 && $includeArchived === true);
+		if (!isset($this->userBoardCache[$userId]) || !$useCache) {
+			$groups = $this->groupManager->getUserGroupIds(
+				$this->userManager->get($userId)
+			);
+			$userBoards = $this->findAllByUser($userId, null, null, $since, $includeArchived);
+			$groupBoards = $this->findAllByGroups($userId, $groups, null, null, $since, $includeArchived);
+			$circleBoards = $this->findAllByCircles($userId, null, null, $since, $includeArchived);
+			$allBoards = array_unique(array_merge($userBoards, $groupBoards, $circleBoards));
+			if ($useCache) {
+				$this->userBoardCache[$userId] = $allBoards;
+			}
+			return $allBoards;
+		}
+		return $this->userBoardCache[$userId];
 	}
 
 	/**
@@ -248,7 +265,7 @@ class BoardMapper extends DeckMapper implements IPermissionMapper {
 				if ($user !== null) {
 					return new User($user);
 				}
-				\OC::$server->getLogger()->debug('User ' . $acl->getId() . ' not found when mapping acl ' . $acl->getParticipant());
+				$this->logger->debug('User ' . $acl->getId() . ' not found when mapping acl ' . $acl->getParticipant());
 				return null;
 			}
 			if ($acl->getType() === Acl::PERMISSION_TYPE_GROUP) {
@@ -256,7 +273,7 @@ class BoardMapper extends DeckMapper implements IPermissionMapper {
 				if ($group !== null) {
 					return new Group($group);
 				}
-				\OC::$server->getLogger()->debug('Group ' . $acl->getId() . ' not found when mapping acl ' . $acl->getParticipant());
+				$this->logger->debug('Group ' . $acl->getId() . ' not found when mapping acl ' . $acl->getParticipant());
 				return null;
 			}
 			if ($acl->getType() === Acl::PERMISSION_TYPE_CIRCLE) {
@@ -268,11 +285,12 @@ class BoardMapper extends DeckMapper implements IPermissionMapper {
 					if ($circle) {
 						return new Circle($circle);
 					}
-				} catch (\Exception $e) {
+				} catch (\Throwable $e) {
+					$this->logger->error('Failed to get circle details when building ACL', ['exception' => $e]);
 				}
 				return null;
 			}
-			\OC::$server->getLogger()->log(ILogger::WARN, 'Unknown permission type for mapping acl ' . $acl->getId());
+			$this->logger->warning('Unknown permission type for mapping acl ' . $acl->getId());
 			return null;
 		});
 	}

lib/Db/Card.php

@@ -49,6 +49,7 @@ class Card extends RelationalEntity {
 	protected $notified = false;
 	protected $deletedAt = 0;
 	protected $commentsUnread = 0;
+	protected $commentsCount = 0;
 	
 	protected $relatedStack = null;
 	protected $relatedBoard = null;
@@ -75,6 +76,7 @@ class Card extends RelationalEntity {
 		$this->addRelation('attachmentCount');
 		$this->addRelation('participants');
 		$this->addRelation('commentsUnread');
+		$this->addRelation('commentsCount');
 		$this->addResolvable('owner');
 		
 		$this->addRelation('relatedStack');

lib/Db/CardMapper.php

@@ -383,6 +383,10 @@ class CardMapper extends QBMapper implements IPermissionMapper {
 		foreach ($query->getDuedate() as $duedate) {
 			$dueDateColumn = $this->databaseType === 'sqlite3' ? $qb->createFunction('DATETIME(`c`.`duedate`)') : 'c.duedate';
 			$date = $duedate->getValue();
+			if ($date === "") {
+				$qb->andWhere($qb->expr()->isNotNull('c.duedate'));
+				continue;
+			}
 			$supportedFilters = ['overdue', 'today', 'week', 'month', 'none'];
 			if (in_array($date, $supportedFilters, true)) {
 				$currentDate = new DateTime();
@@ -430,6 +434,10 @@ class CardMapper extends QBMapper implements IPermissionMapper {
 			foreach ($query->getAssigned() as $index => $assignment) {
 				$qb->innerJoin('c', 'deck_assigned_users', 'au' . $index, $qb->expr()->eq('c.id', 'au' . $index . '.card_id'));
 				$assignedQueryValue = $assignment->getValue();
+				if ($assignedQueryValue === "") {
+					$qb->andWhere($qb->expr()->isNotNull('au' . $index . '.participant'));
+					continue;
+				}
 				$searchUsers = $this->userManager->searchDisplayName($assignment->getValue());
 				$users = array_filter($searchUsers, function (IUser $user) use ($assignedQueryValue) {
 					return (mb_strtolower($user->getDisplayName()) === mb_strtolower($assignedQueryValue) || $user->getUID() === $assignedQueryValue);

lib/Middleware/ExceptionMiddleware.php

@@ -98,9 +98,7 @@ class ExceptionMiddleware extends Middleware {
 				'status' => 500,
 				'message' => $exception->getMessage()
 			];
-			if ($this->config->getSystemValue('loglevel', Util::WARN) === Util::DEBUG) {
-				$this->logger->logException($exception);
-			}
+			$this->logger->logException($exception);
 			if ($this->config->getSystemValue('debug', true) === true) {
 				$response['exception'] = (array) $exception;
 			}

lib/Search/Query/AQueryParameter.php

@@ -37,7 +37,7 @@ class AQueryParameter {
 	
 	public function getValue() {
 		if (is_string($this->value) && mb_strlen($this->value) > 1) {
-			$param = ($this->value[0] === '"' && $this->value[mb_strlen($this->value) - 1] === '"') ? mb_substr($this->value, 1, -1): $this->value;
+			$param = (mb_substr($this->value, 0, 1) === '"' && mb_substr($this->value, -1, 1) === '"') ? mb_substr($this->value, 1, -1): $this->value;
 			return $param;
 		}
 		return $this->value;

lib/Service/AttachmentService.php

@@ -35,6 +35,7 @@ use OCA\Deck\InvalidAttachmentType;
 use OCA\Deck\NoPermissionException;
 use OCA\Deck\NotFoundException;
 use OCA\Deck\StatusException;
+use OCP\AppFramework\Db\IMapperException;
 use OCP\AppFramework\Http\Response;
 use OCP\ICache;
 use OCP\ICacheFactory;
@@ -320,14 +321,10 @@ class AttachmentService {
 	 * Either mark an attachment as deleted for later removal or just remove it depending
 	 * on the IAttachmentService implementation
 	 *
-	 * @param $attachmentId
-	 * @return \OCP\AppFramework\Db\Entity
-	 * @throws \OCA\Deck\NoPermissionException
-	 * @throws \OCP\AppFramework\Db\DoesNotExistException
-	 * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
-	 * @throws BadRequestException
+	 * @throws NoPermissionException
+	 * @throws NotFoundException
 	 */
-	public function delete($cardId, $attachmentId, $type = 'deck_file') {
+	public function delete(int $cardId, int $attachmentId, string $type = 'deck_file'): Attachment {
 		try {
 			$service = $this->getService($type);
 		} catch (InvalidAttachmentType $e) {
@@ -340,40 +337,32 @@ class AttachmentService {
 			$attachment->setType($type);
 			$attachment->setCardId($cardId);
 			$service->extendData($attachment);
-			$service->delete($attachment);
-			$this->changeHelper->cardChanged($attachment->getCardId());
-			$this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $attachment, ActivityManager::SUBJECT_ATTACHMENT_DELETE);
-			return $attachment;
+		} else {
+			try {
+				$attachment = $this->attachmentMapper->find($attachmentId);
+			} catch (IMapperException $e) {
+				throw new NoPermissionException('Permission denied');
+			}
 		}
-
-		try {
-			$attachment = $this->attachmentMapper->find($attachmentId);
-		} catch (\Exception $e) {
-			throw new NoPermissionException('Permission denied');
-		}
-
 		$this->permissionService->checkPermission($this->cardMapper, $attachment->getCardId(), Acl::PERMISSION_EDIT);
-		$this->cache->clear('card-' . $attachment->getCardId());
 
 		if ($service->allowUndo()) {
 			$service->markAsDeleted($attachment);
-			$this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $attachment, ActivityManager::SUBJECT_ATTACHMENT_DELETE);
-			$this->changeHelper->cardChanged($attachment->getCardId());
-			return $this->attachmentMapper->update($attachment);
+			$attachment = $this->attachmentMapper->update($attachment);
+		} else {
+			$service->delete($attachment);
+			if (!$service instanceof ICustomAttachmentService) {
+				$attachment = $this->attachmentMapper->delete($attachment);
+			}
 		}
-		$service->delete($attachment);
 
-		$attachment = $this->attachmentMapper->delete($attachment);
+		$this->cache->clear('card-' . $attachment->getCardId());
 		$this->changeHelper->cardChanged($attachment->getCardId());
 		$this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $attachment, ActivityManager::SUBJECT_ATTACHMENT_DELETE);
 		return $attachment;
 	}
 
-	public function restore($cardId, $attachmentId, $type = 'deck_file') {
-		if (is_numeric($attachmentId) === false) {
-			throw new BadRequestException('attachment id must be a number');
-		}
-
+	public function restore(int $cardId, int $attachmentId, string $type = 'deck_file'): Attachment {
 		try {
 			$attachment = $this->attachmentMapper->find($attachmentId);
 		} catch (\Exception $e) {

lib/Service/CardService.php

@@ -105,8 +105,10 @@ class CardService {
 		$card->setAttachmentCount($this->attachmentService->count($cardId));
 		$user = $this->userManager->get($this->currentUser);
 		$lastRead = $this->commentsManager->getReadMark('deckCard', (string)$card->getId(), $user);
-		$count = $this->commentsManager->getNumberOfCommentsForObject('deckCard', (string)$card->getId(), $lastRead);
-		$card->setCommentsUnread($count);
+		$countUnreadComments = $this->commentsManager->getNumberOfCommentsForObject('deckCard', (string)$card->getId(), $lastRead);
+		$countComments = $this->commentsManager->getNumberOfCommentsForObject('deckCard', (string)$card->getId());
+		$card->setCommentsUnread($countUnreadComments);
+		$card->setCommentsCount($countComments);
 		
 		$stack = $this->stackMapper->find($card->getStackId());
 		$board = $this->boardService->find($stack->getBoardId());

lib/Service/CirclesService.php

@@ -26,6 +26,7 @@ declare(strict_types=1);
 
 namespace OCA\Deck\Service;
 
+use OCA\Circles\Api\v1\Circles;
 use OCP\App\IAppManager;
 
 /**
@@ -53,8 +54,8 @@ class CirclesService {
 		}
 
 		try {
-			\OCA\Circles\Api\v1\Circles::getMember($circleId, $userId, 1, true);
-			return true;
+			$member = \OCA\Circles\Api\v1\Circles::getMember($circleId, $userId, 1, true);
+			return $member->getLevel() >= Circles::LEVEL_MEMBER;
 		} catch (\Exception $e) {
 		}
 		return false;

lib/Service/ConfigService.php

@@ -73,16 +73,20 @@ class ConfigService {
 				if (!$this->groupManager->isAdmin($this->userId)) {
 					throw new NoPermissionException('You must be admin to get the group limit');
 				}
-				$result = $this->getGroupLimit();
-				break;
+				return $this->getGroupLimit();
 			case 'calendar':
-				$result = (bool)$this->config->getUserValue($this->userId, Application::APP_ID, 'calendar', true);
-				break;
+				if ($this->userId === null) {
+					return false;
+				}
+				return (bool)$this->config->getUserValue($this->userId, Application::APP_ID, 'calendar', true);
 		}
-		return $result;
 	}
 
 	public function isCalendarEnabled(int $boardId = null): bool {
+		if ($this->userId === null) {
+			return false;
+		}
+
 		$defaultState = (bool)$this->config->getUserValue($this->userId, Application::APP_ID, 'calendar', true);
 		if ($boardId === null) {
 			return $defaultState;

lib/Service/FilesAppService.php

@@ -23,7 +23,10 @@
 
 namespace OCA\Deck\Service;
 
+use OCA\Deck\Db\Acl;
 use OCA\Deck\Db\Attachment;
+use OCA\Deck\Db\CardMapper;
+use OCA\Deck\NoPermissionException;
 use OCA\Deck\Sharing\DeckShareProvider;
 use OCA\Deck\StatusException;
 use OCP\AppFramework\Http\StreamResponse;
@@ -38,6 +41,7 @@ use OCP\IRequest;
 use OCP\Share\Exceptions\ShareNotFound;
 use OCP\Share\IManager;
 use OCP\Share\IShare;
+use Psr\Log\LoggerInterface;
 
 class FilesAppService implements IAttachmentService, ICustomAttachmentService {
 	private $request;
@@ -48,8 +52,10 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService {
 	private $configService;
 	private $l10n;
 	private $preview;
-	private $permissionService;
 	private $mimeTypeDetector;
+	private $permissionService;
+	private $cardMapper;
+	private $logger;
 
 	public function __construct(
 		IRequest $request,
@@ -59,8 +65,10 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService {
 		ConfigService $configService,
 		DeckShareProvider $shareProvider,
 		IPreview $preview,
-		PermissionService $permissionService,
 		IMimeTypeDetector $mimeTypeDetector,
+		PermissionService $permissionService,
+		CardMapper  $cardMapper,
+		LoggerInterface $logger,
 		string $userId = null
 	) {
 		$this->request = $request;
@@ -72,15 +80,20 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService {
 		$this->userId = $userId;
 		$this->preview = $preview;
 		$this->mimeTypeDetector = $mimeTypeDetector;
+		$this->permissionService = $permissionService;
+		$this->cardMapper = $cardMapper;
+		$this->logger = $logger;
 	}
 
 	public function listAttachments(int $cardId): array {
 		$shares = $this->shareProvider->getSharedWithByType($cardId, IShare::TYPE_DECK, -1, 0);
-		$shares = array_filter($shares, function ($share) {
-			return $share->getPermissions() > 0;
-		});
-		return array_map(function (IShare $share) use ($cardId) {
-			$file = $share->getNode();
+		return array_filter(array_map(function (IShare $share) use ($cardId) {
+			try {
+				$file = $share->getNode();
+			} catch (NotFoundException $e) {
+				$this->logger->debug('Unable to find node for share with ID ' . $share->getId());
+				return null;
+			}
 			$attachment = new Attachment();
 			$attachment->setType('file');
 			$attachment->setId((int)$share->getId());
@@ -89,9 +102,9 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService {
 			$attachment->setData($file->getName());
 			$attachment->setLastModified($file->getMTime());
 			$attachment->setCreatedAt($share->getShareTime()->getTimestamp());
-			$attachment->setDeletedAt(0);
+			$attachment->setDeletedAt($share->getPermissions() === 0 ? $share->getShareTime()->getTimestamp() : 0);
 			return $attachment;
-		}, $shares);
+		}, $shares));
 	}
 
 	public function getAttachmentCount(int $cardId): int {
@@ -125,7 +138,11 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService {
 	public function extendData(Attachment $attachment) {
 		$userFolder = $this->rootFolder->getUserFolder($this->userId);
 		$share = $this->shareProvider->getShareById($attachment->getId());
-		$file = $share->getNode();
+		$files = $userFolder->getById($share->getNode()->getId());
+		if (count($files) === 0) {
+			return $attachment;
+		}
+		$file = array_shift($files);
 		$attachment->setExtendedData([
 			'path' => $userFolder->getRelativePath($file->getPath()),
 			'fileid' => $file->getId(),
@@ -140,6 +157,7 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService {
 	}
 
 	public function display(Attachment $attachment) {
+		// Problem: Folders
 		/** @psalm-suppress InvalidCatch */
 		try {
 			$share = $this->shareProvider->getShareById($attachment->getId());
@@ -161,6 +179,9 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService {
 		$file = $this->getUploadedFile();
 		$fileName = $file['name'];
 
+		// get shares for current card
+		// check if similar filename already exists
+
 		$userFolder = $this->rootFolder->getUserFolder($this->userId);
 		try {
 			$folder = $userFolder->get($this->configService->getAttachmentFolder());
@@ -241,12 +262,16 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService {
 		$file = $share->getNode();
 		$attachment->setData($file->getName());
 
-		if ($file->getOwner() !== null && $file->getOwner()->getUID() === $this->userId) {
-			$file->delete();
+		// Deleting a Nextcloud file attachment will remove the share to the card, keeping the source file untouched
+		// Opt-out of individual shares per user is no longer performed within deck but can still be done through the files app
+		$canEdit = $this->permissionService->checkPermission($this->cardMapper, $attachment->getCardId(), Acl::PERMISSION_EDIT);
+		$isFileOwner = $file->getOwner() !== null && $file->getOwner()->getUID() === $this->userId;
+		if ($isFileOwner || $canEdit) {
+			$this->shareManager->deleteShare($share);
 			return;
 		}
 
-		$this->shareManager->deleteFromSelf($share, $this->userId);
+		throw new NoPermissionException('No permission to remove the attachment from the card');
 	}
 
 	public function allowUndo() {

lib/Service/PermissionService.php

@@ -23,6 +23,8 @@
 
 namespace OCA\Deck\Service;
 
+use OC\Cache\CappedMemoryCache;
+use OCA\Circles\Model\Member;
 use OCA\Deck\Db\Acl;
 use OCA\Deck\Db\AclMapper;
 use OCA\Deck\Db\Board;
@@ -31,7 +33,6 @@ use OCA\Deck\Db\IPermissionMapper;
 use OCA\Deck\Db\User;
 use OCA\Deck\NoPermissionException;
 use OCP\AppFramework\Db\DoesNotExistException;
-use OCP\AppFramework\Db\Entity;
 use OCP\AppFramework\Db\MultipleObjectsReturnedException;
 use OCP\IConfig;
 use OCP\IGroupManager;
@@ -61,6 +62,7 @@ class PermissionService {
 	private $users = [];
 
 	private $circlesEnabled = false;
+	private $boardCache;
 
 	public function __construct(
 		ILogger $logger,
@@ -81,6 +83,8 @@ class PermissionService {
 		$this->config = $config;
 		$this->userId = $userId;
 
+		$this->boardCache = new CappedMemoryCache();
+
 		$this->circlesEnabled = \OC::$server->getAppManager()->isEnabledForUser('circles') &&
 			(version_compare(\OC::$server->getAppManager()->getAppVersion('circles'), '0.17.1') >= 0);
 	}
@@ -149,10 +153,13 @@ class PermissionService {
 			return true;
 		}
 
-		$acls = $this->aclMapper->findAll($boardId);
-		$result = $this->userCan($acls, $permission, $userId);
-		if ($result) {
-			return true;
+		try {
+			$acls = $this->getBoard($boardId)->getAcl();
+			$result = $this->userCan($acls, $permission, $userId);
+			if ($result) {
+				return true;
+			}
+		} catch (DoesNotExistException | MultipleObjectsReturnedException $e) {
 		}
 
 		// Throw NoPermission to not leak information about existing entries
@@ -168,13 +175,24 @@ class PermissionService {
 			$userId = $this->userId;
 		}
 		try {
-			$board = $this->boardMapper->find($boardId);
-			return $board && $userId === $board->getOwner();
+			$board = $this->getBoard($boardId);
+			return $userId === $board->getOwner();
 		} catch (DoesNotExistException | MultipleObjectsReturnedException $e) {
 		}
 		return false;
 	}
 
+	/**
+	 * @throws MultipleObjectsReturnedException
+	 * @throws DoesNotExistException
+	 */
+	private function getBoard($boardId): Board {
+		if (!isset($this->boardCache[$boardId])) {
+			$this->boardCache[$boardId] = $this->boardMapper->find($boardId, false, true);
+		}
+		return $this->boardCache[$boardId];
+	}
+
 	/**
 	 * Check if permission matches the acl rules for current user and groups
 	 *
@@ -194,8 +212,8 @@ class PermissionService {
 
 			if ($this->circlesEnabled && $acl->getType() === Acl::PERMISSION_TYPE_CIRCLE) {
 				try {
-					\OCA\Circles\Api\v1\Circles::getMember($acl->getParticipant(), $this->userId, 1, true);
-					return $acl->getPermission($permission);
+					$member = \OCA\Circles\Api\v1\Circles::getMember($acl->getParticipant(), $this->userId, 1, true);
+					return $member->getLevel() >= Member::LEVEL_MEMBER && $acl->getPermission($permission);
 				} catch (\Exception $e) {
 					$this->logger->info('Member not found in circle that was accessed. This should not happen.');
 				}

lib/Sharing/DeckShareProvider.php

@@ -271,9 +271,9 @@ class DeckShareProvider implements \OCP\Share\IShareProvider {
 		return $share;
 	}
 
-	private function applyBoardPermission($share, $permissions) {
+	private function applyBoardPermission($share, $permissions, $userId) {
 		try {
-			$this->permissionService->checkPermission($this->cardMapper, $share->getSharedWith(), Acl::PERMISSION_EDIT);
+			$this->permissionService->checkPermission($this->cardMapper, $share->getSharedWith(), Acl::PERMISSION_EDIT, $userId);
 		} catch (NoPermissionException $e) {
 			$permissions &= Constants::PERMISSION_ALL - Constants::PERMISSION_UPDATE;
 			$permissions &= Constants::PERMISSION_ALL - Constants::PERMISSION_CREATE;
@@ -281,7 +281,7 @@ class DeckShareProvider implements \OCP\Share\IShareProvider {
 		}
 
 		try {
-			$this->permissionService->checkPermission($this->cardMapper, $share->getSharedWith(), Acl::PERMISSION_SHARE);
+			$this->permissionService->checkPermission($this->cardMapper, $share->getSharedWith(), Acl::PERMISSION_SHARE, $userId);
 		} catch (NoPermissionException $e) {
 			$permissions &= Constants::PERMISSION_ALL - Constants::PERMISSION_SHARE;
 		}
@@ -646,7 +646,7 @@ class DeckShareProvider implements \OCP\Share\IShareProvider {
 			$stmt = $query->execute();
 
 			while ($data = $stmt->fetch()) {
-				$this->applyBoardPermission($shareMap[$data['parent']], (int)$data['permissions']);
+				$this->applyBoardPermission($shareMap[$data['parent']], (int)$data['permissions'], $userId);
 				$shareMap[$data['parent']]->setTarget($data['file_target']);
 			}
 

package.json

@@ -1,7 +1,7 @@
 {
 	"name": "deck",
 	"description": "",
-	"version": "1.0.0",
+	"version": "1.4.4",
 	"authors": [
 		{
 			"name": "Julius Härtl",
@@ -129,4 +129,4 @@
 			"<rootDir>/node_modules/jest-serializer-vue"
 		]
 	}
-}
+}

src/components/board/Board.vue

@@ -32,7 +32,7 @@
 			</div>
 			<EmptyContent v-else-if="isEmpty" key="empty" icon="icon-deck">
 				{{ t('deck', 'No lists available') }}
-				<template #desc>
+				<template v-if="canManage" #desc>
 					{{ t('deck', 'Create a new list to add cards to this board') }}
 					<form @submit.prevent="addNewStack()">
 						<input id="new-stack-input-main"
@@ -110,6 +110,7 @@ export default {
 		}),
 		...mapGetters([
 			'canEdit',
+			'canManage',
 		]),
 		stacksByBoard() {
 			return this.$store.getters.stacksByBoard(this.board.id)

src/components/card/AttachmentList.vue

@@ -22,7 +22,7 @@
 
 <template>
 	<AttachmentDragAndDrop :card-id="cardId" class="drop-upload--sidebar">
-		<div class="button-group">
+		<div class="button-group" v-if="!isReadOnly">
 			<button class="icon-upload" @click="uploadNewFile()">
 				{{ t('deck', 'Upload new files') }}
 			</button>
@@ -49,18 +49,25 @@
 			</li>
 			<li v-for="attachment in attachments"
 				:key="attachment.id"
-				class="attachment">
+				class="attachment"
+				:class="{ 'attachment--deleted': attachment.deletedAt > 0 }">
 				<a class="fileicon"
+					:href="internalLink(attachment)"
 					:style="mimetypeForAttachment(attachment)"
 					@click.prevent="showViewer(attachment)" />
 				<div class="details">
-					<a @click.prevent="showViewer(attachment)">
+					<a :href="internalLink(attachment)" @click.prevent="showViewer(attachment)">
 						<div class="filename">
 							<span class="basename">{{ attachment.data }}</span>
 						</div>
-						<span class="filesize">{{ formattedFileSize(attachment.extendedData.filesize) }}</span>
-						<span class="filedate">{{ relativeDate(attachment.createdAt*1000) }}</span>
-						<span class="filedate">{{ attachment.createdBy }}</span>
+						<div v-if="attachment.deletedAt === 0">
+							<span class="filesize">{{ formattedFileSize(attachment.extendedData.filesize) }}</span>
+							<span class="filedate">{{ relativeDate(attachment.createdAt*1000) }}</span>
+							<span class="filedate">{{ attachment.createdBy }}</span>
+						</div>
+						<div v-else>
+							<span class="attachment--info">{{ t('deck', 'Pending share') }}</span>
+						</div>
 					</a>
 				</div>
 				<Actions v-if="selectable">
@@ -68,12 +75,12 @@
 						{{ t('deck', 'Add this attachment') }}
 					</ActionButton>
 				</Actions>
-				<Actions v-if="removable" :force-menu="true">
+				<Actions v-if="removable && !isReadOnly" :force-menu="true">
 					<ActionLink v-if="attachment.extendedData.fileid" icon="icon-folder" :href="internalLink(attachment)">
 						{{ t('deck', 'Show in Files') }}
 					</ActionLink>
-					<ActionButton v-if="attachment.extendedData.fileid" icon="icon-delete" @click="unshareAttachment(attachment)">
-						{{ t('deck', 'Unshare file') }}
+					<ActionButton v-if="attachment.extendedData.fileid && !isReadOnly" icon="icon-delete" @click="unshareAttachment(attachment)">
+						{{ t('deck', 'Remove attachment') }}
 					</ActionButton>
 
 					<ActionButton v-if="!attachment.extendedData.fileid && attachment.deletedAt === 0" icon="icon-delete" @click="$emit('deleteAttachment', attachment)">
@@ -143,6 +150,7 @@ export default {
 	},
 	computed: {
 		attachments() {
+			// FIXME sort propertly by last modified / deleted at
 			return [...this.$store.getters.attachmentsByCard(this.cardId)].filter(attachment => attachment.deletedAt >= 0).sort((a, b) => b.id - a.id)
 		},
 		mimetypeForAttachment() {
@@ -320,9 +328,10 @@ export default {
 					opacity: 0.7;
 				}
 			}
+			.attachment--info,
 			.filesize, .filedate {
 				font-size: 90%;
-				color: darkgray;
+				color: var(--color-text-maxcontrast);
 			}
 			.app-popover-menu-utils {
 				position: relative;

src/components/card/CardSidebarTabComments.vue

@@ -7,7 +7,11 @@
 			</span>
 		</div>
 
-		<CommentItem v-if="replyTo" :comment="replyTo" :reply="true" />
+		<CommentItem v-if="replyTo"
+			:comment="replyTo"
+			:reply="true"
+			:preview="true"
+			@cancel="cancelReply" />
 		<CommentForm v-model="newComment" @submit="createComment" />
 
 		<ul v-if="getCommentsForCard(card.id).length > 0" id="commentsFeed">
@@ -23,8 +27,8 @@
 		</ul>
 		<div v-else-if="isLoading" class="icon icon-loading" />
 		<div v-else class="emptycontent">
-			<div class="icon-comment" />
-			<p>{{ t('deck', 'No comments yet. Begin the discussion!') }}</p>
+			<div :class="{ 'icon-comment': !error, 'icon-error': error }" />
+			<p>{{ error || t('deck', 'No comments yet. Begin the discussion!') }}</p>
 		</div>
 	</div>
 </template>
@@ -36,6 +40,7 @@ import CommentItem from './CommentItem'
 import CommentForm from './CommentForm'
 import InfiniteLoading from 'vue-infinite-loading'
 import { getCurrentUser } from '@nextcloud/auth'
+
 export default {
 	name: 'CardSidebarTabComments',
 	components: {
@@ -60,6 +65,7 @@ export default {
 			newComment: '',
 			isLoading: false,
 			currentUser: getCurrentUser(),
+			error: null,
 		}
 	},
 	computed: {
@@ -85,19 +91,34 @@ export default {
 	},
 	methods: {
 		async infiniteHandler($state) {
-			await this.loadMore()
-			if (this.hasMoreComments(this.card.id)) {
-				$state.loaded()
-			} else {
+			this.error = null
+			try {
+				await this.loadMore()
+				if (this.hasMoreComments(this.card.id)) {
+					$state.loaded()
+				} else {
+					$state.complete()
+				}
+			} catch (e) {
+				console.error('Failed to fetch more comments during infinite loading', e)
+				this.error = t('deck', 'Failed to load comments')
 				$state.complete()
 			}
 		},
 		async loadComments() {
+			this.$store.dispatch('setReplyTo', null)
+			this.error = null
 			this.isLoading = true
-			await this.$store.dispatch('fetchComments', { cardId: this.card.id })
-			this.isLoading = false
-			if (this.card.commentsUnread > 0) {
-				await this.$store.dispatch('markCommentsAsRead', this.card.id)
+			try {
+				await this.$store.dispatch('fetchComments', { cardId: this.card.id })
+				this.isLoading = false
+				if (this.card.commentsUnread > 0) {
+					await this.$store.dispatch('markCommentsAsRead', this.card.id)
+				}
+			} catch (e) {
+				this.isLoading = false
+				console.error('Failed to fetch more comments during infinite loading', e)
+				this.error = t('deck', 'Failed to load comments')
 			}
 		},
 		async createComment(content) {
@@ -115,6 +136,9 @@ export default {
 			await this.$store.dispatch('fetchMore', { cardId: this.card.id })
 			this.isLoading = false
 		},
+		cancelReply() {
+			this.$store.dispatch('setReplyTo', null)
+		},
 	},
 }
 </script>

src/components/card/CommentForm.vue

@@ -41,6 +41,7 @@
 					</span>
 				</template>
 				<div ref="contentEditable"
+					class="comment-form__contenteditable"
 					contenteditable
 					@keydown.enter="handleKeydown"
 					@paste="onPaste"
@@ -175,6 +176,11 @@ export default {
 <style scoped lang="scss">
 	@import '../../css/comments';
 
+	.comment-form__contenteditable {
+		word-break: break-word;
+		border-radius: var(--border-radius-large)
+	}
+
 	.atwho-wrap {
 		width: 100%;
 		& > div[contenteditable] {

src/components/card/CommentItem.vue

@@ -1,10 +1,22 @@
 <template>
-	<div v-if="reply" class="reply">
-		<span class="reply--hint">{{ t('deck', 'In reply to') }} <UserBubble :user="comment.actorId" :display-name="comment.actorDisplayName" /></span>
-		<RichText class="comment--content"
-			:text="richText(comment)"
-			:arguments="richArgs(comment)"
-			:autolink="true" />
+	<div v-if="reply" class="reply" :class="{ 'reply--preview': preview }">
+		<div class="reply--wrapper">
+			<div class="reply--header">
+				<div class="reply--hint">
+					{{ t('deck', 'In reply to') }}
+					<UserBubble :user="comment.actorId" :display-name="comment.actorDisplayName" />
+				</div>
+				<Actions v-if="preview" class="reply--cancel">
+					<ActionButton icon="icon-close" @click="$emit('cancel')">
+						{{ t('deck', 'Cancel reply') }}
+					</ActionButton>
+				</Actions>
+			</div>
+			<RichText class="comment--content"
+				:text="richText(comment)"
+				:arguments="richArgs(comment)"
+				:autolink="true" />
+		</div>
 	</div>
 	<li v-else class="comment">
 		<template>
@@ -14,13 +26,19 @@
 					{{ comment.actorDisplayName }}
 				</span>
 				<Actions v-show="!edit" :force-menu="true">
-					<ActionButton icon="icon-reply" @click="replyTo()">
+					<ActionButton icon="icon-reply" :close-after-click="true" @click="replyTo()">
 						{{ t('deck', 'Reply') }}
 					</ActionButton>
-					<ActionButton v-if="canEdit" icon="icon-rename" @click="showUpdateForm()">
+					<ActionButton v-if="canEdit"
+						icon="icon-rename"
+						:close-after-click="true"
+						@click="showUpdateForm()">
 						{{ t('deck', 'Update') }}
 					</ActionButton>
-					<ActionButton v-if="canEdit" icon="icon-delete" @click="deleteComment()">
+					<ActionButton v-if="canEdit"
+						icon="icon-delete"
+						:close-after-click="true"
+						@click="deleteComment()">
 						{{ t('deck', 'Delete') }}
 					</ActionButton>
 				</Actions>
@@ -86,6 +104,10 @@ export default {
 			type: Boolean,
 			default: false,
 		},
+		preview: {
+			type: Boolean,
+			default: false,
+		},
 	},
 	data() {
 		return {
@@ -175,20 +197,41 @@ export default {
 	@import '../../css/comments';
 
 	.reply {
-		border-left: 3px solid var(--color-primary-element);
-		padding-left: 5px;
-		margin-left: 2px;
-		margin-bottom: 5px;
+		margin: 0 0 0 44px;
+
+		&.reply--preview {
+			margin: 4px 0;
+			padding: 8px;
+			background-color: var(--color-background-hover);
+			border-radius: var(--border-radius-large);
+
+			.reply--wrapper {
+				margin: 8px;
+			}
+
+			.reply--cancel {
+				margin-right: -12px;
+				margin-top: -12px;
+			}
+		}
+
+		.reply--wrapper {
+			border-left: 4px solid var(--color-border-dark);
+			padding-left: 8px;
+		}
 
 		&::v-deep .rich-text--wrapper {
 			margin-top: -3px;
-			color: var(--color-text-light);
+			color: var(--color-text-lighter);
+		}
+
+		.reply--header {
+			display: flex;
 		}
 
 		.reply--hint {
-			font-size: 0.9em;
 			color: var(--color-text-lighter);
-			vertical-align: top;
+			flex-grow: 1;
 		}
 
 		.comment--content {

src/components/cards/CardBadges.vue

@@ -22,7 +22,13 @@
 
 <template>
 	<div v-if="card" class="badges">
-		<div v-if="card.commentsUnread > 0" class="icon icon-comment" />
+		<div v-if="card.commentsCount > 0"
+			v-tooltip="commentsHint"
+			class="icon icon-comment"
+			:class="{ 'icon-comment--unread': card.commentsUnread > 0 }"
+			@click.stop="openComments">
+			{{ card.commentsCount }}
+		</div>
 
 		<div v-if="card.description && checkListCount > 0" class="card-tasks icon icon-checkmark">
 			{{ checkListCheckedCount }}/{{ checkListCount }}
@@ -58,6 +64,21 @@ export default {
 		checkListCheckedCount() {
 			return (this.card.description.match(/^\s*([*+-]|(\d\.))\s+\[\s*x\s*\](.*)$/gim) || []).length
 		},
+		commentsHint() {
+			if (this.card.commentsUnread > 0) {
+				return t('deck', '{count} comments, {unread} unread', {
+					count: this.card.commentsCount,
+					unread: this.card.commentsUnread
+				})
+			}
+			return null
+		},
+	},
+	methods: {
+		openComments() {
+			const boardId = this.card && this.card.boardId ? this.card.boardId : this.$route.params.id
+			this.$router.push({ name: 'card', params: { id: boardId, cardId: this.card.id, tabId: 'comments' } })
+		},
 	},
 }
 </script>
@@ -70,7 +91,7 @@ export default {
 
 		.icon {
 			opacity: 0.5;
-			padding: 12px 18px;
+			padding: 10px 20px;
 			padding-right: 4px;
 			margin-right: 5px;
 			background-position: left;
@@ -78,8 +99,8 @@ export default {
 			span {
 				margin-left: 18px;
 			}
-			&.icon-edit {
-				opacity: 0.5;
+			&.icon-comment--unread {
+				opacity: 1;
 			}
 		}
 	}

src/components/search/GlobalSearchResults.vue

@@ -22,7 +22,10 @@
 
 <template>
 	<div v-if="searchQuery!==''" class="global-search">
-		<h2><RichText :text="t('deck', 'Search for {searchQuery} in all boards')" :arguments="queryStringArgs" /></h2>
+		<h2>
+			<RichText :text="t('deck', 'Search for {searchQuery} in all boards')" :arguments="queryStringArgs" />
+			<div v-if="loading" class="icon-loading-small" />
+		</h2>
 		<Actions>
 			<ActionButton icon="icon-close" @click="$store.commit('setSearchQuery', '')" />
 		</Actions>
@@ -107,23 +110,38 @@ export default {
 		},
 	},
 	watch: {
-		searchQuery() {
+		async searchQuery() {
 			this.cursor = null
 			this.loading = true
-			this.search()
+			try {
+				await this.search()
+				this.loading = false
+			} catch (e) {
+				if (!axios.isCancel(e)) {
+					console.error('Search request failed', e)
+					this.loading = false
+				}
+			}
 		},
 	},
 	methods: {
-		infiniteHandler($state) {
+		async infiniteHandler($state) {
 			this.loading = true
-			this.search().then((data) => {
+			try {
+				const data = await this.search()
 				if (data.length) {
 					$state.loaded()
 				} else {
 					$state.complete()
 				}
 				this.loading = false
-			})
+			} catch (e) {
+				if (!axios.isCancel(e)) {
+					console.error('Search request failed', e)
+					$state.complete()
+					this.loading = false
+				}
+			}
 		},
 		async search() {
 			if (this.cancel) {
@@ -177,6 +195,13 @@ export default {
 		padding: 10px;
 	}
 
+	h2 > div {
+		display: inline-block;
+
+		&.icon-loading-small {
+			margin-right: 20px;
+		}
+	}
 	h2::v-deep span {
 		background-color: var(--color-background-dark);
 		padding: 3px;

src/css/comments.scss

@@ -48,4 +48,5 @@
 
 .comment--content {
 	margin-left: 44px;
+	word-break: break-word;
 }

src/store/card.js

@@ -92,24 +92,39 @@ export default {
 
 					const filterOutQuotes = (q) => {
 						if (q[0] === '"' && q[q.length - 1] === '"') {
-							return q.substr(1, -1)
+							return q.substr(1, q.length - 2)
 						}
 						return q
 					}
 					for (const match of matches) {
-						let [filter, query] = match.indexOf(':') !== -1 ? match.split(/:(.+)/) : [null, match]
+						let [filter, query] = match.indexOf(':') !== -1 ? match.split(/:(.*)/) : [null, match]
+						const isEmptyQuery = typeof query === 'undefined' || filterOutQuotes(query) === ''
 
 						if (filter === 'title') {
+							if (isEmptyQuery) {
+								continue
+							}
 							hasMatch = hasMatch && card.title.toLowerCase().includes(filterOutQuotes(query).toLowerCase())
 						} else if (filter === 'description') {
+							if (isEmptyQuery) {
+								hasMatch = hasMatch && !!card.description
+								continue
+							}
 							hasMatch = hasMatch && card.description.toLowerCase().includes(filterOutQuotes(query).toLowerCase())
 						} else if (filter === 'list') {
-							const stack = this.getters.stackById(card.stackId)
+							if (isEmptyQuery) {
+								continue
+							}
+							const stack = getters.stackById(card.stackId)
 							if (!stack) {
 								return false
 							}
 							hasMatch = hasMatch && stack.title.toLowerCase().includes(filterOutQuotes(query).toLowerCase())
 						} else if (filter === 'tag') {
+							if (isEmptyQuery) {
+								hasMatch = hasMatch && card.labels.length > 0
+								continue
+							}
 							hasMatch = hasMatch && card.labels.findIndex((label) => label.title.toLowerCase().includes(filterOutQuotes(query).toLowerCase())) !== -1
 						} else if (filter === 'date') {
 							const datediffHour = ((new Date(card.duedate) - new Date()) / 3600 / 1000)
@@ -158,6 +173,10 @@ export default {
 							}
 
 						} else if (filter === 'assigned') {
+							if (isEmptyQuery) {
+								hasMatch = hasMatch && card.assignedUsers.length > 0
+								continue
+							}
 							hasMatch = hasMatch && card.assignedUsers.findIndex((assignment) => {
 								return assignment.participant.primaryKey.toLowerCase() === filterOutQuotes(query).toLowerCase()
 									|| assignment.participant.displayname.toLowerCase() === filterOutQuotes(query).toLowerCase()

tests/psalm-baseline.xml

@@ -105,7 +105,8 @@
     <ParamNameMismatch occurrences="1">
       <code>$boardId</code>
     </ParamNameMismatch>
-    <UndefinedClass occurrences="1">
+    <UndefinedClass occurrences="2">
+      <code>\OCA\Circles\Api\v1\Circles</code>
       <code>\OCA\Circles\Api\v1\Circles</code>
     </UndefinedClass>
   </file>
@@ -170,11 +171,6 @@
       <code>$stackId</code>
     </ParamNameMismatch>
   </file>
-  <file src="lib/Notification/NotificationHelper.php">
-    <InvalidScalarArgument occurrences="1">
-      <code>$board-&gt;getId()</code>
-    </InvalidScalarArgument>
-  </file>
   <file src="lib/Notification/Notifier.php">
     <RedundantCast occurrences="7">
       <code>(string) $l-&gt;t('%s has mentioned you in a comment on "%s".', [$dn, $params[0]])</code>
@@ -196,12 +192,9 @@
       <code>$cardId</code>
       <code>$cardId</code>
     </InvalidScalarArgument>
-    <UndefinedThisPropertyAssignment occurrences="1">
-      <code>$this-&gt;currentUser</code>
-    </UndefinedThisPropertyAssignment>
-    <UndefinedThisPropertyFetch occurrences="1">
-      <code>$this-&gt;currentUser</code>
-    </UndefinedThisPropertyFetch>
+  </file>
+  <file src="lib/Service/AttachmentService.php">
+    <InvalidCatch occurrences="1"/>
   </file>
   <file src="lib/Service/BoardService.php">
     <TooManyArguments occurrences="2">
@@ -265,7 +258,6 @@
     </RedundantCondition>
   </file>
   <file src="lib/Service/FilesAppService.php">
-    <InvalidCatch occurrences="1"/>
     <MissingDependency occurrences="4">
       <code>$this-&gt;rootFolder</code>
       <code>$this-&gt;rootFolder</code>
@@ -274,18 +266,12 @@
     </MissingDependency>
   </file>
   <file src="lib/Service/PermissionService.php">
-    <UndefinedClass occurrences="2">
+    <UndefinedClass occurrences="3">
+      <code>Member</code>
       <code>\OCA\Circles\Api\v1\Circles</code>
       <code>\OCA\Circles\Api\v1\Circles</code>
     </UndefinedClass>
   </file>
-  <file src="lib/Service/SearchService.php">
-    <UndefinedThisPropertyFetch occurrences="3">
-      <code>$this-&gt;l10n</code>
-      <code>$this-&gt;urlGenerator</code>
-      <code>$this-&gt;userManager</code>
-    </UndefinedThisPropertyFetch>
-  </file>
   <file src="lib/Service/StackService.php">
     <UndefinedClass occurrences="1">
       <code>BadRquestException</code>

tests/unit/Db/AclMapperTest.php

@@ -25,6 +25,7 @@ namespace OCA\Deck\Db;
 
 use OCP\IGroupManager;
 use OCP\IUserManager;
+use Psr\Log\LoggerInterface;
 use Test\AppFramework\Db\MapperTestUtility;
 
 /**
@@ -54,7 +55,8 @@ class AclMapperTest extends MapperTestUtility {
 			$this->aclMapper,
 			\OC::$server->query(StackMapper::class),
 			$this->userManager,
-			$this->groupManager
+			$this->groupManager,
+			$this->createMock(LoggerInterface::class)
 		);
 
 		$this->boards = [

tests/unit/Db/BoardMapperTest.php

@@ -26,6 +26,7 @@ namespace OCA\Deck\Db;
 use OCP\IDBConnection;
 use OCP\IGroupManager;
 use OCP\IUserManager;
+use Psr\Log\LoggerInterface;
 use Test\AppFramework\Db\MapperTestUtility;
 
 /**
@@ -61,7 +62,8 @@ class BoardMapperTest extends MapperTestUtility {
 			\OC::$server->query(AclMapper::class),
 			\OC::$server->query(StackMapper::class),
 			$this->userManager,
-			$this->groupManager
+			$this->groupManager,
+			$this->createMock(LoggerInterface::class)
 		);
 		$this->aclMapper = \OC::$server->query(AclMapper::class);
 		$this->labelMapper = \OC::$server->query(LabelMapper::class);

tests/unit/Db/CardTest.php

@@ -83,6 +83,7 @@ class CardTest extends TestCase {
 			'assignedUsers' => null,
 			'deletedAt' => 0,
 			'commentsUnread' => 0,
+			'commentsCount' => 0,
 			'lastEditor' => null,
 			'ETag' => $card->getETag(),
 		], $card->jsonSerialize());
@@ -109,6 +110,7 @@ class CardTest extends TestCase {
 			'assignedUsers' => null,
 			'deletedAt' => 0,
 			'commentsUnread' => 0,
+			'commentsCount' => 0,
 			'lastEditor' => null,
 			'ETag' => $card->getETag(),
 		], $card->jsonSerialize());
@@ -145,6 +147,7 @@ class CardTest extends TestCase {
 			'assignedUsers' => ['user1'],
 			'deletedAt' => 0,
 			'commentsUnread' => 0,
+			'commentsCount' => 0,
 			'lastEditor' => null,
 			'ETag' => $card->getETag(),
 		], $card->jsonSerialize());

tests/unit/Search/Query/AQueryParameterTest.php

@@ -0,0 +1,47 @@
+<?php
+/*
+ * @copyright Copyright (c) 2021 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/>.
+ *
+ */
+
+declare(strict_types=1);
+
+namespace OCA\Deck\Search\Query;
+
+use PHPUnit\Framework\TestCase;
+
+class AQueryParameterTest extends TestCase {
+	public function dataValue() {
+		return [
+			['foo', 'foo'],
+			['spätial character', 'spätial character'],
+			['"spätial character"', 'spätial character'],
+			['"spätial "character"', 'spätial "character'],
+			['"spätial 🐘"', 'spätial 🐘'],
+			['\'spätial character\'', '\'spätial character\''],
+		];
+	}
+
+	/** @dataProvider dataValue */
+	public function testValue($input, $expectedValue) {
+		$parameter = new StringQueryParameter('test', 0, $input);
+		$this->assertEquals($expectedValue, $parameter->getValue());
+	}
+}

tests/unit/Service/CardServiceTest.php

@@ -128,7 +128,7 @@ class CardServiceTest extends TestCase {
 		$this->userManager->expects($this->once())
 			->method('get')
 			->willReturn($user);
-		$this->commentsManager->expects($this->once())
+		$this->commentsManager->expects($this->any())
 			->method('getNumberOfCommentsForObject')
 			->willReturn(0);
 		$boardMock = $this->createMock(Board::class);

tests/unit/Service/PermissionServiceTest.php

@@ -146,7 +146,7 @@ class PermissionServiceTest extends \Test\TestCase {
 	}
 
 	public function testUserIsBoardOwnerNull() {
-		$this->boardMapper->expects($this->once())->method('find')->willReturn(null);
+		$this->boardMapper->expects($this->once())->method('find')->willThrowException(new DoesNotExistException('board does not exist'));
 		$this->assertEquals(false, $this->service->userIsBoardOwner(123));
 	}
 
@@ -225,12 +225,9 @@ class PermissionServiceTest extends \Test\TestCase {
 		$board = new Board();
 		$board->setId($boardId);
 		$board->setOwner($owner);
+		$board->setAcl($this->getAcls($boardId));
 		$this->boardMapper->expects($this->any())->method('find')->willReturn($board);
 
-		// acl check
-		$acls = $this->getAcls($boardId);
-		$this->aclMapper->expects($this->any())->method('findAll')->willReturn($acls);
-
 		$this->shareManager->expects($this->any())
 			->method('sharingDisabledForUser')
 			->willReturn(false);
@@ -250,14 +247,12 @@ class PermissionServiceTest extends \Test\TestCase {
 		$board = new Board();
 		$board->setId($boardId);
 		$board->setOwner($owner);
+		$board->setAcl($this->getAcls($boardId));
 		if ($boardId === null) {
 			$this->boardMapper->expects($this->any())->method('find')->willThrowException(new DoesNotExistException('not found'));
 		} else {
 			$this->boardMapper->expects($this->any())->method('find')->willReturn($board);
 		}
-		$acls = $this->getAcls($boardId);
-		$this->aclMapper->expects($this->any())->method('findAll')->willReturn($acls);
-
 
 		if ($result) {
 			$actual = $this->service->checkPermission($mapper, 1234, $permission);