From 6fa7295b420dc7fececcbc0352ba07e685982024 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 19 Dec 2019 20:26:19 +0100 Subject: [PATCH 1/2] Limit card assignment to users who are participants of the board MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- docs/API.md | 27 +++++++++++++++++++++++++++ lib/Service/CardService.php | 9 ++++++++- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/docs/API.md b/docs/API.md index be877cd66..3ce5b9a15 100644 --- a/docs/API.md +++ b/docs/API.md @@ -651,6 +651,33 @@ The board list endpoint supports setting an `If-Modified-Since` header to limit ##### 200 Success +```json +{ + "id": 3, + "participant": { + "primaryKey": "admin", + "uid": "admin", + "displayname": "admin" + }, + "cardId": 1 +} +``` + +##### 400 Bad request + +```json +{ + "status": 400, + "message": "The user is already assigned to the card" +} +``` + +The request can fail with a bad request response for the following reasons: +- Missing or wrongly formatted request parameters +- The user is already assigned to the card +- The user is not part of the board + + ### PUT /boards/{boardId}/stacks/{stackId}/cards/{cardId}/unassignUser - Assign a user to a card #### Request parameters diff --git a/lib/Service/CardService.php b/lib/Service/CardService.php index 12b9d125e..44a3923ae 100644 --- a/lib/Service/CardService.php +++ b/lib/Service/CardService.php @@ -588,10 +588,17 @@ public function assignUser($cardId, $userId) { $assignments = $this->assignedUsersMapper->find($cardId); foreach ($assignments as $assignment) { if ($assignment->getParticipant() === $userId) { - return false; + throw new BadRequestException('The user is already assigned to the card'); } } + $card = $this->cardMapper->find($cardId); + $boardId = $this->cardMapper->findBoardId($cardId); + $boardUsers = array_keys($this->permissionService->findUsers($boardId)); + if (!in_array($userId, $boardUsers)) { + throw new BadRequestException('The user is not part of the board'); + } + if ($userId !== $this->currentUser) { /* Notifyuser about the card assignment */ From e5edd96b74e182f22d48350a8a1434156a94c296 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 19 Dec 2019 20:54:15 +0100 Subject: [PATCH 2/2] Fix tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- Makefile | 2 +- lib/Service/CardService.php | 2 +- lib/Service/PermissionService.php | 4 +- .../database/AssignedUsersMapperTest.php | 17 +- tests/unit/Service/CardServiceTest.php | 438 ++++++++++-------- 5 files changed, 240 insertions(+), 223 deletions(-) diff --git a/Makefile b/Makefile index f0021202e..29aa5327c 100644 --- a/Makefile +++ b/Makefile @@ -84,7 +84,7 @@ test-unit: ifeq (, $(shell which phpunit 2> /dev/null)) @echo "No phpunit command available, downloading a copy from the web" mkdir -p $(build_tools_directory) - curl -sSL https://phar.phpunit.de/phpunit-5.7.phar -o $(build_tools_directory)/phpunit.phar + curl -sSL https://phar.phpunit.de/phpunit-8.2.phar -o $(build_tools_directory)/phpunit.phar php $(build_tools_directory)/phpunit.phar -c tests/phpunit.xml --coverage-clover build/php-unit.coverage.xml php $(build_tools_directory)/phpunit.phar -c tests/phpunit.integration.xml --coverage-clover build/php-integration.coverage.xml else diff --git a/lib/Service/CardService.php b/lib/Service/CardService.php index 44a3923ae..ac845ec9d 100644 --- a/lib/Service/CardService.php +++ b/lib/Service/CardService.php @@ -594,7 +594,7 @@ public function assignUser($cardId, $userId) { $card = $this->cardMapper->find($cardId); $boardId = $this->cardMapper->findBoardId($cardId); - $boardUsers = array_keys($this->permissionService->findUsers($boardId)); + $boardUsers = array_keys($this->permissionService->findUsers($boardId, true)); if (!in_array($userId, $boardUsers)) { throw new BadRequestException('The user is not part of the board'); } diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php index 2a815fe01..6c6cf7b99 100644 --- a/lib/Service/PermissionService.php +++ b/lib/Service/PermissionService.php @@ -221,9 +221,9 @@ public function userCan(array $acls, $permission, $userId = null) { * @param $boardId * @return array */ - public function findUsers($boardId) { + public function findUsers($boardId, $refresh = false) { // cache users of a board so we don't query them for every cards - if (array_key_exists((string) $boardId, $this->users)) { + if (array_key_exists((string) $boardId, $this->users) && !$refresh) { return $this->users[(string) $boardId]; } try { diff --git a/tests/integration/database/AssignedUsersMapperTest.php b/tests/integration/database/AssignedUsersMapperTest.php index 71347101c..b5d86fee9 100644 --- a/tests/integration/database/AssignedUsersMapperTest.php +++ b/tests/integration/database/AssignedUsersMapperTest.php @@ -102,34 +102,25 @@ public function createBoardWithExampleData() { $this->stacks = $stacks; } - /** - * @covers ::__construct - */ - public function testConstructor() { - //$this->assertAttributeInstanceOf(IDBConnection::class, 'db', $this->assignedUsersMapper); - //$this->assertAttributeEquals(AssignedUsers::class, 'entityClass', $this->assignedUsersMapper); - //$this->assertAttributeEquals('*PREFIX*deck_assigned_users', 'tableName', $this->assignedUsersMapper); - } - /** * @covers ::find */ public function testFind() { $uids = []; $this->cardService->assignUser($this->cards[0]->getId(), self::TEST_USER1); - $this->cardService->assignUser($this->cards[0]->getId(), self::TEST_USER4); + $this->cardService->assignUser($this->cards[0]->getId(), self::TEST_USER2); $assignedUsers = $this->assignedUsersMapper->find($this->cards[0]->getId()); foreach ($assignedUsers as $user) { $uids[$user->getParticipant()] = $user; } $this->assertArrayHasKey(self::TEST_USER1, $uids); - $this->assertArrayNotHasKey(self::TEST_USER2, $uids); + $this->assertArrayHasKey(self::TEST_USER2, $uids); $this->assertArrayNotHasKey(self::TEST_USER3, $uids); - $this->assertArrayHasKey(self::TEST_USER4, $uids); + $this->assertArrayNotHasKey(self::TEST_USER4, $uids); $this->cardService->unassignUser($this->cards[0]->getId(), self::TEST_USER1); - $this->cardService->unassignUser($this->cards[0]->getId(), self::TEST_USER4); + $this->cardService->unassignUser($this->cards[0]->getId(), self::TEST_USER2); } /** diff --git a/tests/unit/Service/CardServiceTest.php b/tests/unit/Service/CardServiceTest.php index ebc85dba9..fad93f12c 100644 --- a/tests/unit/Service/CardServiceTest.php +++ b/tests/unit/Service/CardServiceTest.php @@ -25,6 +25,7 @@ use OCA\Deck\Activity\ActivityManager; +use OCA\Deck\BadRequestException; use OCA\Deck\Db\AssignedUsers; use OCA\Deck\Db\AssignedUsersMapper; use OCA\Deck\Db\Card; @@ -46,22 +47,22 @@ class CardServiceTest extends TestCase { - /** @var CardService|MockObject */ - private $cardService; - /** @var CardMapper|MockObject */ - private $cardMapper; - /** @var StackMapper|MockObject */ - private $stackMapper; - /** @var PermissionService|MockObject */ - private $permissionService; - /** @var NotificationHelper */ - private $notificationHelper; - /** @var AssignedUsersMapper|MockObject */ - private $assignedUsersMapper; - /** @var BoardService|MockObject */ - private $boardService; - /** @var LabelMapper|MockObject */ - private $labelMapper; + /** @var CardService|MockObject */ + private $cardService; + /** @var CardMapper|MockObject */ + private $cardMapper; + /** @var StackMapper|MockObject */ + private $stackMapper; + /** @var PermissionService|MockObject */ + private $permissionService; + /** @var NotificationHelper */ + private $notificationHelper; + /** @var AssignedUsersMapper|MockObject */ + private $assignedUsersMapper; + /** @var BoardService|MockObject */ + private $boardService; + /** @var LabelMapper|MockObject */ + private $labelMapper; private $boardMapper; /** @var AttachmentService|MockObject */ private $attachmentService; @@ -77,15 +78,15 @@ class CardServiceTest extends TestCase { private $changeHelper; public function setUp(): void { - parent::setUp(); - $this->cardMapper = $this->createMock(CardMapper::class); - $this->stackMapper = $this->createMock(StackMapper::class); - $this->boardMapper = $this->createMock(BoardMapper::class); - $this->labelMapper = $this->createMock(LabelMapper::class); - $this->permissionService = $this->createMock(PermissionService::class); - $this->boardService = $this->createMock(BoardService::class); - $this->notificationHelper = $this->createMock(NotificationHelper::class); - $this->assignedUsersMapper = $this->createMock(AssignedUsersMapper::class); + parent::setUp(); + $this->cardMapper = $this->createMock(CardMapper::class); + $this->stackMapper = $this->createMock(StackMapper::class); + $this->boardMapper = $this->createMock(BoardMapper::class); + $this->labelMapper = $this->createMock(LabelMapper::class); + $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->attachmentService = $this->createMock(AttachmentService::class); $this->activityManager = $this->createMock(ActivityManager::class); $this->commentsManager = $this->createMock(ICommentsManager::class); @@ -108,10 +109,10 @@ public function setUp(): void { $this->changeHelper, $this->eventDispatcher, 'user1' - ); - } + ); + } - public function mockActivity($type, $object, $subject) { + public function mockActivity($type, $object, $subject) { // ActivityManager::DECK_OBJECT_BOARD, $newAcl, ActivityManager::SUBJECT_BOARD_SHARE $event = $this->createMock(IEvent::class); $this->activityManager->expects($this->once()) @@ -123,214 +124,244 @@ public function mockActivity($type, $object, $subject) { ->with($event); } - public function testFind() { + public function testFind() { $user = $this->createMock(IUser::class); $this->userManager->expects($this->once()) ->method('get') ->willReturn($user); - $card = new Card(); - $card->setId(1337); - $this->cardMapper->expects($this->any()) - ->method('find') - ->with(123) - ->willReturn($card); - $this->assignedUsersMapper->expects($this->any()) + $card = new Card(); + $card->setId(1337); + $this->cardMapper->expects($this->any()) + ->method('find') + ->with(123) + ->willReturn($card); + $this->assignedUsersMapper->expects($this->any()) ->method('find') ->with(1337) ->willReturn(['user1', 'user2']); $cardExpected = new Card(); $cardExpected->setId(1337); $cardExpected->setAssignedUsers(['user1', 'user2']); - $this->assertEquals($cardExpected, $this->cardService->find(123)); - } + $this->assertEquals($cardExpected, $this->cardService->find(123)); + } - public function testCreate() { - $card = new Card(); - $card->setTitle('Card title'); - $card->setOwner('admin'); - $card->setStackId(123); - $card->setOrder(999); - $card->setType('text'); - $this->cardMapper->expects($this->once()) - ->method('insert') - ->willReturn($card); - $b = $this->cardService->create('Card title', 123, 'text', 999, 'admin'); + public function testCreate() { + $card = new Card(); + $card->setTitle('Card title'); + $card->setOwner('admin'); + $card->setStackId(123); + $card->setOrder(999); + $card->setType('text'); + $this->cardMapper->expects($this->once()) + ->method('insert') + ->willReturn($card); + $b = $this->cardService->create('Card title', 123, 'text', 999, 'admin'); - $this->assertEquals($b->getTitle(), 'Card title'); - $this->assertEquals($b->getOwner(), 'admin'); - $this->assertEquals($b->getType(), 'text'); - $this->assertEquals($b->getOrder(), 999); - $this->assertEquals($b->getStackId(), 123); - } + $this->assertEquals($b->getTitle(), 'Card title'); + $this->assertEquals($b->getOwner(), 'admin'); + $this->assertEquals($b->getType(), 'text'); + $this->assertEquals($b->getOrder(), 999); + $this->assertEquals($b->getStackId(), 123); + } - public function testDelete() { - $cardToBeDeleted = new Card(); - $this->cardMapper->expects($this->once()) - ->method('find') - ->willReturn($cardToBeDeleted); - $this->cardMapper->expects($this->once()) - ->method('update') - ->willReturn($cardToBeDeleted); - $this->cardService->delete(123); - $this->assertTrue($cardToBeDeleted->getDeletedAt() <= time(), 'deletedAt is in the past'); - } + public function testDelete() { + $cardToBeDeleted = new Card(); + $this->cardMapper->expects($this->once()) + ->method('find') + ->willReturn($cardToBeDeleted); + $this->cardMapper->expects($this->once()) + ->method('update') + ->willReturn($cardToBeDeleted); + $this->cardService->delete(123); + $this->assertTrue($cardToBeDeleted->getDeletedAt() <= time(), 'deletedAt is in the past'); + } - public function testUpdate() { - $card = new Card(); - $card->setTitle('title'); - $card->setArchived(false); - $this->cardMapper->expects($this->once())->method('find')->willReturn($card); - $this->cardMapper->expects($this->once())->method('update')->willReturnCallback(function($c) { return $c; }); - $actual = $this->cardService->update(123, 'newtitle', 234, 'text', 999, 'foo', 'admin', '2017-01-01 00:00:00', null); - $this->assertEquals('newtitle', $actual->getTitle()); - $this->assertEquals(234, $actual->getStackId()); - $this->assertEquals('text', $actual->getType()); - $this->assertEquals(999, $actual->getOrder()); - $this->assertEquals('foo', $actual->getDescription()); - $this->assertEquals('2017-01-01T00:00:00+00:00', $actual->getDuedate()); - } + public function testUpdate() { + $card = new Card(); + $card->setTitle('title'); + $card->setArchived(false); + $this->cardMapper->expects($this->once())->method('find')->willReturn($card); + $this->cardMapper->expects($this->once())->method('update')->willReturnCallback(function($c) { return $c; }); + $actual = $this->cardService->update(123, 'newtitle', 234, 'text', 999, 'foo', 'admin', '2017-01-01 00:00:00', null); + $this->assertEquals('newtitle', $actual->getTitle()); + $this->assertEquals(234, $actual->getStackId()); + $this->assertEquals('text', $actual->getType()); + $this->assertEquals(999, $actual->getOrder()); + $this->assertEquals('foo', $actual->getDescription()); + $this->assertEquals('2017-01-01T00:00:00+00:00', $actual->getDuedate()); + } - public function testUpdateArchived() { - $card = new Card(); - $card->setTitle('title'); - $card->setArchived(true); - $this->cardMapper->expects($this->once())->method('find')->willReturn($card); - $this->cardMapper->expects($this->never())->method('update'); + public function testUpdateArchived() { + $card = new Card(); + $card->setTitle('title'); + $card->setArchived(true); + $this->cardMapper->expects($this->once())->method('find')->willReturn($card); + $this->cardMapper->expects($this->never())->method('update'); $this->expectException(StatusException::class); - $this->cardService->update(123, 'newtitle', 234, 'text', 999, 'foo', 'admin', '2017-01-01 00:00:00', null, true); - } + $this->cardService->update(123, 'newtitle', 234, 'text', 999, 'foo', 'admin', '2017-01-01 00:00:00', null, true); + } - public function testRename() { - $card = new Card(); - $card->setTitle('title'); - $card->setArchived(false); - $this->cardMapper->expects($this->once())->method('find')->willReturn($card); - $this->cardMapper->expects($this->once())->method('update')->willReturnCallback(function($c) { return $c; }); - $actual = $this->cardService->rename(123, 'newtitle'); - $this->assertEquals('newtitle', $actual->getTitle()); - } + public function testRename() { + $card = new Card(); + $card->setTitle('title'); + $card->setArchived(false); + $this->cardMapper->expects($this->once())->method('find')->willReturn($card); + $this->cardMapper->expects($this->once())->method('update')->willReturnCallback(function($c) { return $c; }); + $actual = $this->cardService->rename(123, 'newtitle'); + $this->assertEquals('newtitle', $actual->getTitle()); + } - public function testRenameArchived() { - $card = new Card(); - $card->setTitle('title'); - $card->setArchived(true); - $this->cardMapper->expects($this->once())->method('find')->willReturn($card); - $this->cardMapper->expects($this->never())->method('update'); - $this->expectException(StatusException::class); - $this->cardService->rename(123, 'newtitle'); - } + public function testRenameArchived() { + $card = new Card(); + $card->setTitle('title'); + $card->setArchived(true); + $this->cardMapper->expects($this->once())->method('find')->willReturn($card); + $this->cardMapper->expects($this->never())->method('update'); + $this->expectException(StatusException::class); + $this->cardService->rename(123, 'newtitle'); + } - public function dataReorder() { - return [ - [0, 0, [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]], - [0, 9, [1, 2, 3, 4, 5, 6, 7, 8, 9, 0]], - [1, 3, [0, 2, 3, 1, 4, 5, 6, 7, 8, 9]] - ]; - } - /** @dataProvider dataReorder */ - public function testReorder($cardId, $newPosition, $order) { - $cards = $this->getCards(); - $cardsTmp = []; - $this->cardMapper->expects($this->at(0))->method('findAll')->willReturn($cards); - $result = $this->cardService->reorder($cardId, 123, $newPosition); - foreach ($result as $card) { - $actual[$card->getOrder()] = $card->getId(); - } - $this->assertEquals($order, $actual); - } + public function dataReorder() { + return [ + [0, 0, [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]], + [0, 9, [1, 2, 3, 4, 5, 6, 7, 8, 9, 0]], + [1, 3, [0, 2, 3, 1, 4, 5, 6, 7, 8, 9]] + ]; + } + /** @dataProvider dataReorder */ + public function testReorder($cardId, $newPosition, $order) { + $cards = $this->getCards(); + $cardsTmp = []; + $this->cardMapper->expects($this->at(0))->method('findAll')->willReturn($cards); + $result = $this->cardService->reorder($cardId, 123, $newPosition); + foreach ($result as $card) { + $actual[$card->getOrder()] = $card->getId(); + } + $this->assertEquals($order, $actual); + } - private function getCards() { - $cards = []; - for($i=0; $i<10; $i++) { - $cards[$i] = new Card(); - $cards[$i]->setTitle($i); - $cards[$i]->setOrder($i); - $cards[$i]->setId($i); - } - return $cards; - } + private function getCards() { + $cards = []; + for($i=0; $i<10; $i++) { + $cards[$i] = new Card(); + $cards[$i]->setTitle($i); + $cards[$i]->setOrder($i); + $cards[$i]->setId($i); + } + return $cards; + } - public function testReorderArchived() { - $card = new Card(); - $card->setTitle('title'); - $card->setArchived(true); - $this->cardMapper->expects($this->once())->method('findAll')->willReturn([$card]); - $this->cardMapper->expects($this->never())->method('update')->willReturnCallback(function($c) { return $c; }); - $this->expectException(StatusException::class); - $actual = $this->cardService->reorder(123, 234, 1); - } - public function testArchive() { - $card = new Card(); - $this->assertFalse($card->getArchived()); - $this->cardMapper->expects($this->once())->method('find')->willReturn($card); - $this->cardMapper->expects($this->once())->method('update')->willReturnCallback(function($c) { - return $c; - }); - $this->assertTrue($this->cardService->archive(123)->getArchived()); - } - public function testUnarchive() { - $card = new Card(); - $card->setArchived(true); - $this->assertTrue($card->getArchived()); - $this->cardMapper->expects($this->once())->method('find')->willReturn($card); - $this->cardMapper->expects($this->once())->method('update')->willReturnCallback(function($c) { - return $c; - }); - $this->assertFalse($this->cardService->unarchive(123)->getArchived()); - } + public function testReorderArchived() { + $card = new Card(); + $card->setTitle('title'); + $card->setArchived(true); + $this->cardMapper->expects($this->once())->method('findAll')->willReturn([$card]); + $this->cardMapper->expects($this->never())->method('update')->willReturnCallback(function($c) { return $c; }); + $this->expectException(StatusException::class); + $actual = $this->cardService->reorder(123, 234, 1); + } + public function testArchive() { + $card = new Card(); + $this->assertFalse($card->getArchived()); + $this->cardMapper->expects($this->once())->method('find')->willReturn($card); + $this->cardMapper->expects($this->once())->method('update')->willReturnCallback(function($c) { + return $c; + }); + $this->assertTrue($this->cardService->archive(123)->getArchived()); + } + public function testUnarchive() { + $card = new Card(); + $card->setArchived(true); + $this->assertTrue($card->getArchived()); + $this->cardMapper->expects($this->once())->method('find')->willReturn($card); + $this->cardMapper->expects($this->once())->method('update')->willReturnCallback(function($c) { + return $c; + }); + $this->assertFalse($this->cardService->unarchive(123)->getArchived()); + } - public function testAssignLabel() { - $card = new Card(); - $card->setArchived(false); - $this->cardMapper->expects($this->once())->method('find')->willReturn($card); - $this->cardMapper->expects($this->once())->method('assignLabel'); - $this->cardService->assignLabel(123, 999); - } + public function testAssignLabel() { + $card = new Card(); + $card->setArchived(false); + $this->cardMapper->expects($this->once())->method('find')->willReturn($card); + $this->cardMapper->expects($this->once())->method('assignLabel'); + $this->cardService->assignLabel(123, 999); + } - public function testAssignLabelArchived() { - $card = new Card(); - $card->setArchived(true); - $this->cardMapper->expects($this->once())->method('find')->willReturn($card); - $this->cardMapper->expects($this->never())->method('assignLabel'); - $this->expectException(StatusException::class); - $this->cardService->assignLabel(123, 999); - } + public function testAssignLabelArchived() { + $card = new Card(); + $card->setArchived(true); + $this->cardMapper->expects($this->once())->method('find')->willReturn($card); + $this->cardMapper->expects($this->never())->method('assignLabel'); + $this->expectException(StatusException::class); + $this->cardService->assignLabel(123, 999); + } - public function testRemoveLabel() { - $card = new Card(); - $card->setArchived(false); - $this->cardMapper->expects($this->once())->method('find')->willReturn($card); - $this->cardMapper->expects($this->once())->method('removeLabel'); - $this->cardService->removeLabel(123, 999); - } + public function testRemoveLabel() { + $card = new Card(); + $card->setArchived(false); + $this->cardMapper->expects($this->once())->method('find')->willReturn($card); + $this->cardMapper->expects($this->once())->method('removeLabel'); + $this->cardService->removeLabel(123, 999); + } - public function testRemoveLabelArchived() { - $card = new Card(); - $card->setArchived(true); - $this->cardMapper->expects($this->once())->method('find')->willReturn($card); - $this->cardMapper->expects($this->never())->method('removeLabel'); + public function testRemoveLabelArchived() { + $card = new Card(); + $card->setArchived(true); + $this->cardMapper->expects($this->once())->method('find')->willReturn($card); + $this->cardMapper->expects($this->never())->method('removeLabel'); $this->expectException(StatusException::class); - $this->cardService->removeLabel(123, 999); - } + $this->cardService->removeLabel(123, 999); + } - public function testAssignUser() { - $assignments = []; - $this->assignedUsersMapper->expects($this->once()) + public function testAssignUser() { + $assignments = []; + $this->assignedUsersMapper->expects($this->once()) ->method('find') ->with(123) ->willReturn($assignments); - $assignment = new AssignedUsers(); - $assignment->setCardId(123); - $assignment->setParticipant('admin'); - $this->assignedUsersMapper->expects($this->once()) + $assignment = new AssignedUsers(); + $assignment->setCardId(123); + $assignment->setParticipant('admin'); + $this->cardMapper->expects($this->once()) + ->method('findBoardId') + ->willReturn(1); + $this->permissionService->expects($this->once()) + ->method('findUsers') + ->with(1) + ->willReturn(['admin' => 'admin', 'user1' => 'user1']); + $this->assignedUsersMapper->expects($this->once()) ->method('insert') ->with($assignment) ->willReturn($assignment); - $actual = $this->cardService->assignUser(123, 'admin'); - $this->assertEquals($assignment, $actual); + $actual = $this->cardService->assignUser(123, 'admin'); + $this->assertEquals($assignment, $actual); + } + + public function testAssignUserNoParticipant() { + $this->expectException(BadRequestException::class); + $this->expectExceptionMessage('The user is not part of the board'); + $assignments = []; + $this->assignedUsersMapper->expects($this->once()) + ->method('find') + ->with(123) + ->willReturn($assignments); + $assignment = new AssignedUsers(); + $assignment->setCardId(123); + $assignment->setParticipant('admin'); + $this->cardMapper->expects($this->once()) + ->method('findBoardId') + ->willReturn(1); + $this->permissionService->expects($this->once()) + ->method('findUsers') + ->with(1) + ->willReturn(['user2' => 'user2', 'user1' => 'user1']); + $actual = $this->cardService->assignUser(123, 'admin'); } public function testAssignUserExisting() { + $this->expectException(BadRequestException::class); + $this->expectExceptionMessage('The user is already assigned to the card'); $assignment = new AssignedUsers(); $assignment->setCardId(123); $assignment->setParticipant('admin'); @@ -364,13 +395,8 @@ public function testUnassignUserExisting() { $this->assertEquals($assignment, $actual); } - /** - * @expectException \OCA\Deck\NotFoundException - * - * - * - */ public function testUnassignUserNotExisting() { + $this->expectException(NotFoundException::class); $assignment = new AssignedUsers(); $assignment->setCardId(123); $assignment->setParticipant('admin');