[ticket/416] Add handler for module constraints

B3P-416

acp/portal_module.php

@@ -50,6 +50,7 @@ class portal_module
 		$this->log = $phpbb_log;
 		$this->portal_columns = $this->phpbb_container->get('board3.portal.columns');
 		$this->modules_manager = $this->phpbb_container->get('board3.portal.modules.manager');
+		$this->modules_constraints = $this->phpbb_container->get('board3.portal.modules.constraints_handler');
 		define('PORTAL_MODULES_TABLE', $this->phpbb_container->getParameter('board3.portal.modules.table'));
 		define('PORTAL_CONFIG_TABLE', $this->phpbb_container->getParameter('board3.portal.config.table'));
 
@@ -392,14 +393,16 @@ class portal_module
 
 				// Create an array of already installed modules
 				$portal_modules = obtain_portal_modules();
-				$installed_modules = array();
+				$installed_modules = $module_column = array();
 
 				foreach($portal_modules as $cur_module)
 				{
 					$installed_modules[] = $cur_module['module_classname'];
 					// Create an array with the columns the module is in
-					$this->modules_manager->module_column[$cur_module['module_classname']][] = $this->portal_columns->number_to_string($cur_module['module_column']);
+					$module_column[$cur_module['module_classname']][] = $this->portal_columns->number_to_string($cur_module['module_column']);
 				}
