Card,Stack undo delete: CardServiceTest fix

Signed-off-by: Manuel Arno Korfmann <manu@korfmann.info>

Stack,Card undo delete: Test Fix 2

Signed-off-by: Manuel Arno Korfmann <manu@korfmann.info>

Card, Stack undo delete: Test fix 3

Signed-off-by: Manuel Arno Korfmann <manu@korfmann.info>

Card,Stack undo delete: Test fix 4

Signed-off-by: Manuel Arno Korfmann <manu@korfmann.info>

Stack, Card undo delete: Relative time in deleted entity listings

Signed-off-by: Manuel Arno Korfmann <manu@korfmann.info>

Card, Stack undo delete: Test Fix 5

Signed-off-by: Manuel Arno Korfmann <manu@korfmann.info>

Test Fix 6

Signed-off-by: Manuel Arno Korfmann <manu@korfmann.info>

Test Fix 7

Signed-off-by: Manuel Arno Korfmann <manu@korfmann.info>

fix codacy

Signed-off-by: Manuel Arno Korfmann <manu@korfmann.info>

js/controller/BoardController.js

@@ -228,7 +228,7 @@ app.controller('BoardController', function ($rootScope, $scope, $stateParams, St
 				}
 			}
 		);
-	}
+	};
 
 	$scope._cardAndStackUndoDelete = function(deletedCard, associatedDeletedStack) {
 		$scope.stackUndoDelete(associatedDeletedStack).then(function() {

js/service/ApiService.js

@@ -41,7 +41,9 @@ app.factory('ApiService', function ($http, $q) {
 
 	ApiService.prototype.tryAllThenDeleted = function(id) {
 		let object = this.data[id];
-		if (object === undefined) object = this.deleted[id];
+		if (object === undefined) {
+			object = this.deleted[id];
+		} 
 		return object;
 	};
 

lib/Db/LabelMapper.php

@@ -54,6 +54,22 @@ class LabelMapper extends DeckMapper implements IPermissionMapper {
 		return $this->findEntities($sql, [$boardId], $limit, $offset);
 	}
 
+	public function liveOrMemoizedLabelsForBoardId($boardId) {
+		if(is_null($boardId)) {
+			return array();
+		}
+
+		if(!isset($this->memoizedLabelsByBoardId)) {
+			$this->memoizedLabelsByBoardId = array();
+		}
+
+		if(!array_key_exists($boardId, $this->memoizedLabelsByBoardId)) {
+			$this->memoizedLabelsByBoardId[$boardId] = $this->getAssignedLabelsForBoard($boardId);
+		}
+
+		return $this->memoizedLabelsByBoardId[$boardId];
+	}
+
 	public function getAssignedLabelsForBoard($boardId) {
 		$labels = $this->findAssignedLabelsForBoard($boardId);
 		$result = array();

lib/Service/CardService.php

@@ -53,8 +53,11 @@ class CardService {
 		BoardMapper $boardMapper, 
 		PermissionService $permissionService, 
 		BoardService $boardService,
+		NotificationHelper $notificationHelper,
 		AssignedUsersMapper $assignedUsersMapper, 
-		AttachmentService $attachmentService) {
+		AttachmentService $attachmentService,
+		$userId
+	) {
 		$this->cardMapper = $cardMapper;
 		$this->stackMapper = $stackMapper;
 		$this->boardMapper = $boardMapper;

lib/Service/StackService.php

@@ -68,6 +68,12 @@ class StackService {
 
 	private function enrichStackWithCards($stack) {
 		$cards = $this->cardMapper->findAll($stack->id);
+
+		if(is_null($cards)) {
+			return;
+		}
+
+		$labels = $this->labelMapper->liveOrMemoizedLabelsForBoardId($stack->getBoardId());
 		foreach ($cards as $cardIndex => $card) {
 			$assignedUsers = $this->assignedUsersMapper->find($card->getId());
 			$card->setAssignedUsers($assignedUsers);
@@ -76,11 +82,12 @@ class StackService {
 			}
 			$card->setAttachmentCount($this->attachmentService->count($card->getId()));
 		}
+
 		$stack->setCards($cards);
 	}
 
 	private function enrichStacksWithCards($stacks) {
-		foreach ($stacks as $stackIndex => $stack) {
+		foreach ($stacks as $stack) {
 			$this->enrichStackWithCards($stack);
 		}
 	}
@@ -88,7 +95,6 @@ class StackService {
 	public function findAll($boardId) {
 		$this->permissionService->checkPermission(null, $boardId, Acl::PERMISSION_READ);
 		$stacks = $this->stackMapper->findAll($boardId);
-		$labels = $this->labelMapper->getAssignedLabelsForBoard($boardId);
 		$this->enrichStacksWithCards($stacks);
 		return $stacks;
 	}

templates/part.board.sidebarView.php

@@ -126,7 +126,7 @@
     	   <li class='board-detail__deleted-list__item' ng-repeat="deletedStack in stackservice.deleted">
            <span class="icon icon-deck"></span>
            <span>{{deletedStack.title}}</span>
-    			 <span>{{deletedStack.deletedAt}}</span>
+    			 <span>{{deletedStack.deletedAt | relativeDateFilter }}</span>
            <a ng-click="stackUndoDelete(deletedStack)">
              <span class="icon icon-history"></span>
            </a>
@@ -140,7 +140,7 @@
             <span class="icon icon-deck"></span>
     			  <span>{{deletedCard.title}}</span>
     			  <span>{{stackservice.tryAllThenDeleted(deletedCard.stackId).title}}</span>
-    			  <span>{{deletedCard.deletedAt}}</span>
+    			  <span>{{deletedCard.deletedAt | relativeDateFilter }}</span>
         	  <a ng-click="cardUndoDelete(deletedCard)">
               <span class="icon icon-history"></span>
             </a>

tests/unit/Db/CardTest.php

@@ -81,6 +81,7 @@ class CardTest extends TestCase {
 			'attachments' => null,
 			'attachmentCount' => null,
 			'assignedUsers' => null,
+			'deletedAt' => 0
 		], $card->jsonSerialize());
 	}
 	public function testJsonSerializeLabels() {
@@ -103,6 +104,7 @@ class CardTest extends TestCase {
 			'attachments' => null,
 			'attachmentCount' => null,
 			'assignedUsers' => null,
+			'deletedAt' => 0
 		], $card->jsonSerialize());
 	}
 
@@ -135,6 +137,7 @@ class CardTest extends TestCase {
 			'attachments' => null,
 			'attachmentCount' => null,
 			'assignedUsers' => ['user1'],
+			'deletedAt' => 0
 		], $card->jsonSerialize());
 	}
 

tests/unit/Db/StackTest.php

@@ -39,6 +39,7 @@ class StackTest extends \Test\TestCase {
 			'title' => "My Stack",
 			'order' => 1,
 			'boardId' => 1,
+			'deletedAt' => 0
 		], $board->jsonSerialize());
 	}
 	public function testJsonSerializeWithCards() {
@@ -51,6 +52,7 @@ class StackTest extends \Test\TestCase {
 			'order' => 1,
 			'boardId' => 1,
 			'cards' => array("foo", "bar"),
+			'deletedAt' => 0
 		], $board->jsonSerialize());
 	}
 }

tests/unit/Service/CardServiceTest.php

@@ -29,6 +29,7 @@ use OCA\Deck\Db\AssignedUsersMapper;
 use OCA\Deck\Db\Card;
 use OCA\Deck\Db\CardMapper;
 use OCA\Deck\Db\StackMapper;
+use OCA\Deck\Db\BoardMapper;
 use OCA\Deck\NotFoundException;
 use OCA\Deck\Notification\NotificationHelper;
 use OCA\Deck\StatusException;
@@ -55,12 +56,13 @@ class CardServiceTest extends TestCase {
 		parent::setUp();
         $this->cardMapper = $this->createMock(CardMapper::class);
         $this->stackMapper = $this->createMock(StackMapper::class);
+        $this->boardMapper = $this->createMock(BoardMapper::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->cardService = new CardService($this->cardMapper, $this->stackMapper, $this->permissionService, $this->boardService, $this->notificationHelper, $this->assignedUsersMapper, $this->attachmentService, 'userXY');
+        $this->cardService = new CardService($this->cardMapper, $this->stackMapper, $this->boardMapper, $this->permissionService, $this->boardService, $this->notificationHelper, $this->assignedUsersMapper, $this->attachmentService, 'userXY');
     }
 
     public function testFind() {
@@ -100,13 +102,15 @@ class CardServiceTest extends TestCase {
     }
 
     public function testDelete() {
+        $cardToBeDeleted = new Card();
         $this->cardMapper->expects($this->once())
             ->method('find')
-            ->willReturn(new Card());
+            ->willReturn($cardToBeDeleted);
         $this->cardMapper->expects($this->once())
-            ->method('delete')
+            ->method('update')
-            ->willReturn(1);
+            ->willReturn($cardToBeDeleted);
-        $this->assertEquals(1, $this->cardService->delete(123));
+        $this->cardService->delete(123);
+        $this->assertTrue($cardToBeDeleted->getDeletedAt() <= time(), 'deletedAt is in the past');
     }
 
     public function testUpdate() {
@@ -115,7 +119,7 @@ class CardServiceTest extends TestCase {
         $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');
+        $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());
@@ -131,7 +135,7 @@ class CardServiceTest extends TestCase {
         $this->cardMapper->expects($this->once())->method('find')->willReturn($card);
         $this->cardMapper->expects($this->never())->method('update');
         $this->setExpectedException(StatusException::class);
-        $this->cardService->update(123, 'newtitle', 234, 'text', 999, 'foo', 'admin', '2017-01-01 00:00:00');
+        $this->cardService->update(123, 'newtitle', 234, 'text', 999, 'foo', 'admin', '2017-01-01 00:00:00', null);
     }
 
     public function testRename() {

tests/unit/Service/StackServiceTest.php

@@ -28,6 +28,7 @@ namespace OCA\Deck\Service;
 use OCA\Deck\Db\AssignedUsersMapper;
 use OCA\Deck\Db\Card;
 use OCA\Deck\Db\CardMapper;
+use OCA\Deck\Db\BoardMapper;
 use OCA\Deck\Db\Label;
 use OCA\Deck\Db\LabelMapper;
 use OCA\Deck\Db\Stack;
@@ -48,6 +49,8 @@ class StackServiceTest extends TestCase {
 	private $stackMapper;
     /** @var \PHPUnit\Framework\MockObject\MockObject|CardMapper */
 	private $cardMapper;
+    /** @var \PHPUnit\Framework\MockObject\MockObject|BoardMapper */
+	private $boardMapper;
     /** @var \PHPUnit\Framework\MockObject\MockObject|LabelMapper */
 	private $labelMapper;
     /** @var \PHPUnit\Framework\MockObject\MockObject|PermissionService */
@@ -63,16 +66,23 @@ class StackServiceTest extends TestCase {
 		parent::setUp();
 		$this->stackMapper = $this->createMock(StackMapper::class);
 		$this->cardMapper = $this->createMock(CardMapper::class);
-		$this->labelMapper = $this->createMock(LabelMapper::class);
+		$this->boardMapper = $this->createMock(BoardMapper::class);
 		$this->permissionService = $this->createMock(PermissionService::class);
 		$this->boardService = $this->createMock(BoardService::class);
 		$this->assignedUsersMapper = $this->createMock(AssignedUsersMapper::class);
 		$this->attachmentService = $this->createMock(AttachmentService::class);
 
+		$this->labelMapper = $this->getMockBuilder(LabelMapper::class)
+															->setMethodsExcept(['liveOrMemoizedLabelsForBoardId'])
+															->disableOriginalConstructor()
+															->getMock();
+
 		$this->stackService = new StackService(
 			$this->stackMapper,
-            $this->cardMapper,
+      $this->boardMapper,
-            $this->labelMapper,
+      $this->cardMapper,
+      $this->labelMapper,
+
 			$this->permissionService,
 			$this->boardService,
 			$this->assignedUsersMapper,
@@ -130,8 +140,10 @@ class StackServiceTest extends TestCase {
     private function getStacks() {
 	    $s1 = new Stack();
 	    $s1->setId(222);
+			$s1->setBoardId(1);
 	    $s2 = new Stack();
 	    $s2->setId(223);
+			$s1->setBoardId(1);
 	    return [$s1, $s2];
     }
     private function getCards($stackId=0) {
@@ -158,9 +170,12 @@ class StackServiceTest extends TestCase {
 
 	public function testDelete() {
 	    $this->permissionService->expects($this->once())->method('checkPermission');
-        $this->stackMapper->expects($this->once())->method('find')->willReturn(new Stack());
+			$stackToBeDeleted = new Stack();
-	    $this->stackMapper->expects($this->once())->method('delete');
+			$stackToBeDeleted->setId(1);
+      $this->stackMapper->expects($this->once())->method('find')->willReturn($stackToBeDeleted);
+	    $this->stackMapper->expects($this->once())->method('update');
 	    $this->stackService->delete(123);
+			$this->assertTrue($stackToBeDeleted->getDeletedAt() <= time(), "deletedAt is in the past");
     }
 
     public function testUpdate() {
@@ -172,7 +187,7 @@ class StackServiceTest extends TestCase {
         $stack->setTitle('Foo');
         $stack->setBoardId(2);
         $stack->setOrder(1);
-        $result = $this->stackService->update(123, 'Foo', 2, 1);
+        $result = $this->stackService->update(123, 'Foo', 2, 1, null);
         $this->assertEquals($stack, $result);
     }
 

tests/unit/controller/CardControllerTest.php

@@ -76,7 +76,7 @@ class CardControllerTest extends \Test\TestCase {
 			->method('update')
 			->with(1, 'title', 3, 'text', 5, 'foo', $this->userId, '2017-01-01 00:00:00')
 			->willReturn(1);
-		$this->assertEquals(1, $this->controller->update(1, 'title', 3, 'text', 5, 'foo', '2017-01-01 00:00:00'));
+		$this->assertEquals(1, $this->controller->update(1, 'title', 3, 'text', 5, 'foo', '2017-01-01 00:00:00', null));
 	}
 
 	public function testDelete() {

tests/unit/controller/StackControllerTest.php

@@ -81,7 +81,7 @@ class StackControllerTest extends \Test\TestCase {
 			->method('update')
 			->with(1, 2, 3, 4)
 			->willReturn(1);
-		$this->assertEquals(1, $this->controller->update(1, 2, 3, 4));
+		$this->assertEquals(1, $this->controller->update(1, 2, 3, 4, null));
 	}
 
 	public function testReorder() {