From 67fe250248866562c28188a2c828c6764cdf71c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Wed, 4 Nov 2020 19:39:38 +0100 Subject: [PATCH 1/5] Refactor class names to Assignment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Activity/ActivityManager.php | 4 +-- lib/AppInfo/Application20.php | 4 +-- lib/AppInfo/ApplicationLegacy.php | 4 +-- lib/Command/UserExport.php | 4 +-- lib/Db/{AssignedUsers.php => Assignment.php} | 2 +- ...edUsersMapper.php => AssignmentMapper.php} | 23 ++++++++------- lib/Service/AssignmentService.php | 14 +++++----- lib/Service/BoardService.php | 4 +-- lib/Service/CardService.php | 8 +++--- lib/Service/OverviewService.php | 6 ++-- lib/Service/StackService.php | 4 +-- ...apperTest.php => AssignmentMapperTest.php} | 22 +++++++-------- tests/unit/Activity/ActivityManagerTest.php | 4 +-- tests/unit/Command/UserExportTest.php | 4 +-- tests/unit/Service/AssignmentServiceTest.php | 28 +++++++++---------- tests/unit/Service/BoardServiceTest.php | 10 +++---- tests/unit/Service/CardServiceTest.php | 6 ++-- tests/unit/Service/StackServiceTest.php | 6 ++-- 18 files changed, 80 insertions(+), 77 deletions(-) rename lib/Db/{AssignedUsers.php => Assignment.php} (94%) rename lib/Db/{AssignedUsersMapper.php => AssignmentMapper.php} (88%) rename tests/integration/database/{AssignedUsersMapperTest.php => AssignmentMapperTest.php} (94%) diff --git a/lib/Activity/ActivityManager.php b/lib/Activity/ActivityManager.php index 6ad70294b..9d282fdd6 100644 --- a/lib/Activity/ActivityManager.php +++ b/lib/Activity/ActivityManager.php @@ -28,7 +28,7 @@ namespace OCA\Deck\Activity; use InvalidArgumentException; use OCA\Deck\Db\Acl; use OCA\Deck\Db\AclMapper; -use OCA\Deck\Db\AssignedUsers; +use OCA\Deck\Db\Assignment; use OCA\Deck\Db\Attachment; use OCA\Deck\Db\AttachmentMapper; use OCA\Deck\Db\Board; @@ -471,7 +471,7 @@ class ActivityManager { break; case Attachment::class: case Label::class: - case AssignedUsers::class: + case Assignment::class: $objectId = $entity->getCardId(); break; case IComment::class: diff --git a/lib/AppInfo/Application20.php b/lib/AppInfo/Application20.php index 19e5475ed..4e37e58ef 100644 --- a/lib/AppInfo/Application20.php +++ b/lib/AppInfo/Application20.php @@ -33,7 +33,7 @@ use OCA\Deck\Collaboration\Resources\ResourceProviderCard; use OCA\Deck\Dashboard\DeckWidget; use OCA\Deck\Db\Acl; use OCA\Deck\Db\AclMapper; -use OCA\Deck\Db\AssignedUsersMapper; +use OCA\Deck\Db\AssignmentMapper; use OCA\Deck\Db\BoardMapper; use OCA\Deck\Db\CardMapper; use OCA\Deck\Listeners\BeforeTemplateRenderedListener; @@ -132,7 +132,7 @@ class Application20 extends App implements IBootstrap { $aclMapper->delete($acl); } // delete existing user assignments - $assignmentMapper = $container->query(AssignedUsersMapper::class); + $assignmentMapper = $container->query(AssignmentMapper::class); $assignments = $assignmentMapper->findByUserId($user->getUID()); foreach ($assignments as $assignment) { $assignmentMapper->delete($assignment); diff --git a/lib/AppInfo/ApplicationLegacy.php b/lib/AppInfo/ApplicationLegacy.php index eec44dd4b..7c12c4a78 100644 --- a/lib/AppInfo/ApplicationLegacy.php +++ b/lib/AppInfo/ApplicationLegacy.php @@ -30,7 +30,7 @@ use OCA\Deck\Collaboration\Resources\ResourceProvider; use OCA\Deck\Collaboration\Resources\ResourceProviderCard; use OCA\Deck\Db\Acl; use OCA\Deck\Db\AclMapper; -use OCA\Deck\Db\AssignedUsersMapper; +use OCA\Deck\Db\AssignmentMapper; use OCA\Deck\Db\BoardMapper; use OCA\Deck\Db\CardMapper; use OCA\Deck\Middleware\DefaultBoardMiddleware; @@ -113,7 +113,7 @@ class ApplicationLegacy extends App { $aclMapper->delete($acl); } // delete existing user assignments - $assignmentMapper = $container->query(AssignedUsersMapper::class); + $assignmentMapper = $container->query(AssignmentMapper::class); $assignments = $assignmentMapper->findByUserId($user->getUID()); foreach ($assignments as $assignment) { $assignmentMapper->delete($assignment); diff --git a/lib/Command/UserExport.php b/lib/Command/UserExport.php index 61ec06396..b40659ad9 100644 --- a/lib/Command/UserExport.php +++ b/lib/Command/UserExport.php @@ -23,7 +23,7 @@ namespace OCA\Deck\Command; -use OCA\Deck\Db\AssignedUsersMapper; +use OCA\Deck\Db\AssignmentMapper; use OCA\Deck\Db\BoardMapper; use OCA\Deck\Db\CardMapper; use OCA\Deck\Db\StackMapper; @@ -48,7 +48,7 @@ class UserExport extends Command { BoardService $boardService, StackMapper $stackMapper, CardMapper $cardMapper, - AssignedUsersMapper $assignedUsersMapper, + AssignmentMapper $assignedUsersMapper, IUserManager $userManager, IGroupManager $groupManager) { parent::__construct(); diff --git a/lib/Db/AssignedUsers.php b/lib/Db/Assignment.php similarity index 94% rename from lib/Db/AssignedUsers.php rename to lib/Db/Assignment.php index 319c3bd95..56207d36a 100644 --- a/lib/Db/AssignedUsers.php +++ b/lib/Db/Assignment.php @@ -25,7 +25,7 @@ namespace OCA\Deck\Db; use JsonSerializable; -class AssignedUsers extends RelationalEntity implements JsonSerializable { +class Assignment extends RelationalEntity implements JsonSerializable { public $id; protected $participant; protected $cardId; diff --git a/lib/Db/AssignedUsersMapper.php b/lib/Db/AssignmentMapper.php similarity index 88% rename from lib/Db/AssignedUsersMapper.php rename to lib/Db/AssignmentMapper.php index 025570055..61dd4a485 100644 --- a/lib/Db/AssignedUsersMapper.php +++ b/lib/Db/AssignmentMapper.php @@ -33,7 +33,7 @@ use OCP\IDBConnection; use OCP\IGroupManager; use OCP\IUserManager; -class AssignedUsersMapper extends QBMapper implements IPermissionMapper { +class AssignmentMapper extends QBMapper implements IPermissionMapper { /** @var CardMapper */ private $cardMapper; @@ -45,7 +45,8 @@ class AssignedUsersMapper extends QBMapper implements IPermissionMapper { private $circleService; public function __construct(IDBConnection $db, CardMapper $cardMapper, IUserManager $userManager, IGroupManager $groupManager, CirclesService $circleService) { - parent::__construct($db, 'deck_assigned_users', AssignedUsers::class); + parent::__construct($db, 'deck_assigned_users', Assignment::class); + $this->cardMapper = $cardMapper; $this->userManager = $userManager; $this->groupManager = $groupManager; @@ -63,7 +64,7 @@ class AssignedUsersMapper extends QBMapper implements IPermissionMapper { $qb->select('*') ->from('deck_assigned_users') ->where($qb->expr()->eq('card_id', $qb->createNamedParameter($cardId))); - /** @var AssignedUsers[] $users */ + /** @var Assignment[] $users */ $users = $this->findEntities($qb); foreach ($users as &$user) { $this->mapParticipant($user); @@ -76,7 +77,7 @@ class AssignedUsersMapper extends QBMapper implements IPermissionMapper { $qb->select('*') ->from('deck_assigned_users') ->where($qb->expr()->eq('participant', $qb->createNamedParameter($uid))); - /** @var AssignedUsers[] $users */ + /** @var Assignment[] $users */ return $this->findEntities($qb); } @@ -94,19 +95,21 @@ class AssignedUsersMapper extends QBMapper implements IPermissionMapper { * * @param Entity $entity * @return null|Entity + * @throws NotFoundException */ public function insert(Entity $entity): Entity { $origin = $this->getOrigin($entity); if ($origin === null) { throw new NotFoundException('No origin found for assignment'); } - /** @var AssignedUsers $assignment */ + + /** @var Assignment $assignment */ $assignment = parent::insert($entity); $this->mapParticipant($assignment); return $assignment; } - public function mapParticipant(AssignedUsers $assignment): void { + public function mapParticipant(Assignment $assignment): void { $self = $this; $assignment->resolveRelation('participant', function () use (&$self, &$assignment) { return $self->getOrigin($assignment); @@ -115,7 +118,7 @@ class AssignedUsersMapper extends QBMapper implements IPermissionMapper { public function isUserAssigned($cardId, $userId): bool { $assignments = $this->find($cardId); - /** @var AssignedUsers $assignment */ + /** @var Assignment $assignment */ foreach ($assignments as $assignment) { $origin = $this->getOrigin($assignment); if ($origin instanceof User && $assignment->getParticipant() === $userId) { @@ -132,12 +135,12 @@ class AssignedUsersMapper extends QBMapper implements IPermissionMapper { return false; } - private function getOrigin(AssignedUsers $assignment) { - if ($assignment->getType() === AssignedUsers::TYPE_USER) { + private function getOrigin(Assignment $assignment) { + if ($assignment->getType() === Assignment::TYPE_USER) { $origin = $this->userManager->get($assignment->getParticipant()); return $origin ? new User($origin) : null; } - if ($assignment->getType() === AssignedUsers::TYPE_GROUP) { + if ($assignment->getType() === Assignment::TYPE_GROUP) { $origin = $this->groupManager->get($assignment->getParticipant()); return $origin ? new Group($origin) : null; } diff --git a/lib/Service/AssignmentService.php b/lib/Service/AssignmentService.php index b3210bc44..64c1266e3 100644 --- a/lib/Service/AssignmentService.php +++ b/lib/Service/AssignmentService.php @@ -27,8 +27,8 @@ use OCA\Deck\Activity\ActivityManager; use OCA\Deck\BadRequestException; use OCA\Deck\Db\Acl; use OCA\Deck\Db\AclMapper; -use OCA\Deck\Db\AssignedUsers; -use OCA\Deck\Db\AssignedUsersMapper; +use OCA\Deck\Db\Assignment; +use OCA\Deck\Db\AssignmentMapper; use OCA\Deck\Db\CardMapper; use OCA\Deck\Db\ChangeHelper; use OCA\Deck\Event\FTSEvent; @@ -51,7 +51,7 @@ class AssignmentService { */ private $cardMapper; /** - * @var AssignedUsersMapper + * @var AssignmentMapper */ private $assignedUsersMapper; /** @@ -78,7 +78,7 @@ class AssignmentService { public function __construct( PermissionService $permissionService, CardMapper $cardMapper, - AssignedUsersMapper $assignedUsersMapper, + AssignmentMapper $assignedUsersMapper, AclMapper $aclMapper, NotificationHelper $notificationHelper, ActivityManager $activityManager, @@ -106,7 +106,7 @@ class AssignmentService { * @throws MultipleObjectsReturnedException * @throws DoesNotExistException */ - public function assignUser($cardId, $userId, int $type = AssignedUsers::TYPE_USER) { + public function assignUser($cardId, $userId, int $type = Assignment::TYPE_USER) { if (is_numeric($cardId) === false) { throw new BadRequestException('card id must be a number'); } @@ -115,7 +115,7 @@ class AssignmentService { throw new BadRequestException('user id must be provided'); } - if ($type !== AssignedUsers::TYPE_USER && $type !== AssignedUsers::TYPE_GROUP) { + if ($type !== Assignment::TYPE_USER && $type !== Assignment::TYPE_GROUP) { throw new BadRequestException('Invalid type provided for assignemnt'); } @@ -143,7 +143,7 @@ class AssignmentService { $this->notificationHelper->sendCardAssigned($card, $userId); } - $assignment = new AssignedUsers(); + $assignment = new Assignment(); $assignment->setCardId($cardId); $assignment->setParticipant($userId); $assignment->setType($type); diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index cdc348119..7ab87f9cb 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -29,7 +29,7 @@ use OCA\Deck\Activity\ChangeSet; use OCA\Deck\AppInfo\Application; use OCA\Deck\Db\Acl; use OCA\Deck\Db\AclMapper; -use OCA\Deck\Db\AssignedUsersMapper; +use OCA\Deck\Db\AssignmentMapper; use OCA\Deck\Db\ChangeHelper; use OCA\Deck\Db\IPermissionMapper; use OCA\Deck\Db\Label; @@ -80,7 +80,7 @@ class BoardService { AclMapper $aclMapper, PermissionService $permissionService, NotificationHelper $notificationHelper, - AssignedUsersMapper $assignedUsersMapper, + AssignmentMapper $assignedUsersMapper, IUserManager $userManager, IGroupManager $groupManager, ActivityManager $activityManager, diff --git a/lib/Service/CardService.php b/lib/Service/CardService.php index 030c10e5c..0e2d1ead7 100644 --- a/lib/Service/CardService.php +++ b/lib/Service/CardService.php @@ -28,7 +28,7 @@ namespace OCA\Deck\Service; use OCA\Deck\Activity\ActivityManager; use OCA\Deck\Activity\ChangeSet; -use OCA\Deck\Db\AssignedUsersMapper; +use OCA\Deck\Db\AssignmentMapper; use OCA\Deck\Db\Card; use OCA\Deck\Db\CardMapper; use OCA\Deck\Db\Acl; @@ -69,7 +69,7 @@ class CardService { PermissionService $permissionService, BoardService $boardService, NotificationHelper $notificationHelper, - AssignedUsersMapper $assignedUsersMapper, + AssignmentMapper $assignedUsersMapper, AttachmentService $attachmentService, ActivityManager $activityManager, ICommentsManager $commentsManager, @@ -590,7 +590,7 @@ class CardService { */ public function findAllWithDue($userId) { $cards = $this->cardMapper->findAllWithDue($userId); - + return $cards; } @@ -602,7 +602,7 @@ class CardService { */ public function findAssignedCards($userId) { $cards = $this->cardMapper->findAssignedCards($userId); - + return $cards; } } diff --git a/lib/Service/OverviewService.php b/lib/Service/OverviewService.php index 8099108e6..a5df16d69 100644 --- a/lib/Service/OverviewService.php +++ b/lib/Service/OverviewService.php @@ -27,7 +27,7 @@ declare(strict_types=1); namespace OCA\Deck\Service; -use OCA\Deck\Db\AssignedUsersMapper; +use OCA\Deck\Db\AssignmentMapper; use OCA\Deck\Db\Card; use OCA\Deck\Db\CardMapper; use OCP\Comments\ICommentsManager; @@ -45,7 +45,7 @@ class OverviewService { private $labelMapper; /** @var CardMapper */ private $cardMapper; - /** @var AssignedUsersMapper */ + /** @var AssignmentMapper */ private $assignedUsersMapper; /** @var IUserManager */ private $userManager; @@ -60,7 +60,7 @@ class OverviewService { BoardMapper $boardMapper, LabelMapper $labelMapper, CardMapper $cardMapper, - AssignedUsersMapper $assignedUsersMapper, + AssignmentMapper $assignedUsersMapper, IUserManager $userManager, IGroupManager $groupManager, ICommentsManager $commentsManager, diff --git a/lib/Service/StackService.php b/lib/Service/StackService.php index a31d13854..8178d1423 100644 --- a/lib/Service/StackService.php +++ b/lib/Service/StackService.php @@ -28,7 +28,7 @@ use OCA\Deck\Activity\ActivityManager; use OCA\Deck\Activity\ChangeSet; use OCA\Deck\BadRequestException; use OCA\Deck\Db\Acl; -use OCA\Deck\Db\AssignedUsersMapper; +use OCA\Deck\Db\AssignmentMapper; use OCA\Deck\Db\BoardMapper; use OCA\Deck\Db\CardMapper; use OCA\Deck\Db\ChangeHelper; @@ -62,7 +62,7 @@ class StackService { PermissionService $permissionService, BoardService $boardService, CardService $cardService, - AssignedUsersMapper $assignedUsersMapper, + AssignmentMapper $assignedUsersMapper, AttachmentService $attachmentService, ActivityManager $activityManager, EventDispatcherInterface $eventDispatcher, diff --git a/tests/integration/database/AssignedUsersMapperTest.php b/tests/integration/database/AssignmentMapperTest.php similarity index 94% rename from tests/integration/database/AssignedUsersMapperTest.php rename to tests/integration/database/AssignmentMapperTest.php index 5a6a949ce..e593a5d6b 100644 --- a/tests/integration/database/AssignedUsersMapperTest.php +++ b/tests/integration/database/AssignmentMapperTest.php @@ -31,9 +31,9 @@ use OCA\Deck\Service\CardService; /** * @group DB - * @coversDefaultClass OCA\Deck\Db\AssignedUsersMapper + * @coversDefaultClass \OCA\Deck\Db\AssignmentMapper */ -class AssignedUsersMapperTest extends \Test\TestCase { +class AssignmentMapperTest extends \Test\TestCase { private const TEST_USER1 = 'test-share-user1'; private const TEST_USER3 = 'test-share-user3'; private const TEST_USER2 = 'test-share-user2'; @@ -46,7 +46,7 @@ class AssignedUsersMapperTest extends \Test\TestCase { protected $cardService; /** @var StackService */ protected $stackService; - /** @var AssignedUsersMapper */ + /** @var AssignmentMapper */ protected $assignedUsersMapper; /** @var AssignmentService */ private $assignmentService; @@ -85,7 +85,7 @@ class AssignedUsersMapperTest extends \Test\TestCase { $this->stackService = \OC::$server->query(StackService::class); $this->cardService = \OC::$server->query(CardService::class); $this->assignmentService = \OC::$server->query(AssignmentService::class); - $this->assignedUsersMapper = \OC::$server->query(AssignedUsersMapper::class); + $this->assignedUsersMapper = \OC::$server->query(AssignmentMapper::class); $this->createBoardWithExampleData(); } @@ -146,10 +146,10 @@ class AssignedUsersMapperTest extends \Test\TestCase { * @covers ::insert */ public function testInsert() { - $assignment = new AssignedUsers(); + $assignment = new Assignment(); $assignment->setCardId($this->cards[1]->getId()); $assignment->setParticipant(self::TEST_USER4); - $assignment->setType(AssignedUsers::TYPE_USER); + $assignment->setType(Assignment::TYPE_USER); $this->assignedUsersMapper->insert($assignment); $actual = $this->assignedUsersMapper->find($this->cards[1]->getId()); @@ -162,7 +162,7 @@ class AssignedUsersMapperTest extends \Test\TestCase { * @covers ::insert */ public function testInsertInvalidUser() { - $assignment = new AssignedUsers(); + $assignment = new Assignment(); $assignment->setCardId($this->cards[1]->getId()); $assignment->setParticipant('invalid-username'); $assignment->setType(AssignedUsers::TYPE_USER); @@ -174,17 +174,17 @@ class AssignedUsersMapperTest extends \Test\TestCase { * @covers ::mapParticipant */ public function testMapParticipant() { - $assignment = new AssignedUsers(); + $assignment = new Assignment(); $assignment->setCardId($this->cards[1]->getId()); $assignment->setParticipant(self::TEST_USER4); - $assignment->setType(AssignedUsers::TYPE_USER); + $assignment->setType(Assignment::TYPE_USER); $this->assignedUsersMapper->mapParticipant($assignment); $this->assertInstanceOf(User::class, $assignment->resolveParticipant()); - $assignment = new AssignedUsers(); + $assignment = new Assignment(); $assignment->setCardId($this->cards[1]->getId()); $assignment->setParticipant('invalid-username'); - $assignment->setType(AssignedUsers::TYPE_USER); + $assignment->setType(Assignment::TYPE_USER); $this->assignedUsersMapper->mapParticipant($assignment); $this->assertEquals('invalid-username', $assignment->resolveParticipant()); } diff --git a/tests/unit/Activity/ActivityManagerTest.php b/tests/unit/Activity/ActivityManagerTest.php index a93177e35..e91ca6cae 100644 --- a/tests/unit/Activity/ActivityManagerTest.php +++ b/tests/unit/Activity/ActivityManagerTest.php @@ -24,7 +24,7 @@ namespace OCA\Deck\Activity; use OCA\Deck\Db\AclMapper; -use OCA\Deck\Db\AssignedUsers; +use OCA\Deck\Db\Assignment; use OCA\Deck\Db\Attachment; use OCA\Deck\Db\AttachmentMapper; use OCA\Deck\Db\Board; @@ -210,7 +210,7 @@ class ActivityManagerTest extends TestCase { $label = new Label(); $label->setCardId(3); $label->setBoardId(1); - $assignedUser = new AssignedUsers(); + $assignedUser = new Assignment(); $assignedUser->setCardId(3); return [ diff --git a/tests/unit/Command/UserExportTest.php b/tests/unit/Command/UserExportTest.php index 5527dba84..e8eb39527 100644 --- a/tests/unit/Command/UserExportTest.php +++ b/tests/unit/Command/UserExportTest.php @@ -23,7 +23,7 @@ namespace OCA\Deck\Command; -use OCA\Deck\Db\AssignedUsersMapper; +use OCA\Deck\Db\AssignmentMapper; use OCA\Deck\Db\Board; use OCA\Deck\Db\BoardMapper; use OCA\Deck\Db\Card; @@ -53,7 +53,7 @@ class UserExportTest extends \Test\TestCase { $this->boardService= $this->createMock(BoardService::class); $this->stackMapper= $this->createMock(StackMapper::class); $this->cardMapper= $this->createMock(CardMapper::class); - $this->assignedUserMapper= $this->createMock(AssignedUsersMapper::class); + $this->assignedUserMapper= $this->createMock(AssignmentMapper::class); $this->userManager = $this->createMock(IUserManager::class); $this->groupManager = $this->createMock(IGroupManager::class); $this->userExport = new UserExport($this->boardMapper, $this->boardService, $this->stackMapper, $this->cardMapper, $this->assignedUserMapper, $this->userManager, $this->groupManager); diff --git a/tests/unit/Service/AssignmentServiceTest.php b/tests/unit/Service/AssignmentServiceTest.php index 257f3706a..e12f3398e 100644 --- a/tests/unit/Service/AssignmentServiceTest.php +++ b/tests/unit/Service/AssignmentServiceTest.php @@ -26,8 +26,8 @@ namespace OCA\Deck\Service; use OCA\Deck\Activity\ActivityManager; use OCA\Deck\BadRequestException; use OCA\Deck\Db\AclMapper; -use OCA\Deck\Db\AssignedUsers; -use OCA\Deck\Db\AssignedUsersMapper; +use OCA\Deck\Db\Assignment; +use OCA\Deck\Db\AssignmentMapper; use OCA\Deck\Db\CardMapper; use OCA\Deck\Db\ChangeHelper; use OCA\Deck\NotFoundException; @@ -49,7 +49,7 @@ class AssignmentServiceTest extends TestCase { */ private $cardMapper; /** - * @var MockObject|AssignedUsersMapper + * @var MockObject|AssignmentMapper */ private $assignedUsersMapper; /** @@ -83,7 +83,7 @@ class AssignmentServiceTest extends TestCase { $this->permissionService = $this->createMock(PermissionService::class); $this->cardMapper = $this->createMock(CardMapper::class); $this->notificationHelper = $this->createMock(NotificationHelper::class); - $this->assignedUsersMapper = $this->createMock(AssignedUsersMapper::class); + $this->assignedUsersMapper = $this->createMock(AssignmentMapper::class); $this->activityManager = $this->createMock(ActivityManager::class); $this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->changeHelper = $this->createMock(ChangeHelper::class); @@ -118,10 +118,10 @@ class AssignmentServiceTest extends TestCase { ->method('find') ->with(123) ->willReturn($assignments); - $assignment = new AssignedUsers(); + $assignment = new Assignment(); $assignment->setCardId(123); $assignment->setParticipant('admin'); - $assignment->setType(AssignedUsers::TYPE_USER); + $assignment->setType(Assignment::TYPE_USER); $this->cardMapper->expects($this->once()) ->method('findBoardId') ->willReturn(1); @@ -148,10 +148,10 @@ class AssignmentServiceTest extends TestCase { ->method('find') ->with(123) ->willReturn($assignments); - $assignment = new AssignedUsers(); + $assignment = new Assignment(); $assignment->setCardId(123); $assignment->setParticipant('admin'); - $assignment->setType(AssignedUsers::TYPE_USER); + $assignment->setType(Assignment::TYPE_USER); $this->cardMapper->expects($this->once()) ->method('findBoardId') ->willReturn(1); @@ -168,10 +168,10 @@ class AssignmentServiceTest extends TestCase { public function testAssignUserExisting() { $this->expectException(BadRequestException::class); $this->expectExceptionMessage('The user is already assigned to the card'); - $assignment = new AssignedUsers(); + $assignment = new Assignment(); $assignment->setCardId(123); $assignment->setParticipant('admin'); - $assignment->setType(AssignedUsers::TYPE_USER); + $assignment->setType(Assignment::TYPE_USER); $assignments = [ $assignment ]; @@ -184,10 +184,10 @@ class AssignmentServiceTest extends TestCase { } public function testUnassignUserExisting() { - $assignment = new AssignedUsers(); + $assignment = new Assignment(); $assignment->setCardId(123); $assignment->setParticipant('admin'); - $assignment->setType(AssignedUsers::TYPE_USER); + $assignment->setType(Assignment::TYPE_USER); $assignments = [ $assignment ]; @@ -205,10 +205,10 @@ class AssignmentServiceTest extends TestCase { public function testUnassignUserNotExisting() { $this->expectException(NotFoundException::class); - $assignment = new AssignedUsers(); + $assignment = new Assignment(); $assignment->setCardId(123); $assignment->setParticipant('admin'); - $assignment->setType(AssignedUsers::TYPE_USER); + $assignment->setType(Assignment::TYPE_USER); $assignments = [ $assignment ]; diff --git a/tests/unit/Service/BoardServiceTest.php b/tests/unit/Service/BoardServiceTest.php index ec4920c10..909eb5c21 100644 --- a/tests/unit/Service/BoardServiceTest.php +++ b/tests/unit/Service/BoardServiceTest.php @@ -27,8 +27,8 @@ use OC\L10N\L10N; use OCA\Deck\Activity\ActivityManager; use OCA\Deck\Db\Acl; use OCA\Deck\Db\AclMapper; -use OCA\Deck\Db\AssignedUsers; -use OCA\Deck\Db\AssignedUsersMapper; +use OCA\Deck\Db\Assignment; +use OCA\Deck\Db\AssignmentMapper; use OCA\Deck\Db\Board; use OCA\Deck\Db\BoardMapper; use OCA\Deck\Db\ChangeHelper; @@ -61,7 +61,7 @@ class BoardServiceTest extends TestCase { private $permissionService; /** @var NotificationHelper */ private $notificationHelper; - /** @var AssignedUsersMapper */ + /** @var AssignmentMapper */ private $assignedUsersMapper; /** @var IUserManager */ private $userManager; @@ -85,7 +85,7 @@ class BoardServiceTest extends TestCase { $this->labelMapper = $this->createMock(LabelMapper::class); $this->permissionService = $this->createMock(PermissionService::class); $this->notificationHelper = $this->createMock(NotificationHelper::class); - $this->assignedUsersMapper = $this->createMock(AssignedUsersMapper::class); + $this->assignedUsersMapper = $this->createMock(AssignmentMapper::class); $this->userManager = $this->createMock(IUserManager::class); $this->groupManager = $this->createMock(IGroupManager::class); $this->activityManager = $this->createMock(ActivityManager::class); @@ -390,7 +390,7 @@ class BoardServiceTest extends TestCase { ->method('find') ->with(123) ->willReturn($acl); - $assignment = new AssignedUsers(); + $assignment = new Assignment(); $assignment->setParticipant('admin'); $this->assignedUsersMapper->expects($this->once()) ->method('findByUserId') diff --git a/tests/unit/Service/CardServiceTest.php b/tests/unit/Service/CardServiceTest.php index bf6368299..1d9b17c21 100644 --- a/tests/unit/Service/CardServiceTest.php +++ b/tests/unit/Service/CardServiceTest.php @@ -24,7 +24,7 @@ namespace OCA\Deck\Service; use OCA\Deck\Activity\ActivityManager; -use OCA\Deck\Db\AssignedUsersMapper; +use OCA\Deck\Db\AssignmentMapper; use OCA\Deck\Db\Card; use OCA\Deck\Db\CardMapper; use OCA\Deck\Db\ChangeHelper; @@ -54,7 +54,7 @@ class CardServiceTest extends TestCase { private $permissionService; /** @var NotificationHelper */ private $notificationHelper; - /** @var AssignedUsersMapper|MockObject */ + /** @var AssignmentMapper|MockObject */ private $assignedUsersMapper; /** @var BoardService|MockObject */ private $boardService; @@ -83,7 +83,7 @@ class CardServiceTest extends TestCase { $this->permissionService = $this->createMock(PermissionService::class); $this->boardService = $this->createMock(BoardService::class); $this->notificationHelper = $this->createMock(NotificationHelper::class); - $this->assignedUsersMapper = $this->createMock(AssignedUsersMapper::class); + $this->assignedUsersMapper = $this->createMock(AssignmentMapper::class); $this->attachmentService = $this->createMock(AttachmentService::class); $this->activityManager = $this->createMock(ActivityManager::class); $this->commentsManager = $this->createMock(ICommentsManager::class); diff --git a/tests/unit/Service/StackServiceTest.php b/tests/unit/Service/StackServiceTest.php index 0ec5567ca..2c4d9686f 100644 --- a/tests/unit/Service/StackServiceTest.php +++ b/tests/unit/Service/StackServiceTest.php @@ -24,7 +24,7 @@ namespace OCA\Deck\Service; use OCA\Deck\Activity\ActivityManager; -use OCA\Deck\Db\AssignedUsersMapper; +use OCA\Deck\Db\AssignmentMapper; use OCA\Deck\Db\Card; use OCA\Deck\Db\CardMapper; use OCA\Deck\Db\BoardMapper; @@ -56,7 +56,7 @@ class StackServiceTest extends TestCase { private $labelMapper; /** @var \PHPUnit\Framework\MockObject\MockObject|PermissionService */ private $permissionService; - /** @var AssignedUsersMapper|\PHPUnit\Framework\MockObject\MockObject */ + /** @var AssignmentMapper|\PHPUnit\Framework\MockObject\MockObject */ private $assignedUsersMapper; /** @var AttachmentService|\PHPUnit\Framework\MockObject\MockObject */ private $attachmentService; @@ -79,7 +79,7 @@ class StackServiceTest extends TestCase { $this->permissionService = $this->createMock(PermissionService::class); $this->boardService = $this->createMock(BoardService::class); $this->cardService = $this->createMock(CardService::class); - $this->assignedUsersMapper = $this->createMock(AssignedUsersMapper::class); + $this->assignedUsersMapper = $this->createMock(AssignmentMapper::class); $this->attachmentService = $this->createMock(AttachmentService::class); $this->labelMapper = $this->createMock(LabelMapper::class); $this->activityManager = $this->createMock(ActivityManager::class); From d66068cdcd25b0c8abe093378e5f829a034fcf92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Wed, 4 Nov 2020 19:39:59 +0100 Subject: [PATCH 2/5] Move AssignmentMapper to query builder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Db/AssignmentMapper.php | 22 +++++++++++----------- lib/Service/BoardService.php | 2 +- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/Db/AssignmentMapper.php b/lib/Db/AssignmentMapper.php index 61dd4a485..e4571c2de 100644 --- a/lib/Db/AssignmentMapper.php +++ b/lib/Db/AssignmentMapper.php @@ -32,6 +32,7 @@ use OCP\AppFramework\Db\QBMapper; use OCP\IDBConnection; use OCP\IGroupManager; use OCP\IUserManager; +use PDO; class AssignmentMapper extends QBMapper implements IPermissionMapper { @@ -57,27 +58,27 @@ class AssignmentMapper extends QBMapper implements IPermissionMapper { * FIXME: rename this since it returns multiple entities otherwise the naming is confusing with Entity::find * * @param $cardId - * @return array|Entity + * @return Assignment[] */ - public function find($cardId) { + + public function find($cardId): array { $qb = $this->db->getQueryBuilder(); $qb->select('*') ->from('deck_assigned_users') - ->where($qb->expr()->eq('card_id', $qb->createNamedParameter($cardId))); - /** @var Assignment[] $users */ + ->where($qb->expr()->eq('card_id', $qb->createNamedParameter($cardId, PDO::PARAM_INT))); $users = $this->findEntities($qb); - foreach ($users as &$user) { + foreach ($users as $user) { $this->mapParticipant($user); } return $users; } - public function findByUserId($uid) { + public function findByParticipant(string $participant, $type = Assignment::TYPE_USER): array { $qb = $this->db->getQueryBuilder(); $qb->select('*') ->from('deck_assigned_users') - ->where($qb->expr()->eq('participant', $qb->createNamedParameter($uid))); - /** @var Assignment[] $users */ + ->where($qb->expr()->eq('participant', $qb->createNamedParameter($participant, PDO::PARAM_STR))) + ->andWhere($qb->expr()->eq('type', $qb->createNamedParameter($type, PDO::PARAM_INT))); return $this->findEntities($qb); } @@ -94,7 +95,7 @@ class AssignmentMapper extends QBMapper implements IPermissionMapper { * Check if user exists before assigning it to a card * * @param Entity $entity - * @return null|Entity + * @return null|Assignment * @throws NotFoundException */ public function insert(Entity $entity): Entity { @@ -118,7 +119,6 @@ class AssignmentMapper extends QBMapper implements IPermissionMapper { public function isUserAssigned($cardId, $userId): bool { $assignments = $this->find($cardId); - /** @var Assignment $assignment */ foreach ($assignments as $assignment) { $origin = $this->getOrigin($assignment); if ($origin instanceof User && $assignment->getParticipant() === $userId) { @@ -144,7 +144,7 @@ class AssignmentMapper extends QBMapper implements IPermissionMapper { $origin = $this->groupManager->get($assignment->getParticipant()); return $origin ? new Group($origin) : null; } - if ($assignment->getType() === AssignedUsers::TYPE_CIRCLE) { + if ($assignment->getType() === Assignment::TYPE_CIRCLE) { $origin = $this->circleService->getCircle($assignment->getParticipant()); return $origin ? new Circle($origin) : null; } diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index 7ab87f9cb..4bfa6e0e8 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -631,7 +631,7 @@ class BoardService { $acl = $this->aclMapper->find($id); $this->boardMapper->mapAcl($acl); if ($acl->getType() === Acl::PERMISSION_TYPE_USER) { - $assignements = $this->assignedUsersMapper->findByUserId($acl->getParticipant()); + $assignements = $this->assignedUsersMapper->findByParticipant($acl->getParticipant()); foreach ($assignements as $assignement) { $this->assignedUsersMapper->delete($assignement); } From 9b366857ab6df25c8ae15e491cc85862d94c659b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Wed, 4 Nov 2020 19:43:40 +0100 Subject: [PATCH 3/5] Rename find to findAll MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Command/UserExport.php | 2 +- lib/Db/AssignmentMapper.php | 6 +----- lib/Service/AssignmentService.php | 4 ++-- lib/Service/CardService.php | 4 ++-- lib/Service/OverviewService.php | 2 +- lib/Service/StackService.php | 2 +- tests/integration/database/AssignmentMapperTest.php | 6 +++--- 7 files changed, 11 insertions(+), 15 deletions(-) diff --git a/lib/Command/UserExport.php b/lib/Command/UserExport.php index b40659ad9..1d73e8dcf 100644 --- a/lib/Command/UserExport.php +++ b/lib/Command/UserExport.php @@ -99,7 +99,7 @@ class UserExport extends Command { $cards = $this->cardMapper->findAllByStack($stack->getId()); foreach ($cards as $card) { $fullCard = $this->cardMapper->find($card->getId()); - $assignedUsers = $this->assignedUsersMapper->find($card->getId()); + $assignedUsers = $this->assignedUsersMapper->findAll($card->getId()); $fullCard->setAssignedUsers($assignedUsers); $data[$board->getId()]['stacks'][$stack->getId()]['cards'][] = (array)$fullCard->jsonSerialize(); } diff --git a/lib/Db/AssignmentMapper.php b/lib/Db/AssignmentMapper.php index e4571c2de..0491cff1e 100644 --- a/lib/Db/AssignmentMapper.php +++ b/lib/Db/AssignmentMapper.php @@ -55,13 +55,9 @@ class AssignmentMapper extends QBMapper implements IPermissionMapper { } /** - * FIXME: rename this since it returns multiple entities otherwise the naming is confusing with Entity::find - * - * @param $cardId * @return Assignment[] */ - - public function find($cardId): array { + public function findAll(int $cardId): array { $qb = $this->db->getQueryBuilder(); $qb->select('*') ->from('deck_assigned_users') diff --git a/lib/Service/AssignmentService.php b/lib/Service/AssignmentService.php index 64c1266e3..aaf0cec37 100644 --- a/lib/Service/AssignmentService.php +++ b/lib/Service/AssignmentService.php @@ -120,7 +120,7 @@ class AssignmentService { } $this->permissionService->checkPermission($this->cardMapper, $cardId, Acl::PERMISSION_EDIT); - $assignments = $this->assignedUsersMapper->find($cardId); + $assignments = $this->assignedUsersMapper->findAll($cardId); foreach ($assignments as $assignment) { if ($assignment->getParticipant() === $userId && $assignment->getType() === $type) { throw new BadRequestException('The user is already assigned to the card'); @@ -179,7 +179,7 @@ class AssignmentService { throw new BadRequestException('user must be provided'); } - $assignments = $this->assignedUsersMapper->find($cardId); + $assignments = $this->assignedUsersMapper->findAll($cardId); foreach ($assignments as $assignment) { if ($assignment->getParticipant() === $userId && $assignment->getType() === $type) { $assignment = $this->assignedUsersMapper->delete($assignment); diff --git a/lib/Service/CardService.php b/lib/Service/CardService.php index 0e2d1ead7..eab133554 100644 --- a/lib/Service/CardService.php +++ b/lib/Service/CardService.php @@ -98,7 +98,7 @@ class CardService { public function enrich($card) { $cardId = $card->getId(); $this->cardMapper->mapOwner($card); - $card->setAssignedUsers($this->assignedUsersMapper->find($cardId)); + $card->setAssignedUsers($this->assignedUsersMapper->findAll($cardId)); $card->setLabels($this->labelMapper->findAssignedLabelsForCard($cardId)); $card->setAttachmentCount($this->attachmentService->count($cardId)); $user = $this->userManager->get($this->currentUser); @@ -136,7 +136,7 @@ class CardService { $this->permissionService->checkPermission($this->cardMapper, $cardId, Acl::PERMISSION_READ); $card = $this->cardMapper->find($cardId); - $assignedUsers = $this->assignedUsersMapper->find($card->getId()); + $assignedUsers = $this->assignedUsersMapper->findAll($card->getId()); $attachments = $this->attachmentService->findAll($cardId, true); $card->setAssignedUsers($assignedUsers); $card->setAttachments($attachments); diff --git a/lib/Service/OverviewService.php b/lib/Service/OverviewService.php index a5df16d69..6d514cd32 100644 --- a/lib/Service/OverviewService.php +++ b/lib/Service/OverviewService.php @@ -80,7 +80,7 @@ class OverviewService { $cardId = $card->getId(); $this->cardMapper->mapOwner($card); - $card->setAssignedUsers($this->assignedUsersMapper->find($cardId)); + $card->setAssignedUsers($this->assignedUsersMapper->findAll($cardId)); $card->setLabels($this->labelMapper->findAssignedLabelsForCard($cardId)); $card->setAttachmentCount($this->attachmentService->count($cardId)); diff --git a/lib/Service/StackService.php b/lib/Service/StackService.php index 8178d1423..b5dc22213 100644 --- a/lib/Service/StackService.php +++ b/lib/Service/StackService.php @@ -118,7 +118,7 @@ class StackService { $stack = $this->stackMapper->find($stackId); $cards = $this->cardMapper->findAll($stackId); foreach ($cards as $cardIndex => $card) { - $assignedUsers = $this->assignedUsersMapper->find($card->getId()); + $assignedUsers = $this->assignedUsersMapper->findAll($card->getId()); $card->setAssignedUsers($assignedUsers); $card->setAttachmentCount($this->attachmentService->count($card->getId())); } diff --git a/tests/integration/database/AssignmentMapperTest.php b/tests/integration/database/AssignmentMapperTest.php index e593a5d6b..56b53e27f 100644 --- a/tests/integration/database/AssignmentMapperTest.php +++ b/tests/integration/database/AssignmentMapperTest.php @@ -107,14 +107,14 @@ class AssignmentMapperTest extends \Test\TestCase { } /** - * @covers ::find + * @covers ::findAll */ public function testFind() { $uids = []; $this->assignmentService->assignUser($this->cards[0]->getId(), self::TEST_USER1); $this->assignmentService->assignUser($this->cards[0]->getId(), self::TEST_USER2); - $assignedUsers = $this->assignedUsersMapper->find($this->cards[0]->getId()); + $assignedUsers = $this->assignedUsersMapper->findAll($this->cards[0]->getId()); foreach ($assignedUsers as $user) { $uids[$user->getParticipant()] = $user; } @@ -152,7 +152,7 @@ class AssignmentMapperTest extends \Test\TestCase { $assignment->setType(Assignment::TYPE_USER); $this->assignedUsersMapper->insert($assignment); - $actual = $this->assignedUsersMapper->find($this->cards[1]->getId()); + $actual = $this->assignedUsersMapper->findAll($this->cards[1]->getId()); $this->assertEquals(1, count($actual)); $this->assertEquals($this->cards[1]->getId(), $actual[0]->getCardId()); $this->assertEquals(self::TEST_USER4, $actual[0]->getParticipant()); From f031717a543cfd041459de93af6a03e2eb829b9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Wed, 4 Nov 2020 19:52:16 +0100 Subject: [PATCH 4/5] Type hint IPermissionMapper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Db/AclMapper.php | 6 +++--- lib/Db/AssignmentMapper.php | 4 ++-- lib/Db/AttachmentMapper.php | 4 ++-- lib/Db/BoardMapper.php | 4 ++-- lib/Db/CardMapper.php | 4 ++-- lib/Db/IPermissionMapper.php | 4 ++-- lib/Db/LabelMapper.php | 4 ++-- lib/Db/StackMapper.php | 4 ++-- 8 files changed, 17 insertions(+), 17 deletions(-) diff --git a/lib/Db/AclMapper.php b/lib/Db/AclMapper.php index e0ed3c838..732fb54f8 100644 --- a/lib/Db/AclMapper.php +++ b/lib/Db/AclMapper.php @@ -35,19 +35,19 @@ class AclMapper extends DeckMapper implements IPermissionMapper { return $this->findEntities($sql, [$boardId], $limit, $offset); } - public function isOwner($userId, $aclId) { + public function isOwner($userId, $aclId): bool { $sql = 'SELECT owner FROM `*PREFIX*deck_boards` WHERE `id` IN (SELECT board_id FROM `*PREFIX*deck_board_acl` WHERE id = ?)'; $stmt = $this->execute($sql, [$aclId]); $row = $stmt->fetch(); return ($row['owner'] === $userId); } - public function findBoardId($aclId) { + public function findBoardId($aclId): ?int { $entity = $this->find($aclId); return $entity->getBoardId(); } - public function findByParticipant($type, $participant) { + public function findByParticipant($type, $participant): array { $sql = 'SELECT * from *PREFIX*deck_board_acl WHERE type = ? AND participant = ?'; return $this->findEntities($sql, [$type, $participant]); } diff --git a/lib/Db/AssignmentMapper.php b/lib/Db/AssignmentMapper.php index 0491cff1e..85a9cf197 100644 --- a/lib/Db/AssignmentMapper.php +++ b/lib/Db/AssignmentMapper.php @@ -79,11 +79,11 @@ class AssignmentMapper extends QBMapper implements IPermissionMapper { } - public function isOwner($userId, $cardId) { + public function isOwner($userId, $cardId): bool { return $this->cardMapper->isOwner($userId, $cardId); } - public function findBoardId($cardId) { + public function findBoardId($cardId): ?int { return $this->cardMapper->findBoardId($cardId); } diff --git a/lib/Db/AttachmentMapper.php b/lib/Db/AttachmentMapper.php index 69681b0c1..46e63f7c4 100644 --- a/lib/Db/AttachmentMapper.php +++ b/lib/Db/AttachmentMapper.php @@ -156,7 +156,7 @@ class AttachmentMapper extends DeckMapper implements IPermissionMapper { * @param $id int|string unique entity identifier * @return boolean */ - public function isOwner($userId, $id) { + public function isOwner($userId, $id): bool { try { $attachment = $this->find($id); return $this->cardMapper->isOwner($userId, $attachment->getCardId()); @@ -172,7 +172,7 @@ class AttachmentMapper extends DeckMapper implements IPermissionMapper { * @param $id int|string unique entity identifier * @return int|null id of Board */ - public function findBoardId($id) { + public function findBoardId($id): ?int { try { $attachment = $this->find($id); } catch (\Exception $e) { diff --git a/lib/Db/BoardMapper.php b/lib/Db/BoardMapper.php index 72ac5b4ec..1ef690b8c 100644 --- a/lib/Db/BoardMapper.php +++ b/lib/Db/BoardMapper.php @@ -220,12 +220,12 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { return parent::delete($entity); } - public function isOwner($userId, $boardId) { + public function isOwner($userId, $boardId): bool { $board = $this->find($boardId); return ($board->getOwner() === $userId); } - public function findBoardId($id) { + public function findBoardId($id): ?int { return $id; } diff --git a/lib/Db/CardMapper.php b/lib/Db/CardMapper.php index c8d8f4d2a..7bcf65741 100644 --- a/lib/Db/CardMapper.php +++ b/lib/Db/CardMapper.php @@ -308,7 +308,7 @@ class CardMapper extends QBMapper implements IPermissionMapper { $qb->execute(); } - public function isOwner($userId, $cardId) { + public function isOwner($userId, $cardId): bool { $sql = 'SELECT owner FROM `*PREFIX*deck_boards` WHERE `id` IN (SELECT board_id FROM `*PREFIX*deck_stacks` WHERE id IN (SELECT stack_id FROM `*PREFIX*deck_cards` WHERE id = ?))'; $stmt = $this->db->prepare($sql); $stmt->bindParam(1, $cardId, \PDO::PARAM_INT); @@ -317,7 +317,7 @@ class CardMapper extends QBMapper implements IPermissionMapper { return ($row['owner'] === $userId); } - public function findBoardId($cardId) { + public function findBoardId($cardId): ?int { $sql = 'SELECT id FROM `*PREFIX*deck_boards` WHERE `id` IN (SELECT board_id FROM `*PREFIX*deck_stacks` WHERE id IN (SELECT stack_id FROM `*PREFIX*deck_cards` WHERE id = ?))'; $stmt = $this->db->prepare($sql); $stmt->bindParam(1, $cardId, \PDO::PARAM_INT); diff --git a/lib/Db/IPermissionMapper.php b/lib/Db/IPermissionMapper.php index 2b94405b8..39a900253 100644 --- a/lib/Db/IPermissionMapper.php +++ b/lib/Db/IPermissionMapper.php @@ -33,7 +33,7 @@ interface IPermissionMapper { * @param $id int|string unique entity identifier * @return boolean */ - public function isOwner($userId, $id); + public function isOwner($userId, $id): bool; /** * Query boardId for Entity of given $id @@ -41,5 +41,5 @@ interface IPermissionMapper { * @param $id int|string unique entity identifier * @return int|null id of Board */ - public function findBoardId($id); + public function findBoardId($id): ?int; } diff --git a/lib/Db/LabelMapper.php b/lib/Db/LabelMapper.php index ec0b279a0..26f89bf3a 100644 --- a/lib/Db/LabelMapper.php +++ b/lib/Db/LabelMapper.php @@ -92,14 +92,14 @@ class LabelMapper extends DeckMapper implements IPermissionMapper { $stmt->execute(); } - public function isOwner($userId, $labelId) { + public function isOwner($userId, $labelId): bool { $sql = 'SELECT owner FROM `*PREFIX*deck_boards` WHERE `id` IN (SELECT board_id FROM `*PREFIX*deck_labels` WHERE id = ?)'; $stmt = $this->execute($sql, [$labelId]); $row = $stmt->fetch(); return ($row['owner'] === $userId); } - public function findBoardId($labelId) { + public function findBoardId($labelId): ?int { $entity = $this->find($labelId); return $entity->getBoardId(); } diff --git a/lib/Db/StackMapper.php b/lib/Db/StackMapper.php index 58b343929..88cffeeba 100644 --- a/lib/Db/StackMapper.php +++ b/lib/Db/StackMapper.php @@ -67,14 +67,14 @@ class StackMapper extends DeckMapper implements IPermissionMapper { return parent::delete($entity); } - public function isOwner($userId, $stackId) { + public function isOwner($userId, $stackId): bool { $sql = 'SELECT owner FROM `*PREFIX*deck_boards` WHERE `id` IN (SELECT board_id FROM `*PREFIX*deck_stacks` WHERE id = ?)'; $stmt = $this->execute($sql, [$stackId]); $row = $stmt->fetch(); return ($row['owner'] === $userId); } - public function findBoardId($stackId) { + public function findBoardId($stackId): ?int { $entity = $this->find($stackId); return $entity->getBoardId(); } From a461699d42a49c021de43d5984534adbe025041d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Tue, 10 Nov 2020 14:40:32 +0100 Subject: [PATCH 5/5] Adjust rename in tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Db/AssignmentMapper.php | 2 +- lib/Notification/NotificationHelper.php | 12 ++++++------ tests/integration/database/AssignmentMapperTest.php | 10 +++++----- tests/unit/Service/AssignmentServiceTest.php | 10 +++++----- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/lib/Db/AssignmentMapper.php b/lib/Db/AssignmentMapper.php index 85a9cf197..1abe144ec 100644 --- a/lib/Db/AssignmentMapper.php +++ b/lib/Db/AssignmentMapper.php @@ -114,7 +114,7 @@ class AssignmentMapper extends QBMapper implements IPermissionMapper { } public function isUserAssigned($cardId, $userId): bool { - $assignments = $this->find($cardId); + $assignments = $this->findAll($cardId); foreach ($assignments as $assignment) { $origin = $this->getOrigin($assignment); if ($origin instanceof User && $assignment->getParticipant() === $userId) { diff --git a/lib/Notification/NotificationHelper.php b/lib/Notification/NotificationHelper.php index 299e1332d..723923925 100644 --- a/lib/Notification/NotificationHelper.php +++ b/lib/Notification/NotificationHelper.php @@ -26,7 +26,7 @@ namespace OCA\Deck\Notification; use DateTime; use OCA\Deck\AppInfo\Application; use OCA\Deck\Db\Acl; -use OCA\Deck\Db\AssignedUsersMapper; +use OCA\Deck\Db\AssignmentMapper; use OCA\Deck\Db\Board; use OCA\Deck\Db\BoardMapper; use OCA\Deck\Db\CardMapper; @@ -44,8 +44,8 @@ class NotificationHelper { protected $cardMapper; /** @var BoardMapper */ protected $boardMapper; - /** @var AssignedUsersMapper */ - protected $assignedUsersMapper; + /** @var AssignmentMapper */ + protected $assignmentMapper; /** @var PermissionService */ protected $permissionService; /** @var IConfig */ @@ -62,7 +62,7 @@ class NotificationHelper { public function __construct( CardMapper $cardMapper, BoardMapper $boardMapper, - AssignedUsersMapper $assignedUsersMapper, + AssignmentMapper $assignmentMapper, PermissionService $permissionService, IConfig $config, IManager $notificationManager, @@ -71,7 +71,7 @@ class NotificationHelper { ) { $this->cardMapper = $cardMapper; $this->boardMapper = $boardMapper; - $this->assignedUsersMapper = $assignedUsersMapper; + $this->assignmentMapper = $assignmentMapper; $this->permissionService = $permissionService; $this->config = $config; $this->notificationManager = $notificationManager; @@ -107,7 +107,7 @@ class NotificationHelper { if ($user->getUID() === $board->getOwner() && count($board->getAcl()) === 0) { // Notify if all or assigned is configured for unshared boards $shouldNotify = true; - } elseif ($notificationSetting === ConfigService::SETTING_BOARD_NOTIFICATION_DUE_ASSIGNED && $this->assignedUsersMapper->isUserAssigned($card->getId(), $user->getUID())) { + } elseif ($notificationSetting === ConfigService::SETTING_BOARD_NOTIFICATION_DUE_ASSIGNED && $this->assignmentMapper->isUserAssigned($card->getId(), $user->getUID())) { // Notify if the user is assigned and has the assigned setting selected $shouldNotify = true; } diff --git a/tests/integration/database/AssignmentMapperTest.php b/tests/integration/database/AssignmentMapperTest.php index 56b53e27f..6999e1093 100644 --- a/tests/integration/database/AssignmentMapperTest.php +++ b/tests/integration/database/AssignmentMapperTest.php @@ -165,7 +165,7 @@ class AssignmentMapperTest extends \Test\TestCase { $assignment = new Assignment(); $assignment->setCardId($this->cards[1]->getId()); $assignment->setParticipant('invalid-username'); - $assignment->setType(AssignedUsers::TYPE_USER); + $assignment->setType(Assignment::TYPE_USER); $this->expectException(NotFoundException::class); $this->assignedUsersMapper->insert($assignment); } @@ -190,10 +190,10 @@ class AssignmentMapperTest extends \Test\TestCase { } public function testIsUserAssigned() { - $assignment = new AssignedUsers(); + $assignment = new Assignment(); $assignment->setCardId($this->cards[1]->getId()); $assignment->setParticipant(self::TEST_USER4); - $assignment->setType(AssignedUsers::TYPE_USER); + $assignment->setType(Assignment::TYPE_USER); $this->assertFalse($this->assignedUsersMapper->isUserAssigned($this->cards[1]->getId(), self::TEST_USER4)); $assignment = $this->assignedUsersMapper->insert($assignment); @@ -205,10 +205,10 @@ class AssignmentMapperTest extends \Test\TestCase { } public function testIsUserAssignedGroup() { - $assignment = new AssignedUsers(); + $assignment = new Assignment(); $assignment->setCardId($this->cards[1]->getId()); $assignment->setParticipant('group'); - $assignment->setType(AssignedUsers::TYPE_GROUP); + $assignment->setType(Assignment::TYPE_GROUP); $this->assertFalse($this->assignedUsersMapper->isUserAssigned($this->cards[1]->getId(), self::TEST_USER1)); $this->assertFalse($this->assignedUsersMapper->isUserAssigned($this->cards[1]->getId(), self::TEST_USER2)); $this->assertFalse($this->assignedUsersMapper->isUserAssigned($this->cards[1]->getId(), self::TEST_USER3)); diff --git a/tests/unit/Service/AssignmentServiceTest.php b/tests/unit/Service/AssignmentServiceTest.php index e12f3398e..56414c54a 100644 --- a/tests/unit/Service/AssignmentServiceTest.php +++ b/tests/unit/Service/AssignmentServiceTest.php @@ -115,7 +115,7 @@ class AssignmentServiceTest extends TestCase { public function testAssignUser() { $assignments = []; $this->assignedUsersMapper->expects($this->once()) - ->method('find') + ->method('findAll') ->with(123) ->willReturn($assignments); $assignment = new Assignment(); @@ -145,7 +145,7 @@ class AssignmentServiceTest extends TestCase { $this->expectExceptionMessage('The user is not part of the board'); $assignments = []; $this->assignedUsersMapper->expects($this->once()) - ->method('find') + ->method('findAll') ->with(123) ->willReturn($assignments); $assignment = new Assignment(); @@ -176,7 +176,7 @@ class AssignmentServiceTest extends TestCase { $assignment ]; $this->assignedUsersMapper->expects($this->once()) - ->method('find') + ->method('findAll') ->with(123) ->willReturn($assignments); $actual = $this->assignmentService->assignUser(123, 'admin'); @@ -192,7 +192,7 @@ class AssignmentServiceTest extends TestCase { $assignment ]; $this->assignedUsersMapper->expects($this->once()) - ->method('find') + ->method('findAll') ->with(123) ->willReturn($assignments); $this->assignedUsersMapper->expects($this->once()) @@ -213,7 +213,7 @@ class AssignmentServiceTest extends TestCase { $assignment ]; $this->assignedUsersMapper->expects($this->once()) - ->method('find') + ->method('findAll') ->with(123) ->willReturn($assignments); $this->expectException(NotFoundException::class);