[ticket/216] Add move_module_left and move_module_right to unit tests

These methods are now covered by unit tests. They might need to be
stripped a little bit as the code coverage report currently states that
some part of the code is never hit.

B3P-216

acp/portal_module.php

@@ -952,7 +952,7 @@ 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)
 	{
 		$module_data = $this->get_move_module_data($module_id);
 		
@@ -963,7 +963,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)))
 			{
@@ -1043,11 +1043,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');
+		$this->handle_after_move(true);
-		redirect($this->u_action); // redirect in order to get rid of excessive URL parameters
 	}
 
 	/**
@@ -1055,7 +1054,7 @@ 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)
 	{
 		$module_data = $this->get_move_module_data($module_id);
 		
@@ -1066,7 +1065,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)))
 			{
@@ -1146,11 +1145,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');
+		$this->handle_after_move(true);
-		redirect($this->u_action); // redirect in order to get rid of excessive URL parameters
 	}
 
 	/**

tests/acp/fixtures/modules.xml

@@ -8,26 +8,44 @@
 		<row>
 			<value>1</value>
 			<value>\board3\portal\modules\clock</value>
-			<value>2</value>
+			<value>1</value>
 			<value>1</value>
 		</row>
 		<row>
 			<value>2</value>
 			<value>\board3\portal\modules\birthday_list</value>
+			<value>1</value>
 			<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>

tests/acp/move_module_test.php

@@ -28,10 +28,13 @@ 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', 'get', 'put'));
 		$cache->expects($this->any())
@@ -65,7 +68,7 @@ class phpbb_acp_move_module_test extends \board3\portal\tests\testframework\data
 		$module_data = $this->portal_module->get_move_module_data(1);
 		$this->assertEquals(array(
 			'module_order'		=> 1,
-			'module_column'		=> 2,
+			'module_column'		=> 1,
 			'module_classname'	=> '\board3\portal\modules\clock',
 		), $module_data);
 	}
@@ -73,9 +76,10 @@ class phpbb_acp_move_module_test extends \board3\portal\tests\testframework\data
 	public function data_get_last_module_order()
 	{
 		return array(
-			array(1, 1),
+			array(3, 1),
-			array(2, 2),
+			array(1, 2),
-			array(2, 4),
+			array(2, 3),
+			array(1, 4),
 		);
 	}
 
@@ -105,6 +109,10 @@ class phpbb_acp_move_module_test extends \board3\portal\tests\testframework\data
 		$this->portal_module->move_module_down(1);
 		$this->assertTrue(self::$redirected);
 
+		self::$redirected = false;
+		$this->portal_module->move_module_down(1);
+		$this->assertTrue(self::$redirected);
+
 		$this->setExpectedTriggerError(E_USER_NOTICE, 'UNABLE_TO_MOVE_ROW');
 		self::$redirected = false;
 		$this->portal_module->move_module_down(1);
@@ -141,6 +149,78 @@ class phpbb_acp_move_module_test extends \board3\portal\tests\testframework\data
 			$this->assertTrue(self::$redirected);
 		}
 	}
+
+	public function data_move_module_right()
+	{
+		return array(
+			array(2, 1, 1, 1),
+			array(6, 1, 2, 2),
+			array(7, 4, 0, 0),
+		);
+	}
+
+	/**
+	* @dataProvider data_move_module_right
+	*/
+	public function test_move_module_right($module_id, $column_start, $move_right, $move_left)
+	{
+		if ($column_start > 3)
+		{
+			$this->setExpectedTriggerError(E_USER_ERROR, 'CLASS_NOT_FOUND');
+			$this->portal_module->move_module_right($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);
+			$column_start++;
+		}
+		$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(2, 1, 1, 1),
+			array(6, 1, 2, 2),
+			array(7, 4, 0, 0),
+		);
+	}
+
+	/**
+	* @dataProvider data_move_module_left
+	*/
+	public function test_move_module_left($module_id, $column_start, $move_right, $move_left)
+	{
+		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);
+			$column_start++;
+		}
+
+		// 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);
+			$column_start--;
+		}
+		$this->setExpectedTriggerError(E_USER_NOTICE, 'UNABLE_TO_MOVE');
+		$this->portal_module->move_module_left($module_id);
+	}
 }
 
 function redirect($url)