summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/Model/Subtask.php58
-rw-r--r--app/Template/subtask/show.php8
-rw-r--r--tests/units/SubtaskTest.php39
3 files changed, 89 insertions, 16 deletions
diff --git a/app/Model/Subtask.php b/app/Model/Subtask.php
index 492f3a77..f7929af3 100644
--- a/app/Model/Subtask.php
+++ b/app/Model/Subtask.php
@@ -228,6 +228,46 @@ class Subtask extends Base
}
/**
+ * Get subtasks with consecutive positions
+ *
+ * If you remove a subtask, the positions are not anymore consecutives
+ *
+ * @access public
+ * @param integer $task_id
+ * @return array
+ */
+ public function getNormalizedPositions($task_id)
+ {
+ $subtasks = $this->db->hashtable(self::TABLE)->eq('task_id', $task_id)->asc('position')->getAll('id', 'position');
+ $position = 1;
+
+ foreach ($subtasks as $subtask_id => $subtask_position) {
+ $subtasks[$subtask_id] = $position++;
+ }
+
+ return $subtasks;
+ }
+
+ /**
+ * Save the new positions for a set of subtasks
+ *
+ * @access public
+ * @param array $subtasks Hashmap of column_id/column_position
+ * @return boolean
+ */
+ public function savePositions(array $subtasks)
+ {
+ return $this->db->transaction(function ($db) use ($subtasks) {
+
+ foreach ($subtasks as $subtask_id => $position) {
+ if (! $db->table(self::TABLE)->eq('id', $subtask_id)->update(array('position' => $position))) {
+ return false;
+ }
+ }
+ });
+ }
+
+ /**
* Move a subtask down, increment the position value
*
* @access public
@@ -237,7 +277,7 @@ class Subtask extends Base
*/
public function moveDown($task_id, $subtask_id)
{
- $subtasks = $this->db->hashtable(self::TABLE)->eq('task_id', $task_id)->asc('position')->getAll('id', 'position');
+ $subtasks = $this->getNormalizedPositions($task_id);
$positions = array_flip($subtasks);
if (isset($subtasks[$subtask_id]) && $subtasks[$subtask_id] < count($subtasks)) {
@@ -245,12 +285,7 @@ class Subtask extends Base
$position = ++$subtasks[$subtask_id];
$subtasks[$positions[$position]]--;
- $this->db->startTransaction();
- $this->db->table(self::TABLE)->eq('id', $subtask_id)->update(array('position' => $position));
- $this->db->table(self::TABLE)->eq('id', $positions[$position])->update(array('position' => $subtasks[$positions[$position]]));
- $this->db->closeTransaction();
-
- return true;
+ return $this->savePositions($subtasks);
}
return false;
@@ -266,7 +301,7 @@ class Subtask extends Base
*/
public function moveUp($task_id, $subtask_id)
{
- $subtasks = $this->db->hashtable(self::TABLE)->eq('task_id', $task_id)->asc('position')->getAll('id', 'position');
+ $subtasks = $this->getNormalizedPositions($task_id);
$positions = array_flip($subtasks);
if (isset($subtasks[$subtask_id]) && $subtasks[$subtask_id] > 1) {
@@ -274,12 +309,7 @@ class Subtask extends Base
$position = --$subtasks[$subtask_id];
$subtasks[$positions[$position]]++;
- $this->db->startTransaction();
- $this->db->table(self::TABLE)->eq('id', $subtask_id)->update(array('position' => $position));
- $this->db->table(self::TABLE)->eq('id', $positions[$position])->update(array('position' => $subtasks[$positions[$position]]));
- $this->db->closeTransaction();
-
- return true;
+ return $this->savePositions($subtasks);
}
return false;
diff --git a/app/Template/subtask/show.php b/app/Template/subtask/show.php
index c7ac652a..e5368265 100644
--- a/app/Template/subtask/show.php
+++ b/app/Template/subtask/show.php
@@ -1,4 +1,8 @@
<?php if (! empty($subtasks)): ?>
+
+<?php $first_position = $subtasks[0]['position']; ?>
+<?php $last_position = $subtasks[count($subtasks) - 1]['position']; ?>
+
<div id="subtasks" class="task-show-section">
<div class="page-header">
@@ -40,12 +44,12 @@
<?php if (! isset($not_editable)): ?>
<td>
<ul>
- <?php if ($subtask['position'] > 1): ?>
+ <?php if ($subtask['position'] != $first_position): ?>
<li>
<?= $this->a(t('Move Up'), 'subtask', 'movePosition', array('project_id' => $project['id'], 'task_id' => $subtask['task_id'], 'subtask_id' => $subtask['id'], 'direction' => 'up'), true) ?>
</li>
<?php endif ?>
- <?php if ($subtask['position'] != 0 && $subtask['position'] != count($subtasks)): ?>
+ <?php if ($subtask['position'] != $last_position): ?>
<li>
<?= $this->a(t('Move Down'), 'subtask', 'movePosition', array('project_id' => $project['id'], 'task_id' => $subtask['task_id'], 'subtask_id' => $subtask['id'], 'direction' => 'down'), true) ?>
</li>
diff --git a/tests/units/SubtaskTest.php b/tests/units/SubtaskTest.php
index eb1a3fd3..791f8089 100644
--- a/tests/units/SubtaskTest.php
+++ b/tests/units/SubtaskTest.php
@@ -24,6 +24,7 @@ class SubTaskTest extends Base
$this->assertEquals(2, $s->create(array('title' => 'subtask #2', 'task_id' => 1)));
$this->assertEquals(3, $s->create(array('title' => 'subtask #3', 'task_id' => 1)));
+ // Check positions
$subtask = $s->getById(1);
$this->assertNotEmpty($subtask);
$this->assertEquals(1, $subtask['position']);
@@ -36,8 +37,10 @@ class SubTaskTest extends Base
$this->assertNotEmpty($subtask);
$this->assertEquals(3, $subtask['position']);
+ // Move up
$this->assertTrue($s->moveUp(1, 2));
+ // Check positions
$subtask = $s->getById(1);
$this->assertNotEmpty($subtask);
$this->assertEquals(2, $subtask['position']);
@@ -50,7 +53,24 @@ class SubTaskTest extends Base
$this->assertNotEmpty($subtask);
$this->assertEquals(3, $subtask['position']);
+ // We can't move up #2
$this->assertFalse($s->moveUp(1, 2));
+
+ // Test remove
+ $this->assertTrue($s->remove(1));
+ $this->assertTrue($s->moveUp(1, 3));
+
+ // Check positions
+ $subtask = $s->getById(1);
+ $this->assertEmpty($subtask);
+
+ $subtask = $s->getById(2);
+ $this->assertNotEmpty($subtask);
+ $this->assertEquals(2, $subtask['position']);
+
+ $subtask = $s->getById(3);
+ $this->assertNotEmpty($subtask);
+ $this->assertEquals(1, $subtask['position']);
}
public function testMoveDown()
@@ -66,8 +86,10 @@ class SubTaskTest extends Base
$this->assertEquals(2, $s->create(array('title' => 'subtask #2', 'task_id' => 1)));
$this->assertEquals(3, $s->create(array('title' => 'subtask #3', 'task_id' => 1)));
+ // Move down #1
$this->assertTrue($s->moveDown(1, 1));
+ // Check positions
$subtask = $s->getById(1);
$this->assertNotEmpty($subtask);
$this->assertEquals(2, $subtask['position']);
@@ -80,7 +102,24 @@ class SubTaskTest extends Base
$this->assertNotEmpty($subtask);
$this->assertEquals(3, $subtask['position']);
+ // We can't move down #3
$this->assertFalse($s->moveDown(1, 3));
+
+ // Test remove
+ $this->assertTrue($s->remove(1));
+ $this->assertTrue($s->moveDown(1, 2));
+
+ // Check positions
+ $subtask = $s->getById(1);
+ $this->assertEmpty($subtask);
+
+ $subtask = $s->getById(2);
+ $this->assertNotEmpty($subtask);
+ $this->assertEquals(2, $subtask['position']);
+
+ $subtask = $s->getById(3);
+ $this->assertNotEmpty($subtask);
+ $this->assertEquals(1, $subtask['position']);
}
public function testDuplicate()
generated by cgit v1.2.3 (git 2.39.1) at 2024-10-21 03:25:13 +0000