Handle the case of tasks with bad previous positions

3 files changed, 73 insertions, 28 deletions

diff --git a/app/Controller/Task.php b/app/Controller/Task.php
index 85c3306e..7210be5f 100644
--- a/app/Controller/Task.php
+++ b/app/Controller/Task.php
@@ -37,10 +37,6 @@ class Task extends Base
             'category_id' => $this->request->getIntegerParam('category_id'),
         );
 
-        if ($values['column_id'] == 0) {
-            $values['column_id'] = $this->board->getFirstColumn($values['project_id']);
-        }
-
         list($valid,) = $this->task->validateCreation($values);
 
         if ($valid && $this->task->create($values)) {
diff --git a/app/Model/Task.php b/app/Model/Task.php
index 6eb24281..09333008 100644
--- a/app/Model/Task.php
+++ b/app/Model/Task.php
@@ -395,6 +395,11 @@ class Task extends Base
 
         // Prepare data
         $this->prepare($values);
+
+        if (empty($values['column_id'])) {
+            $values['column_id'] = $this->board->getFirstColumn($values['project_id']);
+        }
+
         $values['date_creation'] = time();
         $values['date_modification'] = $values['date_creation'];
         $values['position'] = $this->countByColumnId($values['project_id'], $values['column_id']) + 1;
@@ -553,20 +558,20 @@ class Task extends Base
             return false;
         }
 
-        // 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();
-
-        $board = $this->board->getColumnsList($project_id);
+        $board = $this->db->table(Board::TABLE)->eq('project_id', $project_id)->asc('position')->findAllByColumn('id');
         $columns = array();
         $task_id = (int) $task_id;
 
         // Prepare the columns
-        foreach ($board as $board_column_id => $board_column_name) {
-            $columns[$board_column_id] = array();
+        foreach ($board as $board_column_id) {
+
+            $columns[$board_column_id] = $this->db->table(self::TABLE)
+                          ->eq('is_active', 1)
+                          ->eq('project_id', $project_id)
+                          ->eq('column_id', $board_column_id)
+                          ->neq('id', $task_id)
+                          ->asc('position')
+                          ->findAllByColumn('id');
         }
 
         // The column must exists
@@ -574,18 +579,6 @@ class Task extends Base
             return false;
         }
 
-        // Sort everything by column
-        foreach ($tasks as &$task) {
-            if ($task['id'] != $task_id) {
-                $columns[$task['column_id']][$task['position'] - 1] = (int) $task['id'];
-            }
-        }
-
-        // Sort all tasks by position
-        foreach ($columns as &$column) {
-            ksort($column);
-        }
-
         // We put our task to the new position
         array_splice($columns[$column_id], $position - 1, 0, $task_id); // print_r($columns);
 
@@ -687,7 +680,6 @@ class Task extends Base
             new Validators\Required('color_id', t('The color is required')),
             new Validators\Required('project_id', t('The project is required')),
             new Validators\Integer('project_id', t('This value must be an integer')),
-            new Validators\Required('column_id', t('The column is required')),
             new Validators\Integer('column_id', t('This value must be an integer')),
             new Validators\Integer('owner_id', t('This value must be an integer')),
             new Validators\Integer('creator_id', t('This value must be an integer')),
diff --git a/tests/units/TaskTest.php b/tests/units/TaskTest.php
index 8121a575..1269352e 100644
--- a/tests/units/TaskTest.php
+++ b/tests/units/TaskTest.php
@@ -9,6 +9,63 @@ use Model\User;
 
 class TaskTest extends Base
 {
+    public function testCreation()
+    {
+        $t = new Task($this->registry);
+        $p = new Project($this->registry);
+
+        $this->assertEquals(1, $p->create(array('name' => 'Project #1')));
+        $this->assertEquals(1, $t->create(array('title' => 'Task #1', 'project_id' => 1, 'column_id' => 1)));
+
+        $task = $t->getById(1);
+        $this->assertEquals(1, $task['id']);
+        $this->assertEquals(1, $task['column_id']);
+        $this->assertEquals(1, $task['position']);
+        $this->assertEquals(time(), $task['date_creation']);
+        $this->assertEquals(time(), $task['date_modification']);
+
+        $this->assertEquals(2, $t->create(array('title' => 'Task #2', 'project_id' => 1)));
+
+        $task = $t->getById(2);
+        $this->assertEquals(2, $task['id']);
+        $this->assertEquals(1, $task['column_id']);
+        $this->assertEquals(2, $task['position']);
+        $this->assertEquals(time(), $task['date_creation']);
+        $this->assertEquals(time(), $task['date_modification']);
+    }
+
+    public function testMoveTaskWithBadPreviousPosition()
+    {
+        $t = new Task($this->registry);
+        $p = new Project($this->registry);
+
+        $this->assertEquals(1, $p->create(array('name' => 'Project #1')));
+        $this->assertEquals(1, $this->registry->db->table('tasks')->insert(array('title' => 'A', 'column_id' => 1, 'project_id' => 1, 'position' => 1)));
+
+        // Both tasks have the same position
+        $this->assertEquals(2, $this->registry->db->table('tasks')->insert(array('title' => 'B', 'column_id' => 2, 'project_id' => 1, 'position' => 1)));
+        $this->assertEquals(3, $this->registry->db->table('tasks')->insert(array('title' => 'C', 'column_id' => 2, 'project_id' => 1, 'position' => 1)));
+
+        // Move the first column to the last position of the 2nd column
+        $this->assertTrue($t->movePosition(1, 1, 2, 3));
+
+        // Check tasks position
+        $task = $t->getById(2);
+        $this->assertEquals(2, $task['id']);
+        $this->assertEquals(2, $task['column_id']);
+        $this->assertEquals(1, $task['position']);
+
+        $task = $t->getById(3);
+        $this->assertEquals(3, $task['id']);
+        $this->assertEquals(2, $task['column_id']);
+        $this->assertEquals(2, $task['position']);
+
+        $task = $t->getById(1);
+        $this->assertEquals(1, $task['id']);
+        $this->assertEquals(2, $task['column_id']);
+        $this->assertEquals(3, $task['position']);
+    }
+
     public function testMoveTaskTop()
     {
         $t = new Task($this->registry);
@@ -20,7 +77,7 @@ class TaskTest extends Base
         $this->assertEquals(3, $t->create(array('title' => 'Task #3', 'project_id' => 1, 'column_id' => 1)));
         $this->assertEquals(4, $t->create(array('title' => 'Task #4', 'project_id' => 1, 'column_id' => 1)));
 
-        // Move the last task to hte top
+        // Move the last task to the top
         $this->assertTrue($t->movePosition(1, 4, 1, 1));
 
         // Check tasks position