diff --git a/lib/Service/AssignmentService.php b/lib/Service/AssignmentService.php index 4d9a97486d..f388771a7b 100644 --- a/lib/Service/AssignmentService.php +++ b/lib/Service/AssignmentService.php @@ -134,7 +134,12 @@ public function assignUser(int $cardId, string $userId, int $type = Assignment:: $this->changeHelper->cardChanged($cardId); $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $card, ActivityManager::SUBJECT_CARD_USER_ASSIGN, ['assigneduser' => $userId]); - $this->eventDispatcher->dispatchTyped(new CardUpdatedEvent($card)); + $cardBefore = clone $card; + $cardBefore->setAssignedUsers($assignments); + + $newAssignments = $this->assignedUsersMapper->findAll($cardId); + $card->setAssignedUsers($newAssignments); + $this->eventDispatcher->dispatchTyped(new CardUpdatedEvent($card, $cardBefore)); return $assignment; } @@ -161,8 +166,12 @@ public function unassignUser(int $cardId, string $userId, int $type = 0): Assign } $this->changeHelper->cardChanged($cardId); + $cardBefore = clone $card; + $cardBefore->setAssignedUsers($assignments); - $this->eventDispatcher->dispatchTyped(new CardUpdatedEvent($card)); + $newAssignments = $this->assignedUsersMapper->findAll($cardId); + $card->setAssignedUsers($newAssignments); + $this->eventDispatcher->dispatchTyped(new CardUpdatedEvent($card, $cardBefore)); return $assignment; } diff --git a/lib/Service/CardService.php b/lib/Service/CardService.php index 3b9f97d42b..53bd11201a 100644 --- a/lib/Service/CardService.php +++ b/lib/Service/CardService.php @@ -400,11 +400,11 @@ public function rename(int $id, string $title): Card { $changes = new ChangeSet($card); $card->setTitle($title); $this->changeHelper->cardChanged($card->getId(), false); - $update = $this->cardMapper->update($card); + $newCard = $this->cardMapper->update($card); - $this->eventDispatcher->dispatchTyped(new CardUpdatedEvent($card, $changes->getBefore())); + $this->eventDispatcher->dispatchTyped(new CardUpdatedEvent($newCard, $changes->getBefore())); - return $update; + return $newCard; } /** @@ -485,7 +485,7 @@ public function archive(int $id): Card { $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $newCard, ActivityManager::SUBJECT_CARD_UPDATE_ARCHIVE); $this->changeHelper->cardChanged($id, false); - $this->eventDispatcher->dispatchTyped(new CardUpdatedEvent($card, $changes->getBefore())); + $this->eventDispatcher->dispatchTyped(new CardUpdatedEvent($newCard, $changes->getBefore())); return $newCard; } @@ -512,7 +512,7 @@ public function unarchive(int $id): Card { $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $newCard, ActivityManager::SUBJECT_CARD_UPDATE_UNARCHIVE); $this->changeHelper->cardChanged($id, false); - $this->eventDispatcher->dispatchTyped(new CardUpdatedEvent($card, $changes->getBefore())); + $this->eventDispatcher->dispatchTyped(new CardUpdatedEvent($newCard, $changes->getBefore())); return $newCard; } @@ -537,7 +537,7 @@ public function done(int $id): Card { $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $newCard, ActivityManager::SUBJECT_CARD_UPDATE_DONE); $this->changeHelper->cardChanged($id, false); - $this->eventDispatcher->dispatchTyped(new CardUpdatedEvent($card, $changes->getBefore())); + $this->eventDispatcher->dispatchTyped(new CardUpdatedEvent($newCard, $changes->getBefore())); return $newCard; } @@ -563,7 +563,7 @@ public function undone(int $id): Card { $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $newCard, ActivityManager::SUBJECT_CARD_UPDATE_UNDONE); $this->changeHelper->cardChanged($id, false); - $this->eventDispatcher->dispatchTyped(new CardUpdatedEvent($card, $changes->getBefore())); + $this->eventDispatcher->dispatchTyped(new CardUpdatedEvent($newCard, $changes->getBefore())); return $newCard; } @@ -592,12 +592,13 @@ public function assignLabel(int $cardId, int $labelId): Card { if ($label->getBoardId() !== $this->cardMapper->findBoardId($card->getId())) { throw new StatusException('Operation not allowed. Label does not exist.'); } + $this->cardMapper->assignLabel($cardId, $labelId); $this->changeHelper->cardChanged($cardId); $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $card, ActivityManager::SUBJECT_LABEL_ASSIGN, ['label' => $label]); - $this->eventDispatcher->dispatchTyped(new CardUpdatedEvent($card)); - return $card; + $newCard = $this->cardMapper->find($cardId); + $this->eventDispatcher->dispatchTyped(new CardUpdatedEvent($newCard, $card)); } /** @@ -621,12 +622,13 @@ public function removeLabel(int $cardId, int $labelId): Card { throw new StatusException('Operation not allowed. This card is archived.'); } $label = $this->labelMapper->find($labelId); + $this->cardMapper->removeLabel($cardId, $labelId); $this->changeHelper->cardChanged($cardId); $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $card, ActivityManager::SUBJECT_LABEL_UNASSING, ['label' => $label]); - $this->eventDispatcher->dispatchTyped(new CardUpdatedEvent($card)); - return $card; + $newCard = $this->cardMapper->find($cardId); + $this->eventDispatcher->dispatchTyped(new CardUpdatedEvent($newCard, $card)); } public function getCardUrl(int $cardId): string { diff --git a/tests/unit/Service/AssignmentServiceTest.php b/tests/unit/Service/AssignmentServiceTest.php index bef8fa60d8..71b999a08e 100644 --- a/tests/unit/Service/AssignmentServiceTest.php +++ b/tests/unit/Service/AssignmentServiceTest.php @@ -31,6 +31,7 @@ use OCA\Deck\Db\AssignmentMapper; use OCA\Deck\Db\CardMapper; use OCA\Deck\Db\ChangeHelper; +use OCA\Deck\Event\CardUpdatedEvent; use OCA\Deck\NotFoundException; use OCA\Deck\Notification\NotificationHelper; use OCA\Deck\Validators\AssignmentServiceValidator; @@ -123,14 +124,14 @@ public function mockActivity($type, $object, $subject) { public function testAssignUser() { $assignments = []; - $this->assignedUsersMapper->expects($this->once()) - ->method('findAll') - ->with(123) - ->willReturn($assignments); $assignment = new Assignment(); $assignment->setCardId(123); $assignment->setParticipant('admin'); $assignment->setType(Assignment::TYPE_USER); + $this->assignedUsersMapper->expects($this->exactly(2)) + ->method('findAll') + ->with(123) + ->willReturn([], [$assignment]); $this->cardMapper->expects($this->once()) ->method('findBoardId') ->willReturn(1); @@ -145,6 +146,15 @@ public function testAssignUser() { ->method('insert') ->with($assignment) ->willReturn($assignment); + $this->eventDispatcher + ->expects($this->once()) + ->method('dispatchTyped') + ->with($this->callback(function ($event) use ($assignment) { + $this->assertInstanceOf(CardUpdatedEvent::class, $event); + $this->assertNotNull($event->getCard()); + $this->assertNotNull($event->getCardBefore()); + return true; + })); $actual = $this->assignmentService->assignUser(123, 'admin'); $this->assertEquals($assignment, $actual); } @@ -200,14 +210,23 @@ public function testUnassignUserExisting() { $assignments = [ $assignment ]; - $this->assignedUsersMapper->expects($this->once()) + $this->assignedUsersMapper->expects($this->exactly(2)) ->method('findAll') ->with(123) - ->willReturn($assignments); + ->willReturn($assignments, []); $this->assignedUsersMapper->expects($this->once()) ->method('delete') ->with($assignment) ->willReturn($assignment); + $this->eventDispatcher + ->expects($this->once()) + ->method('dispatchTyped') + ->with($this->callback(function ($event) { + $this->assertInstanceOf(CardUpdatedEvent::class, $event); + $this->assertNotNull($event->getCard()); + $this->assertNotNull($event->getCardBefore()); + return true; + })); $actual = $this->assignmentService->unassignUser(123, 'admin'); $this->assertEquals($assignment, $actual); } diff --git a/tests/unit/Service/CardServiceTest.php b/tests/unit/Service/CardServiceTest.php index 041c012ff5..e49f62e465 100644 --- a/tests/unit/Service/CardServiceTest.php +++ b/tests/unit/Service/CardServiceTest.php @@ -36,6 +36,7 @@ use OCA\Deck\Db\LabelMapper; use OCA\Deck\Db\Stack; use OCA\Deck\Db\StackMapper; +use OCA\Deck\Event\CardUpdatedEvent; use OCA\Deck\Model\CardDetails; use OCA\Deck\Notification\NotificationHelper; use OCA\Deck\StatusException; @@ -164,6 +165,23 @@ public function mockActivity($type, $object, $subject) { ->with($event); } + private function expectCardUpdatedEvent(?callable $assertions = null): void { + $this->eventDispatcher + ->expects($this->once()) + ->method('dispatchTyped') + ->with($this->callback(function ($event) use ($assertions) { + $this->assertInstanceOf(CardUpdatedEvent::class, $event); + $this->assertNotNull($event->getCard()); + $this->assertNotNull($event->getCardBefore()); + + if ($assertions !== null) { + $assertions($event); + } + + return true; + })); + } + public function testFind() { $user = $this->createMock(IUser::class); $this->userManager->expects($this->once()) @@ -259,9 +277,9 @@ public function testClone() { $this->cardMapper->expects($this->once()) ->method('update')->willReturn($clonedCard); - $this->cardMapper->expects($this->exactly(2)) + $this->cardMapper->expects($this->exactly(3)) ->method('find') - ->willReturn($card, $clonedCard); + ->willReturn($card, $clonedCard, $clonedCard); $this->cardMapper->expects($this->any()) ->method('findBoardId') @@ -340,6 +358,7 @@ public function testUpdate() { ->method('find') ->with(234) ->willReturn($stack); + $this->expectCardUpdatedEvent(); $actual = $this->cardService->update(123, 'newtitle', 234, 'text', 'admin', 'foo', 999, '2017-01-01 00:00:00', null); $this->assertEquals('newtitle', $actual->getTitle()); $this->assertEquals(234, $actual->getStackId()); @@ -367,6 +386,7 @@ public function testRename() { $this->cardMapper->expects($this->once())->method('update')->willReturnCallback(function ($c) { return $c; }); + $this->expectCardUpdatedEvent(); $actual = $this->cardService->rename(123, 'newtitle'); $this->assertEquals('newtitle', $actual->getTitle()); } @@ -396,6 +416,7 @@ public function testReorder($cardId, $newPosition, $order) { $card = new Card(); $card->setStackId(123); $this->cardMapper->expects($this->once())->method('find')->willReturn($card); + $this->expectCardUpdatedEvent(); $result = $this->cardService->reorder($cardId, 123, $newPosition); foreach ($result as $card) { $actual[$card->getOrder()] = $card->getId(); @@ -433,6 +454,7 @@ public function testArchive() { $this->cardMapper->expects($this->once())->method('update')->willReturnCallback(function ($c) { return $c; }); + $this->expectCardUpdatedEvent(); $this->assertTrue($this->cardService->archive(123)->getArchived()); } public function testUnarchive() { @@ -443,6 +465,7 @@ public function testUnarchive() { $this->cardMapper->expects($this->once())->method('update')->willReturnCallback(function ($c) { return $c; }); + $this->expectCardUpdatedEvent(); $this->assertFalse($this->cardService->unarchive(123)->getArchived()); } @@ -452,7 +475,7 @@ public function testAssignLabel() { $card->setId(123); $label = new Label(); $label->setBoardId(1); - $this->cardMapper->expects($this->once())->method('find')->willReturn($card); + $this->cardMapper->expects($this->exactly(2))->method('find')->willReturn($card, $card); $this->cardMapper->expects($this->once())->method('assignLabel'); $this->cardMapper->expects($this->once()) ->method('findBoardId') @@ -460,6 +483,7 @@ public function testAssignLabel() { $this->labelMapper->expects($this->once()) ->method('find') ->willReturn($label); + $this->expectCardUpdatedEvent(); $this->cardService->assignLabel(123, 999); } @@ -478,11 +502,12 @@ public function testRemoveLabel() { $card->setId(123); $label = new Label(); $label->setBoardId(1); - $this->cardMapper->expects($this->once())->method('find')->willReturn($card); + $this->cardMapper->expects($this->exactly(2))->method('find')->willReturn($card, $card); $this->cardMapper->expects($this->once())->method('removeLabel'); $this->labelMapper->expects($this->once()) ->method('find') ->willReturn($label); + $this->expectCardUpdatedEvent(); $this->cardService->removeLabel(123, 999); }