From f908a016612c47542971d829b9aeed95db829b77 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 16 Feb 2011 18:15:52 +0100 Subject: [PATCH] [ticket/9549] New method move() to move a group more than 1 up/down. PHPBB3-9549 --- phpBB/includes/group_positions.php | 59 +++++++++++-------- .../group_positions/group_positions_test.php | 59 +++++++++++++++++++ 2 files changed, 94 insertions(+), 24 deletions(-) diff --git a/phpBB/includes/group_positions.php b/phpBB/includes/group_positions.php index 951014c0c0..fc44e3249d 100644 --- a/phpBB/includes/group_positions.php +++ b/phpBB/includes/group_positions.php @@ -163,25 +163,7 @@ class phpbb_group_positions */ public function move_up($group_id) { - $current_value = $this->get_group_value($group_id); - - // Only move the group, if it is in the list and not already on top. - if ($current_value > 1) - { - $this->db->sql_transaction('begin'); - - $sql = 'UPDATE ' . GROUPS_TABLE . ' - SET group_' . $this->field . ' = group_' . $this->field . ' + 1 - WHERE group_' . $this->field . ' = ' . ($current_value - 1); - $this->db->sql_query($sql); - - $sql = 'UPDATE ' . GROUPS_TABLE . ' - SET group_' . $this->field . ' = group_' . $this->field . ' - 1 - WHERE group_id = ' . (int) $group_id; - $this->db->sql_query($sql); - - $this->db->sql_transaction('commit'); - } + $this->move($group_id, 1); } /** @@ -192,22 +174,51 @@ class phpbb_group_positions */ public function move_down($group_id) { + $this->move($group_id, -1); + } + + /** + * Moves a group up/down + * + * @param int $group_id group_id of the group to be moved + * @param int $delta number of steps: + * - positive = move up + * - negative = move down + * @return void + */ + public function move($group_id, $delta) + { + if (!is_int($delta) || !$delta) + { + return; + } + + $move_up = ($delta > 0) ? true : false; $current_value = $this->get_group_value($group_id); if ($current_value != self::GROUP_DISABLED) { $this->db->sql_transaction('begin'); + // First we move all groups between our current value and the target value up/down 1, + // so we have a gap for our group to move. $sql = 'UPDATE ' . GROUPS_TABLE . ' - SET group_' . $this->field . ' = group_' . $this->field . ' - 1 - WHERE group_' . $this->field . ' = ' . ($current_value + 1); + SET group_' . $this->field . ' = group_' . $this->field . (($move_up) ? ' + 1' : ' - 1') . ' + WHERE group_' . $this->field . ' > ' . self::GROUP_DISABLED . ' + AND group_' . $this->field . (($move_up) ? ' >= ' : ' <= ') . ($current_value - $delta) . ' + AND group_' . $this->field . (($move_up) ? ' < ' : ' > ') . $current_value; $this->db->sql_query($sql); - if ($this->db->sql_affectedrows() == 1) + // Because there might be fewer groups above/below the group than we wanted to move, + // we use the number of changed groups, to update the group. + $delta = (int) $this->db->sql_affectedrows(); + + if ($delta) { - // Only update when we move another one up, otherwise it was the last. + // And now finally, when we moved some other groups and built a gap, + // we can move the desired group to it. $sql = 'UPDATE ' . GROUPS_TABLE . ' - SET group_' . $this->field . ' = group_' . $this->field . ' + 1 + SET group_' . $this->field . ' = group_' . $this->field . (($move_up) ? ' - ' : ' + ') . $delta . ' WHERE group_id = ' . (int) $group_id; $this->db->sql_query($sql); } diff --git a/tests/group_positions/group_positions_test.php b/tests/group_positions/group_positions_test.php index 7454402194..b68f205b37 100644 --- a/tests/group_positions/group_positions_test.php +++ b/tests/group_positions/group_positions_test.php @@ -224,5 +224,64 @@ class phpbb_group_positions_test extends phpbb_database_test_case $this->assertEquals($expected, $db->sql_fetchrowset($result)); } + + public static function move_data() + { + return array( + array('teampage', 1, 1, array( + array('group_id' => 1, 'group_teampage' => 0, 'group_legend' => 0), + array('group_id' => 2, 'group_teampage' => 1, 'group_legend' => 0), + array('group_id' => 3, 'group_teampage' => 2, 'group_legend' => 1), + )), + array('teampage', 1, -1, array( + array('group_id' => 1, 'group_teampage' => 0, 'group_legend' => 0), + array('group_id' => 2, 'group_teampage' => 1, 'group_legend' => 0), + array('group_id' => 3, 'group_teampage' => 2, 'group_legend' => 1), + )), + array('teampage', 3, 3, array( + array('group_id' => 1, 'group_teampage' => 0, 'group_legend' => 0), + array('group_id' => 2, 'group_teampage' => 2, 'group_legend' => 0), + array('group_id' => 3, 'group_teampage' => 1, 'group_legend' => 1), + )), + array('teampage', 2, 0, array( + array('group_id' => 1, 'group_teampage' => 0, 'group_legend' => 0), + array('group_id' => 2, 'group_teampage' => 1, 'group_legend' => 0), + array('group_id' => 3, 'group_teampage' => 2, 'group_legend' => 1), + )), + array('teampage', 2, -1, array( + array('group_id' => 1, 'group_teampage' => 0, 'group_legend' => 0), + array('group_id' => 2, 'group_teampage' => 2, 'group_legend' => 0), + array('group_id' => 3, 'group_teampage' => 1, 'group_legend' => 1), + )), + array('teampage', 2, -3, array( + array('group_id' => 1, 'group_teampage' => 0, 'group_legend' => 0), + array('group_id' => 2, 'group_teampage' => 2, 'group_legend' => 0), + array('group_id' => 3, 'group_teampage' => 1, 'group_legend' => 1), + )), + array('teampage', 3, -1, array( + array('group_id' => 1, 'group_teampage' => 0, 'group_legend' => 0), + array('group_id' => 2, 'group_teampage' => 1, 'group_legend' => 0), + array('group_id' => 3, 'group_teampage' => 2, 'group_legend' => 1), + )), + ); + } + + /** + * @dataProvider move_data + */ + public function test_move($field, $group_id, $increment, $expected) + { + global $db; + + $db = $this->new_dbal(); + $test_class = new phpbb_group_positions($db, $field); + $test_class->move($group_id, $increment); + + $result = $db->sql_query('SELECT group_id, group_teampage, group_legend + FROM ' . GROUPS_TABLE . ' + ORDER BY group_id ASC'); + + $this->assertEquals($expected, $db->sql_fetchrowset($result)); + } }