From 3a2782b578ac167eab453c643b3a2b107e1c73d6 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 29 Nov 2014 14:33:05 +0100 Subject: [PATCH] [ticket/416] Add one method for moving modules horizontally B3P-416 --- acp/portal_module.php | 4 +- portal/modules/database_handler.php | 6 + portal/modules/manager.php | 222 +++++++----------- tests/unit/acp/move_module_test.php | 14 +- .../modules_manager_confirm_box_test.php | 2 + tests/unit/portal/modules_manager_test.php | 6 + 6 files changed, 109 insertions(+), 145 deletions(-) diff --git a/acp/portal_module.php b/acp/portal_module.php index 569e8ef2..abf3872c 100644 --- a/acp/portal_module.php +++ b/acp/portal_module.php @@ -411,11 +411,11 @@ class portal_module } else if($action == 'move_right') { - $this->modules_manager->move_module_right($module_id); + $this->modules_manager->move_module_horizontal($module_id, \board3\portal\portal\modules\database_handler::MOVE_DIRECTION_RIGHT); } else if($action == 'move_left') { - $this->modules_manager->move_module_left($module_id); + $this->modules_manager->move_module_horizontal($module_id, \board3\portal\portal\modules\database_handler::MOVE_DIRECTION_LEFT); } else if ($action == 'delete') { diff --git a/portal/modules/database_handler.php b/portal/modules/database_handler.php index b058fbe4..9483f82f 100644 --- a/portal/modules/database_handler.php +++ b/portal/modules/database_handler.php @@ -19,6 +19,12 @@ class database_handler /** @var int Move driection down */ const MOVE_DIRECTION_DOWN = 1; + /** @var int Move direction right */ + const MOVE_DIRECTION_RIGHT = 1; + + /** @var int Move direction left */ + const MOVE_DIRECTION_LEFT = -1; + /** @var \phpbb\db\driver\driver_interface */ protected $db; diff --git a/portal/modules/manager.php b/portal/modules/manager.php index 9c805151..d143183f 100644 --- a/portal/modules/manager.php +++ b/portal/modules/manager.php @@ -259,188 +259,138 @@ class manager } /** - * Move module left one column + * Move module horizontally * * @param int $module_id ID of the module that should be moved + * @param int $direction The direction to move the module + * + * @return null */ - public function move_module_left($module_id) + public function move_module_horizontal($module_id, $direction) { $module_data = $this->get_move_module_data($module_id); $this->get_module($module_data['module_classname']); - $move_action = 0; + $move_action = $this->get_horizontal_move_action($module_data, $direction); + $this->check_module_conflict($module_data, $move_action); - if ($module_data !== false && $module_data['module_column'] > $this->portal_columns->string_to_number('left')) - { - if($this->module->columns & $this->portal_columns->string_to_constant($this->portal_columns->number_to_string($module_data['module_column'] - 1))) - { - $move_action = 1; // we move 1 column to the left - } - else if($this->module->columns & $this->portal_columns->string_to_constant($this->portal_columns->number_to_string($module_data['module_column'] - 2)) && $module_data['module_column'] != 2) - { - $move_action = 2; // we move 2 columns to the left - } - else - { - $this->handle_after_move(false); - } - - /** - * moving only 1 column to the left means we will either end up in the left column - * or in the center column. this is not possible when moving 2 columns to the left. - * 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' && $move_action == 1) - { - $column_string = $this->portal_columns->number_to_string($module_data['module_column'] - $move_action); - // we can only move left to the left & center column - if ($column_string == '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; - } - } - - $sql = 'UPDATE ' . PORTAL_MODULES_TABLE . ' + $sql = 'UPDATE ' . PORTAL_MODULES_TABLE . ' SET module_order = module_order + 1 WHERE module_order >= ' . $module_data['module_order'] . ' - AND module_column = ' . ($module_data['module_column'] - $move_action); - $this->db->sql_query($sql); - $updated = $this->db->sql_affectedrows(); + AND module_column = ' . ($module_data['module_column'] + $move_action); + $this->db->sql_query($sql); + $updated = $this->db->sql_affectedrows(); - $sql = 'UPDATE ' . PORTAL_MODULES_TABLE . ' - SET module_column = module_column - ' . $move_action . ' + $sql = 'UPDATE ' . PORTAL_MODULES_TABLE . ' + SET module_column = ' . ($module_data['module_column'] + $move_action) . ' WHERE module_id = ' . (int) $module_id; - $this->db->sql_query($sql); + $this->db->sql_query($sql); - $sql = 'UPDATE ' . PORTAL_MODULES_TABLE . ' + $sql = 'UPDATE ' . PORTAL_MODULES_TABLE . ' SET module_order = module_order - 1 WHERE module_order >= ' . $module_data['module_order'] . ' AND module_column = ' . $module_data['module_column']; - $this->db->sql_query($sql); + $this->db->sql_query($sql); - // the module that needs to moved is in the last row - if(!$updated) - { - $sql = 'SELECT MAX(module_order) as new_order + // the module that needs to moved is in the last row + if (!$updated) + { + $sql = 'SELECT MAX(module_order) as new_order FROM ' . PORTAL_MODULES_TABLE . ' WHERE module_order < ' . $module_data['module_order'] . ' - AND module_column = ' . (int) ($module_data['module_column'] - $move_action); - $this->db->sql_query($sql); - $new_order = $this->db->sql_fetchfield('new_order') + 1; + AND module_column = ' . (int) ($module_data['module_column'] + $move_action); + $this->db->sql_query($sql); + $new_order = $this->db->sql_fetchfield('new_order') + 1; - $sql = 'UPDATE ' . PORTAL_MODULES_TABLE . ' + $sql = 'UPDATE ' . PORTAL_MODULES_TABLE . ' SET module_order = ' . $new_order . ' WHERE module_id = ' . (int) $module_id; - $this->db->sql_query($sql); - } - } - else - { - $this->handle_after_move(false); + $this->db->sql_query($sql); } $this->handle_after_move(true); } /** - * Move module right one column + * Get the horizontal move action (columns to move) * - * @param int $module_id ID of the module that should be moved + * @param array $module_data Array containing the module data + * @param int $direction Direction to move; 1 for right, -1 for left + * + * @return int|null Move action if module can be moved, calls + * handle_after_move() if it can't be moved */ - public function move_module_right($module_id) + public function get_horizontal_move_action($module_data, $direction) { - $module_data = $this->get_move_module_data($module_id); - - $this->get_module($module_data['module_classname']); - - $move_action = 0; - - if ($module_data !== false && $module_data['module_column'] < $this->portal_columns->string_to_number('right')) + if ($this->can_move_horizontally($module_data, $direction)) { - if($this->module->columns & $this->portal_columns->string_to_constant($this->portal_columns->number_to_string($module_data['module_column'] + 1))) + if ($this->module->get_allowed_columns() & $this->portal_columns->string_to_constant($this->portal_columns->number_to_string($module_data['module_column'] + $direction))) { - $move_action = 1; // we move 1 column to the right + return $direction; // we move 1 column } - else if($this->module->columns & $this->portal_columns->string_to_constant($this->portal_columns->number_to_string($module_data['module_column'] + 2)) && $module_data['module_column'] != 2) + else if ($this->module->get_allowed_columns() & $this->portal_columns->string_to_constant($this->portal_columns->number_to_string($module_data['module_column'] + $direction * 2)) && $module_data['module_column'] != $this->portal_columns->string_to_number('center')) { - $move_action = 2; // we move 2 columns to the right + return 2 * $direction; // we move 2 columns } - else + } + + $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'])) { - $this->handle_after_move(false); + trigger_error($this->user->lang['UNABLE_TO_MOVE'] . adm_back_link($this->u_action)); } - - /** - * moving only 1 column to the right means we will either end up in the right column - * or in the center column. this is not possible when moving 2 columns to the right. - * 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' && $move_action == 1) + else if ($column_string == 'center' && !$this->can_move_module(array('top', 'center', 'bottom'), $module_data['module_classname'])) { - $column_string = $this->portal_columns->number_to_string($module_data['module_column'] + $move_action); - - // we can only move right to the right & center column - if ($column_string == 'right' && !$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 left to the center column so we should move to the right column instead - $move_action = 2; - } + // we are moving from the right to the center column so we should move to the left column instead + $move_action = 2 * $move_action; } + } + } - $sql = 'UPDATE ' . PORTAL_MODULES_TABLE . ' - SET module_order = module_order + 1 - WHERE module_order >= ' . (int) $module_data['module_order'] . ' - AND module_column = ' . (int) ($module_data['module_column'] + $move_action); - $this->db->sql_query($sql); - $updated = $this->db->sql_affectedrows(); - - $sql = 'UPDATE ' . PORTAL_MODULES_TABLE . ' - SET module_column = module_column + ' . $move_action . ' - WHERE module_id = ' . (int) $module_id; - $this->db->sql_query($sql); - - $sql = 'UPDATE ' . PORTAL_MODULES_TABLE . ' - SET module_order = module_order - 1 - WHERE module_order >= ' . (int) $module_data['module_order'] . ' - AND module_column = ' . (int) $module_data['module_column']; - $this->db->sql_query($sql); - - // the module that needs to moved is in the last row - if(!$updated) - { - $sql = 'SELECT MAX(module_order) as new_order - FROM ' . PORTAL_MODULES_TABLE . ' - WHERE module_order < ' . (int) $module_data['module_order'] . ' - AND module_column = ' . (int) ($module_data['module_column'] + $move_action); - $this->db->sql_query($sql); - $new_order = $this->db->sql_fetchfield('new_order') + 1; - - $sql = 'UPDATE ' . PORTAL_MODULES_TABLE . ' - SET module_order = ' . (int) $new_order . ' - WHERE module_id = ' . (int) $module_id; - $this->db->sql_query($sql); - } + /** + * 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 { - $this->handle_after_move(false); + return false; } - - $this->handle_after_move(true); } /** diff --git a/tests/unit/acp/move_module_test.php b/tests/unit/acp/move_module_test.php index cde8ceb7..68e4d088 100644 --- a/tests/unit/acp/move_module_test.php +++ b/tests/unit/acp/move_module_test.php @@ -204,7 +204,7 @@ class phpbb_acp_move_module_test extends \board3\portal\tests\testframework\data if ($column_start > 3) { $this->setExpectedTriggerError(E_USER_ERROR, 'CLASS_NOT_FOUND'); - $this->modules_manager->move_module_right($module_id); + $this->modules_manager->move_module_horizontal($module_id, \board3\portal\portal\modules\database_handler::MOVE_DIRECTION_RIGHT); return; } @@ -218,12 +218,12 @@ class phpbb_acp_move_module_test extends \board3\portal\tests\testframework\data { $data = $this->modules_manager->get_move_module_data($module_id); $this->assertEquals($column_start, $data['module_column']); - $this->modules_manager->move_module_right($module_id); + $this->modules_manager->move_module_horizontal($module_id, \board3\portal\portal\modules\database_handler::MOVE_DIRECTION_RIGHT); $column_start++; $this->update_portal_modules(); } $this->setExpectedTriggerError(E_USER_NOTICE, 'UNABLE_TO_MOVE'); - $this->modules_manager->move_module_right($module_id); + $this->modules_manager->move_module_horizontal($module_id, \board3\portal\portal\modules\database_handler::MOVE_DIRECTION_RIGHT); } public function data_move_module_left() @@ -246,7 +246,7 @@ class phpbb_acp_move_module_test extends \board3\portal\tests\testframework\data if ($column_start > 3) { $this->setExpectedTriggerError(E_USER_ERROR, 'CLASS_NOT_FOUND'); - $this->modules_manager->move_module_left($module_id); + $this->modules_manager->move_module_horizontal($module_id, \board3\portal\portal\modules\database_handler::MOVE_DIRECTION_LEFT); return; } @@ -254,7 +254,7 @@ class phpbb_acp_move_module_test extends \board3\portal\tests\testframework\data { $data = $this->modules_manager->get_move_module_data($module_id); $this->assertEquals($column_start, $data['module_column']); - $this->modules_manager->move_module_right($module_id); + $this->modules_manager->move_module_horizontal($module_id, \board3\portal\portal\modules\database_handler::MOVE_DIRECTION_RIGHT); $this->update_portal_modules(); $column_start++; } @@ -271,12 +271,12 @@ class phpbb_acp_move_module_test extends \board3\portal\tests\testframework\data { $data = $this->modules_manager->get_move_module_data($module_id); $this->assertEquals($column_start, $data['module_column']); - $this->modules_manager->move_module_left($module_id); + $this->modules_manager->move_module_horizontal($module_id, \board3\portal\portal\modules\database_handler::MOVE_DIRECTION_LEFT); $this->update_portal_modules(); $column_start--; } $this->setExpectedTriggerError(E_USER_NOTICE, 'UNABLE_TO_MOVE'); - $this->modules_manager->move_module_left($module_id); + $this->modules_manager->move_module_horizontal($module_id, \board3\portal\portal\modules\database_handler::MOVE_DIRECTION_LEFT); } public function data_can_move_module() diff --git a/tests/unit/portal/modules_manager_confirm_box_test.php b/tests/unit/portal/modules_manager_confirm_box_test.php index d4edd40b..6df1158e 100644 --- a/tests/unit/portal/modules_manager_confirm_box_test.php +++ b/tests/unit/portal/modules_manager_confirm_box_test.php @@ -100,6 +100,7 @@ class modules_manager_confirm_box_test extends \board3\portal\tests\testframewor ), self::$meta_refresh); $this->assertEquals(phpbb_acp_move_module_test::$error_type, E_USER_NOTICE); $this->assertEquals(phpbb_acp_move_module_test::$error, 'adm/index.php?i=15&mode=foobar&module_id=6'); + phpbb_acp_move_module_test::$override_trigger_error = false; } public function test_module_delete() @@ -140,6 +141,7 @@ class modules_manager_confirm_box_test extends \board3\portal\tests\testframewor $this->assertNull($this->modules_manager->module_delete(6, 'foobar', 'module_delete', 6)); $this->assertEquals(E_USER_NOTICE, phpbb_acp_move_module_test::$error_type); $this->assertEquals('SUCCESS_DELETEadm/index.php?i=15&mode=foobar', phpbb_acp_move_module_test::$error); + phpbb_acp_move_module_test::$override_trigger_error = false; } } diff --git a/tests/unit/portal/modules_manager_test.php b/tests/unit/portal/modules_manager_test.php index 808b6406..4297de65 100644 --- a/tests/unit/portal/modules_manager_test.php +++ b/tests/unit/portal/modules_manager_test.php @@ -80,4 +80,10 @@ class board3_portal_modules_manager_test extends \board3\portal\tests\testframew { $this->assertNull($this->modules_manager->handle_ajax_request(array('foobar' => true))); } + + public function test_get_horizontal_move_action() + { + $this->setExpectedTriggerError(E_USER_NOTICE, 'UNABLE_TO_MOVE'); + $this->modules_manager->get_horizontal_move_action(array(), 6); + } }