Julian/phpbb_board3-portal_tlw
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #217 from marc1706/ticket/216

Browse Source 
[ticket/216] Remove duplicate code from moving modules in acp and add tests
This commit is contained in:
Marc Alexander
2014-02-18 01:49:27 +01:00
parent cc098fb66c 4655544ed4
commit 3d755c1c95
4 changed files with 388 additions and 114 deletions
Download Patch File Download Diff File Expand all files Collapse all files

255
acp/portal_module.php
View File

@@ -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;
}
}

24
tests/acp/fixtures/modules.xml
View File

@@ -8,7 +8,7 @@
<row>
<value>1</value>
<value>\board3\portal\modules\clock</value>
<value>2</value>
<value>1</value>
<value>1</value>
</row>
<row>
@@ -17,17 +17,35 @@
<value>2</value>
<value>2</value>
</row>
<row>
<value>6</value>
<value>\board3\portal\modules\donation</value>
<value>1</value>
<value>3</value>
</row>
<row>
<value>3</value>
<value>\board3\portal\modules\clock</value>
<value>4</value>
<value>3</value>
<value>1</value>
</row>
<row>
<value>4</value>
<value>\board3\portal\modules\birthday_list</value>
<value>4</value>
<value>3</value>
<value>2</value>
</row>
<row>
<value>5</value>
<value>\board3\portal\modules\welcome</value>
<value>2</value>
<value>1</value>
</row>
<row>
<value>7</value>
<value>\board3\portal\modules\custom</value>
<value>4</value>
<value>1</value>
</row>
</table>
</dataset>

218
tests/acp/move_module_test.php
View File

@@ -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)

5
tests/mock/user.php
View File

@@ -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
{