fixed bugs where labels would dissappear

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

js/service/ApiService.js

@@ -20,7 +20,6 @@
  *
  */
 import app from '../app/App.js';
-
 /** global: oc_defaults */
 app.factory('ApiService', function ($http, $q) {
 	var ApiService = function (http, endpoint) {
@@ -68,6 +67,10 @@ app.factory('ApiService', function ($http, $q) {
 		$http.get(this.generateUrl(scopeId + '/' + this.endpoint + '/deleted')).then(function (response) {
 			var objects = response.data;
 			objects.forEach(function (obj) {
+				if(self.deleted[obj.id] !== undefined) {
+					return;
+				}
+
 				self.deleted[obj.id] = obj;
 
 				if(self.afterFetch !== undefined) {
@@ -139,7 +142,13 @@ app.factory('ApiService', function ($http, $q) {
 
 		$http.delete(this.baseUrl + '/' + id).then(function (response) {
 			self.deleted[id] = self.data[id];
-			self.remove(id);
+			delete self.data[id];
+
+			let deletedAt = response.data.deletedAt
+			if (deletedAt !== undefined) {
+				self.deleted[id].deletedAt = deletedAt;
+			}
+
 			deferred.resolve(response.data);
 
 		}, function (error) {
@@ -154,9 +163,9 @@ app.factory('ApiService', function ($http, $q) {
 
 		var promise = this.update(entity);
 
-		promise.then(function() {
+		promise.then(() => {
 			self.data[entity.id] = entity;
-			self.remove(entity.id, 'deleted');
+			delete this.deleted[entity.id];
 		});
 
 		return promise;
@@ -166,6 +175,7 @@ app.factory('ApiService', function ($http, $q) {
 	ApiService.prototype.clear = function () {
 		this.data = {};
 	};
+
 	ApiService.prototype.add = function (entity) {
 		var element = this.data[entity.id];
 		if (element === undefined) {
@@ -179,11 +189,7 @@ app.factory('ApiService', function ($http, $q) {
 			element.status = {};
 		}
 	};
-	ApiService.prototype.remove = function (id, collection = 'data') {
+
-		if (this[collection][id] !== undefined) {
-			delete this[collection][id];
-		}
-	};
 	ApiService.prototype.addAll = function (entities) {
 		var self = this;
 		angular.forEach(entities, function (entity) {

lib/Db/LabelMapper.php

@@ -54,22 +54,6 @@ 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

@@ -31,6 +31,7 @@ use OCA\Deck\Db\Acl;
 use OCA\Deck\Db\StackMapper;
 use OCA\Deck\Notification\NotificationHelper;
 use OCA\Deck\Db\BoardMapper;
+use OCA\Deck\Db\LabelMapper;
 use OCA\Deck\NotFoundException;
 use OCA\Deck\StatusException;
 
@@ -40,6 +41,7 @@ class CardService {
 	private $cardMapper;
 	private $stackMapper;
 	private $boardMapper;
+	private $labelMapper;
 	private $permissionService;
 	private $boardService;
 	private $notificationHelper;
@@ -51,6 +53,7 @@ class CardService {
 		CardMapper $cardMapper,
 		StackMapper $stackMapper,
 		BoardMapper $boardMapper,
+		LabelMapper $labelMapper,
 		PermissionService $permissionService, 
 		BoardService $boardService,
 		NotificationHelper $notificationHelper,
@@ -61,6 +64,7 @@ class CardService {
 		$this->cardMapper = $cardMapper;
 		$this->stackMapper = $stackMapper;
 		$this->boardMapper = $boardMapper;
+		$this->labelMapper = $labelMapper;
 		$this->permissionService = $permissionService;
 		$this->boardService = $boardService;
 		$this->notificationHelper = $notificationHelper;
@@ -69,9 +73,20 @@ class CardService {
 		$this->currentUser = $userId;
 	}
 
+	public function enrich($card) {
+		$cardId = $card->getId();
+		$card->setAssignedUsers($this->assignedUsersMapper->find($cardId));
+		$card->setLabels($this->labelMapper->findAssignedLabelsForCard($cardId));
+		$card->setAttachmentCount($this->attachmentService->count($cardId));
+	}
+
 	public function fetchDeleted($boardId) {
 		$this->permissionService->checkPermission($this->boardMapper, $boardId, Acl::PERMISSION_READ);
-		return $this->cardMapper->findDeleted($boardId);
+		$cards = $this->cardMapper->findDeleted($boardId);
+		foreach ($cards as $card) {
+			$this->enrich($card);
+		}
+		return $cards;
 	}
 
 	public function find($cardId) {

lib/Service/StackService.php

@@ -43,6 +43,7 @@ class StackService {
 	private $labelMapper;
 	private $permissionService;
 	private $boardService;
+	private $cardService;
 	private $assignedUsersMapper;
 	private $attachmentService;
 
@@ -53,6 +54,7 @@ class StackService {
 		LabelMapper $labelMapper,
 		PermissionService $permissionService,
 		BoardService $boardService,
+		CardService $cardService,
 		AssignedUsersMapper $assignedUsersMapper,
 		AttachmentService $attachmentService
 	) {
@@ -62,6 +64,7 @@ class StackService {
 		$this->labelMapper = $labelMapper;
 		$this->permissionService = $permissionService;
 		$this->boardService = $boardService;
+		$this->cardService = $cardService;
 		$this->assignedUsersMapper = $assignedUsersMapper;
 		$this->attachmentService = $attachmentService;
 	}
@@ -73,14 +76,8 @@ class StackService {
 			return;
 		}
 
-		$labels = $this->labelMapper->liveOrMemoizedLabelsForBoardId($stack->getBoardId());
+		foreach ($cards as $card) {
-		foreach ($cards as $cardIndex => $card) {
+			$this->cardService->enrich($card);
-			$assignedUsers = $this->assignedUsersMapper->find($card->getId());
-			$card->setAssignedUsers($assignedUsers);
-			if (array_key_exists($card->id, $labels)) {
-				$cards[$cardIndex]->setLabels($labels[$card->id]);
-			}
-			$card->setAttachmentCount($this->attachmentService->count($card->getId()));
 		}
 
 		$stack->setCards($cards);

tests/unit/Service/CardServiceTest.php

@@ -30,6 +30,7 @@ use OCA\Deck\Db\Card;
 use OCA\Deck\Db\CardMapper;
 use OCA\Deck\Db\StackMapper;
 use OCA\Deck\Db\BoardMapper;
+use OCA\Deck\Db\LabelMapper;
 use OCA\Deck\NotFoundException;
 use OCA\Deck\Notification\NotificationHelper;
 use OCA\Deck\StatusException;
@@ -51,18 +52,34 @@ class CardServiceTest extends TestCase {
     private $assignedUsersMapper;
   	/** @var BoardService|\PHPUnit\Framework\MockObject\MockObject */
   	private $boardService;
+  	/** @var LabelMapper|\PHPUnit\Framework\MockObject\MockObject */
+  	private $labelMapper;
+	private $boardMapper;
+	private $attachmentService;
 
 	public function setUp() {
 		    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->cardService = new CardService($this->cardMapper, $this->stackMapper, $this->boardMapper, $this->permissionService, $this->boardService, $this->notificationHelper, $this->assignedUsersMapper, $this->attachmentService, 'userXY');
+        $this->cardService = new CardService(
+			$this->cardMapper,
+			$this->stackMapper,
+			$this->boardMapper,
+			$this->labelMapper,
+			$this->permissionService,
+			$this->boardService,
+			$this->notificationHelper,
+			$this->assignedUsersMapper,
+			$this->attachmentService,
+			'user1'
+        );
     }
 
     public function testFind() {

tests/unit/Service/StackServiceTest.php

@@ -61,6 +61,8 @@ class StackServiceTest extends TestCase {
 	private $attachmentService;
 	/** @var BoardService|\PHPUnit\Framework\MockObject\MockObject */
 	private $boardService;
+	/** @var CardService|\PHPUnit\Framework\MockObject\MockObject */
+	private $cardService;
 
 	public function setUp() {
 		parent::setUp();
@@ -69,13 +71,10 @@ class StackServiceTest extends TestCase {
 		$this->boardMapper = $this->createMock(BoardMapper::class);
 		$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->attachmentService = $this->createMock(AttachmentService::class);
-
+		$this->labelMapper = $this->createMock(LabelMapper::class);
-		$this->labelMapper = $this->getMockBuilder(LabelMapper::class)
-															->setMethodsExcept(['liveOrMemoizedLabelsForBoardId'])
-															->disableOriginalConstructor()
-															->getMock();
 
 		$this->stackService = new StackService(
 			$this->stackMapper,
@@ -85,6 +84,7 @@ class StackServiceTest extends TestCase {
 
 			$this->permissionService,
 			$this->boardService,
+			$this->cardService,
 			$this->assignedUsersMapper,
 			$this->attachmentService
 		);
@@ -93,9 +93,16 @@ class StackServiceTest extends TestCase {
 	public function testFindAll() {
         $this->permissionService->expects($this->once())->method('checkPermission');
         $this->stackMapper->expects($this->once())->method('findAll')->willReturn($this->getStacks());
-        $this->labelMapper->expects($this->once())->method('getAssignedLabelsForBoard')->willReturn($this->getLabels());
+    		$this->cardService->expects($this->atLeastOnce())->method('enrich')->will(
+					$this->returnCallback(
+						function($card) {
+							$card->setLabels($this->getLabels()[$card->getId()]);
+						}
+					)
+				);
         $this->cardMapper->expects($this->any())->method('findAll')->willReturn($this->getCards(222));
 
+
         $actual = $this->stackService->findAll(123);
         for($stackId=0; $stackId<3; $stackId++) {
             for ($cardId=0;$cardId<10;$cardId++) {