[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

@@ -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;
 		}
 

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_legend = $legend->get_group_value($group_id);
-			$current_teampage = phpbb_group_positions::get_group_value('teampage', $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 . '
+			$legend->delete_group($group_id, true);
-				SET group_teampage = group_teampage - 1
-				WHERE group_teampage > ' . $current_legend;
-			$db->sql_query($sql);
 			$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 . '
+			$teampage->delete_group($group_id, true);
-				SET group_teampage = group_teampage - 1
-				WHERE group_teampage > ' . $current_teampage;
-			$db->sql_query($sql);
 			$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);
+	$legend = new phpbb_group_positions($db, 'legend');
-	phpbb_group_positions::delete_group('teampage', $group_id);
+	$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 . "

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.
+	* phpbb-database object
-	* @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
 	*/
-	static function get_group_value($field, $group_id)
+	public $db = null;
-	{
-		global $db;
 
-		$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);
+		$result = $this->db->sql_query($sql);
-		$current_value = $db->sql_fetchfield('group_' . $field);
+		$current_value = $this->db->sql_fetchfield('group_' . $this->field);
-		$db->sql_freeresult($result);
+		$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_' . $this->field . '
-
-		$sql = 'SELECT group_' . $field . '
 			FROM ' . GROUPS_TABLE . '
-			ORDER BY group_' . $field . ' DESC';
+			ORDER BY group_' . $this->field . ' DESC';
-		$result = $db->sql_query_limit($sql, 1);
+		$result = $this->db->sql_query_limit($sql, 1);
-		$group_count = (int) $db->sql_fetchfield('group_' . $field);
+		$group_count = (int) $this->db->sql_fetchfield('group_' . $this->field);
-		$db->sql_freeresult($result);
+		$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
+	* @param	int		$group_id	group_id of the group to be added
-	* @return void
+	* @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 . '
+				SET group_' . $this->field . ' = ' . $next_value . '
-				WHERE group_' . $field . ' = ' . self::GROUP_DISABLED . '
+				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
+	* @param	int		$group_id		group_id of the group to be deleted
-	* @return void
+	* @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;
+			$this->db->sql_transaction('begin');
-
-			$db->sql_transaction('begin');
 
 			$sql = 'UPDATE ' . GROUPS_TABLE . '
-				SET group_' . $field . ' = group_' . $field . ' - 1
+				SET group_' . $this->field . ' = group_' . $this->field . ' - 1
-				WHERE group_' . $field . ' > ' . $current_value;
+				WHERE group_' . $this->field . ' > ' . $current_value;
-			$db->sql_query($sql);
+			$this->db->sql_query($sql);
 
-			$sql = 'UPDATE ' . GROUPS_TABLE . '
+			if (!$skip_group)
-				SET group_' . $field . ' = ' . self::GROUP_DISABLED . '
+			{
-				WHERE group_id = ' . (int) $group_id;
+				$sql = 'UPDATE ' . GROUPS_TABLE . '
-			$db->sql_query($sql);
+					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
+	* @param	int		$group_id	group_id of the group to be moved
-	* @return void
+	* @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;
+			$this->db->sql_transaction('begin');
-
-			$db->sql_transaction('begin');
 
 			$sql = 'UPDATE ' . GROUPS_TABLE . '
-				SET group_' . $field . ' = group_' . $field . ' + 1
+				SET group_' . $this->field . ' = group_' . $this->field . ' + 1
-				WHERE group_' . $field . ' = ' . ($current_value - 1);
+				WHERE group_' . $this->field . ' = ' . ($current_value - 1);
-			$db->sql_query($sql);
+			$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
+	* @param	int		$group_id	group_id of the group to be moved
-	* @return void
+	* @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;
+			$this->db->sql_transaction('begin');
-
-			$db->sql_transaction('begin');
 
 			$sql = 'UPDATE ' . GROUPS_TABLE . '
-				SET group_' . $field . ' = group_' . $field . ' - 1
+				SET group_' . $this->field . ' = group_' . $this->field . ' - 1
-				WHERE group_' . $field . ' = ' . ($current_value + 1);
+				WHERE group_' . $this->field . ' = ' . ($current_value + 1);
-			$db->sql_query($sql);
+			$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)
 		{

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(

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 . '