Merge branch '3.1.x' into 3.2.x

Conflicts: phpBB/phpbb/db/migration/tool/module.php
author: Marc Alexander <admin@m-a-styles.de> 2016-08-31 21:41:17 +0200
committer: Marc Alexander <admin@m-a-styles.de> 2016-08-31 21:41:17 +0200
commit: dff950162cd4d79d5e199e833e46a98d381397dc (patch)
tree: 344a365547b8a99a7f57cd8dcba79cc1fc839643
parent: 05e6ed04ed48e321990125059e64e2d1592d69c4 (diff)
parent: f008708d5cebfea7ef6efef4c876e99f819bff71 (diff)
download: forums-dff950162cd4d79d5e199e833e46a98d381397dc.tar
forums-dff950162cd4d79d5e199e833e46a98d381397dc.tar.gz
forums-dff950162cd4d79d5e199e833e46a98d381397dc.tar.bz2
forums-dff950162cd4d79d5e199e833e46a98d381397dc.tar.xz
forums-dff950162cd4d79d5e199e833e46a98d381397dc.zip
4 files changed, 211 insertions, 67 deletions
diff --git a/phpBB/language/en/migrator.php b/phpBB/language/en/migrator.php
index 78364319a1..8a82d40be5 100644
--- a/phpBB/language/en/migrator.php
+++ b/phpBB/language/en/migrator.php
@@ -69,9 +69,12 @@ $lang = array_merge($lang, array(
 	'MIGRATION_INVALID_DATA_UNDEFINED_METHOD'		=> 'A migration is invalid. An undefined migration tool method was encountered.',
 
 	'MODULE_ERROR'						=> 'An error occurred while creating a module: %s',
+	'MODULE_EXISTS'						=> 'A module already exists: %s',
+	'MODULE_EXIST_MULTIPLE'				=> 'Several modules with the given parent module langname already exist: %s. Try using before/after keys to clarify the module placement.',
 	'MODULE_INFO_FILE_NOT_EXIST'		=> 'A required module info file is missing: %2$s',
 	'MODULE_NOT_EXIST'					=> 'A required module does not exist: %s',
 
+	'PARENT_MODULE_FIND_ERROR'			=> 'Unable to determine the parent module identifier: %s',
 	'PERMISSION_NOT_EXIST'				=> 'The permission setting "%s" unexpectedly does not exist.',
 
 	'ROLE_NOT_EXIST'					=> 'The permission role "%s" unexpectedly does not exist.',
diff --git a/phpBB/phpbb/db/migration/tool/module.php b/phpBB/phpbb/db/migration/tool/module.php
index 90ed63e2e6..a7dffbb7f2 100644
--- a/phpBB/phpbb/db/migration/tool/module.php
+++ b/phpBB/phpbb/db/migration/tool/module.php
@@ -41,6 +41,9 @@ class module implements \phpbb\db\migration\tool\tool_interface
 	/** @var string */
 	protected $modules_table;
 
+	/** @var array */
+	protected $module_categories = array();
+
 	/**
 	* Constructor
 	*
@@ -94,30 +97,8 @@ class module implements \phpbb\db\migration\tool\tool_interface
 		$parent_sql = '';
 		if ($parent !== false)
 		{
-			// Allows '' to be sent as 0
-			$parent = $parent ?: 0;
-
-			if (!is_numeric($parent))
-			{
-				$sql = 'SELECT module_id
-					FROM ' . $this->modules_table . "
-					WHERE module_langname = '" . $this->db->sql_escape($parent) . "'
-						AND module_class = '" . $this->db->sql_escape($class) . "'";
-				$result = $this->db->sql_query($sql);
-				$module_id = $this->db->sql_fetchfield('module_id');
-				$this->db->sql_freeresult($result);
-
-				if (!$module_id)
-				{
-					return false;
-				}
-
-				$parent_sql = 'AND parent_id = ' . (int) $module_id;
-			}
-			else
-			{
-				$parent_sql = 'AND parent_id = ' . (int) $parent;
-			}
+			$parent = $this->get_parent_module_id($parent, $module);
+			$parent_sql = 'AND parent_id = ' . (int) $parent;
 		}
 
 		$sql = 'SELECT module_id
@@ -180,15 +161,14 @@ class module implements \phpbb\db\migration\tool\tool_interface
 	{
 		global $user, $phpbb_log;
 
-		// Allows '' to be sent as 0
-		$parent = $parent ?: 0;
-
 		// allow sending the name as a string in $data to create a category
 		if (!is_array($data))
 		{
 			$data = array('module_langname' => $data);
 		}
 
+		$parent = $data['parent_id'] = $this->get_parent_module_id($parent, $data);
+
 		if (!isset($data['module_langname']))
 		{
 			// The "automatic" way
@@ -218,31 +198,14 @@ class module implements \phpbb\db\migration\tool\tool_interface
 		}
 
 		// The "manual" way
-		if (!is_numeric($parent))
-		{
-			$sql = 'SELECT module_id
-				FROM ' . $this->modules_table . "
-				WHERE module_langname = '" . $this->db->sql_escape($parent) . "'
-					AND module_class = '" . $this->db->sql_escape($class) . "'";
-			$result = $this->db->sql_query($sql);
-			$module_id = $this->db->sql_fetchfield('module_id');
-			$this->db->sql_freeresult($result);
-
-			if (!$module_id)
-			{
-				throw new \phpbb\db\migration\exception('MODULE_NOT_EXIST', $parent);
-			}
-
-			$parent = $data['parent_id'] = $module_id;
-		}
-		else if (!$this->exists($class, false, $parent))
+		if (!$this->exists($class, false, $parent))
 		{
 			throw new \phpbb\db\migration\exception('MODULE_NOT_EXIST', $parent);
 		}
 
 		if ($this->exists($class, $parent, $data['module_langname']))
 		{
-			return;
+			throw new \phpbb\db\migration\exception('MODULE_EXISTS', $module_id);
 		}
 
 		$module_data = array(
@@ -374,26 +337,8 @@ class module implements \phpbb\db\migration\tool\tool_interface
 			$parent_sql = '';
 			if ($parent !== false)
 			{
-				// Allows '' to be sent as 0
-				$parent = ($parent) ?: 0;
-
-				if (!is_numeric($parent))
-				{
-					$sql = 'SELECT module_id
-						FROM ' . $this->modules_table . "
-						WHERE module_langname = '" . $this->db->sql_escape($parent) . "'
-							AND module_class = '" . $this->db->sql_escape($class) . "'";
-					$result = $this->db->sql_query($sql);
-					$module_id = $this->db->sql_fetchfield('module_id');
-					$this->db->sql_freeresult($result);
-
-					// we know it exists from the module_exists check
-					$parent_sql = 'AND parent_id = ' . (int) $module_id;
-				}
-				else
-				{
-					$parent_sql = 'AND parent_id = ' . (int) $parent;
-				}
+				$parent = $this->get_parent_module_id($parent, $module);
+				$parent_sql = 'AND parent_id = ' . (int) $parent;
 			}
 
 			$module_ids = array();
@@ -475,4 +420,110 @@ class module implements \phpbb\db\migration\tool\tool_interface
 
 		return array_pop($module);
 	}
+
+	/**
+	* Get the list of installed module categories
+	*	key - module_id
+	*	value - module_langname
+	*
+	* @return null
+	*/
+	protected function get_categories_list()
+	{
+		// Select the top level categories
+		// and 2nd level [sub]categories which exist for ACP only
+		$sql = 'SELECT m2.module_id, m2.module_langname
+			FROM ' . $this->modules_table . ' m1, ' . $this->modules_table . " m2
+			WHERE m1.parent_id = 0
+				AND (m1.module_id = m2.module_id
+				OR m2.module_class = 'acp' AND m2.parent_id = m1.module_id)
+			ORDER BY m1.module_id, m2.module_id ASC";
+
+		$result = $this->db->sql_query($sql);
+		while ($row = $this->db->sql_fetchrow($result))
+		{
+			$this->module_categories[(int) $row['module_id']] = $row['module_langname'];
+		}
+		$this->db->sql_freeresult($result);
+	}
+
+	/**
+	* Get parent module id
+	*
+	* @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
+	* @return int The parent module_id
+	* @throws \phpbb\db\migration\exception
+	*/
+	public function get_parent_module_id($parent_id, $data = '')
+	{
+		// Allow '' to be sent as 0
+		$parent_id = $parent_id ?: 0;
+
+		// If automatic adding is in action, convert array back to string to simplify things
+		if (is_array($data) && sizeof($data) == 1)
+		{
+			$data = $data['module_langname'];
+		}
+
+		if (!is_numeric($parent_id))
+		{
+			// Refresh the $module_categories array
+			$this->get_categories_list();
+
+			// Search for the parent module_langname
+			$ids = array_keys($this->module_categories, $parent_id);
+
+			switch (sizeof($ids))
+			{
+				// No parent with the given module_langname exist
+				case 0:
+					throw new \phpbb\db\migration\exception('MODULE_NOT_EXIST', $parent_id);
+				break;
+
+				// Return the module id
+				case 1:
+					$parent_id = (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'])))
+					{
+						$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)
+						{
+							throw 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
+						throw new \phpbb\db\migration\exception('MODULE_EXIST_MULTIPLE', $parent_id);
+					}
+				break;
+			}
+		}
+
+		return $parent_id;
+	}
 }
diff --git a/tests/dbal/fixtures/migrator_module.xml b/tests/dbal/fixtures/migrator_module.xml
index 32afe7e6f3..e172d7a145 100644
--- a/tests/dbal/fixtures/migrator_module.xml
+++ b/tests/dbal/fixtures/migrator_module.xml
@@ -20,7 +20,7 @@
 			<value>acp</value>
 			<value>0</value>
 			<value>1</value>
-			<value>4</value>
+			<value>6</value>
 			<value>ACP_CAT</value>
 			<value></value>
 			<value></value>
@@ -38,5 +38,57 @@
 			<value>test</value>
 			<value></value>
 		</row>
+		<row>
+			<value>3</value>
+			<value>1</value>
+			<value>1</value>
+			<value></value>
+			<value>acp</value>
+			<value>1</value>
+			<value>4</value>
+			<value>5</value>
+			<value>ACP_FORUM_BASED_PERMISSIONS</value>
+			<value></value>
+			<value></value>
+		</row>
+		<row>
+			<value>4</value>
+			<value>1</value>
+			<value>1</value>
+			<value></value>
+			<value>acp</value>
+			<value>0</value>
+			<value>7</value>
+			<value>12</value>
+			<value>ACP_CAT_FORUMS</value>
+			<value></value>
+			<value></value>
+		</row>
+		<row>
+			<value>5</value>
+			<value>1</value>
+			<value>1</value>
+			<value></value>
+			<value>acp</value>
+			<value>4</value>
+			<value>8</value>
+			<value>11</value>
+			<value>ACP_FORUM_BASED_PERMISSIONS</value>
+			<value></value>
+			<value></value>
+		</row>
+		<row>
+			<value>6</value>
+			<value>1</value>
+			<value>1</value>
+			<value></value>
+			<value>acp</value>
+			<value>5</value>
+			<value>9</value>
+			<value>10</value>
+			<value>ACP_FORUM_BASED_PERMISSIONS_CHILD_1</value>
+			<value></value>
+			<value></value>
+		</row>
 	</table>
 </dataset>
diff --git a/tests/dbal/migrator_tool_module_test.php b/tests/dbal/migrator_tool_module_test.php
index c2252c8d10..1744b7e92d 100644
--- a/tests/dbal/migrator_tool_module_test.php
+++ b/tests/dbal/migrator_tool_module_test.php
@@ -121,6 +121,44 @@ class phpbb_dbal_migrator_tool_module_test extends phpbb_database_test_case
 			$this->fail($e);
 		}
 		$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
+		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'		=> '',
+				'after'				=> 'ACP_FORUM_BASED_PERMISSIONS_CHILD_1',
+			));
+		}
+		catch (Exception $e)
+		{
+			$this->fail($e);
+		}
+		$this->assertEquals(true, $this->tool->exists('acp', 'ACP_FORUM_BASED_PERMISSIONS', 'ACP_NEW_PERMISSIONS_MODULE'));
 	}
 
 	public function test_remove()
author	Marc Alexander <admin@m-a-styles.de>	2016-08-31 21:41:17 +0200
committer	Marc Alexander <admin@m-a-styles.de>	2016-08-31 21:41:17 +0200
commit	dff950162cd4d79d5e199e833e46a98d381397dc (patch)
tree	344a365547b8a99a7f57cd8dcba79cc1fc839643
parent	05e6ed04ed48e321990125059e64e2d1592d69c4 (diff)
parent	f008708d5cebfea7ef6efef4c876e99f819bff71 (diff)
download	forums-dff950162cd4d79d5e199e833e46a98d381397dc.tar forums-dff950162cd4d79d5e199e833e46a98d381397dc.tar.gz forums-dff950162cd4d79d5e199e833e46a98d381397dc.tar.bz2 forums-dff950162cd4d79d5e199e833e46a98d381397dc.tar.xz forums-dff950162cd4d79d5e199e833e46a98d381397dc.zip