From 2ef4b55af427d90412544e77916e9449db7dbbcd Mon Sep 17 00:00:00 2001 From: Manuel Arno Korfmann Date: Mon, 9 Jul 2018 23:49:42 +0200 Subject: [PATCH 01/10] cards soft delete wip Signed-off-by: Manuel Arno Korfmann cards: softdelete done; undo delete wip Signed-off-by: Manuel Arno Korfmann show deleted cards in board settings sidebar wip Signed-off-by: Manuel Arno Korfmann CardMapper#findDeleted: fix bug in entity property assigning Signed-off-by: Manuel Arno Korfmann --- appinfo/database.xml | 8 ++++++++ appinfo/routes.php | 1 + js/controller/BoardController.js | 12 ++++++++++++ js/service/ApiService.js | 12 ++++++++++++ js/service/CardService.js | 18 ++++++++++++++++++ lib/Controller/CardController.php | 9 +++++++++ lib/Db/Card.php | 2 ++ lib/Db/CardMapper.php | 9 ++++++++- lib/Service/CardService.php | 9 ++++++++- templates/part.board.sidebarView.php | 10 ++++++++++ 10 files changed, 88 insertions(+), 2 deletions(-) diff --git a/appinfo/database.xml b/appinfo/database.xml index 3eb4495f9..e9b7586ab 100644 --- a/appinfo/database.xml +++ b/appinfo/database.xml @@ -167,6 +167,14 @@ boolean false + + deleted_at + integer + 0 + 8 + false + true + deck_cards_stack_id_index diff --git a/appinfo/routes.php b/appinfo/routes.php index cdd2b9d79..47a550f5b 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -50,6 +50,7 @@ return [ ['name' => 'card#create', 'url' => '/cards', 'verb' => 'POST'], ['name' => 'card#update', 'url' => '/cards/{cardId}', 'verb' => 'PUT'], ['name' => 'card#delete', 'url' => '/cards/{cardId}', 'verb' => 'DELETE'], + ['name' => 'card#deleted', 'url' => '/cards/deleted/{boardId}', 'verb' => 'GET'], ['name' => 'card#rename', 'url' => '/cards/{cardId}/rename', 'verb' => 'PUT'], ['name' => 'card#reorder', 'url' => '/cards/{cardId}/reorder', 'verb' => 'PUT'], ['name' => 'card#archive', 'url' => '/cards/{cardId}/archive', 'verb' => 'PUT'], diff --git a/js/controller/BoardController.js b/js/controller/BoardController.js index d8b4c16b7..bce35642f 100644 --- a/js/controller/BoardController.js +++ b/js/controller/BoardController.js @@ -42,6 +42,8 @@ app.controller('BoardController', function ($rootScope, $scope, $stateParams, St $scope.board = BoardService.getCurrent(); $scope.uploader = FileService.uploader; + $scope.deletedCards = []; + // workaround for $stateParams changes not being propagated $scope.$watch(function() { return $state.params; @@ -136,6 +138,14 @@ app.controller('BoardController', function ($rootScope, $scope, $stateParams, St }); }; + $scope.loadDeletedCards = function() { + CardService.fetchDeleted($scope.id).then(function (data) { + $scope.deletedCards = data; + }, function (error) { + $scope.statusservice.setError('Error occured', error); + }); + } + $scope.loadDefault = function () { StackService.fetchAll($scope.id).then(function (data) { $scope.statusservice.releaseWaiting(); @@ -193,6 +203,7 @@ app.controller('BoardController', function ($rootScope, $scope, $stateParams, St } CardService.delete(card.id).then(function () { StackService.removeCard(card); + $scope.loadDeletedCards(); }); }); }; @@ -384,4 +395,5 @@ app.controller('BoardController', function ($rootScope, $scope, $stateParams, St return card.attachmentCount; }; + $scope.loadDeletedCards(); }); diff --git a/js/service/ApiService.js b/js/service/ApiService.js index 051479417..2705739a7 100644 --- a/js/service/ApiService.js +++ b/js/service/ApiService.js @@ -114,6 +114,18 @@ app.factory('ApiService', function ($http, $q) { }; + ApiService.prototype.softDelete = function (id) { + var deferred = $q.defer(); + var self = this; + + $http.delete(this.baseUrl + '/' + id).then(function (response) { + self.data[id].deletedAt = response.data.deletedAt; + deferred.resolve(response.data); + }, function (error) { + deferred.reject('Deleting ' + self.endpoint + ' failed'); + }); + return deferred.promise; + }; // methods for managing data ApiService.prototype.clear = function () { diff --git a/js/service/CardService.js b/js/service/CardService.js index 51edc4e1d..614ef1e8e 100644 --- a/js/service/CardService.js +++ b/js/service/CardService.js @@ -27,6 +27,8 @@ app.factory('CardService', function (ApiService, $http, $q) { }; CardService.prototype = angular.copy(ApiService.prototype); + CardService.prototype.delete = CardService.prototype.softDelete; + CardService.prototype.reorder = function (card, order) { var deferred = $q.defer(); var self = this; @@ -172,6 +174,22 @@ app.factory('CardService', function (ApiService, $http, $q) { return deferred.promise; }; + CardService.prototype.fetchDeleted = function (boardId) { + + var deferred = $q.defer(); + var self = this; + $http.get(this.baseUrl + '/deleted/' + boardId).then(function (response) { + var objects = response.data; + return objects; + deferred.resolve(self.data); + }, function (error) { + deferred.reject('Fetching ' + self.endpoint + ' failed'); + }); + return deferred.promise; + + }; + + var service = new CardService($http, 'cards', $q); return service; }); diff --git a/lib/Controller/CardController.php b/lib/Controller/CardController.php index aeadcb6f9..1084ece45 100644 --- a/lib/Controller/CardController.php +++ b/lib/Controller/CardController.php @@ -104,6 +104,15 @@ class CardController extends Controller { return $this->cardService->delete($cardId); } + /** + * @NoAdminRequired + * @param $boardId + * @return \OCP\AppFramework\Db\Entity + */ + public function deleted($boardId) { + return $this->cardService->fetchDeleted($boardId); + } + /** * @NoAdminRequired * @param $cardId diff --git a/lib/Db/Card.php b/lib/Db/Card.php index 21f65d79e..f065fc10a 100644 --- a/lib/Db/Card.php +++ b/lib/Db/Card.php @@ -42,6 +42,7 @@ class Card extends RelationalEntity { protected $archived = false; protected $duedate; protected $notified = false; + protected $deletedAt; private $databaseType = 'sqlite'; @@ -58,6 +59,7 @@ class Card extends RelationalEntity { $this->addType('createdAt', 'integer'); $this->addType('archived', 'boolean'); $this->addType('notified', 'boolean'); + $this->addType('deletedAt', 'integer'); $this->addRelation('labels'); $this->addRelation('assignedUsers'); $this->addRelation('attachments'); diff --git a/lib/Db/CardMapper.php b/lib/Db/CardMapper.php index e0234fc43..7cb5d1f57 100644 --- a/lib/Db/CardMapper.php +++ b/lib/Db/CardMapper.php @@ -124,6 +124,13 @@ class CardMapper extends DeckMapper implements IPermissionMapper { return $this->findEntities($sql, [$stackId], $limit, $offset); } + public function findDeleted($boardId, $limit = null, $offset = null) { + $sql = 'SELECT c.* FROM `*PREFIX*deck_cards` c + INNER JOIN `*PREFIX*deck_stacks` s ON s.id = c.stack_id + WHERE `s`.`board_id` = ? AND NOT c.archived AND NOT c.deleted_at = 0 AND c.deleted_at <= ? ORDER BY `c`.`order`'; + return $this->findEntities($sql, [$boardId, time()], $limit, $offset); + } + public function findAllArchived($stackId, $limit = null, $offset = null) { $sql = 'SELECT * FROM `*PREFIX*deck_cards` WHERE `stack_id`=? AND archived ORDER BY `last_modified`'; return $this->findEntities($sql, [$stackId], $limit, $offset); @@ -197,4 +204,4 @@ class CardMapper extends DeckMapper implements IPermissionMapper { } -} \ No newline at end of file +} diff --git a/lib/Service/CardService.php b/lib/Service/CardService.php index a9b7ae7a2..8b040f4e7 100644 --- a/lib/Service/CardService.php +++ b/lib/Service/CardService.php @@ -56,6 +56,10 @@ class CardService { $this->currentUser = $userId; } + public function fetchDeleted($boardId) { + return $this->cardMapper->findDeleted($boardId); + } + public function find($cardId) { $this->permissionService->checkPermission($this->cardMapper, $cardId, Acl::PERMISSION_READ); $card = $this->cardMapper->find($cardId); @@ -89,7 +93,10 @@ class CardService { if ($this->boardService->isArchived($this->cardMapper, $id)) { throw new StatusException('Operation not allowed. This board is archived.'); } - return $this->cardMapper->delete($this->cardMapper->find($id)); + $card = $this->cardMapper->find($id); + $card->setDeletedAt(time()); + $this->cardMapper->update($card); + return $card; } public function update($id, $title, $stackId, $type, $order, $description, $owner, $duedate) { diff --git a/templates/part.board.sidebarView.php b/templates/part.board.sidebarView.php index 7229af2dd..b168d0cfa 100644 --- a/templates/part.board.sidebarView.php +++ b/templates/part.board.sidebarView.php @@ -14,6 +14,7 @@
@@ -118,4 +119,13 @@
+ +
+
    +
  • +{{123}} +{{deletedCard}} +
    • +
