From a2dd8671b67f5b4bd0f9b780d732e32afc8d4af2 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 30 Nov 2014 15:27:24 +0100 Subject: [PATCH] [ticket/416] Add handler for module constraints B3P-416 --- acp/portal_module.php | 11 +- config/services.yml | 11 +- portal/modules/constraints_handler.php | 162 ++++++++++++++++++ portal/modules/manager.php | 119 +------------ tests/unit/acp/move_module_test.php | 17 +- .../modules_manager_confirm_box_test.php | 8 +- tests/unit/portal/modules_manager_test.php | 35 +++- 7 files changed, 238 insertions(+), 125 deletions(-) create mode 100644 portal/modules/constraints_handler.php diff --git a/acp/portal_module.php b/acp/portal_module.php index abf3872c..685c13d3 100644 --- a/acp/portal_module.php +++ b/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; } diff --git a/config/services.yml b/config/services.yml index 9e3a7aab..f560c8d4 100644 --- a/config/services.yml +++ b/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 diff --git a/portal/modules/constraints_handler.php b/portal/modules/constraints_handler.php new file mode 100644 index 00000000..f65041cc --- /dev/null +++ b/portal/modules/constraints_handler.php @@ -0,0 +1,162 @@ +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; + } +} diff --git a/portal/modules/manager.php b/portal/modules/manager.php index e47cec95..b72a7848 100644 --- a/portal/modules/manager.php +++ b/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) ? '&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; - } } diff --git a/tests/unit/acp/move_module_test.php b/tests/unit/acp/move_module_test.php index 68e4d088..ed41b7f8 100644 --- a/tests/unit/acp/move_module_test.php +++ b/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)); } } diff --git a/tests/unit/portal/modules_manager_confirm_box_test.php b/tests/unit/portal/modules_manager_confirm_box_test.php index 6df1158e..aed718de 100644 --- a/tests/unit/portal/modules_manager_confirm_box_test.php +++ b/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 diff --git a/tests/unit/portal/modules_manager_test.php b/tests/unit/portal/modules_manager_test.php index 4297de65..08fd8f32 100644 --- a/tests/unit/portal/modules_manager_test.php +++ b/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; + } }