From 8d12838aedeaa23cccf128e98e93d05507edda4d Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 14 Feb 2011 16:09:09 +0100 Subject: [PATCH] [ticket/9549] Make the class non static and extend delete_group function. delete_group() can now be used, so it does not update the actual group. This can save a query, when you update the group anyway. PHPBB3-9549 --- phpBB/includes/acp/acp_groups.php | 13 +- phpBB/includes/functions_user.php | 34 ++-- phpBB/includes/group_positions.php | 175 ++++++++++-------- phpBB/install/database_update.php | 1 + .../group_positions/group_positions_test.php | 41 +++- 5 files changed, 154 insertions(+), 110 deletions(-) diff --git a/phpBB/includes/acp/acp_groups.php b/phpBB/includes/acp/acp_groups.php index 5e6ebcaa7d..015be3c30e 100644 --- a/phpBB/includes/acp/acp_groups.php +++ b/phpBB/includes/acp/acp_groups.php @@ -819,6 +819,10 @@ class acp_groups // Invalid mode trigger_error($user->lang['NO_MODE'] . adm_back_link($this->u_action), E_USER_WARNING); } + else if ($field) + { + $group_position = new phpbb_group_positions($db, $field); + } switch ($action) { @@ -826,26 +830,25 @@ class acp_groups set_config('legend_sort_groupname', request_var('legend_sort_groupname', 0)); break; - case 'set_config_teampage': set_config('teampage_forums', request_var('teampage_forums', 0)); set_config('teampage_multiple', request_var('teampage_multiple', 0)); break; case 'add': - phpbb_group_positions::add_group($field, $group_id); + $group_position->add_group($group_id); break; case 'delete': - phpbb_group_positions::delete_group($field, $group_id); + $group_position->delete_group($group_id); break; case 'move_up': - phpbb_group_positions::move_up($field, $group_id); + $group_position->move_up($group_id); break; case 'move_down': - phpbb_group_positions::move_down($field, $group_id); + $group_position->move_down($group_id); break; } diff --git a/phpBB/includes/functions_user.php b/phpBB/includes/functions_user.php index fba96f93e9..ab2481a5dd 100644 --- a/phpBB/includes/functions_user.php +++ b/phpBB/includes/functions_user.php @@ -2495,13 +2495,15 @@ function group_create(&$group_id, $type, $name, $desc, $group_attributes, $allow if (!sizeof($error)) { - $current_legend = phpbb_group_positions::GROUP_DISABLED; $current_teampage = phpbb_group_positions::GROUP_DISABLED; + + $legend = new phpbb_group_positions($db, 'legend'); + $teampage = new phpbb_group_positions($db, 'teampage'); if ($group_id) { - $current_legend = phpbb_group_positions::get_group_value('legend', $group_id); - $current_teampage = phpbb_group_positions::get_group_value('teampage', $group_id); + $current_legend = $legend->get_group_value($group_id); + $current_teampage = $teampage->get_group_value($group_id); } if (isset($group_attributes['group_legend'])) @@ -2509,7 +2511,7 @@ function group_create(&$group_id, $type, $name, $desc, $group_attributes, $allow if (($group_id && ($current_legend == phpbb_group_positions::GROUP_DISABLED)) || !$group_id) { // Old group currently not in the legend or new group, add at the end. - $group_attributes['group_legend'] = 1 + phpbb_group_positions::get_group_count('legend'); + $group_attributes['group_legend'] = 1 + $legend->get_group_count(); } else { @@ -2520,10 +2522,7 @@ function group_create(&$group_id, $type, $name, $desc, $group_attributes, $allow else if ($group_id && ($current_legend > phpbb_group_positions::GROUP_DISABLED)) { // Group is removed from the legend - $sql = 'UPDATE ' . GROUPS_TABLE . ' - SET group_teampage = group_teampage - 1 - WHERE group_teampage > ' . $current_legend; - $db->sql_query($sql); + $legend->delete_group($group_id, true); $group_attributes['group_legend'] = phpbb_group_positions::GROUP_DISABLED; } else @@ -2536,7 +2535,7 @@ function group_create(&$group_id, $type, $name, $desc, $group_attributes, $allow if (($group_id && ($current_teampage == phpbb_group_positions::GROUP_DISABLED)) || !$group_id) { // Old group currently not on the teampage or new group, add at the end. - $group_attributes['group_teampage'] = 1 + phpbb_group_positions::get_group_count('teampage'); + $group_attributes['group_teampage'] = 1 + $teampage->get_group_count(); } else { @@ -2547,10 +2546,7 @@ function group_create(&$group_id, $type, $name, $desc, $group_attributes, $allow else if ($group_id && ($current_teampage > phpbb_group_positions::GROUP_DISABLED)) { // Group is removed from the teampage - $sql = 'UPDATE ' . GROUPS_TABLE . ' - SET group_teampage = group_teampage - 1 - WHERE group_teampage > ' . $current_teampage; - $db->sql_query($sql); + $teampage->delete_group($group_id, true); $group_attributes['group_teampage'] = phpbb_group_positions::GROUP_DISABLED; } else @@ -2558,6 +2554,10 @@ function group_create(&$group_id, $type, $name, $desc, $group_attributes, $allow $group_attributes['group_teampage'] = phpbb_group_positions::GROUP_DISABLED; } + // Unset the objects, we don't need them anymore. + unset($legend); + unset($teampage); + $user_ary = array(); $sql_ary = array( 'group_name' => (string) $name, @@ -2783,8 +2783,12 @@ function group_delete($group_id, $group_name = false) while ($start); // Delete group from legend and teampage - phpbb_group_positions::delete_group('legend', $group_id); - phpbb_group_positions::delete_group('teampage', $group_id); + $legend = new phpbb_group_positions($db, 'legend'); + $legend->delete_group($group_id); + unset($legend); + $teampage = new phpbb_group_positions($db, 'teampage'); + $teampage->delete_group($group_id); + unset($teampage); // Delete group $sql = 'DELETE FROM ' . GROUPS_TABLE . " diff --git a/phpBB/includes/group_positions.php b/phpBB/includes/group_positions.php index 1749ca0578..e63e17c384 100644 --- a/phpBB/includes/group_positions.php +++ b/phpBB/includes/group_positions.php @@ -31,21 +31,41 @@ class phpbb_group_positions const GROUP_DISABLED = 0; /** - * Returns the group_{$field} for a given group, if the group exists. - * @param string $field name of the field to be selected - * @param int $group_id group_id of the group to be selected - * @return int position of the group + * phpbb-database object */ - static function get_group_value($field, $group_id) - { - global $db; + public $db = null; - $sql = 'SELECT group_' . $field . ' + /** + * Name of the field we want to handle: either 'teampage' or 'legend' + */ + private $field = ''; + + /** + * Constructor + */ + public function __construct ($db, $field) + { + if (!in_array($field, array('teampage', 'legend'))) + { + + } + $this->db = $db; + $this->field = $field; + } + + /** + * Returns the group_{$this->field} for a given group, if the group exists. + * @param int $group_id group_id of the group to be selected + * @return int position of the group + */ + public function get_group_value($group_id) + { + $sql = 'SELECT group_' . $this->field . ' FROM ' . GROUPS_TABLE . ' WHERE group_id = ' . (int) $group_id; - $result = $db->sql_query($sql); - $current_value = $db->sql_fetchfield('group_' . $field); - $db->sql_freeresult($result); + $result = $this->db->sql_query($sql); + $current_value = $this->db->sql_fetchfield('group_' . $this->field); + $this->db->sql_freeresult($result); if ($current_value === false) { @@ -59,149 +79,144 @@ class phpbb_group_positions /** * Get number of groups, displayed on the teampage/legend - * @param string $field name of the field to be counted - * @return int value of the last group displayed + * + * @return int value of the last group displayed */ - static function get_group_count($field) + public function get_group_count() { - global $db; - - $sql = 'SELECT group_' . $field . ' + $sql = 'SELECT group_' . $this->field . ' FROM ' . GROUPS_TABLE . ' - ORDER BY group_' . $field . ' DESC'; - $result = $db->sql_query_limit($sql, 1); - $group_count = (int) $db->sql_fetchfield('group_' . $field); - $db->sql_freeresult($result); + ORDER BY group_' . $this->field . ' DESC'; + $result = $this->db->sql_query_limit($sql, 1); + $group_count = (int) $this->db->sql_fetchfield('group_' . $this->field); + $this->db->sql_freeresult($result); return $group_count; } /** * Addes a group by group_id - * @param string $field name of the field the group is added to - * @param int $group_id group_id of the group to be added - * @return void + * + * @param int $group_id group_id of the group to be added + * @return void */ - static function add_group($field, $group_id) + public function add_group($group_id) { - $current_value = self::get_group_value($field, $group_id); + $current_value = $this->get_group_value($group_id); if ($current_value == self::GROUP_DISABLED) { - global $db; - // Group is currently not displayed, add it at the end. - $next_value = 1 + self::get_group_count($field, $field); + $next_value = 1 + $this->get_group_count(); $sql = 'UPDATE ' . GROUPS_TABLE . ' - SET group_' . $field . ' = ' . $next_value . ' - WHERE group_' . $field . ' = ' . self::GROUP_DISABLED . ' + SET group_' . $this->field . ' = ' . $next_value . ' + WHERE group_' . $this->field . ' = ' . self::GROUP_DISABLED . ' AND group_id = ' . (int) $group_id; - $db->sql_query($sql); + $this->db->sql_query($sql); } } /** * Deletes a group by group_id - * @param string $field name of the field the group is deleted from - * @param int $group_id group_id of the group to be deleted - * @return void + * + * @param int $group_id group_id of the group to be deleted + * @param bool $skip_group Skip the group itself, to save the query, when you need to update it anyway. + * @return void */ - static function delete_group($field, $group_id) + public function delete_group($group_id, $skip_group = false) { - $current_value = self::get_group_value($field, $group_id); + $current_value = $this->get_group_value($group_id); if ($current_value != self::GROUP_DISABLED) { - global $db; - - $db->sql_transaction('begin'); + $this->db->sql_transaction('begin'); $sql = 'UPDATE ' . GROUPS_TABLE . ' - SET group_' . $field . ' = group_' . $field . ' - 1 - WHERE group_' . $field . ' > ' . $current_value; - $db->sql_query($sql); + SET group_' . $this->field . ' = group_' . $this->field . ' - 1 + WHERE group_' . $this->field . ' > ' . $current_value; + $this->db->sql_query($sql); - $sql = 'UPDATE ' . GROUPS_TABLE . ' - SET group_' . $field . ' = ' . self::GROUP_DISABLED . ' - WHERE group_id = ' . (int) $group_id; - $db->sql_query($sql); + if (!$skip_group) + { + $sql = 'UPDATE ' . GROUPS_TABLE . ' + SET group_' . $this->field . ' = ' . self::GROUP_DISABLED . ' + WHERE group_id = ' . (int) $group_id; + $this->db->sql_query($sql); + } - $db->sql_transaction('commit'); + $this->db->sql_transaction('commit'); } } /** * Moves a group up by group_id - * @param string $field name of the field the group is moved by - * @param int $group_id group_id of the group to be moved - * @return void + * + * @param int $group_id group_id of the group to be moved + * @return void */ - static function move_up($field, $group_id) + public function move_up($group_id) { - $current_value = self::get_group_value($field, $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) { - global $db; - - $db->sql_transaction('begin'); + $this->db->sql_transaction('begin'); $sql = 'UPDATE ' . GROUPS_TABLE . ' - SET group_' . $field . ' = group_' . $field . ' + 1 - WHERE group_' . $field . ' = ' . ($current_value - 1); - $db->sql_query($sql); + 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_' . $field . ' = ' . ($current_value - 1) . ' + SET group_' . $this->field . ' = ' . ($current_value - 1) . ' WHERE group_id = ' . (int) $group_id; - $db->sql_query($sql); + $this->db->sql_query($sql); - $db->sql_transaction('commit'); + $this->db->sql_transaction('commit'); } } /** * Moves a group down by group_id - * @param string $field name of the field the group is moved by - * @param int $group_id group_id of the group to be moved - * @return void + * + * @param int $group_id group_id of the group to be moved + * @return void */ - static function move_down($field, $group_id) + public function move_down($group_id) { - $current_value = self::get_group_value($field, $group_id); + $current_value = $this->get_group_value($group_id); if ($current_value != self::GROUP_DISABLED) { - global $db; - - $db->sql_transaction('begin'); + $this->db->sql_transaction('begin'); $sql = 'UPDATE ' . GROUPS_TABLE . ' - SET group_' . $field . ' = group_' . $field . ' - 1 - WHERE group_' . $field . ' = ' . ($current_value + 1); - $db->sql_query($sql); + SET group_' . $this->field . ' = group_' . $this->field . ' - 1 + WHERE group_' . $this->field . ' = ' . ($current_value + 1); + $this->db->sql_query($sql); - if ($db->sql_affectedrows() == 1) + if ($this->db->sql_affectedrows() == 1) { // Only update when we move another one up, otherwise it was the last. $sql = 'UPDATE ' . GROUPS_TABLE . ' - SET group_' . $field . ' = ' . ($current_value + 1) . ' + SET group_' . $this->field . ' = ' . ($current_value + 1) . ' WHERE group_id = ' . (int) $group_id; - $db->sql_query($sql); + $this->db->sql_query($sql); } - $db->sql_transaction('commit'); + $this->db->sql_transaction('commit'); } } /** * Get group type language var - * @param int $group_type group_type from the groups-table - * @return string name of the language variable for the given group-type. + * + * @param int $group_type group_type from the groups-table + * @return string name of the language variable for the given group-type. */ - static function group_type_language($group_type) + static public function group_type_language($group_type) { switch ($group_type) { diff --git a/phpBB/install/database_update.php b/phpBB/install/database_update.php index 9e8a063f44..792e33245e 100644 --- a/phpBB/install/database_update.php +++ b/phpBB/install/database_update.php @@ -920,6 +920,7 @@ function database_update_info() '3.0.7-PL1' => array(), // No changes from 3.0.8-RC1 to 3.0.8 '3.0.8-RC1' => array(), + // Changes from 3.1.0-dev to 3.1.0-A1 '3.1.0-dev' => array( 'add_columns' => array( diff --git a/tests/group_positions/group_positions_test.php b/tests/group_positions/group_positions_test.php index 6955a084a2..449217c9b7 100644 --- a/tests/group_positions/group_positions_test.php +++ b/tests/group_positions/group_positions_test.php @@ -34,7 +34,8 @@ class phpbb_group_positions_test extends phpbb_database_test_case $db = $this->new_dbal(); - $this->assertEquals($expected, phpbb_group_positions::get_group_value($field, $group_id)); + $test_class = new phpbb_group_positions($db, $field); + $this->assertEquals($expected, $test_class->get_group_value($group_id)); } public static function get_group_count_data() @@ -54,7 +55,8 @@ class phpbb_group_positions_test extends phpbb_database_test_case $db = $this->new_dbal(); - $this->assertEquals($expected, phpbb_group_positions::get_group_count($field)); + $test_class = new phpbb_group_positions($db, $field); + $this->assertEquals($expected, $test_class->get_group_count()); } public static function add_group_data() @@ -81,7 +83,8 @@ class phpbb_group_positions_test extends phpbb_database_test_case global $db; $db = $this->new_dbal(); - phpbb_group_positions::add_group($field, $group_id); + $test_class = new phpbb_group_positions($db, $field); + $test_class->add_group($group_id); $result = $db->sql_query('SELECT group_id, group_teampage, group_legend FROM ' . GROUPS_TABLE . ' @@ -93,33 +96,49 @@ class phpbb_group_positions_test extends phpbb_database_test_case public static function delete_group_data() { return array( - array('teampage', 1, array( + array('teampage', 1, false, 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, array( + array('teampage', 2, false, array( array('group_id' => 1, 'group_teampage' => 0, 'group_legend' => 0), array('group_id' => 2, 'group_teampage' => 0, 'group_legend' => 0), array('group_id' => 3, 'group_teampage' => 1, 'group_legend' => 1), )), - array('teampage', 3, array( + array('teampage', 3, false, 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' => 0, 'group_legend' => 1), )), + array('teampage', 1, true, 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, true, 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' => 1, 'group_legend' => 1), + )), + array('teampage', 3, true, 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 delete_group_data */ - public function test_delete_group($field, $group_id, $expected) + public function test_delete_group($field, $group_id, $skip_group, $expected) { global $db; $db = $this->new_dbal(); - phpbb_group_positions::delete_group($field, $group_id); + $test_class = new phpbb_group_positions($db, $field); + $test_class->delete_group($group_id, $skip_group); $result = $db->sql_query('SELECT group_id, group_teampage, group_legend FROM ' . GROUPS_TABLE . ' @@ -157,7 +176,8 @@ class phpbb_group_positions_test extends phpbb_database_test_case global $db; $db = $this->new_dbal(); - phpbb_group_positions::move_up($field, $group_id); + $test_class = new phpbb_group_positions($db, $field); + $test_class->move_up($group_id); $result = $db->sql_query('SELECT group_id, group_teampage, group_legend FROM ' . GROUPS_TABLE . ' @@ -195,7 +215,8 @@ class phpbb_group_positions_test extends phpbb_database_test_case global $db; $db = $this->new_dbal(); - phpbb_group_positions::move_down($field, $group_id); + $test_class = new phpbb_group_positions($db, $field); + $test_class->move_down($group_id); $result = $db->sql_query('SELECT group_id, group_teampage, group_legend FROM ' . GROUPS_TABLE . '