diff options
-rw-r--r-- | app/Controller/Board.php | 34 | ||||
-rw-r--r-- | app/Model/Board.php | 26 | ||||
-rw-r--r-- | app/Model/Task.php | 112 | ||||
-rw-r--r-- | assets/js/app.js | 30 | ||||
-rw-r--r-- | docs/api-json-rpc.markdown | 6 | ||||
-rw-r--r-- | jsonrpc.php | 4 | ||||
-rwxr-xr-x | scripts/create-random-tasks.php | 26 | ||||
-rw-r--r-- | tests/functionals/ApiTest.php | 16 | ||||
-rw-r--r-- | tests/units/ActionTest.php | 34 | ||||
-rw-r--r-- | tests/units/TaskTest.php | 161 |
10 files changed, 344 insertions, 105 deletions
diff --git a/app/Controller/Board.php b/app/Controller/Board.php index e2c10f58..4724cae5 100644 --- a/app/Controller/Board.php +++ b/app/Controller/Board.php @@ -396,27 +396,31 @@ class Board extends Base */ public function save() { - if ($this->request->isAjax()) { + $project_id = $this->request->getIntegerParam('project_id'); - $project_id = $this->request->getIntegerParam('project_id'); - $values = $this->request->getValues(); + if ($project_id > 0 && $this->request->isAjax()) { - if ($project_id > 0 && ! $this->project->isUserAllowed($project_id, $this->acl->getUserId())) { - $this->response->text('Not Authorized', 401); + if (! $this->project->isUserAllowed($project_id, $this->acl->getUserId())) { + $this->response->status(401); } - if (isset($values['positions'])) { - $this->board->saveTasksPosition($values['positions'], $values['selected_task_id']); + $values = $this->request->getValues(); + + if ($this->task->movePosition($project_id, $values['task_id'], $values['column_id'], $values['position'])) { + + $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); diff --git a/app/Model/Board.php b/app/Model/Board.php index 8fb30e70..07020600 100644 --- a/app/Model/Board.php +++ b/app/Model/Board.php @@ -21,32 +21,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 * * @access public diff --git a/app/Model/Task.php b/app/Model/Task.php index df6e4426..07a2a2fd 100644 --- a/app/Model/Task.php +++ b/app/Model/Task.php @@ -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(); + + // 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 $this->update($values, $trigger_events); + return true; } /** diff --git a/assets/js/app.js b/assets/js/app.js index 70e2ca38..a08ac8a8 100644 --- a/assets/js/app.js +++ b/assets/js/app.js @@ -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(); diff --git a/docs/api-json-rpc.markdown b/docs/api-json-rpc.markdown index c06943bb..4d598d0e 100644 --- a/docs/api-json-rpc.markdown +++ b/docs/api-json-rpc.markdown @@ -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** diff --git a/jsonrpc.php b/jsonrpc.php index c645650f..22b44df6 100644 --- a/jsonrpc.php +++ b/jsonrpc.php @@ -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 diff --git a/scripts/create-random-tasks.php b/scripts/create-random-tasks.php new file mode 100755 index 00000000..4d665178 --- /dev/null +++ b/scripts/create-random-tasks.php @@ -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); + } +} diff --git a/tests/functionals/ApiTest.php b/tests/functionals/ApiTest.php index 6e23fd52..b41db6fc 100644 --- a/tests/functionals/ApiTest.php +++ b/tests/functionals/ApiTest.php @@ -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)); + } } diff --git a/tests/units/ActionTest.php b/tests/units/ActionTest.php index 19226449..b07af992 100644 --- a/tests/units/ActionTest.php +++ b/tests/units/ActionTest.php @@ -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']); - } + }*/ } diff --git a/tests/units/TaskTest.php b/tests/units/TaskTest.php index e1a976c1..a45127e2 100644 --- a/tests/units/TaskTest.php +++ b/tests/units/TaskTest.php @@ -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)); } } |