diff --git a/js/controller/BoardController.js b/js/controller/BoardController.js index cd863b014..aa82d15f3 100644 --- a/js/controller/BoardController.js +++ b/js/controller/BoardController.js @@ -311,7 +311,7 @@ app.controller('BoardController', function ($rootScope, $scope, $stateParams, St $scope.refreshData(); }); }; - + $scope.aclUpdate = function (acl) { BoardService.updateAcl(acl); }; diff --git a/js/service/ApiService.js b/js/service/ApiService.js index 098507dd8..89910ad89 100644 --- a/js/service/ApiService.js +++ b/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) { @@ -43,7 +42,7 @@ app.factory('ApiService', function ($http, $q) { let object = this.data[id]; if (object === undefined) { object = this.deleted[id]; - } + } return object; }; @@ -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) { diff --git a/lib/Db/LabelMapper.php b/lib/Db/LabelMapper.php index 6757c8ab6..3107b4310 100644 --- a/lib/Db/LabelMapper.php +++ b/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(); diff --git a/lib/Service/CardService.php b/lib/Service/CardService.php index 1167d8ba0..fcf691ecc 100644 --- a/lib/Service/CardService.php +++ b/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; @@ -49,8 +51,9 @@ class CardService { public function __construct( CardMapper $cardMapper, - StackMapper $stackMapper, - BoardMapper $boardMapper, + 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) { diff --git a/lib/Service/StackService.php b/lib/Service/StackService.php index 79232a4d1..b64766e79 100644 --- a/lib/Service/StackService.php +++ b/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 $cardIndex => $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())); + foreach ($cards as $card) { + $this->cardService->enrich($card); } $stack->setCards($cards); diff --git a/tests/unit/Service/CardServiceTest.php b/tests/unit/Service/CardServiceTest.php index 941230700..74f4ee0bf 100644 --- a/tests/unit/Service/CardServiceTest.php +++ b/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; @@ -49,20 +50,36 @@ class CardServiceTest extends TestCase { private $notificationHelper; /** @var AssignedUsersMapper|\PHPUnit\Framework\MockObject\MockObject */ private $assignedUsersMapper; - /** @var BoardService|\PHPUnit\Framework\MockObject\MockObject */ - private $boardService; + /** @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(); + 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() { diff --git a/tests/unit/Service/StackServiceTest.php b/tests/unit/Service/StackServiceTest.php index 0793a68ac..e013de63d 100644 --- a/tests/unit/Service/StackServiceTest.php +++ b/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->getMockBuilder(LabelMapper::class) - ->setMethodsExcept(['liveOrMemoizedLabelsForBoardId']) - ->disableOriginalConstructor() - ->getMock(); + $this->labelMapper = $this->createMock(LabelMapper::class); $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++) {