summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--ChangeLog1
-rw-r--r--app/Controller/BoardPopover.php4
-rw-r--r--app/Controller/Task.php4
-rw-r--r--app/Core/Base.php1
-rw-r--r--app/Helper/UserHelper.php33
-rw-r--r--app/Model/TaskFinder.php2
-rw-r--r--app/Model/TaskPermission.php34
-rw-r--r--app/ServiceProvider/ClassProvider.php1
-rw-r--r--app/Template/board/task_menu.php18
-rw-r--r--app/Template/board/task_private.php4
-rw-r--r--app/Template/task/dropdown.php18
-rw-r--r--app/Template/task/remove.php4
-rw-r--r--app/Template/task/sidebar.php2
-rw-r--r--tests/units/Helper/UserHelperTest.php94
-rw-r--r--tests/units/Model/TaskPermissionTest.php103
15 files changed, 152 insertions, 171 deletions
diff --git a/ChangeLog b/ChangeLog
index e33bee40..27b4a3dd 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 63dab302..10584137 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 902a32d6..072df87b 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 2b619af5..c065ea2a 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 c3369dfd..b39d7b62 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
@@ -42,6 +43,17 @@ class UserHelper extends Base
}
/**
+ * 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
*
* @access public
@@ -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 a1aa0f58..28ddb88f 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 b1e02589..00000000
--- a/app/Model/TaskPermission.php
+++ /dev/null
@@ -1,34 +0,0 @@
-<?php
-
-namespace Kanboard\Model;
-
-use Kanboard\Core\Security\Role;
-
-/**
- * Task permission model
- *
- * @package model
- * @author Frederic Guillot
- */
-class TaskPermission extends Base
-{
- /**
- * Return true if the user can remove a task
- *
- * Regular users can't remove tasks from other people
- *
- * @public
- * @param array $task
- * @return bool
- */
- public function canRemoveTask(array $task)
- {
- if ($this->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 18c1d578..54e2ad78 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 c0d97cda..00000000
--- a/app/Template/board/task_menu.php
+++ /dev/null
@@ -1,18 +0,0 @@
-<span class="dropdown">
- <a href="#" class="dropdown-menu"><?= '#'.$task['id'] ?> <i class="fa fa-caret-down"></i></a>
- <ul>
- <li><i class="fa fa-user fa-fw"></i>&nbsp;<?= $this->url->link(t('Change assignee'), 'BoardPopover', 'changeAssignee', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?></li>
- <li><i class="fa fa-tag fa-fw"></i>&nbsp;<?= $this->url->link(t('Change category'), 'BoardPopover', 'changeCategory', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?></li>
- <li><i class="fa fa-align-left fa-fw"></i>&nbsp;<?= $this->url->link(t('Change description'), 'taskmodification', 'description', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?></li>
- <li><i class="fa fa-pencil-square-o fa-fw"></i>&nbsp;<?= $this->url->link(t('Edit this task'), 'taskmodification', 'edit', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?></li>
- <li><i class="fa fa-comment-o fa-fw"></i>&nbsp;<?= $this->url->link(t('Add a comment'), 'comment', 'create', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?></li>
- <li><i class="fa fa-code-fork fa-fw"></i>&nbsp;<?= $this->url->link(t('Add internal link'), 'TaskInternalLink', 'create', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?></li>
- <li><i class="fa fa-external-link fa-fw"></i>&nbsp;<?= $this->url->link(t('Add external link'), 'TaskExternalLink', 'find', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?></li>
- <li><i class="fa fa-camera fa-fw"></i>&nbsp;<?= $this->url->link(t('Add a screenshot'), 'BoardPopover', 'screenshot', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?></li>
- <?php if ($task['is_active'] == 1): ?>
- <li><i class="fa fa-close fa-fw"></i>&nbsp;<?= $this->url->link(t('Close this task'), 'taskstatus', 'close', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?></li>
- <?php else: ?>
- <li><i class="fa fa-check-square-o fa-fw"></i>&nbsp;<?= $this->url->link(t('Open this task'), 'taskstatus', 'open', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?></li>
- <?php endif ?>
- </ul>
-</span> \ No newline at end of file
diff --git a/app/Template/board/task_private.php b/app/Template/board/task_private.php
index 19bcbcfa..57623042 100644
--- a/app/Template/board/task_private.php
+++ b/app/Template/board/task_private.php
@@ -17,7 +17,7 @@
<div class="task-board-collapsed">
<div class="task-board-saving-icon" style="display: none;"><i class="fa fa-spinner fa-pulse"></i></div>
<?php if ($this->user->hasProjectAccess('taskmodification', 'edit', $task['project_id'])): ?>
- <?= $this->render('board/task_menu', array('task' => $task)) ?>
+ <?= $this->render('task/dropdown', array('task' => $task)) ?>
<?php else: ?>
<strong><?= '#'.$task['id'] ?></strong>
<?php endif ?>
@@ -33,7 +33,7 @@
<div class="task-board-expanded">
<div class="task-board-saving-icon" style="display: none;"><i class="fa fa-spinner fa-pulse fa-2x"></i></div>
<?php if ($this->user->hasProjectAccess('taskmodification', 'edit', $task['project_id'])): ?>
- <?= $this->render('board/task_menu', array('task' => $task)) ?>
+ <?= $this->render('task/dropdown', array('task' => $task)) ?>
<?php else: ?>
<strong><?= '#'.$task['id'] ?></strong>
<?php endif ?>
diff --git a/app/Template/task/dropdown.php b/app/Template/task/dropdown.php
index 6fea3728..567249df 100644
--- a/app/Template/task/dropdown.php
+++ b/app/Template/task/dropdown.php
@@ -8,6 +8,14 @@
</li>
<?php endif ?>
<li>
+ <i class="fa fa-user fa-fw"></i>
+ <?= $this->url->link(t('Change assignee'), 'BoardPopover', 'changeAssignee', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?>
+ </li>
+ <li>
+ <i class="fa fa-tag fa-fw"></i>
+ <?= $this->url->link(t('Change category'), 'BoardPopover', 'changeCategory', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?>
+ </li>
+ <li>
<i class="fa fa-pencil-square-o fa-fw"></i>
<?= $this->url->link(t('Edit the task'), 'taskmodification', 'edit', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?>
</li>
@@ -32,6 +40,10 @@
<?= $this->url->link(t('Add a comment'), 'comment', 'create', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?>
</li>
<li>
+ <i class="fa fa-camera fa-fw"></i>
+ <?= $this->url->link(t('Add a screenshot'), 'BoardPopover', 'screenshot', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?>
+ </li>
+ <li>
<i class="fa fa-files-o fa-fw"></i>
<?= $this->url->link(t('Duplicate'), 'taskduplication', 'duplicate', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?>
</li>
@@ -43,6 +55,12 @@
<i class="fa fa-clone fa-fw"></i>
<?= $this->url->link(t('Move to another project'), 'taskduplication', 'move', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?>
</li>
+ <?php if ($this->user->canRemoveTask($task)): ?>
+ <li>
+ <i class="fa fa-trash-o fa-fw"></i>
+ <?= $this->url->link(t('Remove'), 'task', 'remove', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?>
+ </li>
+ <?php endif ?>
<?php if (isset($task['is_active'])): ?>
<li>
<?php if ($task['is_active'] == 1): ?>
diff --git a/app/Template/task/remove.php b/app/Template/task/remove.php
index eb0809b1..b869b646 100644
--- a/app/Template/task/remove.php
+++ b/app/Template/task/remove.php
@@ -8,8 +8,8 @@
</p>
<div class="form-actions">
- <?= $this->url->link(t('Yes'), 'task', 'remove', array('task_id' => $task['id'], 'project_id' => $task['project_id'], 'confirmation' => 'yes'), true, 'btn btn-red') ?>
+ <?= $this->url->link(t('Yes'), 'task', 'remove', array('task_id' => $task['id'], 'project_id' => $task['project_id'], 'confirmation' => 'yes'), true, 'btn btn-red popover-link') ?>
<?= t('or') ?>
<?= $this->url->link(t('cancel'), 'task', 'show', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'close-popover') ?>
</div>
-</div> \ No newline at end of file
+</div>
diff --git a/app/Template/task/sidebar.php b/app/Template/task/sidebar.php
index a2d73b8c..46f9e1a2 100644
--- a/app/Template/task/sidebar.php
+++ b/app/Template/task/sidebar.php
@@ -87,7 +87,7 @@
<?= $this->url->link(t('Open this task'), 'taskstatus', 'open', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?>
<?php endif ?>
</li>
- <?php if ($this->task->canRemove($task)): ?>
+ <?php if ($this->user->canRemoveTask($task)): ?>
<li>
<i class="fa fa-trash-o fa-fw"></i>
<?= $this->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 9a9832b2..135f8ca6 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 82cd581e..00000000
--- a/tests/units/Model/TaskPermissionTest.php
+++ /dev/null
@@ -1,103 +0,0 @@
-<?php
-
-require_once __DIR__.'/../Base.php';
-
-use Kanboard\Model\TaskCreation;
-use Kanboard\Model\TaskFinder;
-use Kanboard\Model\TaskPermission;
-use Kanboard\Model\Project;
-use Kanboard\Model\User;
-use Kanboard\Core\User\UserSession;
-
-class TaskPermissionTest extends Base
-{
- public function testPrepareCreation()
- {
- $tc = new TaskCreation($this->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));
- }
-}
generated by cgit v1.2.3 (git 2.46.0) at 2024-11-26 13:56:39 +0000