Merge branch 'ticket/15339' into ticket/15339-33

phpBB/phpbb/db/migration/data/v30x/release_3_0_6_rc1.php

@@ -156,6 +156,7 @@ class release_3_0_6_rc1 extends \phpbb\db\migration\migration
 					'module_langname'	=> 'ACP_FEED_SETTINGS',
 					'module_mode'		=> 'feed',
 					'module_auth'		=> 'acl_a_board',
+					'after'				=> array('signature', 'ACP_SIGNATURE_SETTINGS'),
 				),
 			)),
 			array('module.add', array(
@@ -167,6 +168,7 @@ class release_3_0_6_rc1 extends \phpbb\db\migration\migration
 					'module_mode'		=> 'warnings',
 					'module_auth'		=> 'acl_a_user',
 					'module_display'	=> false,
+					'after'				=> array('feedback', 'ACP_USER_FEEDBACK'),
 				),
 			)),
 			array('module.add', array(
@@ -187,6 +189,7 @@ class release_3_0_6_rc1 extends \phpbb\db\migration\migration
 					'module_langname'	=> 'ACP_FORUM_PERMISSIONS_COPY',
 					'module_mode'		=> 'setting_forum_copy',
 					'module_auth'		=> 'acl_a_fauth && acl_a_authusers && acl_a_authgroups && acl_a_mauth',
+					'after'				=> array('setting_forum_local', 'ACP_FORUM_PERMISSIONS'),
 				),
 			)),
 			array('module.add', array(

phpBB/phpbb/db/migration/data/v30x/release_3_0_8_rc1.php

@@ -39,6 +39,7 @@ class release_3_0_8_rc1 extends \phpbb\db\migration\migration
 					'module_langname'	=> 'ACP_POST_SETTINGS',
 					'module_mode'		=> 'post',
 					'module_auth'		=> 'acl_a_board',
+					'after'				=> array('message', 'ACP_MESSAGE_SETTINGS'),
 				),
 			)),
 			array('config.add', array('load_unreads_search', 1)),

phpBB/phpbb/db/migration/tool/module.php

@@ -84,9 +84,11 @@ class module implements \phpbb\db\migration\tool\tool_interface
 	*		Use false to ignore the parent check and check class wide.
 	* @param int|string $module The module_id|module_langname you would like to
 	* 		check for to see if it exists
-	* @return bool true/false if module exists
+	* @param bool $lazy Checks lazily if the module exists. Returns true if it exists in at
+	*       least one given parent.
+	* @return bool true if module exists in *all* given parents, false if not
 	*/
-	public function exists($class, $parent, $module)
+	public function exists($class, $parent, $module, $lazy = false)
 	{
 		// the main root directory should return true
 		if (!$module)
@@ -94,18 +96,23 @@ class module implements \phpbb\db\migration\tool\tool_interface
 			return true;
 		}
 
-		$parent_sql = '';
+		$parent_sqls = [];
 		if ($parent !== false)
 		{
-			$parent = $this->get_parent_module_id($parent, $module, false);
+			$parents = $this->get_parent_module_id($parent, $module, false);
-			if ($parent === false)
+			if ($parents === false)
 			{
 				return false;
 			}
 
-			$parent_sql = 'AND parent_id = ' . (int) $parent;
+			foreach ((array) $parents as $parent_id)
+			{
+				$parent_sqls[] = 'AND parent_id = ' . (int) $parent_id;
+			}
 		}
 
+		foreach ($parent_sqls as $parent_sql)
+		{
 			$sql = 'SELECT module_id
 				FROM ' . $this->modules_table . "
 				WHERE module_class = '" . $this->db->sql_escape($class) . "'
@@ -115,12 +122,18 @@ class module implements \phpbb\db\migration\tool\tool_interface
 			$module_id = $this->db->sql_fetchfield('module_id');
 			$this->db->sql_freeresult($result);
 
-		if ($module_id)
+			if (!$lazy && !$module_id)
+			{
+				return false;
+			}
+			else if ($lazy && $module_id)
 			{
 				return true;
 			}
+		}
 