+				$this->modules_constraints->set_module_column($module_column);
+				unset($module_column);
 
 				if ($action == 'move_up')
 				{
@@ -582,7 +585,7 @@ class portal_module
 							{
 								$column_string = $this->portal_columns->number_to_string($row['module_column'] + 1); // move 1 right
 
-								if ($column_string == 'right' && !$this->modules_manager->can_move_module(array('left', 'right'), $row['module_classname']))
+								if ($column_string == 'right' && !$this->modules_constraints->can_move_module(array('left', 'right'), $row['module_classname']))
 								{
 									$move_right = false;
 								}
@@ -612,7 +615,7 @@ class portal_module
 							{
 								$column_string = $this->portal_columns->number_to_string($row['module_column'] - 1); // move 1 left
 
-								if ($column_string == 'left' && !$this->modules_manager->can_move_module(array('left', 'right'), $row['module_classname']))
+								if ($column_string == 'left' && !$this->modules_constraints->can_move_module(array('left', 'right'), $row['module_classname']))
 								{
 									$move_left = false;
 								}

config/services.yml

@@ -103,11 +103,18 @@ services:
             - @dbal.conn
             - @board3.portal.columns
             - @board3.portal.helper
-            - @board.portal.modules.database_handler
+            - @board3.portal.modules.constraints_handler
+            - @board3.portal.modules.database_handler
             - @request
             - @user
 
-    board.portal.modules.database_handler:
+    board3.portal.modules.database_handler:
         class: board3\portal\portal\modules\database_handler
         arguments:
             - @dbal.conn
+
+    board3.portal.modules.constraints_handler:
+        class: board3\portal\portal\modules\constraints_handler
+        arguments:
+            - @board3.portal.columns
+            - @user

portal/modules/constraints_handler.php

@@ -0,0 +1,162 @@
+<?php
+/**
+ *
+ * @package Board3 Portal v2.1
+ * @copyright (c) 2014 Board3 Group ( www.board3.de )
+ * @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2
+ *
+ */
+
+namespace board3\portal\portal\modules;
+
+use board3\portal\portal\columns;
+
+class constraints_handler
+{
+	/** @var \board3\portal\portal\columns */
+	protected $portal_columns;
+
+	/** @var \phpbb\user */
+	protected $user;
+
+	/** @var string Form action (link) */
+	protected $u_action;
+
+	/** @var array Array with info about modules and their columns */
+	public $module_column = array();
+
+	/**
+	 * Constructor for constraints handler
+	 *
+	 * @param columns $portal_columns Portal columns
+	 * @param \phpbb\user $user phpBB user object
+	 */
+	public function __construct(columns $portal_columns, $user)
+	{
+		$this->portal_columns = $portal_columns;
+		$this->user = $user;
+	}
+
+	/**
+	 * Set u_action for module
+	 *
+	 * @param string $u_action u_action for module
+	 */
+	public function set_u_action($u_action)
+	{
+		$this->u_action = $u_action;
+	}
+
+	/**
+	 * Set module columns info
+	 *
+	 * @param array $module_column Array with info about modules and their columns
+	 */
+	public function set_module_column($module_column = array())
+	{
+		$this->module_column = $module_column;
+	}
+
+	/**
+	 * Check if there is conflict between the move action and existing modules
+	 *
+	 * @param array $module_data The module's data
+	 * @param int $move_action The move action
+	 *
+	 * @return null
+	 */
+	public function check_module_conflict($module_data, &$move_action)
+	{
+		/**
+		 * Moving only 1 column means we will either end up in a side column
+		 * or in the center column. This is not possible when moving 2 columns.
+		 * Therefore we only need to check if we won't end up with a duplicate
+		 * module in the new column (side columns (left & right) or center
+		 * columns (top, center, bottom)). Of course this does not apply to
+		 * custom modules.
+		 */
+		if ($module_data['module_classname'] != '\board3\portal\modules\custom' && abs($move_action) == 1)
+		{
+			$column_string = $this->portal_columns->number_to_string($module_data['module_column'] + $move_action);
+
+			// we can only move horizontally to center or side columns
+			if (in_array($column_string, array('right', 'left')) && !$this->can_move_module(array('right', 'left'), $module_data['module_classname']))
+			{
+				trigger_error($this->user->lang['UNABLE_TO_MOVE'] . adm_back_link($this->u_action));
+			}
+			else if ($column_string == 'center' && !$this->can_move_module(array('top', 'center', 'bottom'), $module_data['module_classname']))
+			{
+				// we are moving from the right to the center column so we should move to the left column instead
+				$move_action = 2 * $move_action;
+			}
+		}
+	}
+
+	/**
+	 * Check if module can be moved horizontally
+	 *
+	 * @param array $module_data Module's module data
+	 * @param int $direction Direction to move the module
+	 *
+	 * @return bool True if module can be moved, false if not
+	 */
+	public function can_move_horizontally($module_data, $direction)
+	{
+		if (isset($module_data['module_column']))
+		{
+			return ($direction === database_handler::MOVE_DIRECTION_RIGHT) ? $module_data['module_column'] < $this->portal_columns->string_to_number('right') : $module_data['module_column'] > $this->portal_columns->string_to_number('left');
+		}
+		else
+		{
+			return false;
+		}
+	}
+
+	/**
+	 * Check if module can be moved to desired column(s)
+	 *
+	 * @param array|int	$target_column Column(s) the module should be moved to
+	 * @param string		$module_class Class of the module
+	 * @return bool		True if module can be moved to desired column,
+	 *			false if not
+	 */
+	public function can_move_module($target_column, $module_class)
+	{
+		$submit = true;
+
+		if (is_array($target_column))
+		{
+			foreach ($target_column as $column)
+			{
+				if (!$this->can_move_module($column, $module_class))
+				{
+					$submit = false;
+				}
+			}
+		}
+
+		// do we want to add the module to the side columns or to the center columns?
+		if (in_array($target_column, array('left', 'right')))
+		{
+			// does the module already exist in the side columns?
+			if (isset($this->module_column[$module_class]) &&
+				(in_array('left', $this->module_column[$module_class]) || in_array('right', $this->module_column[$module_class])))
+			{
+				$submit = false;
+			}
+		}
+		else if (in_array($target_column, array('center', 'top', 'bottom')))
+		{
+			// does the module already exist in the center columns?
+			if (isset($this->module_column[$module_class]) &&
+				(in_array('center', $this->module_column[$module_class]) ||
+					in_array('top', $this->module_column[$module_class]) ||
+					in_array('bottom', $this->module_column[$module_class])))
+			{
+				$submit = false;
+			}
+		}
+
+		return $submit;
+	}
+}

portal/modules/manager.php

@@ -26,6 +26,9 @@ class manager
 	/** @var \board3\portal\includes\helper */
 	protected $portal_helper;
 
+	/** @var \board3\portal\portal\modules\constraints_handler */
+	protected $constraints_handler;
+
 	/** @var \board3\portal\portal\modules\database_handler */
 	protected $database_handler;
 
@@ -47,9 +50,6 @@ class manager
 	/** @var string class of acp module */
 	protected $acp_class;
 
-	/** @var array Array of module columns */
-	public $module_column = array();
-
 	/**
 	 * Constructor for modules manager
 	 *
@@ -57,16 +57,18 @@ class manager
 	 * @param \phpbb\db\driver\driver_interface $db Database driver
 	 * @param \board3\portal\portal\columns $portal_columns Portal columns helper
 	 * @param \board3\portal\includes\helper $portal_helper Portal helper
+	 * @param \board3\portal\portal\modules\constraints_handler $constraints_handler Modules constraints handler
 	 * @param \board3\portal\portal\modules\database_handler $database_handler Modules database handler
 	 * @param \phpbb\request\request_interface $request phpBB request
 	 * @param \phpbb\user $user phpBB user
 	 */
-	public function __construct($cache, driver_interface $db, columns $portal_columns, helper $portal_helper, database_handler $database_handler, request_interface $request, $user)
+	public function __construct($cache, driver_interface $db, columns $portal_columns, helper $portal_helper, constraints_handler $constraints_handler, database_handler $database_handler, request_interface $request, $user)
 	{
 		$this->cache = $cache;
 		$this->db = $db;
 		$this->portal_columns = $portal_columns;
 		$this->portal_helper = $portal_helper;
+		$this->constraints_handler = $constraints_handler;
 		$this->database_handler = $database_handler;
 		$this->request = $request;
 		$this->user = $user;
@@ -82,6 +84,7 @@ class manager
 	public function set_u_action($u_action)
 	{
 		$this->u_action = $u_action;
+		$this->constraints_handler->set_u_action($u_action);
 
 		return $this;
 	}
@@ -279,7 +282,7 @@ class manager
 		$this->get_module($module_data['module_classname']);
 
 		$move_action = $this->get_horizontal_move_action($module_data, $direction);
-		$this->check_module_conflict($module_data, $move_action);
+		$this->constraints_handler->check_module_conflict($module_data, $move_action);
 
 		$this->database_handler->move_module_horizontal($module_id, $module_data, $move_action);
 
@@ -297,7 +300,7 @@ class manager
 	 */
 	public function get_horizontal_move_action($module_data, $direction)
 	{
-		if ($this->can_move_horizontally($module_data, $direction))
+		if ($this->constraints_handler->can_move_horizontally($module_data, $direction))
 		{
 			if ($this->module->get_allowed_columns() & $this->portal_columns->string_to_constant($this->portal_columns->number_to_string($module_data['module_column'] + $direction)))
 			{
@@ -312,61 +315,6 @@ class manager
 		$this->handle_after_move(false);
 	}
 
-	/**
-	 * Check if there is conflict between the move action and existing modules
-	 *
-	 * @param array $module_data The module's data
-	 * @param int $move_action The move action
-	 *
-	 * @return null
-	 */
-	protected function check_module_conflict($module_data, &$move_action)
-	{
-		/**
-		 * Moving only 1 column means we will either end up in a side column
-		 * or in the center column. This is not possible when moving 2 columns.
-		 * Therefore we only need to check if we won't end up with a duplicate
-		 * module in the new column (side columns (left & right) or center
-		 * columns (top, center, bottom)). Of course this does not apply to
-		 * custom modules.
-		 */
-		if ($module_data['module_classname'] != '\board3\portal\modules\custom' && abs($move_action) == 1)
-		{
-			$column_string = $this->portal_columns->number_to_string($module_data['module_column'] + $move_action);
-
-			// we can only move horizontally to center or side columns
-			if (in_array($column_string, array('right', 'left')) && !$this->can_move_module(array('right', 'left'), $module_data['module_classname']))
-			{
-				trigger_error($this->user->lang['UNABLE_TO_MOVE'] . adm_back_link($this->u_action));
-			}
-			else if ($column_string == 'center' && !$this->can_move_module(array('top', 'center', 'bottom'), $module_data['module_classname']))
-			{
-				// we are moving from the right to the center column so we should move to the left column instead
-				$move_action = 2 * $move_action;
-			}
-		}
-	}
-
-	/**
-	 * Check if module can be moved horizontally
-	 *
-	 * @param array $module_data Module's module data
-	 * @param int $direction Direction to move the module
-	 *
-	 * @return bool True if module can be moved, false if not
-	 */
-	protected function can_move_horizontally($module_data, $direction)
-	{
-		if (isset($module_data['module_column']))
-		{
-			return ($direction === database_handler::MOVE_DIRECTION_RIGHT) ? $module_data['module_column'] < $this->portal_columns->string_to_number('right') : $module_data['module_column'] > $this->portal_columns->string_to_number('left');
-		}
-		else
-		{
-			return false;
-		}
-	}
-
 	/**
 	 * Delete module
 	 *
@@ -442,53 +390,4 @@ class manager
 	{
 		return preg_replace(array('/i=[0-9]+/', '/mode=[a-zA-Z0-9_]+/'), array('i=%5C' . str_replace('\\', '%5C', $this->acp_class), 'mode=' . $mode), $this->u_action) . (($module_id) ? '&amp;module_id=' . $module_id : '');
 	}
-
-	/**
-	 * Check if module can be moved to desired column(s)
-	 *
-	 * @param array|int	$target_column Column(s) the module should be
-	 *				moved to
-	 * @param string		$module_class Class of the module
-	 * @return bool		True if module can be moved to desired column,
-	 *			false if not
-	 */
-	public function can_move_module($target_column, $module_class)
-	{
-		$submit = true;
-
-		if (is_array($target_column))
-		{
-			foreach ($target_column as $column)
-			{
-				if (!$this->can_move_module($column, $module_class))
-				{
-					$submit = false;
-				}
-			}
-		}
-
-		// do we want to add the module to the side columns or to the center columns?
-		if (in_array($target_column, array('left', 'right')))
-		{
-			// does the module already exist in the side columns?
-			if (isset($this->module_column[$module_class]) &&
-				(in_array('left', $this->module_column[$module_class]) || in_array('right', $this->module_column[$module_class])))
-			{
-				$submit = false;
-			}
-		}
-		else if (in_array($target_column, array('center', 'top', 'bottom')))
-		{
-			// does the module already exist in the center columns?
-			if (isset($this->module_column[$module_class]) &&
-				(in_array('center', $this->module_column[$module_class]) ||
-					in_array('top', $this->module_column[$module_class]) ||
-					in_array('bottom', $this->module_column[$module_class])))
-			{
-				$submit = false;
-			}
-		}
-
-		return $submit;
-	}
 }

tests/unit/acp/move_module_test.php

@@ -25,6 +25,9 @@ class phpbb_acp_move_module_test extends \board3\portal\tests\testframework\data
 	/** @var \board3\portal\portal\columns */
 	protected $portal_columns;
 
+	/** @var \board3\portal\portal\modules\constraints_handler */
+	protected $constraints_handler;
+
 	public function setUp()
 	{
 		parent::setUp();
@@ -73,19 +76,21 @@ class phpbb_acp_move_module_test extends \board3\portal\tests\testframework\data
 			'UNABLE_TO_MOVE_ROW'	=> 'UNABLE_TO_MOVE_ROW',
 		));
 		$this->database_handler = new \board3\portal\portal\modules\database_handler($db);
-		$this->modules_manager = new \board3\portal\portal\modules\manager($cache, $db, $this->portal_columns, $portal_helper, $this->database_handler, $request, $user);
+		$this->constraints_handler = new \board3\portal\portal\modules\constraints_handler($this->portal_columns, $user);
+		$this->modules_manager = new \board3\portal\portal\modules\manager($cache, $db, $this->portal_columns, $portal_helper, $this->constraints_handler, $this->database_handler, $request, $user);
 		$phpbb_container->set('board3.portal.modules.manager', $this->modules_manager);
+		$phpbb_container->set('board3.portal.modules.constraints_handler', $this->constraints_handler);
 		$this->portal_module = new \board3\portal\acp\portal_module();
 		$this->update_portal_modules();
 	}
 
 	protected function update_portal_modules()
 	{
-		$this->modules_manager->module_column = array();
+		$this->constraints_handler->module_column = array();
 		$portal_modules = obtain_portal_modules();
 		foreach($portal_modules as $cur_module)
 		{
-			$this->modules_manager->module_column[$cur_module['module_classname']][] = $this->portal_columns->number_to_string($cur_module['module_column']);
+			$this->constraints_handler->module_column[$cur_module['module_classname']][] = $this->portal_columns->number_to_string($cur_module['module_column']);
 		}
 	}
 
@@ -211,7 +216,7 @@ class phpbb_acp_move_module_test extends \board3\portal\tests\testframework\data
 		if ($add_to_column)
 		{
 			$module_data = $this->modules_manager->get_move_module_data($module_id);
-			$this->modules_manager->module_column[$module_data['module_classname']][] = $this->portal_columns->number_to_string($add_to_column);
+			$this->constraints_handler->module_column[$module_data['module_classname']][] = $this->portal_columns->number_to_string($add_to_column);
 		}
 
 		for ($i = 1; $i <= $move_right; $i++)
@@ -262,7 +267,7 @@ class phpbb_acp_move_module_test extends \board3\portal\tests\testframework\data
 		if ($add_to_column)
 		{
 			$module_data = $this->modules_manager->get_move_module_data($module_id);
-			$this->modules_manager->module_column[$module_data['module_classname']][] = $this->portal_columns->number_to_string($add_to_column);
+			$this->constraints_handler->module_column[$module_data['module_classname']][] = $this->portal_columns->number_to_string($add_to_column);
 		}
 
 		// We always start in the right column = 3
@@ -297,7 +302,7 @@ class phpbb_acp_move_module_test extends \board3\portal\tests\testframework\data
 	public function test_can_move_module($expected, $target_column, $module_class)
 	{
 		$this->update_portal_modules();
-		$this->assertEquals($expected, $this->modules_manager->can_move_module($target_column, $module_class));
+		$this->assertEquals($expected, $this->constraints_handler->can_move_module($target_column, $module_class));
 	}
 }
 

tests/unit/portal/modules_manager_confirm_box_test.php

@@ -23,6 +23,9 @@ class modules_manager_confirm_box_test extends \board3\portal\tests\testframewor
 	/** @var \board3\portal\portal\modules\manager */
 	protected $modules_manager;
 
+	/** @var \board3\portal\portal\modules\constraints_handler */
+	protected $constraints_handler;
+
 	public function getDataSet()
 	{
 		return $this->createXMLDataSet(dirname(__FILE__) . '/../acp/fixtures/modules.xml');
@@ -74,7 +77,8 @@ class modules_manager_confirm_box_test extends \board3\portal\tests\testframewor
 		));
 
 		$this->database_handler = new \board3\portal\portal\modules\database_handler($db);
-		$this->modules_manager = new \board3\portal\portal\modules\manager($this->cache, $db, $this->portal_columns, $this->portal_helper, $this->database_handler, $request, $user);
+		$this->constraints_handler = new \board3\portal\portal\modules\constraints_handler($this->portal_columns, $user);
+		$this->modules_manager = new \board3\portal\portal\modules\manager($this->cache, $db, $this->portal_columns, $this->portal_helper, $this->constraints_handler, $this->database_handler, $request, $user);
 		$portal_config = array();
 	}
 
