Task move position refactoring

This commit is contained in:
Frédéric Guillot 2014-09-04 20:14:26 -07:00
parent 749136361e
commit 954bed954f
10 changed files with 344 additions and 105 deletions

View File

@ -396,27 +396,31 @@ class Board extends Base
*/
public function save()
{
if ($this->request->isAjax()) {
$project_id = $this->request->getIntegerParam('project_id');
if ($project_id > 0 && $this->request->isAjax()) {
if (! $this->project->isUserAllowed($project_id, $this->acl->getUserId())) {
$this->response->status(401);
}
$project_id = $this->request->getIntegerParam('project_id');
$values = $this->request->getValues();
if ($project_id > 0 && ! $this->project->isUserAllowed($project_id, $this->acl->getUserId())) {
$this->response->text('Not Authorized', 401);
}
if ($this->task->movePosition($project_id, $values['task_id'], $values['column_id'], $values['position'])) {
if (isset($values['positions'])) {
$this->board->saveTasksPosition($values['positions'], $values['selected_task_id']);
$this->response->html(
$this->template->load('board_show', array(
'current_project_id' => $project_id,
'board' => $this->board->get($project_id),
'categories' => $this->category->getList($project_id, false),
)),
201
);
}
else {
$this->response->html(
$this->template->load('board_show', array(
'current_project_id' => $project_id,
'board' => $this->board->get($project_id),
'categories' => $this->category->getList($project_id, false),
)),
201
);
$this->response->status(400);
}
}
else {
$this->response->status(401);

View File

@ -20,32 +20,6 @@ class Board extends Base
*/
const TABLE = 'columns';
/**
* Save task positions for each column
*
* @access public
* @param array $positions [['task_id' => X, 'column_id' => X, 'position' => X], ...]
* @param integer $selected_task_id The selected task id
* @return boolean
*/
public function saveTasksPosition(array $positions, $selected_task_id)
{
$this->db->startTransaction();
foreach ($positions as $value) {
// We trigger events only for the selected task
if (! $this->task->movePosition($value['task_id'], $value['column_id'], $value['position'], $value['task_id'] == $selected_task_id)) {
$this->db->cancelTransaction();
return false;
}
}
$this->db->closeTransaction();
return true;
}
/**
* Create a board with default columns, must be executed inside a transaction
*

View File

@ -421,10 +421,9 @@ class Task extends Base
*
* @access public
* @param array $values Form values
* @param boolean $trigger_events Flag to trigger events
* @return boolean
*/
public function update(array $values, $trigger_events = true)
public function update(array $values)
{
// Fetch original task
$original_task = $this->getById($values['id']);
@ -436,21 +435,14 @@ class Task extends Base
// Prepare data
$this->prepare($values);
$updated_task = $values;
$updated_task['date_modification'] = time();
unset($updated_task['id']);
// We update the modification date only for the selected task to highlight recent moves
if ($trigger_events) {
$updated_task['date_modification'] = time();
}
$result = $this->db->table(self::TABLE)->eq('id', $values['id'])->update($updated_task);
// Trigger events
if ($result && $trigger_events) {
if ($this->db->table(self::TABLE)->eq('id', $values['id'])->update($updated_task)) {
$this->triggerUpdateEvents($original_task, $updated_task);
}
return $result;
return true;
}
/**
@ -548,23 +540,99 @@ class Task extends Base
* Move a task to another column or to another position
*
* @access public
* @param integer $project_id Project id
* @param integer $task_id Task id
* @param integer $column_id Column id
* @param integer $position Position (must be greater than 1)
* @param boolean $trigger_events Flag to trigger events
* @param integer $position Position (must be >= 1)
* @return boolean
*/
public function movePosition($task_id, $column_id, $position, $trigger_events = true)
public function movePosition($project_id, $task_id, $column_id, $position)
{
$this->event->clearTriggeredEvents();
// The position can't be lower than 1
if ($position < 1) {
return false;
}
$values = array(
'id' => $task_id,
'column_id' => $column_id,
'position' => $position,
);
// We fetch all tasks on the board
$tasks = $this->db->table(self::TABLE)
->columns('id', 'column_id', 'position')
->eq('is_active', 1)
->eq('project_id', $project_id)
->findAll();
return $this->update($values, $trigger_events);
// We sort everything by column and position
$board = $this->board->getColumnsList($project_id);
$columns = array();
foreach ($board as $board_column_id => $board_column_name) {
$columns[$board_column_id] = array();
}
foreach ($tasks as &$task) {
if ($task['id'] != $task_id) {
$columns[$task['column_id']][$task['position'] - 1] = (int) $task['id'];
}
}
// The column must exists
if (! isset($columns[$column_id])) {
return false;
}
// We put our task to the new position
array_splice($columns[$column_id], $position - 1, 0, $task_id); // print_r($columns);
// We save the new positions for all tasks
return $this->savePositions($task_id, $columns);
}
/**
* Save task positions
*
* @access private
* @param integer $moved_task_id Id of the moved task
* @param array $columns Sorted tasks
* @return boolean
*/
private function savePositions($moved_task_id, array $columns)
{
$this->db->startTransaction();
foreach ($columns as $column_id => $column) {
$position = 1;
ksort($column);
foreach ($column as $task_id) {
if ($task_id == $moved_task_id) {
// Events will be triggered only for that task
$result = $this->update(array(
'id' => $task_id,
'position' => $position,
'column_id' => $column_id
));
}
else {
$result = $this->db->table(self::TABLE)->eq('id', $task_id)->update(array(
'position' => $position,
'column_id' => $column_id
));
}
$position++;
if (! $result) {
$this->db->cancelTransaction();
return false;
}
}
}
$this->db->closeTransaction();
return true;
}
/**

View File

@ -85,7 +85,11 @@ Kanboard.Board = (function() {
connectWith: ".column",
placeholder: "draggable-placeholder",
stop: function(event, ui) {
board_save(ui.item.attr('data-task-id'));
board_save(
ui.item.attr('data-task-id'),
ui.item.parent().attr("data-column-id"),
ui.item.index() + 1
);
}
});
@ -126,30 +130,20 @@ Kanboard.Board = (function() {
}
// Save and refresh the board
function board_save(selected_task_id)
function board_save(taskId, columnId, position)
{
var data = [];
var boardSelector = $("#board");
var projectId = boardSelector.attr("data-project-id");
board_unload_events();
$(".column").each(function() {
var columnId = $(this).attr("data-column-id");
$("#column-" + columnId + " .task-board").each(function(index) {
data.push({
"task_id": parseInt($(this).attr("data-task-id")),
"position": index + 1,
"column_id": parseInt(columnId)
});
});
});
$.ajax({
cache: false,
url: "?controller=board&action=save&project_id=" + projectId,
data: {"positions": data, "csrf_token": boardSelector.attr("data-csrf-token"), "selected_task_id": selected_task_id},
data: {
"task_id": taskId,
"column_id": columnId,
"position": position,
"csrf_token": boardSelector.attr("data-csrf-token"),
},
type: "POST",
success: function(data) {
$("#board").remove();

View File

@ -257,6 +257,12 @@ Procedures
- Result on success: **true**
- Result on failure: **false**
### moveTaskPosition
- Purpose: **Move a task to another column or another position**
- Parameters: **project_id** (integer), **task_id** (integer), **column_id** (integer), **position** (integer)
- Result on success: **true**
- Result on failure: **false**

View File

@ -146,6 +146,10 @@ $server->register('removeTask', function($task_id) use ($task) {
return $task->remove($task_id);
});
$server->register('moveTaskPosition', function($project_id, $task_id, $column_id, $position) use ($task) {
return $task->movePosition($project_id, $task_id, $column_id, $position);
});
/**
* User procedures

26
scripts/create-random-tasks.php Executable file
View File

@ -0,0 +1,26 @@
#!/usr/bin/env php
<?php
require __DIR__.'/../app/common.php';
use Model\Task;
$task_per_column = 250;
$taskModel = new Task($registry);
foreach (array(1, 2, 3, 4) as $column_id) {
for ($i = 1; $i <= $task_per_column; $i++) {
$task = array(
'title' => 'Task #'.$i.'-'.$column_id,
'project_id' => 1,
'column_id' => $column_id,
'owner_id' => rand(0, 1),
'color_id' => rand(0, 1) === 0 ? 'green' : 'purple',
'score' => rand(0, 21),
);
$taskModel->create($task);
}
}

View File

@ -396,23 +396,19 @@ class Api extends PHPUnit_Framework_TestCase
$this->assertTrue(is_array($subtasks));
$this->assertEquals(1, count($subtasks));
}
/*
public function testAutomaticActions()
public function testMoveTaskPosition()
{
$task = array(
'title' => 'Task #1',
'title' => 'Task to move',
'color_id' => 'blue',
'owner_id' => 0,
'owner_id' => 1,
'project_id' => 1,
'column_id' => 1,
);
$this->assertTrue($this->client->createTask($task));
$tasks = $this->client->getAllTasks(1, array(1));
$task = $tasks[count($tasks) - 1];
$task['column_id'] = 3;
$this->assertTrue($this->client->updateTask($task));
}*/
$this->assertTrue($this->client->moveTaskPosition(1, 2, 3, 1));
}
}

View File

@ -9,7 +9,7 @@ use Model\Task;
use Model\Category;
class ActionTest extends Base
{
{/*
public function testFetchActions()
{
$action = new Action($this->registry);
@ -84,7 +84,7 @@ class ActionTest extends Base
$this->assertEquals(1, $t1['column_id']);
// We move our task
$task->movePosition(1, 4, 1);
$task->movePosition(1, 1, 4, 1);
$this->assertTrue($this->registry->event->isEventTriggered(Task::EVENT_MOVE_COLUMN));
$this->assertTrue($this->registry->event->isEventTriggered(Task::EVENT_UPDATE));
@ -94,7 +94,7 @@ class ActionTest extends Base
$this->assertEquals(4, $t1['column_id']);
$this->assertEquals(0, $t1['is_active']);
}
*/
public function testEventMovePosition()
{
$task = new Task($this->registry);
@ -140,7 +140,7 @@ class ActionTest extends Base
$this->assertTrue($this->registry->event->hasListener(Task::EVENT_MOVE_POSITION, 'Action\TaskAssignColorCategory'));
// Our task should have the color red and position=0
// Our task should have the color red and position=1
$t1 = $task->getById(1);
$this->assertEquals(1, $t1['position']);
$this->assertEquals(1, $t1['is_active']);
@ -152,12 +152,9 @@ class ActionTest extends Base
$this->assertEquals('yellow', $t1['color_id']);
// We move our tasks
$task->movePosition(1, 1, 2); // task #1 to position 2
$task->movePosition(2, 1, 1); // task #2 to position 1
$this->assertTrue($task->movePosition(1, 1, 1, 10)); // task #1 to the end of the column
$this->assertTrue($this->registry->event->isEventTriggered(Task::EVENT_MOVE_POSITION));
// Both tasks should be green
$t1 = $task->getById(1);
$this->assertEquals(2, $t1['position']);
$this->assertEquals(1, $t1['is_active']);
@ -166,9 +163,24 @@ class ActionTest extends Base
$t1 = $task->getById(2);
$this->assertEquals(1, $t1['position']);
$this->assertEquals(1, $t1['is_active']);
$this->assertEquals('yellow', $t1['color_id']);
$this->registry->event->clearTriggeredEvents();
$this->assertTrue($task->movePosition(1, 2, 1, 44)); // task #2 to position 1
$this->assertTrue($this->registry->event->isEventTriggered(Task::EVENT_MOVE_POSITION));
$this->assertEquals('Action\TaskAssignColorCategory', $this->registry->event->getLastListenerExecuted());
$t1 = $task->getById(1);
$this->assertEquals(1, $t1['position']);
$this->assertEquals(1, $t1['is_active']);
$this->assertEquals('green', $t1['color_id']);
$t1 = $task->getById(2);
$this->assertEquals(2, $t1['position']);
$this->assertEquals(1, $t1['is_active']);
$this->assertEquals('green', $t1['color_id']);
}
/*
public function testExecuteMultipleActions()
{
$task = new Task($this->registry);
@ -223,7 +235,7 @@ class ActionTest extends Base
$this->assertEquals(1, $t1['project_id']);
// We move our task
$task->movePosition(1, 4, 1);
$task->movePosition(1, 1, 4, 1);
$this->assertTrue($this->registry->event->isEventTriggered(Task::EVENT_CLOSE));
$this->assertTrue($this->registry->event->isEventTriggered(Task::EVENT_MOVE_COLUMN));
@ -240,5 +252,5 @@ class ActionTest extends Base
$this->assertEquals(1, $t2['is_active']);
$this->assertEquals(2, $t2['project_id']);
$this->assertEquals('unit_test', $t2['title']);
}
}*/
}

View File

@ -9,6 +9,160 @@ use Model\User;
class TaskTest extends Base
{
public function testMovePosition()
{
$t = new Task($this->registry);
$p = new Project($this->registry);
$this->assertEquals(1, $p->create(array('name' => 'Project #1')));
$counter = 1;
$task_per_column = 5;
foreach (array(1, 2, 3, 4) as $column_id) {
for ($i = 1; $i <= $task_per_column; $i++, $counter++) {
$task = array(
'title' => 'Task #'.$i.'-'.$column_id,
'project_id' => 1,
'column_id' => $column_id,
'owner_id' => 0,
);
$this->assertEquals($counter, $t->create($task));
$task = $t->getById($counter);
$this->assertNotFalse($task);
$this->assertNotEmpty($task);
$this->assertEquals($i, $task['position']);
}
}
// We move task id #4, column 1, position 4 to the column 2, position 3
$this->assertTrue($t->movePosition(1, 4, 2, 3));
// We check the new position of the task
$task = $t->getById(4);
$this->assertEquals(4, $task['id']);
$this->assertEquals(2, $task['column_id']);
$this->assertEquals(3, $task['position']);
// The tasks before have the correct position
$task = $t->getById(3);
$this->assertEquals(3, $task['id']);
$this->assertEquals(1, $task['column_id']);
$this->assertEquals(3, $task['position']);
$task = $t->getById(7);
$this->assertEquals(7, $task['id']);
$this->assertEquals(2, $task['column_id']);
$this->assertEquals(2, $task['position']);
// The tasks after have the correct position
$task = $t->getById(5);
$this->assertEquals(5, $task['id']);
$this->assertEquals(1, $task['column_id']);
$this->assertEquals(4, $task['position']);
$task = $t->getById(8);
$this->assertEquals(8, $task['id']);
$this->assertEquals(2, $task['column_id']);
$this->assertEquals(4, $task['position']);
// The number of tasks per column
$this->assertEquals($task_per_column - 1, $t->countByColumnId(1, 1));
$this->assertEquals($task_per_column + 1, $t->countByColumnId(1, 2));
$this->assertEquals($task_per_column, $t->countByColumnId(1, 3));
$this->assertEquals($task_per_column, $t->countByColumnId(1, 4));
// We move task id #1, column 1, position 1 to the column 4, position 6 (last position)
$this->assertTrue($t->movePosition(1, 1, 4, $task_per_column + 1));
// We check the new position of the task
$task = $t->getById(1);
$this->assertEquals(1, $task['id']);
$this->assertEquals(4, $task['column_id']);
$this->assertEquals($task_per_column + 1, $task['position']);
// The tasks before have the correct position
$task = $t->getById(20);
$this->assertEquals(20, $task['id']);
$this->assertEquals(4, $task['column_id']);
$this->assertEquals($task_per_column, $task['position']);
// The tasks after have the correct position
$task = $t->getById(2);
$this->assertEquals(2, $task['id']);
$this->assertEquals(1, $task['column_id']);
$this->assertEquals(1, $task['position']);
// The number of tasks per column
$this->assertEquals($task_per_column - 2, $t->countByColumnId(1, 1));
$this->assertEquals($task_per_column + 1, $t->countByColumnId(1, 2));
$this->assertEquals($task_per_column, $t->countByColumnId(1, 3));
$this->assertEquals($task_per_column + 1, $t->countByColumnId(1, 4));
// Our previous moved task should stay at the same place
$task = $t->getById(4);
$this->assertEquals(4, $task['id']);
$this->assertEquals(2, $task['column_id']);
$this->assertEquals(3, $task['position']);
// Test wrong position number
$this->assertFalse($t->movePosition(1, 2, 3, 0));
$this->assertFalse($t->movePosition(1, 2, 3, -2));
// Wrong column
$this->assertFalse($t->movePosition(1, 2, 22, 2));
// Test position greater than the last position
$this->assertTrue($t->movePosition(1, 11, 1, 22));
$task = $t->getById(11);
$this->assertEquals(11, $task['id']);
$this->assertEquals(1, $task['column_id']);
$this->assertEquals($t->countByColumnId(1, 1), $task['position']);
$task = $t->getById(5);
$this->assertEquals(5, $task['id']);
$this->assertEquals(1, $task['column_id']);
$this->assertEquals($t->countByColumnId(1, 1) - 1, $task['position']);
$task = $t->getById(4);
$this->assertEquals(4, $task['id']);
$this->assertEquals(2, $task['column_id']);
$this->assertEquals(3, $task['position']);
$this->assertEquals($task_per_column - 1, $t->countByColumnId(1, 1));
$this->assertEquals($task_per_column + 1, $t->countByColumnId(1, 2));
$this->assertEquals($task_per_column - 1, $t->countByColumnId(1, 3));
$this->assertEquals($task_per_column + 1, $t->countByColumnId(1, 4));
// Our previous moved task should stay at the same place
$task = $t->getById(4);
$this->assertEquals(4, $task['id']);
$this->assertEquals(2, $task['column_id']);
$this->assertEquals(3, $task['position']);
// Test moving task to position 1
$this->assertTrue($t->movePosition(1, 14, 1, 1));
$task = $t->getById(14);
$this->assertEquals(14, $task['id']);
$this->assertEquals(1, $task['column_id']);
$this->assertEquals(1, $task['position']);
$task = $t->getById(2);
$this->assertEquals(2, $task['id']);
$this->assertEquals(1, $task['column_id']);
$this->assertEquals(2, $task['position']);
$this->assertEquals($task_per_column, $t->countByColumnId(1, 1));
$this->assertEquals($task_per_column + 1, $t->countByColumnId(1, 2));
$this->assertEquals($task_per_column - 2, $t->countByColumnId(1, 3));
$this->assertEquals($task_per_column + 1, $t->countByColumnId(1, 4));
}
public function testExport()
{
$t = new Task($this->registry);
@ -271,15 +425,16 @@ class TaskTest extends Base
$this->assertTrue($this->registry->event->isEventTriggered(Task::EVENT_OPEN));
// We change the column of our task
$this->assertTrue($t->movePosition(1, 2, 1));
$this->assertTrue($t->movePosition(1, 1, 2, 1));
$this->assertTrue($this->registry->event->isEventTriggered(Task::EVENT_MOVE_COLUMN));
// We change the position of our task
$this->assertTrue($t->movePosition(1, 2, 2));
$this->assertEquals(2, $t->create(array('title' => 'test 2', 'project_id' => 1, 'column_id' => 2)));
$this->assertTrue($t->movePosition(1, 1, 2, 2));
$this->assertTrue($this->registry->event->isEventTriggered(Task::EVENT_MOVE_POSITION));
// We change the column and the position of our task
$this->assertTrue($t->movePosition(1, 1, 3));
$this->assertTrue($t->movePosition(1, 1, 1, 1));
$this->assertTrue($this->registry->event->isEventTriggered(Task::EVENT_MOVE_COLUMN));
}
}