Improve column restrictions

This commit is contained in:
Frederic Guillot 2016-09-11 18:32:47 -04:00
parent d8f6d85683
commit c84df535b6
No known key found for this signature in database
GPG Key ID: 92D77191BA7FBC99
9 changed files with 66 additions and 94 deletions

View File

@ -30,6 +30,10 @@ class TaskMovePositionController extends BaseController
$task = $this->getTask();
$values = $this->request->getJson();
if (! $this->helper->projectRole->canMoveTask($task['project_id'], $task['column_id'], $values['column_id'])) {
throw new AccessForbiddenException(e("You don't have the permission to move this task"));
}
$result = $this->taskPositionModel->movePosition(
$task['project_id'],
$task['id'],

View File

@ -38,17 +38,18 @@ class ColumnMoveRestrictionCacheDecorator
}
/**
* Proxy method to get column Ids
* Proxy method to get sortable columns
*
* @param int $project_id
* @return array|mixed
*/
public function getAllSrcColumns($project_id, $role)
public function getSortableColumns($project_id, $role)
{
$key = $this->cachePrefix.$project_id.$role;
$columnIds = $this->cache->get($key);
if ($columnIds === null) {
$columnIds = $this->columnMoveRestrictionModel->getAllSrcColumns($project_id, $role);
$columnIds = $this->columnMoveRestrictionModel->getSortableColumns($project_id, $role);
$this->cache->set($key, $columnIds);
}

View File

@ -24,7 +24,7 @@ class BoardSwimlaneFormatter extends BaseFormatter implements FormatterInterface
* @param array $swimlanes
* @return $this
*/
public function withSwimlanes($swimlanes)
public function withSwimlanes(array $swimlanes)
{
$this->swimlanes = $swimlanes;
return $this;
@ -37,7 +37,7 @@ class BoardSwimlaneFormatter extends BaseFormatter implements FormatterInterface
* @param array $columns
* @return $this
*/
public function withColumns($columns)
public function withColumns(array $columns)
{
$this->columns = $columns;
return $this;

View File

@ -26,27 +26,47 @@ class ProjectRoleHelper extends Base
}
/**
* Return true if the task can be moved by the connected user
* Return true if the task can be moved by the logged user
*
* @param array $task
* @return bool
*/
public function isDraggable(array $task)
public function isDraggable(array &$task)
{
if ($task['is_active'] == 1 && $this->helper->user->hasProjectAccess('BoardViewController', 'save', $task['project_id'])) {
$role = $this->getProjectUserRole($task['project_id']);
if ($this->role->isCustomProjectRole($role)) {
$srcColumnIds = $this->columnMoveRestrictionCacheDecorator->getAllSrcColumns($task['project_id'], $role);
return isset($srcColumnIds[$task['column_id']]);
}
return true;
return $this->isSortableColumn($task['project_id'], $task['column_id'], 'src_column_id');
}
return false;
}
/**
* Return true is the column is sortable
*
* @param int $project_id
* @param int $column_id
* @param string $field
* @return bool
*/
public function isSortableColumn($project_id, $column_id, $field)
{
$role = $this->getProjectUserRole($project_id);
if ($this->role->isCustomProjectRole($role)) {
$sortableColumns = $this->columnMoveRestrictionCacheDecorator->getSortableColumns($project_id, $role);
foreach ($sortableColumns as $column) {
if ($column[$field] == $column_id) {
return true;
}
}
return empty($sortableColumns);
}
return true;
}
/**
* Check if the user can move a task
*
@ -60,12 +80,19 @@ class ProjectRoleHelper extends Base
$role = $this->getProjectUserRole($project_id);
if ($this->role->isCustomProjectRole($role)) {
return $this->columnMoveRestrictionModel->isAllowed(
$project_id,
$role,
$src_column_id,
$dst_column_id
);
if ($src_column_id == $dst_column_id) {
return true;
}
$sortableColumns = $this->columnMoveRestrictionCacheDecorator->getSortableColumns($project_id, $role);
foreach ($sortableColumns as $column) {
if ($column['src_column_id'] == $src_column_id && $column['dst_column_id'] == $dst_column_id) {
return true;
}
}
return empty($sortableColumns);
}
return true;

View File

@ -14,26 +14,6 @@ class ColumnMoveRestrictionModel extends Base
{
const TABLE = 'column_has_move_restrictions';
/**
* Check if the custom project role is allowed to move a task
*
* @param int $project_id
* @param string $role
* @param int $src_column_id
* @param int $dst_column_id
* @return int
*/
public function isAllowed($project_id, $role, $src_column_id, $dst_column_id)
{
return $this->db->table(self::TABLE)
->left(ProjectRoleModel::TABLE, 'pr', 'role_id', self::TABLE, 'role_id')
->eq(self::TABLE.'.project_id', $project_id)
->eq(self::TABLE.'.src_column_id', $src_column_id)
->eq(self::TABLE.'.dst_column_id', $dst_column_id)
->eq('pr.role', $role)
->exists();
}
/**
* Fetch one restriction
*
@ -91,20 +71,21 @@ class ColumnMoveRestrictionModel extends Base
}
/**
* Get all source column Ids
* Get all sortable column Ids
*
* @param int $project_id
* @param string $role
* @return array
*/
public function getAllSrcColumns($project_id, $role)
public function getSortableColumns($project_id, $role)
{
return $this->db
->hashtable(self::TABLE)
->table(self::TABLE)
->columns(self::TABLE.'.src_column_id', self::TABLE.'.dst_column_id')
->left(ProjectRoleModel::TABLE, 'pr', 'role_id', self::TABLE, 'role_id')
->eq(self::TABLE.'.project_id', $project_id)
->eq('pr.role', $role)
->getAll('src_column_id', 'src_column_id');
->findAll();
}
/**

View File

@ -8,7 +8,12 @@
>
<!-- tasks list -->
<div class="board-task-list board-column-expanded" data-column-id="<?= $column['id'] ?>" data-swimlane-id="<?= $swimlane['id'] ?>" data-task-limit="<?= $column['task_limit'] ?>">
<div
class="board-task-list board-column-expanded <?= $this->projectRole->isSortableColumn($column['project_id'], $column['id'], 'dst_column_id') ? 'sortable-column' : '' ?>"
data-column-id="<?= $column['id'] ?>"
data-swimlane-id="<?= $swimlane['id'] ?>"
data-task-limit="<?= $column['task_limit'] ?>">
<?php foreach ($column['tasks'] as $task): ?>
<?= $this->render($not_editable ? 'board/task_public' : 'board/task_private', array(
'project' => $project,

File diff suppressed because one or more lines are too long

View File

@ -16,7 +16,7 @@ Kanboard.BoardDragAndDrop.prototype.dragAndDrop = function() {
var params = {
forcePlaceholderSize: true,
tolerance: "pointer",
connectWith: ".board-task-list",
connectWith: ".sortable-column",
placeholder: "draggable-placeholder",
items: ".draggable-item",
stop: function(event, ui) {

View File

@ -54,32 +54,6 @@ class ColumnMoveRestrictionModelTest extends Base
$this->assertEquals(3, $restriction['dst_column_id']);
}
public function testGetSrcColumns()
{
$projectModel = new ProjectModel($this->container);
$projectRoleModel = new ProjectRoleModel($this->container);
$columnMoveRestrictionModel = new ColumnMoveRestrictionModel($this->container);
$this->assertEquals(1, $projectModel->create(array('name' => 'Test')));
$this->assertEquals(2, $projectModel->create(array('name' => 'Test')));
$this->assertEquals(1, $projectRoleModel->create(1, 'Role A'));
$this->assertEquals(2, $projectRoleModel->create(1, 'Role B'));
$this->assertEquals(3, $projectRoleModel->create(2, 'Role C'));
$this->assertEquals(1, $columnMoveRestrictionModel->create(1, 1, 2, 3));
$this->assertEquals(2, $columnMoveRestrictionModel->create(1, 2, 3, 4));
$columnIds = $columnMoveRestrictionModel->getAllSrcColumns(1, 'Role A');
$this->assertEquals(2, $columnIds[2]);
$this->assertEmpty($columnMoveRestrictionModel->getAllSrcColumns(2, 'Role A'));
$this->assertEmpty($columnMoveRestrictionModel->getAllSrcColumns(2, 'Role C'));
$columnIds = $columnMoveRestrictionModel->getAllSrcColumns(1, 'Role B');
$this->assertEquals(3, $columnIds[3]);
}
public function testGetAll()
{
$projectModel = new ProjectModel($this->container);
@ -115,24 +89,4 @@ class ColumnMoveRestrictionModelTest extends Base
$this->assertEquals(3, $restrictions[1]['src_column_id']);
$this->assertEquals(4, $restrictions[1]['dst_column_id']);
}
public function testIsAllowed()
{
$projectModel = new ProjectModel($this->container);
$projectRoleModel = new ProjectRoleModel($this->container);
$columnMoveRestrictionModel = new ColumnMoveRestrictionModel($this->container);
$this->assertEquals(1, $projectModel->create(array('name' => 'Test')));
$this->assertEquals(2, $projectModel->create(array('name' => 'Test')));
$this->assertEquals(1, $projectRoleModel->create(1, 'Role A'));
$this->assertEquals(2, $projectRoleModel->create(1, 'Role B'));
$this->assertEquals(3, $projectRoleModel->create(2, 'Role C'));
$this->assertEquals(1, $columnMoveRestrictionModel->create(1, 1, 2, 3));
$this->assertEquals(2, $columnMoveRestrictionModel->create(1, 2, 3, 4));
$this->assertFalse($columnMoveRestrictionModel->isAllowed(1, 'Role A', 1, 2));
$this->assertTrue($columnMoveRestrictionModel->isAllowed(1, 'Role A', 2, 3));
}
}