@@ -122,7 +126,7 @@ class modules_manager_confirm_box_test extends \board3\portal\tests\testframewor
 		$this->cache->expects($this->any())
 			->method('purge');
 		$this->request->overwrite('module_classname', '\board3\portal\modules\donation');
-		$this->modules_manager = new \board3\portal\portal\modules\manager($this->cache, $this->db, $this->portal_columns, $this->portal_helper, $this->database_handler, $this->request, $this->user);
+		$this->modules_manager = new \board3\portal\portal\modules\manager($this->cache, $this->db, $this->portal_columns, $this->portal_helper, $this->constraints_handler, $this->database_handler, $this->request, $this->user);
 		$this->modules_manager->set_u_action('adm/index.php?i=15&mode=foobar')->set_acp_class('foo\bar');
 
 		// Trigger confirm box creation

tests/unit/portal/modules_manager_test.php

@@ -7,6 +7,8 @@
  *
  */
 
+namespace board3\portal\portal\modules;
+
 class board3_portal_modules_manager_test extends \board3\portal\tests\testframework\database_test_case
 {
 	protected $portal_columns;
@@ -15,6 +17,9 @@ class board3_portal_modules_manager_test extends \board3\portal\tests\testframew
 	/** @var \board3\portal\portal\modules\manager */
 	protected $modules_manager;
 
+	/** @var \board3\portal\portal\modules\constraints_handler */
+	protected $constraints_handler;
+
 	public function getDataSet()
 	{
 		return $this->createXMLDataSet(dirname(__FILE__) . '/../acp/fixtures/modules.xml');
@@ -22,6 +27,8 @@ class board3_portal_modules_manager_test extends \board3\portal\tests\testframew
 
 	public function setUp()
 	{
+		global $cache, $db;
+
 		parent::setUp();
 
 		$user = new \board3\portal\tests\mock\user();
@@ -57,7 +64,13 @@ class board3_portal_modules_manager_test extends \board3\portal\tests\testframew
 			'UNABLE_TO_MOVE_ROW'	=> 'UNABLE_TO_MOVE_ROW',
 		));
 		$this->database_handler = new \board3\portal\portal\modules\database_handler($db);
-		$this->modules_manager = new \board3\portal\portal\modules\manager($cache, $db, $this->portal_columns, $portal_helper, $this->database_handler, $request, $user);
+		$this->constraints_handler = new \board3\portal\portal\modules\constraints_handler($this->portal_columns, $user);
+		$this->modules_manager = new \board3\portal\portal\modules\manager($cache, $db, $this->portal_columns, $portal_helper, $this->constraints_handler, $this->database_handler, $request, $user);
+		$portal_modules = obtain_portal_modules();
+		foreach($portal_modules as $cur_module)
+		{
+			$this->constraints_handler->module_column[$cur_module['module_classname']][] = $this->portal_columns->number_to_string($cur_module['module_column']);
+		}
 	}
 
 	public function test_set_u_action()
@@ -86,4 +99,24 @@ class board3_portal_modules_manager_test extends \board3\portal\tests\testframew
 		$this->setExpectedTriggerError(E_USER_NOTICE, 'UNABLE_TO_MOVE');
 		$this->modules_manager->get_horizontal_move_action(array(), 6);
 	}
+
+	public function test_set_module_column()
+	{
+		$module_column = $this->constraints_handler->module_column;
+		$this->constraints_handler->set_module_column(array());
+		$this->assertEquals(array(), $this->constraints_handler->module_column);
+		$this->constraints_handler->set_module_column($module_column);
+		$this->assertEquals($module_column, $this->constraints_handler->module_column);
+	}
+
+	public function test_check_module_conflict()
+	{
+		phpbb_acp_move_module_test::$override_trigger_error = true;
+		phpbb_acp_move_module_test::$error = '';
+		phpbb_acp_move_module_test::$error_type = 0;
+		$move_action = 1;
+		$this->constraints_handler->check_module_conflict($this->modules_manager->get_move_module_data(2), $move_action);
+		$this->assertEquals('UNABLE_TO_MOVE', phpbb_acp_move_module_test::$error);
+		phpbb_acp_move_module_test::$override_trigger_error = false;
+	}
 }