From eb0cd9685d677a50dbb9075c1ef4b4a9b27febb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakob=20R=C3=B6hrl?= Date: Tue, 18 Dec 2018 16:32:30 +0100 Subject: [PATCH 1/8] =?UTF-8?q?Signed-off-by:=20Jakob=20R=C3=B6hrl=20?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fix for issue586 --- lib/Service/LabelService.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/Service/LabelService.php b/lib/Service/LabelService.php index e46ee39ab..0291133df 100644 --- a/lib/Service/LabelService.php +++ b/lib/Service/LabelService.php @@ -90,6 +90,14 @@ class LabelService { throw new BadRequestException('board id must be a number'); } + $boardLabels = $this->labelMapper->findAll($boardId); + foreach($boardLabels as $boardLabel) { + if ($boardLabel->getTitle() === $title) { + throw new BadRequestException('title must be unique'); + break; + } + } + $this->permissionService->checkPermission(null, $boardId, Acl::PERMISSION_MANAGE); if ($this->boardService->isArchived(null, $boardId)) { throw new StatusException('Operation not allowed. This board is archived.'); From ab73f58fd81ae521ecadefd7ba11f913662f9375 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakob=20R=C3=B6hrl?= Date: Thu, 20 Dec 2018 13:26:09 +0100 Subject: [PATCH 2/8] added private function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jakob Röhrl --- lib/Service/LabelService.php | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/lib/Service/LabelService.php b/lib/Service/LabelService.php index 0291133df..44d2c255d 100644 --- a/lib/Service/LabelService.php +++ b/lib/Service/LabelService.php @@ -90,15 +90,10 @@ class LabelService { throw new BadRequestException('board id must be a number'); } - $boardLabels = $this->labelMapper->findAll($boardId); - foreach($boardLabels as $boardLabel) { - if ($boardLabel->getTitle() === $title) { - throw new BadRequestException('title must be unique'); - break; - } - } - $this->permissionService->checkPermission(null, $boardId, Acl::PERMISSION_MANAGE); + + $this->checkDuplicateTitle($boardId, $title); + if ($this->boardService->isArchived(null, $boardId)) { throw new StatusException('Operation not allowed. This board is archived.'); } @@ -160,14 +155,33 @@ class LabelService { } $this->permissionService->checkPermission($this->labelMapper, $id, Acl::PERMISSION_MANAGE); + + $label = $this->find($id); + $this->checkDuplicateTitle($label->getBoardId(), $title); + if ($this->boardService->isArchived($this->labelMapper, $id)) { throw new StatusException('Operation not allowed. This board is archived.'); } - $label = $this->find($id); + $label->setTitle($title); $label->setColor($color); $this->changeHelper->boardChanged($label->getBoardId()); return $this->labelMapper->update($label); } + /** + * @param $boardId + * @param $title + * @throws BadRequestException + */ + private function checkDuplicateTitle($boardId, $title) { + $boardLabels = $this->labelMapper->findAll($boardId); + foreach($boardLabels as $boardLabel) { + if ($boardLabel->getTitle() === $title) { + throw new BadRequestException('title must be unique'); + break; + } + } + } + } From f01cd506f7bc89f7c168fcf07a9b068dd61e9f2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakob=20R=C3=B6hrl?= Date: Thu, 24 Jan 2019 10:19:05 +0100 Subject: [PATCH 3/8] new try MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jakob Röhrl --- js/controller/BoardController.js | 10 ++++++++-- lib/Service/LabelService.php | 22 ++++++++++++++++++++-- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/js/controller/BoardController.js b/js/controller/BoardController.js index e6599f2f8..6273875cb 100644 --- a/js/controller/BoardController.js +++ b/js/controller/BoardController.js @@ -323,7 +323,7 @@ app.controller('BoardController', function ($rootScope, $scope, $stateParams, St // remove from board data var i = BoardService.getCurrent().labels.indexOf(label); BoardService.getCurrent().labels.splice(i, 1); - + // remove from cards var cards = CardService.data; for (var card in cards) { @@ -346,11 +346,17 @@ app.controller('BoardController', function ($rootScope, $scope, $stateParams, St BoardService.getCurrent().labels.push(data); $scope.status.createLabel = false; $scope.newLabel = {}; + }).catch(err => { + OC.Notification.showTemporary('Duplicate label name is not allowed'); }); }; + $scope.labelUpdate = function (label) { label.edit = false; - LabelService.update(label); + LabelService.update(label).catch(err => { + label.title('XXX'); + OC.Notification.showTemporary('Duplicate label name is not allowed'); + }); // update labels in UI var cards = CardService.data; diff --git a/lib/Service/LabelService.php b/lib/Service/LabelService.php index 44d2c255d..8bc041ca5 100644 --- a/lib/Service/LabelService.php +++ b/lib/Service/LabelService.php @@ -92,7 +92,14 @@ class LabelService { $this->permissionService->checkPermission(null, $boardId, Acl::PERMISSION_MANAGE); - $this->checkDuplicateTitle($boardId, $title); + //$this->checkDuplicateTitle($boardId, $title); + $boardLabels = $this->labelMapper->findAll($boardId); + foreach($boardLabels as $boardLabel) { + if ($boardLabel->getTitle() === $title) { + throw new BadRequestException('title must be unique'); + break; + } + } if ($this->boardService->isArchived(null, $boardId)) { throw new StatusException('Operation not allowed. This board is archived.'); @@ -157,7 +164,18 @@ class LabelService { $this->permissionService->checkPermission($this->labelMapper, $id, Acl::PERMISSION_MANAGE); $label = $this->find($id); - $this->checkDuplicateTitle($label->getBoardId(), $title); + //$this->checkDuplicateTitle($label->getBoardId(), $title); + + $boardLabels = $this->labelMapper->findAll($label->getBoardId()); + foreach($boardLabels as $boardLabel) { + if ($boardLabel->getId() === $label->getId()) { + continue; + } + if ($boardLabel->getTitle() === $title) { + throw new BadRequestException('title must be unique'); + break; + } + } if ($this->boardService->isArchived($this->labelMapper, $id)) { throw new StatusException('Operation not allowed. This board is archived.'); From e5d3c16a80be9c3053e07c982236d26cb01493ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakob=20R=C3=B6hrl?= Date: Thu, 31 Jan 2019 14:39:46 +0100 Subject: [PATCH 4/8] show different error messages and UI reset after failed update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jakob Röhrl --- js/controller/BoardController.js | 10 +++++++--- js/service/ApiService.js | 4 ++-- lib/Service/LabelService.php | 2 +- templates/part.board.sidebarView.php | 2 +- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/js/controller/BoardController.js b/js/controller/BoardController.js index 6273875cb..65b4f9e68 100644 --- a/js/controller/BoardController.js +++ b/js/controller/BoardController.js @@ -347,15 +347,19 @@ app.controller('BoardController', function ($rootScope, $scope, $stateParams, St $scope.status.createLabel = false; $scope.newLabel = {}; }).catch(err => { - OC.Notification.showTemporary('Duplicate label name is not allowed'); + OC.Notification.showTemporary(err); }); }; + $scope.labelUpdateBefore = function (label) { + label.renameTitle = label.title; + }; + $scope.labelUpdate = function (label) { label.edit = false; LabelService.update(label).catch(err => { - label.title('XXX'); - OC.Notification.showTemporary('Duplicate label name is not allowed'); + label.title = label.renameTitle; + OC.Notification.showTemporary(err); }); // update labels in UI diff --git a/js/service/ApiService.js b/js/service/ApiService.js index 077ed4dd7..d5599e69a 100644 --- a/js/service/ApiService.js +++ b/js/service/ApiService.js @@ -119,7 +119,7 @@ app.factory('ApiService', function ($http, $q) { self.add(response.data); deferred.resolve(response.data); }, function (error) { - deferred.reject('Fetching' + self.endpoint + ' failed'); + deferred.reject(error.data.message); }); return deferred.promise; }; @@ -131,7 +131,7 @@ app.factory('ApiService', function ($http, $q) { self.add(response.data); deferred.resolve(response.data); }, function (error) { - deferred.reject('Updating ' + self.endpoint + ' failed'); + deferred.reject(error.data.message); }); return deferred.promise; diff --git a/lib/Service/LabelService.php b/lib/Service/LabelService.php index 8bc041ca5..a3a9639b9 100644 --- a/lib/Service/LabelService.php +++ b/lib/Service/LabelService.php @@ -153,7 +153,7 @@ class LabelService { throw new BadRequestException('label id must be a number'); } - if ($title === false || $title === null) { + if ($title === false || $title === null || $title === "") { throw new BadRequestException('title must be provided'); } diff --git a/templates/part.board.sidebarView.php b/templates/part.board.sidebarView.php index db35568be..928510b01 100644 --- a/templates/part.board.sidebarView.php +++ b/templates/part.board.sidebarView.php @@ -105,7 +105,7 @@ t('Update tag')); ?> - t('Edit tag')); ?> + t('Edit tag')); ?> t('Delete tag')); ?>
  • From ee20841ad61d778f61ccf139a1bedcb938e17cf6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakob=20R=C3=B6hrl?= Date: Fri, 15 Feb 2019 10:39:33 +0100 Subject: [PATCH 5/8] fix failed unit test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jakob Röhrl --- lib/Service/LabelService.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/Service/LabelService.php b/lib/Service/LabelService.php index a3a9639b9..2fc5658e3 100644 --- a/lib/Service/LabelService.php +++ b/lib/Service/LabelService.php @@ -93,6 +93,7 @@ class LabelService { $this->permissionService->checkPermission(null, $boardId, Acl::PERMISSION_MANAGE); //$this->checkDuplicateTitle($boardId, $title); + $boardLabels = array(); $boardLabels = $this->labelMapper->findAll($boardId); foreach($boardLabels as $boardLabel) { if ($boardLabel->getTitle() === $title) { @@ -166,6 +167,7 @@ class LabelService { $label = $this->find($id); //$this->checkDuplicateTitle($label->getBoardId(), $title); + $boardLabels = array(); $boardLabels = $this->labelMapper->findAll($label->getBoardId()); foreach($boardLabels as $boardLabel) { if ($boardLabel->getId() === $label->getId()) { From d11917e4ffd14c729c49cd89f7bc4a92c842c093 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakob=20R=C3=B6hrl?= Date: Wed, 20 Feb 2019 14:15:31 +0100 Subject: [PATCH 6/8] now the tests are working MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jakob Röhrl --- lib/Service/LabelService.php | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/lib/Service/LabelService.php b/lib/Service/LabelService.php index 2fc5658e3..6b8e09b55 100644 --- a/lib/Service/LabelService.php +++ b/lib/Service/LabelService.php @@ -93,12 +93,13 @@ class LabelService { $this->permissionService->checkPermission(null, $boardId, Acl::PERMISSION_MANAGE); //$this->checkDuplicateTitle($boardId, $title); - $boardLabels = array(); $boardLabels = $this->labelMapper->findAll($boardId); - foreach($boardLabels as $boardLabel) { - if ($boardLabel->getTitle() === $title) { - throw new BadRequestException('title must be unique'); - break; + if (is_array($boardLabels) || is_object($boardLabels)) { + foreach($boardLabels as $boardLabel) { + if ($boardLabel->getTitle() === $title) { + throw new BadRequestException('title must be unique'); + break; + } } } @@ -169,13 +170,15 @@ class LabelService { $boardLabels = array(); $boardLabels = $this->labelMapper->findAll($label->getBoardId()); - foreach($boardLabels as $boardLabel) { - if ($boardLabel->getId() === $label->getId()) { - continue; - } - if ($boardLabel->getTitle() === $title) { - throw new BadRequestException('title must be unique'); - break; + if (is_array($boardLabels) || is_object($boardLabels)) { + foreach($boardLabels as $boardLabel) { + if ($boardLabel->getId() === $label->getId()) { + continue; + } + if ($boardLabel->getTitle() === $title) { + throw new BadRequestException('title must be unique'); + break; + } } } From 4481fc1aceededc9fdd9844de5009c7748d9e7c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Tue, 5 Mar 2019 09:27:28 +0100 Subject: [PATCH 7/8] Remove unused is_object MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Service/LabelService.php | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/Service/LabelService.php b/lib/Service/LabelService.php index 6b8e09b55..5529fa9f2 100644 --- a/lib/Service/LabelService.php +++ b/lib/Service/LabelService.php @@ -94,7 +94,7 @@ class LabelService { //$this->checkDuplicateTitle($boardId, $title); $boardLabels = $this->labelMapper->findAll($boardId); - if (is_array($boardLabels) || is_object($boardLabels)) { + if (\is_array($boardLabels)) { foreach($boardLabels as $boardLabel) { if ($boardLabel->getTitle() === $title) { throw new BadRequestException('title must be unique'); @@ -164,13 +164,12 @@ class LabelService { } $this->permissionService->checkPermission($this->labelMapper, $id, Acl::PERMISSION_MANAGE); - + $label = $this->find($id); //$this->checkDuplicateTitle($label->getBoardId(), $title); - - $boardLabels = array(); + $boardLabels = $this->labelMapper->findAll($label->getBoardId()); - if (is_array($boardLabels) || is_object($boardLabels)) { + if (\is_array($boardLabels)) { foreach($boardLabels as $boardLabel) { if ($boardLabel->getId() === $label->getId()) { continue; @@ -181,7 +180,7 @@ class LabelService { } } } - + if ($this->boardService->isArchived($this->labelMapper, $id)) { throw new StatusException('Operation not allowed. This board is archived.'); } From ee93d64fd03c7aa8571c6b77ff08749a917f41a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Tue, 5 Mar 2019 09:29:28 +0100 Subject: [PATCH 8/8] Fix codacy warnings and remove unused code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- js/controller/BoardController.js | 4 ++-- lib/Service/LabelService.php | 17 ----------------- 2 files changed, 2 insertions(+), 19 deletions(-) diff --git a/js/controller/BoardController.js b/js/controller/BoardController.js index 65b4f9e68..41729c8aa 100644 --- a/js/controller/BoardController.js +++ b/js/controller/BoardController.js @@ -346,7 +346,7 @@ app.controller('BoardController', function ($rootScope, $scope, $stateParams, St BoardService.getCurrent().labels.push(data); $scope.status.createLabel = false; $scope.newLabel = {}; - }).catch(err => { + }).catch((err) => { OC.Notification.showTemporary(err); }); }; @@ -357,7 +357,7 @@ app.controller('BoardController', function ($rootScope, $scope, $stateParams, St $scope.labelUpdate = function (label) { label.edit = false; - LabelService.update(label).catch(err => { + LabelService.update(label).catch((err) => { label.title = label.renameTitle; OC.Notification.showTemporary(err); }); diff --git a/lib/Service/LabelService.php b/lib/Service/LabelService.php index 5529fa9f2..f596b19a2 100644 --- a/lib/Service/LabelService.php +++ b/lib/Service/LabelService.php @@ -92,7 +92,6 @@ class LabelService { $this->permissionService->checkPermission(null, $boardId, Acl::PERMISSION_MANAGE); - //$this->checkDuplicateTitle($boardId, $title); $boardLabels = $this->labelMapper->findAll($boardId); if (\is_array($boardLabels)) { foreach($boardLabels as $boardLabel) { @@ -166,7 +165,6 @@ class LabelService { $this->permissionService->checkPermission($this->labelMapper, $id, Acl::PERMISSION_MANAGE); $label = $this->find($id); - //$this->checkDuplicateTitle($label->getBoardId(), $title); $boardLabels = $this->labelMapper->findAll($label->getBoardId()); if (\is_array($boardLabels)) { @@ -191,19 +189,4 @@ class LabelService { return $this->labelMapper->update($label); } - /** - * @param $boardId - * @param $title - * @throws BadRequestException - */ - private function checkDuplicateTitle($boardId, $title) { - $boardLabels = $this->labelMapper->findAll($boardId); - foreach($boardLabels as $boardLabel) { - if ($boardLabel->getTitle() === $title) { - throw new BadRequestException('title must be unique'); - break; - } - } - } - }