-		return false;
+		// Returns true, if modules exist in all parents and false otherwise
+		return !$lazy;
 	}
 
 	/**
@@ -172,7 +185,7 @@ class module implements \phpbb\db\migration\tool\tool_interface
 			$data = array('module_langname' => $data);
 		}
 
-		$parent = $data['parent_id'] = $this->get_parent_module_id($parent, $data);
+		$parents = (array) $this->get_parent_module_id($parent, $data);
 
 		if (!isset($data['module_langname']))
 		{
@@ -195,13 +208,20 @@ class module implements \phpbb\db\migration\tool\tool_interface
 					);
 
 					// Run the "manual" way with the data we've collected.
+					foreach ($parents as $parent)
+					{
 						$this->add($class, $parent, $new_module);
 					}
 				}
+			}
 
 			return;
 		}
 
+		foreach ($parents as $parent)
+		{
+			$data['parent_id'] = $parent;
+
 			// The "manual" way
 			if (!$this->exists($class, false, $parent))
 			{
@@ -235,13 +255,27 @@ class module implements \phpbb\db\migration\tool\tool_interface
 				// Move the module if requested above/below an existing one
 				if (isset($data['before']) && $data['before'])
 				{
+					$before_mode = $before_langname = '';
+					if (is_array($data['before']))
+					{
+						// Restore legacy-legacy behaviour from phpBB 3.0
+						list($before_mode, $before_langname) = $data['before'];
+					}
+					else
+					{
+						// Legacy behaviour from phpBB 3.1+
+						$before_langname = $data['before'];
+					}
+
 					$sql = 'SELECT left_id
 					FROM ' . $this->modules_table . "
 					WHERE module_class = '" . $this->db->sql_escape($class) . "'
 						AND parent_id = " . (int) $parent . "
-						AND module_langname = '" . $this->db->sql_escape($data['before']) . "'";
+						AND module_langname = '" . $this->db->sql_escape($before_langname) . "'"
-				$this->db->sql_query($sql);
+						. (($before_mode) ? " AND module_mode = '" . $this->db->sql_escape($before_mode) . "'" : '');
+					$result = $this->db->sql_query($sql);
 					$to_left = (int) $this->db->sql_fetchfield('left_id');
+					$this->db->sql_freeresult($result);
 
 					$sql = 'UPDATE ' . $this->modules_table . "
 					SET left_id = left_id + 2, right_id = right_id + 2
@@ -258,13 +292,27 @@ class module implements \phpbb\db\migration\tool\tool_interface
 				}
 				else if (isset($data['after']) && $data['after'])
 				{
+					$after_mode = $after_langname = '';
+					if (is_array($data['after']))
+					{
+						// Restore legacy-legacy behaviour from phpBB 3.0
+						list($after_mode, $after_langname) = $data['after'];
+					}
+					else
+					{
+						// Legacy behaviour from phpBB 3.1+
+						$after_langname = $data['after'];
+					}
+
 					$sql = 'SELECT right_id
 					FROM ' . $this->modules_table . "
 					WHERE module_class = '" . $this->db->sql_escape($class) . "'
 						AND parent_id = " . (int) $parent . "
-						AND module_langname = '" . $this->db->sql_escape($data['after']) . "'";
+						AND module_langname = '" . $this->db->sql_escape($after_langname) . "'"
-				$this->db->sql_query($sql);
+						. (($after_mode) ? " AND module_mode = '" . $this->db->sql_escape($after_mode) . "'" : '');
+					$result = $this->db->sql_query($sql);
 					$to_right = (int) $this->db->sql_fetchfield('right_id');
+					$this->db->sql_freeresult($result);
 
 					$sql = 'UPDATE ' . $this->modules_table . "
 					SET left_id = left_id + 2, right_id = right_id + 2
@@ -285,6 +333,7 @@ class module implements \phpbb\db\migration\tool\tool_interface
 				// Error
 				throw new \phpbb\db\migration\exception('MODULE_ERROR', $e->getMessage());
 			}
+		}
 
 		// Clear the Modules Cache
 		$this->cache->destroy("_modules_$class");
@@ -334,7 +383,7 @@ class module implements \phpbb\db\migration\tool\tool_interface
 		}
 		else
 		{
-			if (!$this->exists($class, $parent, $module))
+			if (!$this->exists($class, $parent, $module, true))
 			{
 				return;
 			}
@@ -342,8 +391,8 @@ class module implements \phpbb\db\migration\tool\tool_interface
 			$parent_sql = '';
 			if ($parent !== false)
 			{
-				$parent = $this->get_parent_module_id($parent, $module);
+				$parents = (array) $this->get_parent_module_id($parent, $module);
-				$parent_sql = 'AND parent_id = ' . (int) $parent;
+				$parent_sql = 'AND ' . $this->db->sql_in_set('parent_id', $parents);
 			}
 
 			$module_ids = array();
@@ -457,14 +506,11 @@ class module implements \phpbb\db\migration\tool\tool_interface
 	* @param string|int $parent_id The parent module_id|module_langname
 	* @param int|string|array $data The module_id, module_langname for existance checking or module data array for adding
 	* @param bool $throw_exception The flag indicating if exception should be thrown on error
-	* @return mixed The int parent module_id or false
+	* @return mixed The int parent module_id, an array of int parent module_id values or false
 	* @throws \phpbb\db\migration\exception
 	*/
 	public function get_parent_module_id($parent_id, $data = '', $throw_exception = true)
 	{
-		// Initialize exception object placeholder
-		$exception = false;
-
 		// Allow '' to be sent as 0
 		$parent_id = $parent_id ?: 0;
 
@@ -486,61 +532,26 @@ class module implements \phpbb\db\migration\tool\tool_interface
 			{
 				// No parent with the given module_langname exist
 				case 0:
-					$exception = new \phpbb\db\migration\exception('MODULE_NOT_EXIST', $parent_id);
+					if ($throw_exception)
+					{
+						throw new \phpbb\db\migration\exception('MODULE_NOT_EXIST', $parent_id);
+					}
+
+					return false;
 				break;
 
 				// Return the module id
 				case 1:
-					$parent_id = (int) $ids[0];
+					return (int) $ids[0];
 				break;
 
-				// Several modules with the given module_langname were found
-				// Try to determine the parent_id by the neighbour module parent
 				default:
-					if (is_array($data) && (isset($data['before']) || isset($data['after'])))
+					// This represents the old behaviour of phpBB 3.0
-					{
+					return $ids;
-						$neighbour_module_langname = isset($data['before']) ? $data['before'] : $data['after'];
-						$sql = 'SELECT parent_id
-							FROM ' . $this->modules_table . "
-							WHERE module_langname = '" . $this->db->sql_escape($neighbour_module_langname) . "'
-								AND " . $this->db->sql_in_set('parent_id', $ids);
-						$result = $this->db->sql_query($sql);
-						$parent_id = (int) $this->db->sql_fetchfield('parent_id');
-						if (!$parent_id)
-						{
-							$exception = new \phpbb\db\migration\exception('PARENT_MODULE_FIND_ERROR', $data['parent_id']);
-						}
-					}
-					else if (!empty($data) && !is_array($data))
-					{
-						// The module_langname is set, checking for the module existance
-						// As more than 1 parents were found already, there's no way for null parent_id here
-						$sql = 'SELECT m2.module_id as module_parent_id
-							FROM ' . $this->modules_table . ' m1, ' . $this->modules_table . " m2
-							WHERE " . ((is_numeric($data)) ? 'm1.module_id = ' . (int) $data : "m1.module_langname = '" . $this->db->sql_escape($data)) . "'
-								AND m2.module_id = m1.parent_id
-								AND " . $this->db->sql_in_set('m2.module_id', $ids);
-						$result = $this->db->sql_query($sql);
-						$parent_id = (int) $this->db->sql_fetchfield('module_parent_id');
-					}
-					else
-					{
-						//Unable to get the parent module id, throwing an exception
-						$exception = new \phpbb\db\migration\exception('MODULE_EXIST_MULTIPLE', $parent_id);
-					}
 				break;
 			}
 		}
 
