diff options
| -rw-r--r-- | ChangeLog | 4 | ||||
| -rw-r--r-- | app/Model/TaskPosition.php | 180 | ||||
| -rw-r--r-- | composer.json | 2 | ||||
| -rw-r--r-- | composer.lock | 18 | ||||
| -rw-r--r-- | tests/units/Model/TaskPositionTest.php | 4 |
5 files changed, 81 insertions, 127 deletions
@@ -6,6 +6,10 @@ New features: - Added report to compare working hours between open and closed tasks - Added the possiblity to define custom routes from plugins +Improvements: + +- Improving performance during task position change (3 times faster than before) + Bug fixes: - Fix wrong constant name that cause a PHP error in project management section diff --git a/app/Model/TaskPosition.php b/app/Model/TaskPosition.php index da363cb3..0a8f6c4c 100644 --- a/app/Model/TaskPosition.php +++ b/app/Model/TaskPosition.php @@ -26,28 +26,34 @@ class TaskPosition extends Base */ public function movePosition($project_id, $task_id, $column_id, $position, $swimlane_id = 0, $fire_events = true) { + $result = false; + if ($position < 1) { - return false; + return $result; } $task = $this->taskFinder->getById($task_id); - // Ignore closed tasks if ($task['is_active'] == Task::STATUS_CLOSED) { return true; } - $result = false; + $this->db->startTransaction(); - if ($task['swimlane_id'] != $swimlane_id) { - $result = $this->saveSwimlaneChange($project_id, $task_id, $position, $task['column_id'], $column_id, $task['swimlane_id'], $swimlane_id); - } elseif ($task['column_id'] != $column_id) { - $result = $this->saveColumnChange($project_id, $task_id, $position, $swimlane_id, $task['column_id'], $column_id); + if ($task['swimlane_id'] != $swimlane_id || $task['column_id'] != $column_id) { + $result = $this->moveTaskToAnotherColumn($task, $swimlane_id, $column_id, $position); } elseif ($task['position'] != $position) { - $result = $this->savePositionChange($project_id, $task_id, $position, $column_id, $swimlane_id); + $result = $this->moveTaskWithinSameColumn($task, $position); } - if ($result && $fire_events) { + if (! $result) { + $this->db->cancelTransaction(); + return false; + } + + $this->db->closeTransaction(); + + if ($fire_events) { $this->fireEvents($task, $column_id, $position, $swimlane_id); } @@ -55,145 +61,87 @@ class TaskPosition extends Base } /** - * Move a task to another swimlane + * Move a task to another column/swimlane * * @access private - * @param integer $project_id - * @param integer $task_id - * @param integer $position - * @param integer $original_column_id - * @param integer $new_column_id - * @param integer $original_swimlane_id - * @param integer $new_swimlane_id + * @param array $task + * @param integer $swimlane_id + * @param integer $column_id + * @param integer $position * @return boolean */ - private function saveSwimlaneChange($project_id, $task_id, $position, $original_column_id, $new_column_id, $original_swimlane_id, $new_swimlane_id) + private function moveTaskToAnotherColumn(array $task, $swimlane_id, $column_id, $position) { - $this->db->startTransaction(); - $r1 = $this->saveTaskPositions($project_id, $task_id, 0, $original_column_id, $original_swimlane_id); - $r2 = $this->saveTaskPositions($project_id, $task_id, $position, $new_column_id, $new_swimlane_id); - $this->db->closeTransaction(); + $results = array(); + $max = $this->getQuery($task['project_id'], $swimlane_id, $column_id)->count(); + $position = $max > 0 && $position > $max ? $max + 1 : $position; - return $r1 && $r2; - } + $results[] = $this->getQuery($task['project_id'], $task['swimlane_id'], $task['column_id'])->gt('position', $task['position'])->decrement('position', 1); + $results[] = $this->getQuery($task['project_id'], $swimlane_id, $column_id)->gte('position', $position)->increment('position', 1); + $results[] = $this->updateTaskPosition($task['id'], $swimlane_id, $column_id, $position); - /** - * Move a task to another column - * - * @access private - * @param integer $project_id - * @param integer $task_id - * @param integer $position - * @param integer $swimlane_id - * @param integer $original_column_id - * @param integer $new_column_id - * @return boolean - */ - private function saveColumnChange($project_id, $task_id, $position, $swimlane_id, $original_column_id, $new_column_id) - { - $this->db->startTransaction(); - $r1 = $this->saveTaskPositions($project_id, $task_id, 0, $original_column_id, $swimlane_id); - $r2 = $this->saveTaskPositions($project_id, $task_id, $position, $new_column_id, $swimlane_id); - $this->db->closeTransaction(); - - return $r1 && $r2; + return !in_array(false, $results, true); } /** - * Move a task to another position in the same column + * Move a task within the same column * * @access private - * @param integer $project_id - * @param integer $task_id - * @param integer $position - * @param integer $column_id - * @param integer $swimlane_id + * @param array $task + * @param integer $position * @return boolean */ - private function savePositionChange($project_id, $task_id, $position, $column_id, $swimlane_id) + private function moveTaskWithinSameColumn(array $task, $position) { - $this->db->startTransaction(); - $result = $this->saveTaskPositions($project_id, $task_id, $position, $column_id, $swimlane_id); - $this->db->closeTransaction(); + $results = array(); + $max = $this->getQuery($task['project_id'], $task['swimlane_id'], $task['column_id'])->count(); + $position = $max > 0 && $position > $max ? $max : $position; + + if ($position >= $max) { + $results[] = $this->getQuery($task['project_id'], $task['swimlane_id'], $task['column_id'])->lte('position', $position)->decrement('position', 1); + } else { + $results[] = $this->getQuery($task['project_id'], $task['swimlane_id'], $task['column_id'])->gte('position', $position)->increment('position', 1); + } - return $result; + $results[] = $this->updateTaskPosition($task['id'], $task['swimlane_id'], $task['column_id'], $position); + + return !in_array(false, $results, true); } /** - * Save all task positions for one column + * Update final task position * * @access private - * @param integer $project_id - * @param integer $task_id - * @param integer $position - * @param integer $column_id - * @param integer $swimlane_id + * @param integer $task_id + * @param integer $swimlane_id + * @param integer $column_id + * @param integer $position * @return boolean */ - private function saveTaskPositions($project_id, $task_id, $position, $column_id, $swimlane_id) + private function updateTaskPosition($task_id, $swimlane_id, $column_id, $position) { - $tasks_ids = $this->db->table(Task::TABLE) + return $this->db->table(Task::TABLE) + ->eq('id', $task_id) ->eq('is_active', 1) - ->eq('swimlane_id', $swimlane_id) - ->eq('project_id', $project_id) - ->eq('column_id', $column_id) - ->neq('id', $task_id) - ->asc('position') - ->asc('id') - ->findAllByColumn('id'); - - $offset = 1; - - foreach ($tasks_ids as $current_task_id) { - - // Insert the new task - if ($position == $offset) { - if (! $this->saveTaskPosition($task_id, $offset, $column_id, $swimlane_id)) { - return false; - } - $offset++; - } - - // Rewrite other tasks position - if (! $this->saveTaskPosition($current_task_id, $offset, $column_id, $swimlane_id)) { - return false; - } - - $offset++; - } - - // Insert the new task at the bottom and normalize bad position - if ($position >= $offset && ! $this->saveTaskPosition($task_id, $offset, $column_id, $swimlane_id)) { - return false; - } - - return true; + ->update(array('position' => $position, 'column_id' => $column_id, 'swimlane_id' => $swimlane_id)); } /** - * Save new task position + * Get common query * * @access private - * @param integer $task_id - * @param integer $position - * @param integer $column_id - * @param integer $swimlane_id - * @return boolean + * @param integer $project_id + * @param integer $swimlane_id + * @param integer $column_id + * @return \PicoDb\Table */ - private function saveTaskPosition($task_id, $position, $column_id, $swimlane_id) + private function getQuery($project_id, $swimlane_id, $column_id) { - $result = $this->db->table(Task::TABLE)->eq('id', $task_id)->update(array( - 'position' => $position, - 'column_id' => $column_id, - 'swimlane_id' => $swimlane_id, - )); - - if (! $result) { - $this->db->cancelTransaction(); - return false; - } - - return true; + return $this->db->table(Task::TABLE) + ->eq('project_id', $project_id) + ->eq('swimlane_id', $swimlane_id) + ->eq('column_id', $column_id) + ->eq('is_active', 1); } /** diff --git a/composer.json b/composer.json index 06637abc..f75a4829 100644 --- a/composer.json +++ b/composer.json @@ -16,7 +16,7 @@ "erusev/parsedown" : "1.6.0", "fabiang/xmpp" : "0.6.1", "fguillot/json-rpc" : "1.0.3", - "fguillot/picodb" : "1.0.3", + "fguillot/picodb" : "dev-master", "fguillot/simpleLogger" : "1.0.0", "fguillot/simple-validator" : "1.0.0", "league/html-to-markdown" : "~4.0", diff --git a/composer.lock b/composer.lock index 4af69a06..228805d0 100644 --- a/composer.lock +++ b/composer.lock @@ -4,8 +4,8 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file", "This file is @generated automatically" ], - "hash": "a003844ed3fef854d64490d07064c66f", - "content-hash": "2189f3f4d11741a635dbd27a5750b71e", + "hash": "964754ad3b7c394f96169a14865ffeee", + "content-hash": "c379b500a20d7025a0b872c7ec8837d3", "packages": [ { "name": "christian-riesen/base32", @@ -297,16 +297,16 @@ }, { "name": "fguillot/picodb", - "version": "v1.0.3", + "version": "dev-master", "source": { "type": "git", "url": "https://github.com/fguillot/picoDb.git", - "reference": "0b4640ebe531a89d2faa36dfabe30a194f1fe903" + "reference": "cd377f1bc944086600f8b1bab975981e3d27f7f1" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/fguillot/picoDb/zipball/0b4640ebe531a89d2faa36dfabe30a194f1fe903", - "reference": "0b4640ebe531a89d2faa36dfabe30a194f1fe903", + "url": "https://api.github.com/repos/fguillot/picoDb/zipball/cd377f1bc944086600f8b1bab975981e3d27f7f1", + "reference": "cd377f1bc944086600f8b1bab975981e3d27f7f1", "shasum": "" }, "require": { @@ -330,7 +330,7 @@ ], "description": "Minimalist database query builder", "homepage": "https://github.com/fguillot/picoDb", - "time": "2015-11-30 02:57:18" + "time": "2015-12-23 13:56:10" }, { "name": "fguillot/simple-validator", @@ -884,7 +884,9 @@ ], "aliases": [], "minimum-stability": "stable", - "stability-flags": [], + "stability-flags": { + "fguillot/picodb": 20 + }, "prefer-stable": false, "prefer-lowest": false, "platform": { diff --git a/tests/units/Model/TaskPositionTest.php b/tests/units/Model/TaskPositionTest.php index 42612f44..5f045768 100644 --- a/tests/units/Model/TaskPositionTest.php +++ b/tests/units/Model/TaskPositionTest.php @@ -106,7 +106,7 @@ class TaskPositionTest extends Base $this->assertEquals(1, $tc->create(array('title' => 'Task #1', 'project_id' => 1, 'column_id' => 1))); $this->assertEquals(2, $tc->create(array('title' => 'Task #2', 'project_id' => 1, 'column_id' => 1))); - // We move the task 2 to the column 3 + // We move the task 1 to the column 3 $this->assertTrue($tp->movePosition(1, 1, 3, 1)); // Check tasks position @@ -235,7 +235,7 @@ class TaskPositionTest extends Base $this->assertEquals(3, $tc->create(array('title' => 'Task #3', 'project_id' => 1, 'column_id' => 1))); $this->assertEquals(4, $tc->create(array('title' => 'Task #4', 'project_id' => 1, 'column_id' => 1))); - // Move the last task to the bottom + // Move the first task to the bottom $this->assertTrue($tp->movePosition(1, 1, 1, 4)); // Check tasks position |