diff --git a/acp/portal_module.php b/acp/portal_module.php index f4577839..1b9fcc10 100644 --- a/acp/portal_module.php +++ b/acp/portal_module.php @@ -24,6 +24,7 @@ class portal_module protected $c_class; protected $db, $user, $cache, $template, $display_vars, $config, $phpbb_root_path, $phpbb_admin_path, $phpEx, $phpbb_container; protected $root_path, $mod_version_check, $request; + public $module_column = array(); /** @var \phpbb\di\service_collection Portal modules */ protected $modules; @@ -145,7 +146,7 @@ class portal_module 'MODULE_SHOW_IMAGE' => (in_array(column_num_string($module_data['module_column']), array('center', 'top', 'bottom'))) ? false : true, )); - if($module_data['module_classname'] != 'custom') + if($module_data['module_classname'] != '\board3\portal\modules\custom') { $groups_ary = explode(',', $module_data['module_group_ids']); @@ -282,7 +283,7 @@ class portal_module if(isset($module_name)) { - if ($module_data['module_classname'] !== 'custom') + if ($module_data['module_classname'] !== '\board3\portal\modules\custom') { add_log('admin', 'LOG_PORTAL_CONFIG', $module_name); } @@ -382,13 +383,13 @@ class portal_module // Create an array of already installed modules $portal_modules = obtain_portal_modules(); - $installed_modules = $module_column = array(); + $installed_modules = array(); foreach($portal_modules as $cur_module) { $installed_modules[] = $cur_module['module_classname']; // Create an array with the columns the module is in - $module_column[$cur_module['module_classname']][] = column_num_string($cur_module['module_column']); + $this->module_column[$cur_module['module_classname']][] = column_num_string($cur_module['module_column']); } if ($action == 'move_up') @@ -426,23 +427,11 @@ class portal_module // do we want to add the module to the side columns or to the center columns? if (in_array($column_string, array('left', 'right'))) { - // does the module already exist in the side columns? - if (isset($module_column[$module_classname]) && - (in_array('left', $module_column[$module_classname]) || in_array('right', $module_column[$module_classname]))) - { - $submit = false; - } + $submit = $this->can_move_module(array('left', 'right'), $module_classname); } elseif (in_array($column_string, array('center', 'top', 'bottom'))) { - // does the module already exist in the center columns? - if (isset($module_column[$module_classname]) && - (in_array('center', $module_column[$module_classname]) || - in_array('top', $module_column[$module_classname]) || - in_array('bottom', $module_column[$module_classname]))) - { - $submit = false; - } + $submit = $this->can_move_module(array('center', 'top', 'bottom'), $module_classname); } // do not install if module already exists in that column @@ -525,8 +514,7 @@ class portal_module if (in_array($column_string, array('left', 'right'))) { // does the module already exist in the side columns? - if (isset($module_column[$module_class]) && - (in_array('left', $module_column[$module_class]) || in_array('right', $module_column[$module_class]))) + if (!$this->can_move_module(array('left', 'right'), $module_class)) { continue; } @@ -534,10 +522,7 @@ class portal_module elseif (in_array($column_string, array('center', 'top', 'bottom'))) { // does the module already exist in the center columns? - if (isset($module_column[$module_class]) && - (in_array('center', $module_column[$module_class]) || - in_array('top', $module_column[$module_class]) || - in_array('bottom', $module_column[$module_class]))) + if (!$this->can_move_module(array('center', 'top', 'bottom'), $module_class)) { continue; } @@ -628,14 +613,11 @@ class portal_module * this only applies to modules in the center column as the side modules * will automatically skip the center column when moving if they need to */ - if ($row['module_classname'] != 'custom') + if ($row['module_classname'] != '\board3\portal\modules\custom') { $column_string = column_num_string($row['module_column'] + 1); // move 1 right - if ($column_string == 'right' && - isset($module_column[$row['module_classname']]) && - (in_array('left', $module_column[$row['module_classname']]) || - in_array('right', $module_column[$row['module_classname']]))) + if ($column_string == 'right' && !$this->can_move_module(array('left', 'right'), $row['module_classname'])) { $move_right = false; } @@ -661,14 +643,11 @@ class portal_module * this only applies to modules in the center column as the side modules * will automatically skip the center column when moving if they need to */ - if ($row['module_classname'] != 'custom') + if ($row['module_classname'] != '\board3\portal\modules\custom') { $column_string = column_num_string($row['module_column'] - 1); // move 1 left - if ($column_string == 'left' && - isset($module_column[$row['module_classname']]) && - (in_array('left', $module_column[$row['module_classname']]) || - in_array('right', $module_column[$row['module_classname']]))) + if ($column_string == 'left' && !$this->can_move_module(array('left', 'right'), $row['module_classname'])) { $move_left = false; } @@ -829,6 +808,65 @@ class portal_module } } + /** + * Get module_data required for moving it + * + * @param int $module_id ID of the module that should be moved + * @return array|null Module_data or empty if not successful + */ + public function get_move_module_data($module_id) + { + $sql = 'SELECT module_order, module_column, module_classname + FROM ' . PORTAL_MODULES_TABLE . ' + WHERE module_id = ' . (int) $module_id; + $result = $this->db->sql_query_limit($sql, 1); + $module_data = $this->db->sql_fetchrow($result); + $this->db->sql_freeresult($result); + + return $module_data; + } + + /** + * Handle output after moving module + * + * @param bool $success Whether moving module was successful + * @param bool $is_row Whether the module move was inside a row + * @return void + */ + public function handle_after_move($success = true, $is_row = false) + { + if (!$success) + { + trigger_error($this->user->lang['UNABLE_TO_MOVE' . (($is_row) ? '_ROW' : '')] . adm_back_link($this->u_action)); + } + + $this->cache->destroy('portal_modules'); + redirect($this->u_action); // redirect in order to get rid of excessive URL parameters + } + + /** + * Get the module order to the last module in the column + * + * @param int $module_column Module column to check + * @return int Module order of the last module in the column + */ + public function get_last_module_order($module_column) + { + $modules = obtain_portal_modules(); + $last_order = 1; + foreach ($modules as $cur_module) + { + if ($cur_module['module_column'] != $module_column) + { + continue; + } + + $last_order = max($last_order, $cur_module['module_order']); + } + + return $last_order; + } + /** * Move module up one row * @@ -836,12 +874,7 @@ class portal_module */ public function move_module_up($module_id) { - $sql = 'SELECT module_order, module_column - FROM ' . PORTAL_MODULES_TABLE . ' - WHERE module_id = ' . (int) $module_id; - $result = $this->db->sql_query_limit($sql, 1); - $module_data = $this->db->sql_fetchrow($result); - $this->db->sql_freeresult($result); + $module_data = $this->get_move_module_data($module_id); if (($module_data !== false) && ($module_data['module_order'] > 1)) { @@ -860,13 +893,8 @@ class portal_module $this->db->sql_query($sql); } } - else - { - trigger_error($this->user->lang['UNABLE_TO_MOVE_ROW'] . adm_back_link($this->u_action)); - } - - $this->cache->destroy('portal_modules'); - redirect($this->u_action); // redirect in order to get rid of excessive URL parameters + + $this->handle_after_move($updated, true); } /** @@ -874,16 +902,11 @@ class portal_module * * @param int $module_id ID of the module that should be moved */ - protected function move_module_down($module_id) + public function move_module_down($module_id) { - $sql = 'SELECT module_order, module_column - FROM ' . PORTAL_MODULES_TABLE . ' - WHERE module_id = ' . (int) $module_id; - $result = $this->db->sql_query_limit($sql, 1); - $module_data = $this->db->sql_fetchrow($result); - $this->db->sql_freeresult($result); + $module_data = $this->get_move_module_data($module_id); - if ($module_data !== false) + if ($module_data !== false && $this->get_last_module_order($module_data['module_column']) != $module_data['module_order']) { $sql = 'UPDATE ' . PORTAL_MODULES_TABLE . ' SET module_order = module_order - 1 @@ -899,13 +922,8 @@ class portal_module $this->db->sql_query($sql); } } - else - { - trigger_error($this->user->lang['UNABLE_TO_MOVE_ROW'] . adm_back_link($this->u_action)); - } - - $this->cache->destroy('portal_modules'); - redirect($this->u_action); // redirect in order to get rid of excessive URL parameters + + $this->handle_after_move($updated, true); } /** @@ -913,14 +931,9 @@ class portal_module * * @param int $module_id ID of the module that should be moved */ - protected function move_module_left($module_id) + public function move_module_left($module_id) { - $sql = 'SELECT module_order, module_column, module_classname - FROM ' . PORTAL_MODULES_TABLE . ' - WHERE module_id = ' . (int) $module_id; - $result = $this->db->sql_query_limit($sql, 1); - $module_data = $this->db->sql_fetchrow($result); - $this->db->sql_freeresult($result); + $module_data = $this->get_move_module_data($module_id); if (!isset($this->modules[$module_data['module_classname']])) { @@ -929,7 +942,7 @@ class portal_module $this->c_class = $this->modules[$module_data['module_classname']]; - if ($module_data !== false) + if ($module_data !== false && $module_data['module_column'] > column_string_num('left')) { if($this->c_class->columns & column_string_const(column_num_string($module_data['module_column'] - 1))) { @@ -941,7 +954,7 @@ class portal_module } else { - // @todo: need an error handle here (i.e. trigger_error()) + $this->handle_after_move(false); } /** @@ -951,22 +964,15 @@ class portal_module * 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'] != 'custom' && $move_action == 1) + if ($module_data['module_classname'] != '\board3\portal\modules\custom' && $move_action == 1) { $column_string = column_num_string($module_data['module_column'] - $move_action); // we can only move left to the left & center column - if ($column_string == 'left' && - isset($module_column[$module_data['module_classname']]) && - (in_array('left', $module_column[$module_data['module_classname']]) || - in_array('right', $module_column[$module_data['module_classname']]))) + 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)); } - elseif ($column_string == 'center' && - isset($module_column[$module_data['module_classname']]) && - (in_array('center', $module_column[$module_data['module_classname']]) || - in_array('top', $module_column[$module_data['module_classname']]) || - in_array('bottom', $module_column[$module_data['module_classname']]))) + elseif ($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; @@ -1009,11 +1015,10 @@ class portal_module } else { - trigger_error($this->user->lang['UNABLE_TO_MOVE'] . adm_back_link($this->u_action)); + $this->handle_after_move(false); } - $this->cache->destroy('portal_modules'); - redirect($this->u_action); // redirect in order to get rid of excessive URL parameters + $this->handle_after_move(true); } /** @@ -1021,14 +1026,9 @@ class portal_module * * @param int $module_id ID of the module that should be moved */ - protected function move_module_right($module_id) + public function move_module_right($module_id) { - $sql = 'SELECT module_order, module_column, module_classname - FROM ' . PORTAL_MODULES_TABLE . ' - WHERE module_id = ' . (int) $module_id; - $result = $this->db->sql_query_limit($sql, 1); - $module_data = $this->db->sql_fetchrow($result); - $this->db->sql_freeresult($result); + $module_data = $this->get_move_module_data($module_id); if (!isset($this->modules[$module_data['module_classname']])) { @@ -1037,7 +1037,7 @@ class portal_module $this->c_class = $this->modules[$module_data['module_classname']]; - if ($module_data !== false) + if ($module_data !== false && $module_data['module_column'] < column_string_num('right')) { if($this->c_class->columns & column_string_const(column_num_string($module_data['module_column'] + 1))) { @@ -1049,7 +1049,7 @@ class portal_module } else { - // @todo: need an error handle here + $this->handle_after_move(false); } /** @@ -1059,22 +1059,15 @@ class portal_module * 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'] != 'custom' && $move_action == 1) + if ($module_data['module_classname'] != '\board3\portal\modules\custom' && $move_action == 1) { $column_string = column_num_string($module_data['module_column'] + $move_action); // we can only move right to the right & center column - if ($column_string == 'right' && - isset($module_column[$module_data['module_classname']]) && - (in_array('left', $module_column[$module_data['module_classname']]) || - in_array('right', $module_column[$module_data['module_classname']]))) + 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)); } - elseif ($column_string == 'center' && - isset($module_column[$module_data['module_classname']]) && - (in_array('center', $module_column[$module_data['module_classname']]) || - in_array('top', $module_column[$module_data['module_classname']]) || - in_array('bottom', $module_column[$module_data['module_classname']]))) + elseif ($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; @@ -1117,11 +1110,10 @@ class portal_module } else { - trigger_error($this->user->lang['UNABLE_TO_MOVE'] . adm_back_link($this->u_action)); + $this->handle_after_move(false); } - - $this->cache->destroy('portal_modules'); - redirect($this->u_action); // redirect in order to get rid of excessive URL parameters + + $this->handle_after_move(true); } /** @@ -1220,4 +1212,53 @@ class portal_module } } } + + /** + * 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; + } + } + elseif (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/acp/fixtures/modules.xml b/tests/acp/fixtures/modules.xml index c2f193c4..a1358060 100644 --- a/tests/acp/fixtures/modules.xml +++ b/tests/acp/fixtures/modules.xml @@ -8,7 +8,7 @@ 1 \board3\portal\modules\clock - 2 + 1 1 @@ -17,17 +17,35 @@ 2 2 + + 6 + \board3\portal\modules\donation + 1 + 3 + 3 \board3\portal\modules\clock - 4 + 3 1 4 \board3\portal\modules\birthday_list - 4 + 3 2 + + 5 + \board3\portal\modules\welcome + 2 + 1 + + + 7 + \board3\portal\modules\custom + 4 + 1 + diff --git a/tests/acp/move_module_test.php b/tests/acp/move_module_test.php index 892975a9..dd123fd1 100644 --- a/tests/acp/move_module_test.php +++ b/tests/acp/move_module_test.php @@ -28,20 +28,45 @@ class phpbb_acp_move_module_test extends \board3\portal\tests\testframework\data ->disableOriginalConstructor() ->getMock()); // Mock module service collection + $config = new \phpbb\config\config(array()); $phpbb_container->set('board3.module_collection', array( new \board3\portal\modules\clock(), - new \board3\portal\modules\birthday_list(new \phpbb\config\config(array()), $template, $this->db, $user), + new \board3\portal\modules\birthday_list($config, $template, $this->db, $user), + new \board3\portal\modules\welcome($config, new \phpbb_mock_request, $this->db, $user, $phpbb_root_path, $phpEx), + new \board3\portal\modules\donation($config, $template, $user), )); - $cache = $this->getMock('\phpbb\cache\cache', array('destroy', 'sql_exists')); + $cache = $this->getMock('\phpbb\cache\cache', array('destroy', 'sql_exists', 'get', 'put')); $cache->expects($this->any()) ->method('destroy') ->with($this->equalTo('portal_modules')); + $cache->expects($this->any()) + ->method('get') + ->with($this->anything()) + ->will($this->returnValue(false)); $cache->expects($this->any()) ->method('sql_exists') ->with($this->anything()); + $cache->expects($this->any()) + ->method('put') + ->with($this->anything()); $db = $this->db; + $user->set(array( + 'UNABLE_TO_MOVE' => 'UNABLE_TO_MOVE', + 'UNABLE_TO_MOVE_ROW' => 'UNABLE_TO_MOVE_ROW', + )); $this->portal_module = new \board3\portal\acp\portal_module(); + $this->update_portal_modules(); + } + + protected function update_portal_modules() + { + $this->portal_module->module_column = array(); + $portal_modules = obtain_portal_modules(); + foreach($portal_modules as $cur_module) + { + $this->portal_module->module_column[$cur_module['module_classname']][] = column_num_string($cur_module['module_column']); + } } public function getDataSet() @@ -49,17 +74,204 @@ class phpbb_acp_move_module_test extends \board3\portal\tests\testframework\data return $this->createXMLDataSet(dirname(__FILE__) . '/fixtures/modules.xml'); } + public function test_get_move_module_data() + { + $module_data = $this->portal_module->get_move_module_data(1); + $this->assertEquals(array( + 'module_order' => 1, + 'module_column' => 1, + 'module_classname' => '\board3\portal\modules\clock', + ), $module_data); + } + + public function data_get_last_module_order() + { + return array( + array(3, 1), + array(2, 2), + array(2, 3), + array(1, 4), + ); + } + + /** + * @dataProvider data_get_last_module_order + */ + public function test_get_last_module_order($expected, $column) + { + $this->assertEquals($expected, $this->portal_module->get_last_module_order($column)); + } + public function test_move_module_up() { self::$redirected = false; $this->portal_module->move_module_up(2); $this->assertTrue(self::$redirected); - $this->setExpectedTriggerError(E_USER_NOTICE); + $this->setExpectedTriggerError(E_USER_NOTICE, 'UNABLE_TO_MOVE_ROW'); self::$redirected = false; $this->portal_module->move_module_up(2); $this->assertFalse(self::$redirected); } + + public function test_move_module_down() + { + self::$redirected = false; + $this->portal_module->move_module_down(3); + $this->assertTrue(self::$redirected); + + $this->setExpectedTriggerError(E_USER_NOTICE, 'UNABLE_TO_MOVE_ROW'); + self::$redirected = false; + $this->portal_module->move_module_down(3); + $this->assertFalse(self::$redirected); + } + + public function data_handle_after_move() + { + return array( + array(true, false, false), + array(false, false, 'UNABLE_TO_MOVE'), + array(false, true, 'UNABLE_TO_MOVE_ROW'), + ); + } + + /** + * @dataProvider data_handle_after_move + */ + public function test_handle_after_move($success, $is_row, $error) + { + if ($error) + { + $this->setExpectedTriggerError(E_USER_NOTICE, $error); + } + else + { + self::$redirected = false; + } + + $this->portal_module->handle_after_move($success, $is_row); + + if (!$error) + { + $this->assertTrue(self::$redirected); + } + } + + public function data_move_module_right() + { + return array( + array(6, 1, 2), + array(6, 1, 1, 2), + array(7, 4, 0), + array(5, 2, 0), + array(1, 1, 1, 3), + array(2, 2, 0), + ); + } + + /** + * @dataProvider data_move_module_right + */ + public function test_move_module_right($module_id, $column_start, $move_right, $add_to_column = false) + { + if ($column_start > 3) + { + $this->setExpectedTriggerError(E_USER_ERROR, 'CLASS_NOT_FOUND'); + $this->portal_module->move_module_right($module_id); + return; + } + + if ($add_to_column) + { + $module_data = $this->portal_module->get_move_module_data($module_id); + $this->portal_module->module_column[$module_data['module_classname']][] = column_num_string($add_to_column); + } + + for ($i = 1; $i <= $move_right; $i++) + { + $data = $this->portal_module->get_move_module_data($module_id); + $this->assertEquals($column_start, $data['module_column']); + $this->portal_module->move_module_right($module_id); + $column_start++; + $this->update_portal_modules(); + } + $this->setExpectedTriggerError(E_USER_NOTICE, 'UNABLE_TO_MOVE'); + $this->portal_module->move_module_right($module_id); + } + + public function data_move_module_left() + { + return array( + array(1, 1, 1, 1), + array(6, 1, 2, 2), + array(5, 2, 0, 0), + array(6, 1, 2, 1, 2), + array(7, 4, 0, 0), + array(2, 2, 0, 0, 1), + ); + } + + /** + * @dataProvider data_move_module_left + */ + public function test_move_module_left($module_id, $column_start, $move_right, $move_left, $add_to_column = false) + { + if ($column_start > 3) + { + $this->setExpectedTriggerError(E_USER_ERROR, 'CLASS_NOT_FOUND'); + $this->portal_module->move_module_left($module_id); + return; + } + + for ($i = 1; $i <= $move_right; $i++) + { + $data = $this->portal_module->get_move_module_data($module_id); + $this->assertEquals($column_start, $data['module_column']); + $this->portal_module->move_module_right($module_id); + $this->update_portal_modules(); + $column_start++; + } + + if ($add_to_column) + { + $module_data = $this->portal_module->get_move_module_data($module_id); + $this->portal_module->module_column[$module_data['module_classname']][] = column_num_string($add_to_column); + } + + // We always start in the right column = 3 + $column_start = 3; + for ($i = 1; $i <= $move_left; $i++) + { + $data = $this->portal_module->get_move_module_data($module_id); + $this->assertEquals($column_start, $data['module_column']); + $this->portal_module->move_module_left($module_id); + $this->update_portal_modules(); + $column_start--; + } + $this->setExpectedTriggerError(E_USER_NOTICE, 'UNABLE_TO_MOVE'); + $this->portal_module->move_module_left($module_id); + } + + public function data_can_move_module() + { + return array( + array(false, 'left', '\board3\portal\modules\clock'), + array(false, 'right', '\board3\portal\modules\clock'), + array(true, 'center', '\board3\portal\modules\clock'), + array(true, array('top', 'bottom', 'center'), '\board3\portal\modules\clock'), + array(false, array('left', 'right'), '\board3\portal\modules\clock'), + array(false, 'center', '\board3\portal\modules\birthday_list'), + ); + } + + /** + * @dataProvider data_can_move_module + */ + public function test_can_move_module($expected, $target_column, $module_class) + { + $this->update_portal_modules(); + $this->assertEquals($expected, $this->portal_module->can_move_module($target_column, $module_class)); + } } function redirect($url) diff --git a/tests/mock/user.php b/tests/mock/user.php index baed62e0..8106e5fc 100644 --- a/tests/mock/user.php +++ b/tests/mock/user.php @@ -34,7 +34,10 @@ class user extends \PHPUnit_Framework_TestCase { include_once(dirname(__FILE__) . '/../../language/en/' . $file . '.php'); - $this->set($lang); + if (isset($lang)) + { + $this->set($lang); + } } else {