+
From f2795f120bc034c88d9d02dc6de3b77f2b5a4f53 Mon Sep 17 00:00:00 2001 From: Manuel Arno Korfmann Date: Wed, 11 Jul 2018 22:13:31 +0200 Subject: [PATCH 02/10] show deleted cards in sidebar tab (styling still wip) Signed-off-by: Manuel Arno Korfmann card undo delete done, styling still wip Signed-off-by: Manuel Arno Korfmann fix Codacy findings Signed-off-by: Manuel Arno Korfmann --- js/controller/BoardController.js | 20 ++++++++++++++++++-- js/service/ApiService.js | 24 ++++++++++++------------ js/service/CardService.js | 10 ++++++---- lib/Controller/CardController.php | 5 +++-- lib/Db/CardMapper.php | 6 +++--- lib/Service/BoardService.php | 2 +- lib/Service/CardService.php | 16 ++++++++++++++-- lib/Service/StackService.php | 2 +- templates/part.board.sidebarView.php | 9 +++++++-- 9 files changed, 65 insertions(+), 29 deletions(-) diff --git a/js/controller/BoardController.js b/js/controller/BoardController.js index bce35642f..9b5e2bc59 100644 --- a/js/controller/BoardController.js +++ b/js/controller/BoardController.js @@ -144,7 +144,7 @@ app.controller('BoardController', function ($rootScope, $scope, $stateParams, St }, function (error) { $scope.statusservice.setError('Error occured', error); }); - } + }; $scope.loadDefault = function () { StackService.fetchAll($scope.id).then(function (data) { @@ -203,10 +203,25 @@ app.controller('BoardController', function ($rootScope, $scope, $stateParams, St } CardService.delete(card.id).then(function () { StackService.removeCard(card); - $scope.loadDeletedCards(); + $scope.deletedCards.push(card); }); }); }; + + $scope.cardUndoDelete = function (deletedCard) { + CardService.undoDelete(deletedCard); + StackService.addCard(deletedCard); + $scope.removeFromDeletedCards(deletedCard); + }; + + $scope.removeFromDeletedCards = function(deletedCard) { + for(var i=0;i<$scope.deletedCards.length;i++) { + if($scope.deletedCards[i].id === deletedCard.id) { + $scope.deletedCards.splice(i, 1); + } + } + }; + $scope.cardArchive = function (card) { CardService.archive(card); StackService.removeCard(card); @@ -247,6 +262,7 @@ app.controller('BoardController', function ($rootScope, $scope, $stateParams, St // TODO: remove from cards }; $scope.labelCreate = function (label) { + alert(label); label.boardId = $scope.id; LabelService.create(label).then(function (data) { $scope.newStack.title = ''; diff --git a/js/service/ApiService.js b/js/service/ApiService.js index 2705739a7..e6af62f65 100644 --- a/js/service/ApiService.js +++ b/js/service/ApiService.js @@ -114,18 +114,18 @@ app.factory('ApiService', function ($http, $q) { }; - ApiService.prototype.softDelete = function (id) { - var deferred = $q.defer(); - var self = this; - - $http.delete(this.baseUrl + '/' + id).then(function (response) { - self.data[id].deletedAt = response.data.deletedAt; - deferred.resolve(response.data); - }, function (error) { - deferred.reject('Deleting ' + self.endpoint + ' failed'); - }); - return deferred.promise; - }; + ApiService.prototype.softDelete = function (id) { + var deferred = $q.defer(); + var self = this; + + $http.delete(this.baseUrl + '/' + id).then(function (response) { + self.data[id].deletedAt = response.data.deletedAt; + deferred.resolve(response.data); + }, function (error) { + deferred.reject('Deleting ' + self.endpoint + ' failed'); + }); + return deferred.promise; + }; // methods for managing data ApiService.prototype.clear = function () { diff --git a/js/service/CardService.js b/js/service/CardService.js index 614ef1e8e..840d21156 100644 --- a/js/service/CardService.js +++ b/js/service/CardService.js @@ -29,6 +29,11 @@ app.factory('CardService', function (ApiService, $http, $q) { CardService.prototype.delete = CardService.prototype.softDelete; + CardService.prototype.undoDelete = function(card) { + card.deletedAt = 0; + this.update(card); + }; + CardService.prototype.reorder = function (card, order) { var deferred = $q.defer(); var self = this; @@ -175,18 +180,15 @@ app.factory('CardService', function (ApiService, $http, $q) { }; CardService.prototype.fetchDeleted = function (boardId) { - var deferred = $q.defer(); var self = this; $http.get(this.baseUrl + '/deleted/' + boardId).then(function (response) { var objects = response.data; - return objects; - deferred.resolve(self.data); + deferred.resolve(objects); }, function (error) { deferred.reject('Fetching ' + self.endpoint + ' failed'); }); return deferred.promise; - }; diff --git a/lib/Controller/CardController.php b/lib/Controller/CardController.php index 1084ece45..22502f27e 100644 --- a/lib/Controller/CardController.php +++ b/lib/Controller/CardController.php @@ -89,10 +89,11 @@ class CardController extends Controller { * @param $order * @param $description * @param $duedate + * @param $deletedAt * @return \OCP\AppFramework\Db\Entity */ - public function update($id, $title, $stackId, $type, $order, $description, $duedate) { - return $this->cardService->update($id, $title, $stackId, $type, $order, $description, $this->userId, $duedate); + public function update($id, $title, $stackId, $type, $order, $description, $duedate, $deletedAt) { + return $this->cardService->update($id, $title, $stackId, $type, $order, $description, $this->userId, $duedate, $deletedAt); } /** diff --git a/lib/Db/CardMapper.php b/lib/Db/CardMapper.php index 7cb5d1f57..31f363398 100644 --- a/lib/Db/CardMapper.php +++ b/lib/Db/CardMapper.php @@ -120,15 +120,15 @@ class CardMapper extends DeckMapper implements IPermissionMapper { public function findAll($stackId, $limit = null, $offset = null) { $sql = 'SELECT * FROM `*PREFIX*deck_cards` - WHERE `stack_id` = ? AND NOT archived ORDER BY `order`'; + WHERE `stack_id` = ? AND NOT archived AND deleted_at = 0 ORDER BY `order`'; return $this->findEntities($sql, [$stackId], $limit, $offset); } public function findDeleted($boardId, $limit = null, $offset = null) { $sql = 'SELECT c.* FROM `*PREFIX*deck_cards` c INNER JOIN `*PREFIX*deck_stacks` s ON s.id = c.stack_id - WHERE `s`.`board_id` = ? AND NOT c.archived AND NOT c.deleted_at = 0 AND c.deleted_at <= ? ORDER BY `c`.`order`'; - return $this->findEntities($sql, [$boardId, time()], $limit, $offset); + WHERE `s`.`board_id` = ? AND NOT c.archived AND NOT c.deleted_at = 0 ORDER BY `c`.`order`'; + return $this->findEntities($sql, [$boardId], $limit, $offset); } public function findAllArchived($stackId, $limit = null, $offset = null) { diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index 3f9a905cc..ab6baf225 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -256,4 +256,4 @@ class BoardService { return $this->aclMapper->delete($acl); } -} \ No newline at end of file +} diff --git a/lib/Service/CardService.php b/lib/Service/CardService.php index 8b040f4e7..723bc43cd 100644 --- a/lib/Service/CardService.php +++ b/lib/Service/CardService.php @@ -30,6 +30,7 @@ use OCA\Deck\Db\CardMapper; use OCA\Deck\Db\Acl; use OCA\Deck\Db\StackMapper; use OCA\Deck\Notification\NotificationHelper; +use OCA\Deck\Db\BoardMapper; use OCA\Deck\NotFoundException; use OCA\Deck\StatusException; @@ -38,6 +39,7 @@ class CardService { private $cardMapper; private $stackMapper; + private $boardMapper; private $permissionService; private $boardService; private $notificationHelper; @@ -45,9 +47,17 @@ class CardService { private $attachmentService; private $currentUser; - public function __construct(CardMapper $cardMapper, StackMapper $stackMapper, PermissionService $permissionService, BoardService $boardService, NotificationHelper $notificationHelper, AssignedUsersMapper $assignedUsersMapper, AttachmentService $attachmentService, $userId) { + public function __construct( + CardMapper $cardMapper, + StackMapper $stackMapper, + BoardMapper $boardMapper, + PermissionService $permissionService, + BoardService $boardService, + AssignedUsersMapper $assignedUsersMapper, + AttachmentService $attachmentService) { $this->cardMapper = $cardMapper; $this->stackMapper = $stackMapper; + $this->boardMapper = $boardMapper; $this->permissionService = $permissionService; $this->boardService = $boardService; $this->notificationHelper = $notificationHelper; @@ -57,6 +67,7 @@ class CardService { } public function fetchDeleted($boardId) { + $this->permissionService->checkPermission($this->boardMapper, $boardId, Acl::PERMISSION_READ); return $this->cardMapper->findDeleted($boardId); } @@ -99,7 +110,7 @@ class CardService { return $card; } - public function update($id, $title, $stackId, $type, $order, $description, $owner, $duedate) { + public function update($id, $title, $stackId, $type, $order, $description, $owner, $duedate, $deletedAt) { $this->permissionService->checkPermission($this->cardMapper, $id, Acl::PERMISSION_EDIT); if ($this->boardService->isArchived($this->cardMapper, $id)) { throw new StatusException('Operation not allowed. This board is archived.'); @@ -115,6 +126,7 @@ class CardService { $card->setOwner($owner); $card->setDescription($description); $card->setDuedate($duedate); + $card->setDeletedAt($deletedAt); return $this->cardMapper->update($card); } diff --git a/lib/Service/StackService.php b/lib/Service/StackService.php index 30e40cc6e..581543623 100644 --- a/lib/Service/StackService.php +++ b/lib/Service/StackService.php @@ -154,4 +154,4 @@ class StackService { return $result; } -} \ No newline at end of file +} diff --git a/templates/part.board.sidebarView.php b/templates/part.board.sidebarView.php index b168d0cfa..4e92bb427 100644 --- a/templates/part.board.sidebarView.php +++ b/templates/part.board.sidebarView.php @@ -123,8 +123,13 @@
  • -{{123}} -{{deletedCard}} +
    +
    Title
    +
    {{deletedCard.title}}
    +
    Stack
    +
    {{stackservice.data[deletedCard.stackId].title}}
    +
    +
    t('Undo delete')); ?>
From ef4ce31c47a5ef70d1a4d00f2d4cd182ac067f2c Mon Sep 17 00:00:00 2001 From: Manuel Arno Korfmann Date: Mon, 16 Jul 2018 00:09:43 +0200 Subject: [PATCH 03/10] refactoring and stack undo delete early wip Signed-off-by: Manuel Arno Korfmann stack soft delete done Signed-off-by: Manuel Arno Korfmann stack undo delete done Signed-off-by: Manuel Arno Korfmann stack undo: code review remarks and fixes Signed-off-by: Manuel Arno Korfmann --- appinfo/database.xml | 8 ++++ appinfo/routes.php | 1 + css/style.scss | 8 +++- js/controller/BoardController.js | 69 ++++++++++++++++------------ js/service/ApiService.js | 40 ++++++++++------ js/service/CardService.js | 28 ++--------- js/service/StackService.js | 30 ++---------- lib/Controller/StackController.php | 15 +++++- lib/Db/Stack.php | 4 +- lib/Db/StackMapper.php | 13 ++++-- lib/Service/StackService.php | 20 +++++++- templates/part.board.mainView.php | 2 +- templates/part.board.sidebarView.php | 44 ++++++++++++------ 13 files changed, 164 insertions(+), 118 deletions(-) diff --git a/appinfo/database.xml b/appinfo/database.xml index e9b7586ab..91c9076ba 100644 --- a/appinfo/database.xml +++ b/appinfo/database.xml @@ -77,6 +77,14 @@ 8 false + + deleted_at + integer + 0 + 8 + false + true + deck_stacks_board_id_index diff --git a/appinfo/routes.php b/appinfo/routes.php index 47a550f5b..de62cb0cc 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -43,6 +43,7 @@ return [ ['name' => 'stack#update', 'url' => '/stacks/{stackId}', 'verb' => 'PUT'], ['name' => 'stack#reorder', 'url' => '/stacks/{stackId}/reorder', 'verb' => 'PUT'], ['name' => 'stack#delete', 'url' => '/stacks/{stackId}', 'verb' => 'DELETE'], + ['name' => 'stack#deleted', 'url' => '/stacks/deleted/{boardId}', 'verb' => 'GET'], ['name' => 'stack#archived', 'url' => '/stacks/{boardId}/archived', 'verb' => 'GET'], // cards diff --git a/css/style.scss b/css/style.scss index a94b60938..efd70ef3e 100644 --- a/css/style.scss +++ b/css/style.scss @@ -1215,12 +1215,18 @@ input.input-inline { .tabHeaders { clear: both; - overflow: hidden; + overflow: initial; margin-bottom: 0; } .tabsContainer { margin-top: 15px; + height: 100%; + + .tab { + height: 100%; + overflow: scroll; + } } .ui-select-offscreen { diff --git a/js/controller/BoardController.js b/js/controller/BoardController.js index 9b5e2bc59..048190d47 100644 --- a/js/controller/BoardController.js +++ b/js/controller/BoardController.js @@ -4,20 +4,20 @@ * @author Julius Härtl * * @license GNU AGPL version 3 or any later version - * + * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as * published by the Free Software Foundation, either version 3 of the * License, or (at your option) any later version. - * + * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU Affero General Public License for more details. - * + * * You should have received a copy of the GNU Affero General Public License * along with this program. If not, see . - * + * */ import app from '../app/App.js'; @@ -42,7 +42,17 @@ app.controller('BoardController', function ($rootScope, $scope, $stateParams, St $scope.board = BoardService.getCurrent(); $scope.uploader = FileService.uploader; - $scope.deletedCards = []; + $scope.deletedCards = {}; + $scope.deletedStacks = {}; + + $scope.$watch(function() { + return $state.current; + }, function(currentState) { + if(currentState.name === 'board.detail') { + $scope.loadDeletedEntity(CardService, 'deletedCards'); + $scope.loadDeletedEntity(StackService, 'deletedStacks'); + } + }); // workaround for $stateParams changes not being propagated $scope.$watch(function() { @@ -138,9 +148,11 @@ app.controller('BoardController', function ($rootScope, $scope, $stateParams, St }); }; - $scope.loadDeletedCards = function() { - CardService.fetchDeleted($scope.id).then(function (data) { - $scope.deletedCards = data; + $scope.loadDeletedEntity = function(service, scopeKey) { + service.fetchDeleted($scope.id).then(function (data) { + for(i=0;i * * @license GNU AGPL version 3 or any later version - * + * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as * published by the Free Software Foundation, either version 3 of the * License, or (at your option) any later version. - * + * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU Affero General Public License for more details. - * + * * You should have received a copy of the GNU Affero General Public License * along with this program. If not, see . - * + * */ import app from '../app/App.js'; @@ -48,6 +48,19 @@ app.factory('ApiService', function ($http, $q) { return deferred.promise; }; + ApiService.prototype.fetchDeleted = function (scopeId) { + var deferred = $q.defer(); + var self = this; + $http.get(this.baseUrl + '/deleted/' + scopeId).then(function (response) { + var objects = response.data; + deferred.resolve(objects); + }, function (error) { + deferred.reject('Fetching ' + self.endpoint + ' failed'); + }); + return deferred.promise; + }; + + ApiService.prototype.fetchOne = function (id) { this.id = id; @@ -111,20 +124,19 @@ app.factory('ApiService', function ($http, $q) { deferred.reject('Deleting ' + self.endpoint + ' failed'); }); return deferred.promise; - }; - ApiService.prototype.softDelete = function (id) { - var deferred = $q.defer(); + ApiService.prototype.undoDelete = function(entity) { var self = this; - - $http.delete(this.baseUrl + '/' + id).then(function (response) { - self.data[id].deletedAt = response.data.deletedAt; - deferred.resolve(response.data); - }, function (error) { - deferred.reject('Deleting ' + self.endpoint + ' failed'); + entity.deletedAt = 0; + + var promise = this.update(entity); + + promise.then(function() { + self.data[entity.id] = entity; }); - return deferred.promise; + + return promise; }; // methods for managing data diff --git a/js/service/CardService.js b/js/service/CardService.js index 840d21156..eb62785f0 100644 --- a/js/service/CardService.js +++ b/js/service/CardService.js @@ -4,20 +4,20 @@ * @author Julius Härtl * * @license GNU AGPL version 3 or any later version - * + * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as * published by the Free Software Foundation, either version 3 of the * License, or (at your option) any later version. - * + * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU Affero General Public License for more details. - * + * * You should have received a copy of the GNU Affero General Public License * along with this program. If not, see . - * + * */ import app from '../app/App.js'; @@ -27,13 +27,6 @@ app.factory('CardService', function (ApiService, $http, $q) { }; CardService.prototype = angular.copy(ApiService.prototype); - CardService.prototype.delete = CardService.prototype.softDelete; - - CardService.prototype.undoDelete = function(card) { - card.deletedAt = 0; - this.update(card); - }; - CardService.prototype.reorder = function (card, order) { var deferred = $q.defer(); var self = this; @@ -179,19 +172,6 @@ app.factory('CardService', function (ApiService, $http, $q) { return deferred.promise; }; - CardService.prototype.fetchDeleted = function (boardId) { - var deferred = $q.defer(); - var self = this; - $http.get(this.baseUrl + '/deleted/' + boardId).then(function (response) { - var objects = response.data; - deferred.resolve(objects); - }, function (error) { - deferred.reject('Fetching ' + self.endpoint + ' failed'); - }); - return deferred.promise; - }; - - var service = new CardService($http, 'cards', $q); return service; }); diff --git a/js/service/StackService.js b/js/service/StackService.js index a4671a331..a5ee1d0a9 100644 --- a/js/service/StackService.js +++ b/js/service/StackService.js @@ -4,20 +4,20 @@ * @author Julius Härtl * * @license GNU AGPL version 3 or any later version - * + * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as * published by the Free Software Foundation, either version 3 of the * License, or (at your option) any later version. - * + * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU Affero General Public License for more details. - * + * * You should have received a copy of the GNU Affero General Public License * along with this program. If not, see . - * + * */ import app from '../app/App.js'; @@ -27,6 +27,7 @@ app.factory('StackService', function (ApiService, CardService, $http, $q) { ApiService.call(this, $http, ep, $q); }; StackService.prototype = angular.copy(ApiService.prototype); + StackService.prototype.fetchAll = function (boardId) { var deferred = $q.defer(); var self = this; @@ -129,27 +130,6 @@ app.factory('StackService', function (ApiService, CardService, $http, $q) { } }; - // FIXME: Should not show popup but proper undo mechanism - StackService.prototype.delete = function (id) { - var deferred = $q.defer(); - var self = this; - - OC.dialogs.confirm(t('deck', 'Are you sure you want to delete the stack with all of its data?'), t('deck', 'Delete'), function(state) { - if (!state) { - return; - } - $http.delete(self.baseUrl + '/' + id).then(function (response) { - self.remove(id); - deferred.resolve(response.data); - - }, function (error) { - deferred.reject('Deleting ' + self.endpoint + ' failed'); - }); - }); - return deferred.promise; - }; - var service = new StackService($http, 'stacks', $q); return service; }); - diff --git a/lib/Controller/StackController.php b/lib/Controller/StackController.php index 3e2e358ed..3063d858f 100644 --- a/lib/Controller/StackController.php +++ b/lib/Controller/StackController.php @@ -74,10 +74,11 @@ class StackController extends Controller { * @param $title * @param $boardId * @param $order + * @param $deletedAt * @return \OCP\AppFramework\Db\Entity */ - public function update($id, $title, $boardId, $order) { - return $this->stackService->update($id, $title, $boardId, $order); + public function update($id, $title, $boardId, $order, $deletedAt) { + return $this->stackService->update($id, $title, $boardId, $order, $deletedAt); } /** @@ -98,4 +99,14 @@ class StackController extends Controller { public function delete($stackId) { return $this->stackService->delete($stackId); } + + /** + * @NoAdminRequired + * @param $boardId + * @return \OCP\AppFramework\Db\Entity + */ + public function deleted($boardId) { + return $this->stackService->fetchDeleted($boardId); + } + } diff --git a/lib/Db/Stack.php b/lib/Db/Stack.php index 8a3d00ffb..b90604126 100644 --- a/lib/Db/Stack.php +++ b/lib/Db/Stack.php @@ -27,12 +27,14 @@ class Stack extends RelationalEntity { protected $title; protected $boardId; + protected $deletedAt; protected $cards = array(); protected $order; public function __construct() { $this->addType('id', 'integer'); $this->addType('boardId', 'integer'); + $this->addType('deletedAt', 'integer'); $this->addType('order', 'integer'); } @@ -47,4 +49,4 @@ class Stack extends RelationalEntity { } return $json; } -} \ No newline at end of file +} diff --git a/lib/Db/StackMapper.php b/lib/Db/StackMapper.php index 51d8bf29c..c98d87cc0 100644 --- a/lib/Db/StackMapper.php +++ b/lib/Db/StackMapper.php @@ -51,10 +51,17 @@ class StackMapper extends DeckMapper implements IPermissionMapper { public function findAll($boardId, $limit = null, $offset = null) { - $sql = 'SELECT * FROM `*PREFIX*deck_stacks` WHERE `board_id` = ? ORDER BY `order`'; + $sql = 'SELECT * FROM `*PREFIX*deck_stacks` WHERE `board_id` = ? AND deleted_at = 0 ORDER BY `order`'; return $this->findEntities($sql, [$boardId], $limit, $offset); } - + + + public function findDeleted($boardId, $limit = null, $offset = null) { + $sql = 'SELECT * FROM `*PREFIX*deck_stacks` s + WHERE `s`.`board_id` = ? AND NOT s.deleted_at = 0'; + return $this->findEntities($sql, [$boardId], $limit, $offset); + } + public function delete(Entity $entity) { // delete cards on stack @@ -73,4 +80,4 @@ class StackMapper extends DeckMapper implements IPermissionMapper { $entity = $this->find($stackId); return $entity->getBoardId(); } -} \ No newline at end of file +} diff --git a/lib/Service/StackService.php b/lib/Service/StackService.php index 581543623..a4ca46eec 100644 --- a/lib/Service/StackService.php +++ b/lib/Service/StackService.php @@ -25,6 +25,7 @@ namespace OCA\Deck\Service; use OCA\Deck\Db\Acl; use OCA\Deck\Db\CardMapper; +use OCA\Deck\Db\BoardMapper; use OCA\Deck\Db\LabelMapper; use OCA\Deck\Db\AssignedUsersMapper; use OCA\Deck\Db\Stack; @@ -38,6 +39,7 @@ class StackService { private $stackMapper; private $cardMapper; + private $boardMapper; private $labelMapper; private $permissionService; private $boardService; @@ -46,6 +48,7 @@ class StackService { public function __construct( StackMapper $stackMapper, + BoardMapper $boardMapper, CardMapper $cardMapper, LabelMapper $labelMapper, PermissionService $permissionService, @@ -54,6 +57,7 @@ class StackService { AttachmentService $attachmentService ) { $this->stackMapper = $stackMapper; + $this->boardMapper = $boardMapper; $this->cardMapper = $cardMapper; $this->labelMapper = $labelMapper; $this->permissionService = $permissionService; @@ -81,6 +85,11 @@ class StackService { return $stacks; } + public function fetchDeleted($boardId) { + $this->permissionService->checkPermission($this->boardMapper, $boardId, Acl::PERMISSION_READ); + return $this->stackMapper->findDeleted($boardId); + } + public function findAllArchived($boardId) { $this->permissionService->checkPermission(null, $boardId, Acl::PERMISSION_READ); $stacks = $this->stackMapper->findAll($boardId); @@ -115,10 +124,16 @@ class StackService { public function delete($id) { $this->permissionService->checkPermission($this->stackMapper, $id, Acl::PERMISSION_MANAGE); - return $this->stackMapper->delete($this->stackMapper->find($id)); + + $stack = $this->stackMapper->find($id); + $stack->setDeletedAt(time()); + $this->stackMapper->update($stack); + + return $stack; } - public function update($id, $title, $boardId, $order) { + + public function update($id, $title, $boardId, $order, $deletedAt) { $this->permissionService->checkPermission($this->stackMapper, $id, Acl::PERMISSION_MANAGE); if ($this->boardService->isArchived($this->stackMapper, $id)) { throw new StatusException('Operation not allowed. This board is archived.'); @@ -127,6 +142,7 @@ class StackService { $stack->setTitle($title); $stack->setBoardId($boardId); $stack->setOrder($order); + $stack->setDeletedAt($deletedAt); return $this->stackMapper->update($stack); } diff --git a/templates/part.board.mainView.php b/templates/part.board.mainView.php index bc5f483c2..26447c9c2 100644 --- a/templates/part.board.mainView.php +++ b/templates/part.board.mainView.php @@ -52,7 +52,7 @@ + ng-click="stackDelete(s)">
  • {{ statusservice.text }}

    @@ -14,7 +14,8 @@
    @@ -120,17 +121,30 @@
    -
    -
      -
    • -
      -
      Title
      -
      {{deletedCard.title}}
      -
      Stack
      -
      {{stackservice.data[deletedCard.stackId].title}}
      -
      -
      t('Undo delete')); ?>       -
      • -
    +
    + +
    + +
    +
      +
    • +
      +
      Title
      +
      {{deletedCard.title}}
      +
      Stack
      +
      {{stackservice.data[deletedCard.stackId].title}}
      +
      + +
      t('Undo delete')); ?>       +
      • +
    From 95548fba54e596dec331d3bbe0e2a0f83cef0674 Mon Sep 17 00:00:00 2001 From: Manuel Arno Korfmann Date: Wed, 18 Jul 2018 23:19:29 +0200 Subject: [PATCH 04/10] Conditional restoration of deleted stacks on card undo delete Signed-off-by: Manuel Arno Korfmann --- js/controller/BoardController.js | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/js/controller/BoardController.js b/js/controller/BoardController.js index 048190d47..81135dbd9 100644 --- a/js/controller/BoardController.js +++ b/js/controller/BoardController.js @@ -215,7 +215,7 @@ app.controller('BoardController', function ($rootScope, $scope, $stateParams, St } $scope.stackUndoDelete = function (deletedStack) { - StackService.undoDelete(deletedStack).then(function() { + return StackService.undoDelete(deletedStack).then(function() { delete $scope.deletedStacks[deletedStack.id]; }); } @@ -229,8 +229,24 @@ app.controller('BoardController', function ($rootScope, $scope, $stateParams, St $scope.cardUndoDelete = function (deletedCard) { CardService.undoDelete(deletedCard).then(function() { - StackService.addCard(deletedCard); delete $scope.deletedCards[deletedCard.id]; + + var associatedDeletedStack = $scope.deletedStacks[deletedCard.stackId]; + if(associatedDeletedStack !== undefined) { + OC.dialogs.confirm( + t('deck', 'The associated stack is deleted as well, do you want to restore it as well?'), + t('deck', 'Yes'), + function(state) { + + if (state) { + $scope.stackUndoDelete(associatedDeletedStack).then(function() { + StackService.addCard(deletedCard); + }); + } + }); + } else { + StackService.addCard(deletedCard); + } }); }; From 41d30d4fd4107169aa2b684ea9187e05345ed9b1 Mon Sep 17 00:00:00 2001 From: Manuel Arno Korfmann Date: Thu, 19 Jul 2018 00:16:31 +0200 Subject: [PATCH 05/10] stack, card undo delete: refactoring Signed-off-by: Manuel Arno Korfmann stack undo delete: serve cards with deleted and delete actions Signed-off-by: Manuel Arno Korfmann stack, cards undo delete: codacy Signed-off-by: Manuel Arno Korfmann card undo delete: 526#discussion_r204501758, refactoring Signed-off-by: Manuel Arno Korfmann card, stack undo delete: code review fixes #1 Signed-off-by: Manuel Arno Korfmann undo card, stack delete: show deleted stacks name in deleted card listing Signed-off-by: Manuel Arno Korfmann --- appinfo/routes.php | 12 ++--- css/style.scss | 10 ++++ js/controller/BoardController.js | 80 +++++++++++++--------------- js/service/ApiService.js | 37 ++++++++++--- js/service/StackService.js | 4 ++ lib/Db/Card.php | 10 ++-- lib/Db/Stack.php | 2 +- lib/Service/StackService.php | 46 ++++++++++------ templates/part.board.sidebarView.php | 34 ++++++------ 9 files changed, 137 insertions(+), 98 deletions(-) diff --git a/appinfo/routes.php b/appinfo/routes.php index de62cb0cc..90dffb520 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -5,20 +5,20 @@ * @author Julius Härtl * * @license GNU AGPL version 3 or any later version - * + * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as * published by the Free Software Foundation, either version 3 of the * License, or (at your option) any later version. - * + * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU Affero General Public License for more details. - * + * * You should have received a copy of the GNU Affero General Public License * along with this program. If not, see . - * + * */ return [ @@ -43,7 +43,7 @@ return [ ['name' => 'stack#update', 'url' => '/stacks/{stackId}', 'verb' => 'PUT'], ['name' => 'stack#reorder', 'url' => '/stacks/{stackId}/reorder', 'verb' => 'PUT'], ['name' => 'stack#delete', 'url' => '/stacks/{stackId}', 'verb' => 'DELETE'], - ['name' => 'stack#deleted', 'url' => '/stacks/deleted/{boardId}', 'verb' => 'GET'], + ['name' => 'stack#deleted', 'url' => '/{boardId}/stacks/deleted', 'verb' => 'GET'], ['name' => 'stack#archived', 'url' => '/stacks/{boardId}/archived', 'verb' => 'GET'], // cards @@ -51,7 +51,7 @@ return [ ['name' => 'card#create', 'url' => '/cards', 'verb' => 'POST'], ['name' => 'card#update', 'url' => '/cards/{cardId}', 'verb' => 'PUT'], ['name' => 'card#delete', 'url' => '/cards/{cardId}', 'verb' => 'DELETE'], - ['name' => 'card#deleted', 'url' => '/cards/deleted/{boardId}', 'verb' => 'GET'], + ['name' => 'card#deleted', 'url' => '/{boardId}/cards/deleted', 'verb' => 'GET'], ['name' => 'card#rename', 'url' => '/cards/{cardId}/rename', 'verb' => 'PUT'], ['name' => 'card#reorder', 'url' => '/cards/{cardId}/reorder', 'verb' => 'PUT'], ['name' => 'card#archive', 'url' => '/cards/{cardId}/archive', 'verb' => 'PUT'], diff --git a/css/style.scss b/css/style.scss index efd70ef3e..12504fe91 100644 --- a/css/style.scss +++ b/css/style.scss @@ -1170,6 +1170,16 @@ input.input-inline { position: relative; } +.board-detail__deleted-list__item { + display: flex; + flex-direction: row; + justify-content: space-between; + + * { + flex-basis: 20%; + } +} + #board-detail-labels { ul li { input { diff --git a/js/controller/BoardController.js b/js/controller/BoardController.js index 81135dbd9..cccc8f4f9 100644 --- a/js/controller/BoardController.js +++ b/js/controller/BoardController.js @@ -42,15 +42,12 @@ app.controller('BoardController', function ($rootScope, $scope, $stateParams, St $scope.board = BoardService.getCurrent(); $scope.uploader = FileService.uploader; - $scope.deletedCards = {}; - $scope.deletedStacks = {}; - $scope.$watch(function() { return $state.current; }, function(currentState) { if(currentState.name === 'board.detail') { - $scope.loadDeletedEntity(CardService, 'deletedCards'); - $scope.loadDeletedEntity(StackService, 'deletedStacks'); + CardService.fetchDeleted($scope.id); + StackService.fetchDeleted($scope.id); } }); @@ -148,16 +145,6 @@ app.controller('BoardController', function ($rootScope, $scope, $stateParams, St }); }; - $scope.loadDeletedEntity = function(service, scopeKey) { - service.fetchDeleted($scope.id).then(function (data) { - for(i=0;i * * @license GNU AGPL version 3 or any later version - * + * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as * published by the Free Software Foundation, either version 3 of the * License, or (at your option) any later version. - * + * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU Affero General Public License for more details. - * + * * You should have received a copy of the GNU Affero General Public License * along with this program. If not, see . - * + * */ namespace OCA\Deck\Db; @@ -42,7 +42,7 @@ class Card extends RelationalEntity { protected $archived = false; protected $duedate; protected $notified = false; - protected $deletedAt; + protected $deletedAt = 0; private $databaseType = 'sqlite'; diff --git a/lib/Db/Stack.php b/lib/Db/Stack.php index b90604126..f1ba66c9c 100644 --- a/lib/Db/Stack.php +++ b/lib/Db/Stack.php @@ -27,7 +27,7 @@ class Stack extends RelationalEntity { protected $title; protected $boardId; - protected $deletedAt; + protected $deletedAt = 0; protected $cards = array(); protected $order; diff --git a/lib/Service/StackService.php b/lib/Service/StackService.php index a4ca46eec..d0ec926d9 100644 --- a/lib/Service/StackService.php +++ b/lib/Service/StackService.php @@ -5,20 +5,20 @@ * @author Julius Härtl * * @license GNU AGPL version 3 or any later version - * + * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as * published by the Free Software Foundation, either version 3 of the * License, or (at your option) any later version. - * + * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU Affero General Public License for more details. - * + * * You should have received a copy of the GNU Affero General Public License * along with this program. If not, see . - * + * */ namespace OCA\Deck\Service; @@ -66,28 +66,38 @@ class StackService { $this->attachmentService = $attachmentService; } + private function enrichStackWithCards($stack) { + $cards = $this->cardMapper->findAll($stack->id); + 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())); + } + $stack->setCards($cards); + } + + private function enrichStacksWithCards($stacks) { + foreach ($stacks as $stackIndex => $stack) { + $this->enrichStackWithCards($stack); + } + } + public function findAll($boardId) { $this->permissionService->checkPermission(null, $boardId, Acl::PERMISSION_READ); $stacks = $this->stackMapper->findAll($boardId); $labels = $this->labelMapper->getAssignedLabelsForBoard($boardId); - foreach ($stacks as $stackIndex => $stack) { - $cards = $this->cardMapper->findAll($stack->id); - 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())); - } - $stacks[$stackIndex]->setCards($cards); - } + $this->enrichStacksWithCards($stacks); return $stacks; } public function fetchDeleted($boardId) { $this->permissionService->checkPermission($this->boardMapper, $boardId, Acl::PERMISSION_READ); - return $this->stackMapper->findDeleted($boardId); + $stacks = $this->stackMapper->findDeleted($boardId); + $this->enrichStacksWithCards($stacks); + return $stacks; } public function findAllArchived($boardId) { @@ -129,6 +139,8 @@ class StackService { $stack->setDeletedAt(time()); $this->stackMapper->update($stack); + $this->enrichStackWithCards($stack); + return $stack; } diff --git a/templates/part.board.sidebarView.php b/templates/part.board.sidebarView.php index 04f418ba0..c08c36b36 100644 --- a/templates/part.board.sidebarView.php +++ b/templates/part.board.sidebarView.php @@ -122,28 +122,28 @@
    -
      -
    • -
      -
      Title
      -
      {{deletedStack.title}}
      -
      -
      t('Undo delete')); ?>       +
        +
      • + + {{deletedStack.title}} + {{deletedStack.deletedAt}} + + +
    -
      -
    • -
      -
      Title
      -
      {{deletedCard.title}}
      -
      Stack
      -
      {{stackservice.data[deletedCard.stackId].title}}
      -
      - -
      t('Undo delete')); ?>       +
        +
      • + + {{deletedCard.title}} + {{stackservice.tryAllThenDeleted(deletedCard.stackId).title}} + {{deletedCard.deletedAt}} + + +
    From 5ddfb666332faf6e4eee974367ebb414402df50c Mon Sep 17 00:00:00 2001 From: Manuel Arno Korfmann Date: Sat, 28 Jul 2018 04:12:50 +0200 Subject: [PATCH 06/10] Card,Stack undo delete: CardServiceTest fix Signed-off-by: Manuel Arno Korfmann Stack,Card undo delete: Test Fix 2 Signed-off-by: Manuel Arno Korfmann Card, Stack undo delete: Test fix 3 Signed-off-by: Manuel Arno Korfmann Card,Stack undo delete: Test fix 4 Signed-off-by: Manuel Arno Korfmann Stack, Card undo delete: Relative time in deleted entity listings Signed-off-by: Manuel Arno Korfmann Card, Stack undo delete: Test Fix 5 Signed-off-by: Manuel Arno Korfmann Test Fix 6 Signed-off-by: Manuel Arno Korfmann Test Fix 7 Signed-off-by: Manuel Arno Korfmann fix codacy Signed-off-by: Manuel Arno Korfmann --- js/controller/BoardController.js | 2 +- js/service/ApiService.js | 4 ++- lib/Db/LabelMapper.php | 24 ++++++++++++--- lib/Service/CardService.php | 7 +++-- lib/Service/StackService.php | 12 ++++++-- templates/part.board.sidebarView.php | 4 +-- tests/unit/Db/BoardMapperTest.php | 10 +++---- tests/unit/Db/CardTest.php | 13 +++++---- tests/unit/Db/StackTest.php | 12 ++++---- tests/unit/Service/CardServiceTest.php | 20 ++++++++----- tests/unit/Service/StackServiceTest.php | 29 ++++++++++++++----- tests/unit/controller/CardControllerTest.php | 2 +- tests/unit/controller/StackControllerTest.php | 2 +- 13 files changed, 96 insertions(+), 45 deletions(-) diff --git a/js/controller/BoardController.js b/js/controller/BoardController.js index cccc8f4f9..aa53ac3e7 100644 --- a/js/controller/BoardController.js +++ b/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() { diff --git a/js/service/ApiService.js b/js/service/ApiService.js index e37f4be4b..098507dd8 100644 --- a/js/service/ApiService.js +++ b/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; }; diff --git a/lib/Db/LabelMapper.php b/lib/Db/LabelMapper.php index 9ed47266b..6757c8ab6 100644 --- a/lib/Db/LabelMapper.php +++ b/lib/Db/LabelMapper.php @@ -5,20 +5,20 @@ * @author Julius Härtl * * @license GNU AGPL version 3 or any later version - * + * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as * published by the Free Software Foundation, either version 3 of the * License, or (at your option) any later version. - * + * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU Affero General Public License for more details. - * + * * You should have received a copy of the GNU Affero General Public License * along with this program. If not, see . - * + * */ namespace OCA\Deck\Db; @@ -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(); diff --git a/lib/Service/CardService.php b/lib/Service/CardService.php index 723bc43cd..1167d8ba0 100644 --- a/lib/Service/CardService.php +++ b/lib/Service/CardService.php @@ -52,9 +52,12 @@ class CardService { StackMapper $stackMapper, BoardMapper $boardMapper, PermissionService $permissionService, - BoardService $boardService, + BoardService $boardService, + NotificationHelper $notificationHelper, AssignedUsersMapper $assignedUsersMapper, - AttachmentService $attachmentService) { + AttachmentService $attachmentService, + $userId + ) { $this->cardMapper = $cardMapper; $this->stackMapper = $stackMapper; $this->boardMapper = $boardMapper; diff --git a/lib/Service/StackService.php b/lib/Service/StackService.php index d0ec926d9..79232a4d1 100644 --- a/lib/Service/StackService.php +++ b/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; } @@ -140,7 +146,7 @@ class StackService { $this->stackMapper->update($stack); $this->enrichStackWithCards($stack); - + return $stack; } diff --git a/templates/part.board.sidebarView.php b/templates/part.board.sidebarView.php index c08c36b36..2b18ce4f3 100644 --- a/templates/part.board.sidebarView.php +++ b/templates/part.board.sidebarView.php @@ -126,7 +126,7 @@
  • {{deletedStack.title}} - {{deletedStack.deletedAt}} + {{deletedStack.deletedAt | relativeDateFilter }} @@ -140,7 +140,7 @@ {{deletedCard.title}} {{stackservice.tryAllThenDeleted(deletedCard.stackId).title}} - {{deletedCard.deletedAt}} + {{deletedCard.deletedAt | relativeDateFilter }} diff --git a/tests/unit/Db/BoardMapperTest.php b/tests/unit/Db/BoardMapperTest.php index 64c878893..f11eb0363 100644 --- a/tests/unit/Db/BoardMapperTest.php +++ b/tests/unit/Db/BoardMapperTest.php @@ -5,20 +5,20 @@ * @author Julius Härtl * * @license GNU AGPL version 3 or any later version - * + * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as * published by the Free Software Foundation, either version 3 of the * License, or (at your option) any later version. - * + * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU Affero General Public License for more details. - * + * * You should have received a copy of the GNU Affero General Public License * along with this program. If not, see . - * + * */ namespace OCA\Deck\Db; @@ -175,4 +175,4 @@ class BoardMapperTest extends MapperTestUtility { } } -} \ No newline at end of file +} diff --git a/tests/unit/Db/CardTest.php b/tests/unit/Db/CardTest.php index be6472b88..ba8ff7030 100644 --- a/tests/unit/Db/CardTest.php +++ b/tests/unit/Db/CardTest.php @@ -5,20 +5,20 @@ * @author Julius Härtl * * @license GNU AGPL version 3 or any later version - * + * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as * published by the Free Software Foundation, either version 3 of the * License, or (at your option) any later version. - * + * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU Affero General Public License for more details. - * + * * You should have received a copy of the GNU Affero General Public License * along with this program. If not, see . - * + * */ namespace OCA\Deck\Db; @@ -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,7 +137,8 @@ class CardTest extends TestCase { 'attachments' => null, 'attachmentCount' => null, 'assignedUsers' => ['user1'], + 'deletedAt' => 0 ], $card->jsonSerialize()); } -} \ No newline at end of file +} diff --git a/tests/unit/Db/StackTest.php b/tests/unit/Db/StackTest.php index a28a732c6..75873d350 100644 --- a/tests/unit/Db/StackTest.php +++ b/tests/unit/Db/StackTest.php @@ -5,20 +5,20 @@ * @author Julius Härtl * * @license GNU AGPL version 3 or any later version - * + * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as * published by the Free Software Foundation, either version 3 of the * License, or (at your option) any later version. - * + * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU Affero General Public License for more details. - * + * * You should have received a copy of the GNU Affero General Public License * along with this program. If not, see . - * + * */ namespace OCA\Deck\Db; @@ -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()); } -} \ No newline at end of file +} diff --git a/tests/unit/Service/CardServiceTest.php b/tests/unit/Service/CardServiceTest.php index 0f850c006..941230700 100644 --- a/tests/unit/Service/CardServiceTest.php +++ b/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') - ->willReturn(1); - $this->assertEquals(1, $this->cardService->delete(123)); + ->method('update') + ->willReturn($cardToBeDeleted); + $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() { @@ -317,4 +321,4 @@ class CardServiceTest extends TestCase { } -} \ No newline at end of file +} diff --git a/tests/unit/Service/StackServiceTest.php b/tests/unit/Service/StackServiceTest.php index d9cc908fc..0793a68ac 100644 --- a/tests/unit/Service/StackServiceTest.php +++ b/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->labelMapper, + $this->boardMapper, + $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()); - $this->stackMapper->expects($this->once())->method('delete'); + $stackToBeDeleted = new Stack(); + $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); } @@ -207,4 +222,4 @@ class StackServiceTest extends TestCase { return $stack; } -} \ No newline at end of file +} diff --git a/tests/unit/controller/CardControllerTest.php b/tests/unit/controller/CardControllerTest.php index 6f19db775..904f7b03a 100644 --- a/tests/unit/controller/CardControllerTest.php +++ b/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() { diff --git a/tests/unit/controller/StackControllerTest.php b/tests/unit/controller/StackControllerTest.php index 01f0f963e..b209178db 100644 --- a/tests/unit/controller/StackControllerTest.php +++ b/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() { From 4d5353b8d44b72af4f29613d226a539e2dc3113c Mon Sep 17 00:00:00 2001 From: Manuel Arno Korfmann Date: Sun, 5 Aug 2018 10:21:42 +0200 Subject: [PATCH 07/10] renaming and more codacy Signed-off-by: Manuel Arno Korfmann --- js/controller/BoardController.js | 26 ++++++++++++++++---------- templates/part.board.sidebarView.php | 2 +- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/js/controller/BoardController.js b/js/controller/BoardController.js index aa53ac3e7..cd863b014 100644 --- a/js/controller/BoardController.js +++ b/js/controller/BoardController.js @@ -209,43 +209,44 @@ app.controller('BoardController', function ($rootScope, $scope, $stateParams, St }); }; - $scope.cardUndoDelete = function (deletedCard) { + $scope.cardOrCardAndStackUndoDelete = function (deletedCard) { var associatedDeletedStack = $scope.stackservice.deleted[deletedCard.stackId]; if(associatedDeletedStack !== undefined) { - $scope.cardAndStackUndoDelete(deletedCard, associatedDeletedStack); + $scope.cardAndStackUndoDeleteAskForConfirmation(deletedCard, associatedDeletedStack); } else { - $scope._cardUndoDelete(deletedCard); + $scope.cardUndoDelete(deletedCard); } }; - $scope.cardAndStackUndoDelete = function(deletedCard, associatedDeletedStack) { + $scope.cardAndStackUndoDeleteAskForConfirmation = function(deletedCard, associatedDeletedStack) { OC.dialogs.confirm( t('deck', 'The associated stack is deleted as well, it will be restored as well.'), t('deck', 'Restore associated stack'), function(state) { if (state) { - $scope._cardAndStackUndoDelete(deletedCard, associatedDeletedStack); + $scope.cardAndStackUndoDelete(deletedCard, associatedDeletedStack); } } ); }; - $scope._cardAndStackUndoDelete = function(deletedCard, associatedDeletedStack) { + $scope.cardAndStackUndoDelete = function(deletedCard, associatedDeletedStack) { $scope.stackUndoDelete(associatedDeletedStack).then(function() { - $scope._cardUndoDelete(deletedCard); + $scope.cardUndoDelete(deletedCard); }); - } + }; - $scope._cardUndoDelete = function(deletedCard) { + $scope.cardUndoDelete = function(deletedCard) { CardService.undoDelete(deletedCard).then(function() { StackService.addCard(deletedCard); }); - } + }; $scope.cardArchive = function (card) { CardService.archive(card); StackService.removeCard(card); }; + $scope.isCurrentUserAssigned = function (card) { if (! CardService.get(card.id).assignedUsers) { return false; @@ -255,6 +256,7 @@ app.controller('BoardController', function ($rootScope, $scope, $stateParams, St }); return userList.length === 1; }; + $scope.cardAssignToMe = function (card) { CardService.assignUser(card, OC.getCurrentUser().uid) .then( @@ -263,6 +265,7 @@ app.controller('BoardController', function ($rootScope, $scope, $stateParams, St // TODO: remove this jquery call. Fix and use appPopoverMenuUtils instead $('.popovermenu').addClass('hidden'); }; + $scope.cardUnassignFromMe = function (card) { CardService.unassignUser(card, OC.getCurrentUser().uid); StackService.updateCard(card); @@ -281,6 +284,7 @@ app.controller('BoardController', function ($rootScope, $scope, $stateParams, St BoardService.getCurrent().labels.splice(i, 1); // TODO: remove from cards }; + $scope.labelCreate = function (label) { label.boardId = $scope.id; LabelService.create(label).then(function (data) { @@ -300,12 +304,14 @@ app.controller('BoardController', function ($rootScope, $scope, $stateParams, St BoardService.addAcl(sharee); $scope.status.addSharee = null; }; + $scope.aclDelete = function (acl) { BoardService.deleteAcl(acl).then(function(data) { $scope.loadDefault(); $scope.refreshData(); }); }; + $scope.aclUpdate = function (acl) { BoardService.updateAcl(acl); }; diff --git a/templates/part.board.sidebarView.php b/templates/part.board.sidebarView.php index 2b18ce4f3..6fa652929 100644 --- a/templates/part.board.sidebarView.php +++ b/templates/part.board.sidebarView.php @@ -141,7 +141,7 @@ {{deletedCard.title}} {{stackservice.tryAllThenDeleted(deletedCard.stackId).title}} {{deletedCard.deletedAt | relativeDateFilter }} - +
    • From 94e1b86eaf8c2b219f507fe842d77b063b455c91 Mon Sep 17 00:00:00 2001 From: Manuel Arno Korfmann Date: Sun, 5 Aug 2018 22:33:42 +0200 Subject: [PATCH 08/10] fixed bugs where labels would dissappear Signed-off-by: Manuel Arno Korfmann --- js/controller/BoardController.js | 2 +- js/service/ApiService.js | 26 +++++++++++++++--------- lib/Db/LabelMapper.php | 16 --------------- lib/Service/CardService.php | 21 ++++++++++++++++--- lib/Service/StackService.php | 13 +++++------- tests/unit/Service/CardServiceTest.php | 27 ++++++++++++++++++++----- tests/unit/Service/StackServiceTest.php | 19 +++++++++++------ 7 files changed, 75 insertions(+), 49 deletions(-) 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++) { From 241a654f289fc7d8e5a7148e52af3228241353f9 Mon Sep 17 00:00:00 2001 From: Manuel Arno Korfmann Date: Sun, 5 Aug 2018 22:36:25 +0200 Subject: [PATCH 09/10] codacy fix Signed-off-by: Manuel Arno Korfmann --- js/service/ApiService.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/service/ApiService.js b/js/service/ApiService.js index 89910ad89..7cf2c2a37 100644 --- a/js/service/ApiService.js +++ b/js/service/ApiService.js @@ -144,7 +144,7 @@ app.factory('ApiService', function ($http, $q) { self.deleted[id] = self.data[id]; delete self.data[id]; - let deletedAt = response.data.deletedAt + let deletedAt = response.data.deletedAt; if (deletedAt !== undefined) { self.deleted[id].deletedAt = deletedAt; } From 43e76ecca31652d13166d90ce317ca7989ab5fc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Wed, 15 Aug 2018 20:46:58 +0200 Subject: [PATCH 10/10] Bump development version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- appinfo/info.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appinfo/info.xml b/appinfo/info.xml index f2ff32e48..1760084d6 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -14,7 +14,7 @@ - 🚀 Get your project organized - 0.5.0-dev1 + 0.5.0-dev2 agpl Julius Härtl Deck