diff --git a/ChangeLog b/ChangeLog index e33bee40e..27b4a3dd9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -11,6 +11,7 @@ New features: Improvements: +* Unify task drop-down menu between different views * Improve LDAP user group membership synchronization * Category and user filters do not append anymore in search field * Added more template hooks diff --git a/app/Controller/BoardPopover.php b/app/Controller/BoardPopover.php index 63dab3021..105841374 100644 --- a/app/Controller/BoardPopover.php +++ b/app/Controller/BoardPopover.php @@ -44,7 +44,7 @@ class BoardPopover extends Base $this->flash->failure(t('Unable to update your task.')); } - $this->response->redirect($this->helper->url->to('board', 'show', array('project_id' => $values['project_id']))); + $this->response->redirect($this->helper->url->to('board', 'show', array('project_id' => $values['project_id'])), true); } /** @@ -81,7 +81,7 @@ class BoardPopover extends Base $this->flash->failure(t('Unable to update your task.')); } - $this->response->redirect($this->helper->url->to('board', 'show', array('project_id' => $values['project_id']))); + $this->response->redirect($this->helper->url->to('board', 'show', array('project_id' => $values['project_id'])), true); } /** diff --git a/app/Controller/Task.php b/app/Controller/Task.php index 902a32d68..072df87be 100644 --- a/app/Controller/Task.php +++ b/app/Controller/Task.php @@ -151,7 +151,7 @@ class Task extends Base { $task = $this->getTask(); - if (! $this->taskPermission->canRemoveTask($task)) { + if (! $this->helper->user->canRemoveTask($task)) { $this->forbidden(); } @@ -164,7 +164,7 @@ class Task extends Base $this->flash->failure(t('Unable to remove this task.')); } - $this->response->redirect($this->helper->url->to('board', 'show', array('project_id' => $task['project_id']))); + $this->response->redirect($this->helper->url->to('board', 'show', array('project_id' => $task['project_id'])), true); } $this->response->html($this->template->render('task/remove', array( diff --git a/app/Core/Base.php b/app/Core/Base.php index 2b619af5b..c065ea2a8 100644 --- a/app/Core/Base.php +++ b/app/Core/Base.php @@ -92,7 +92,6 @@ use Pimple\Container; * @property \Kanboard\Model\TaskFinder $taskFinder * @property \Kanboard\Model\TaskLink $taskLink * @property \Kanboard\Model\TaskModification $taskModification - * @property \Kanboard\Model\TaskPermission $taskPermission * @property \Kanboard\Model\TaskPosition $taskPosition * @property \Kanboard\Model\TaskStatus $taskStatus * @property \Kanboard\Model\TaskMetadata $taskMetadata diff --git a/app/Helper/UserHelper.php b/app/Helper/UserHelper.php index c3369dfd7..b39d7b621 100644 --- a/app/Helper/UserHelper.php +++ b/app/Helper/UserHelper.php @@ -3,6 +3,7 @@ namespace Kanboard\Helper; use Kanboard\Core\Base; +use Kanboard\Core\Security\Role; /** * User helpers @@ -41,6 +42,17 @@ class UserHelper extends Base return mb_strtoupper($initials); } + /** + * Return the user full name + * + * @param array $user User properties + * @return string + */ + public function getFullname(array $user = array()) + { + return $this->user->getFullname(empty($user) ? $this->userSession->getAll() : $user); + } + /** * Get user id * @@ -149,13 +161,24 @@ class UserHelper extends Base } /** - * Return the user full name + * Return true if the user can remove a task * - * @param array $user User properties - * @return string + * Regular users can't remove tasks from other people + * + * @public + * @param array $task + * @return bool */ - public function getFullname(array $user = array()) + public function canRemoveTask(array $task) { - return $this->user->getFullname(empty($user) ? $this->userSession->getAll() : $user); + if (isset($task['creator_id']) && $task['creator_id'] == $this->userSession->getId()) { + return true; + } + + if ($this->userSession->isAdmin() || $this->getProjectUserRole($task['project_id']) === Role::PROJECT_MANAGER) { + return true; + } + + return false; } } diff --git a/app/Model/TaskFinder.php b/app/Model/TaskFinder.php index a1aa0f581..28ddb88fc 100644 --- a/app/Model/TaskFinder.php +++ b/app/Model/TaskFinder.php @@ -71,6 +71,8 @@ class TaskFinder extends Base 'tasks.priority', 'tasks.time_spent', 'tasks.time_estimated', + 'tasks.is_active', + 'tasks.creator_id', 'projects.name AS project_name' ) ->join(Project::TABLE, 'id', 'project_id') diff --git a/app/Model/TaskPermission.php b/app/Model/TaskPermission.php deleted file mode 100644 index b1e025893..000000000 --- a/app/Model/TaskPermission.php +++ /dev/null @@ -1,34 +0,0 @@ -userSession->isAdmin() || $this->projectUserRole->getUserRole($task['project_id'], $this->userSession->getId()) === Role::PROJECT_MANAGER) { - return true; - } elseif (isset($task['creator_id']) && $task['creator_id'] == $this->userSession->getId()) { - return true; - } - - return false; - } -} diff --git a/app/ServiceProvider/ClassProvider.php b/app/ServiceProvider/ClassProvider.php index 18c1d5784..54e2ad784 100644 --- a/app/ServiceProvider/ClassProvider.php +++ b/app/ServiceProvider/ClassProvider.php @@ -63,7 +63,6 @@ class ClassProvider implements ServiceProviderInterface 'TaskFile', 'TaskLink', 'TaskModification', - 'TaskPermission', 'TaskPosition', 'TaskStatus', 'TaskMetadata', diff --git a/app/Template/board/task_menu.php b/app/Template/board/task_menu.php deleted file mode 100644 index c0d97cdad..000000000 --- a/app/Template/board/task_menu.php +++ /dev/null @@ -1,18 +0,0 @@ - - - - \ No newline at end of file diff --git a/app/Template/board/task_private.php b/app/Template/board/task_private.php index 19bcbcfa3..57623042f 100644 --- a/app/Template/board/task_private.php +++ b/app/Template/board/task_private.php @@ -17,7 +17,7 @@
user->hasProjectAccess('taskmodification', 'edit', $task['project_id'])): ?> - render('board/task_menu', array('task' => $task)) ?> + render('task/dropdown', array('task' => $task)) ?> @@ -33,7 +33,7 @@
user->hasProjectAccess('taskmodification', 'edit', $task['project_id'])): ?> - render('board/task_menu', array('task' => $task)) ?> + render('task/dropdown', array('task' => $task)) ?> diff --git a/app/Template/task/dropdown.php b/app/Template/task/dropdown.php index 6fea3728a..567249df5 100644 --- a/app/Template/task/dropdown.php +++ b/app/Template/task/dropdown.php @@ -7,6 +7,14 @@ url->link(t('Set automatically the start date'), 'taskmodification', 'start', array('task_id' => $task['id'], 'project_id' => $task['project_id'])) ?> +
  • + + url->link(t('Change assignee'), 'BoardPopover', 'changeAssignee', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?> +
    • +
  • + + url->link(t('Change category'), 'BoardPopover', 'changeCategory', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?> +
  • url->link(t('Edit the task'), 'taskmodification', 'edit', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?> @@ -31,6 +39,10 @@ url->link(t('Add a comment'), 'comment', 'create', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?>
    • +
  • + + url->link(t('Add a screenshot'), 'BoardPopover', 'screenshot', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?> +
  • url->link(t('Duplicate'), 'taskduplication', 'duplicate', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?> @@ -43,6 +55,12 @@ url->link(t('Move to another project'), 'taskduplication', 'move', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?>
    • + user->canRemoveTask($task)): ?> +
  • + + url->link(t('Remove'), 'task', 'remove', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?> +
    • +
  • diff --git a/app/Template/task/remove.php b/app/Template/task/remove.php index eb0809b10..b869b6469 100644 --- a/app/Template/task/remove.php +++ b/app/Template/task/remove.php @@ -8,8 +8,8 @@

    - url->link(t('Yes'), 'task', 'remove', array('task_id' => $task['id'], 'project_id' => $task['project_id'], 'confirmation' => 'yes'), true, 'btn btn-red') ?> + url->link(t('Yes'), 'task', 'remove', array('task_id' => $task['id'], 'project_id' => $task['project_id'], 'confirmation' => 'yes'), true, 'btn btn-red popover-link') ?> url->link(t('cancel'), 'task', 'show', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'close-popover') ?>
    -
    • \ No newline at end of file +
    diff --git a/app/Template/task/sidebar.php b/app/Template/task/sidebar.php index a2d73b8c9..46f9e1a25 100644 --- a/app/Template/task/sidebar.php +++ b/app/Template/task/sidebar.php @@ -87,7 +87,7 @@ url->link(t('Open this task'), 'taskstatus', 'open', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?> - task->canRemove($task)): ?> + user->canRemoveTask($task)): ?>
  • url->link(t('Remove'), 'task', 'remove', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?> diff --git a/tests/units/Helper/UserHelperTest.php b/tests/units/Helper/UserHelperTest.php index 9a9832b2f..135f8ca61 100644 --- a/tests/units/Helper/UserHelperTest.php +++ b/tests/units/Helper/UserHelperTest.php @@ -2,11 +2,15 @@ require_once __DIR__.'/../Base.php'; +use Kanboard\Core\User\UserSession; use Kanboard\Helper\UserHelper; use Kanboard\Model\Project; use Kanboard\Model\ProjectUserRole; +use Kanboard\Model\TaskCreation; +use Kanboard\Model\TaskFinder; use Kanboard\Model\User as UserModel; use Kanboard\Core\Security\Role; +use Kanboard\Model\User; class UserHelperTest extends Base { @@ -228,4 +232,94 @@ class UserHelperTest extends Base $this->assertFalse($helper->hasProjectAccess('task', 'show', 2)); $this->assertFalse($helper->hasProjectAccess('taskcreation', 'save', 2)); } + + public function testCanRemoveTask() + { + $taskCreationModel = new TaskCreation($this->container); + $taskFinderModel = new TaskFinder($this->container); + $helper = new UserHelper($this->container); + $projectModel = new Project($this->container); + $userModel = new User($this->container); + $userSessionModel = new UserSession($this->container); + + $this->assertNotFalse($userModel->create(array('username' => 'toto', 'password' => '123456'))); + $this->assertNotFalse($userModel->create(array('username' => 'toto2', 'password' => '123456'))); + $this->assertEquals(1, $projectModel->create(array('name' => 'Project #1'))); + $this->assertEquals(1, $taskCreationModel->create(array('title' => 'Task #1', 'project_id' => 1, 'creator_id' => 1))); + $this->assertEquals(2, $taskCreationModel->create(array('title' => 'Task #2', 'project_id' => 1, 'creator_id' => 2))); + $this->assertEquals(3, $taskCreationModel->create(array('title' => 'Task #3', 'project_id' => 1, 'creator_id' => 3))); + $this->assertEquals(4, $taskCreationModel->create(array('title' => 'Task #4', 'project_id' => 1))); + + // User #1 can remove everything + $user = $userModel->getById(1); + $this->assertNotEmpty($user); + $userSessionModel->initialize($user); + + $task = $taskFinderModel->getById(1); + $this->assertNotEmpty($task); + $this->assertTrue($helper->canRemoveTask($task)); + + // User #2 can't remove the task #1 + $user = $userModel->getById(2); + $this->assertNotEmpty($user); + $userSessionModel->initialize($user); + + $task = $taskFinderModel->getById(1); + $this->assertNotEmpty($task); + $this->assertFalse($helper->canRemoveTask($task)); + + // User #1 can remove everything + $user = $userModel->getById(1); + $this->assertNotEmpty($user); + $userSessionModel->initialize($user); + + $task = $taskFinderModel->getById(2); + $this->assertNotEmpty($task); + $this->assertTrue($helper->canRemoveTask($task)); + + // User #2 can remove his own task + $user = $userModel->getbyId(2); + $this->assertNotEmpty($user); + $userSessionModel->initialize($user); + + $task = $taskFinderModel->getById(2); + $this->assertNotEmpty($task); + $this->assertTrue($helper->canRemoveTask($task)); + + // User #1 can remove everything + $user = $userModel->getById(1); + $this->assertNotEmpty($user); + $userSessionModel->initialize($user); + + $task = $taskFinderModel->getById(3); + $this->assertNotEmpty($task); + $this->assertTrue($helper->canRemoveTask($task)); + + // User #2 can't remove the task #3 + $user = $userModel->getById(2); + $this->assertNotEmpty($user); + $userSessionModel->initialize($user); + + $task = $taskFinderModel->getById(3); + $this->assertNotEmpty($task); + $this->assertFalse($helper->canRemoveTask($task)); + + // User #1 can remove everything + $user = $userModel->getById(1); + $this->assertNotEmpty($user); + $userSessionModel->initialize($user); + + $task = $taskFinderModel->getById(4); + $this->assertNotEmpty($task); + $this->assertTrue($helper->canRemoveTask($task)); + + // User #2 can't remove the task #4 + $user = $userModel->getById(2); + $this->assertNotEmpty($user); + $userSessionModel->initialize($user); + + $task = $taskFinderModel->getById(4); + $this->assertNotEmpty($task); + $this->assertFalse($helper->canRemoveTask($task)); + } } diff --git a/tests/units/Model/TaskPermissionTest.php b/tests/units/Model/TaskPermissionTest.php deleted file mode 100644 index 82cd581ef..000000000 --- a/tests/units/Model/TaskPermissionTest.php +++ /dev/null @@ -1,103 +0,0 @@ -container); - $tf = new TaskFinder($this->container); - $tp = new TaskPermission($this->container); - $p = new Project($this->container); - $u = new User($this->container); - $us = new UserSession($this->container); - - $this->assertNotFalse($u->create(array('username' => 'toto', 'password' => '123456'))); - $this->assertNotFalse($u->create(array('username' => 'toto2', 'password' => '123456'))); - $this->assertEquals(1, $p->create(array('name' => 'Project #1'))); - $this->assertEquals(1, $tc->create(array('title' => 'Task #1', 'project_id' => 1, 'creator_id' => 1))); - $this->assertEquals(2, $tc->create(array('title' => 'Task #2', 'project_id' => 1, 'creator_id' => 2))); - $this->assertEquals(3, $tc->create(array('title' => 'Task #3', 'project_id' => 1, 'creator_id' => 3))); - $this->assertEquals(4, $tc->create(array('title' => 'Task #4', 'project_id' => 1))); - - // User #1 can remove everything - $user = $u->getbyId(1); - $this->assertNotEmpty($user); - $us->initialize($user); - - $task = $tf->getbyId(1); - $this->assertNotEmpty($task); - $this->assertTrue($tp->canRemoveTask($task)); - - // User #2 can't remove the task #1 - $user = $u->getbyId(2); - $this->assertNotEmpty($user); - $us->initialize($user); - - $task = $tf->getbyId(1); - $this->assertNotEmpty($task); - $this->assertFalse($tp->canRemoveTask($task)); - - // User #1 can remove everything - $user = $u->getbyId(1); - $this->assertNotEmpty($user); - $us->initialize($user); - - $task = $tf->getbyId(2); - $this->assertNotEmpty($task); - $this->assertTrue($tp->canRemoveTask($task)); - - // User #2 can remove his own task - $user = $u->getbyId(2); - $this->assertNotEmpty($user); - $us->initialize($user); - - $task = $tf->getbyId(2); - $this->assertNotEmpty($task); - $this->assertTrue($tp->canRemoveTask($task)); - - // User #1 can remove everything - $user = $u->getbyId(1); - $this->assertNotEmpty($user); - $us->initialize($user); - - $task = $tf->getbyId(3); - $this->assertNotEmpty($task); - $this->assertTrue($tp->canRemoveTask($task)); - - // User #2 can't remove the task #3 - $user = $u->getbyId(2); - $this->assertNotEmpty($user); - $us->initialize($user); - - $task = $tf->getbyId(3); - $this->assertNotEmpty($task); - $this->assertFalse($tp->canRemoveTask($task)); - - // User #1 can remove everything - $user = $u->getbyId(1); - $this->assertNotEmpty($user); - $us->initialize($user); - - $task = $tf->getbyId(4); - $this->assertNotEmpty($task); - $this->assertTrue($tp->canRemoveTask($task)); - - // User #2 can't remove the task #4 - $user = $u->getbyId(2); - $this->assertNotEmpty($user); - $us->initialize($user); - - $task = $tf->getbyId(4); - $this->assertNotEmpty($task); - $this->assertFalse($tp->canRemoveTask($task)); - } -}