-		if ($exception !== false)
-		{
-			if ($throw_exception)
-			{
-				throw $exception;
-			}
-			return false;
-		}
-
 		return $parent_id;
 	}
 }

tests/dbal/migrator_tool_module_test.php

@@ -193,25 +193,6 @@ class phpbb_dbal_migrator_tool_module_test extends phpbb_database_test_case
 		}
 		$this->assertEquals(true, $this->tool->exists('acp', 'ACP_NEW_CAT', 'ACP_NEW_MODULE'));
 
-		// Test adding module when plural parent module_langname exists
-		// PHPBB3-14703
-		// Adding fail
-		try
-		{
-			$this->tool->add('acp', 'ACP_FORUM_BASED_PERMISSIONS', array(
-				'module_basename'	=> 'acp_new_permissions_module',
-				'module_langname'	=> 'ACP_NEW_PERMISSIONS_MODULE',
-				'module_mode'		=> 'test',
-				'module_auth'		=> '',
-			));
-			$this->fail('Exception not thrown');
-		}
-		catch (Exception $e)
-		{
-			$this->assertEquals('phpbb\db\migration\exception', get_class($e));
-			$this->assertEquals('MODULE_EXIST_MULTIPLE', $e->getMessage());
-		}
-
 		// Test adding module when plural parent module_langname exists
 		// PHPBB3-14703
 		// Adding success