Merge pull request #77 from phpbb-extensions/master

Merge master branch into release-1.0.3

composer.json

@@ -49,7 +49,7 @@
 	"extra": {
 		"display-name": "Pages",
 		"soft-require": {
-			"phpbb/phpbb": ">=3.1.0,<3.2.*@dev"
+			"phpbb/phpbb": ">=3.1.3,<3.2.*@dev"
 		},
 		"version-check": {
 			"host": "www.phpbb.com",

controller/main_controller.php

@@ -11,6 +11,7 @@
 namespace phpbb\pages\controller;
 
 use Symfony\Component\DependencyInjection\ContainerInterface;
+use phpbb\exception\http_exception;
 
 /**
 * Main controller
@@ -56,6 +57,7 @@ public function __construct(\phpbb\auth\auth $auth, ContainerInterface $containe
 	*
 	* @param string $route The route name for a page
 	* @return \Symfony\Component\HttpFoundation\Response A Symfony Response object
+	* @throws http_exception
 	* @access public
 	*/
 	public function display($route)
@@ -64,38 +66,33 @@ public function display($route)
 		$this->user->add_lang_ext('phpbb/pages', 'pages_controller');
 
 		// Load the page data to display
-		$display = $this->load_page_data($route);
+		$page = $this->load_page_data($route);
 
-		// Set the page title to use
-		$page_title = ($display) ? $display->get_title() : $this->user->lang('INFORMATION');
+		// Set the page title
+		$page_title = $page->get_title();
 
 		// Assign the page data to template variables
 		$this->template->assign_vars(array(
 			'PAGE_TITLE'	=> $page_title,
-			'PAGE_CONTENT'	=> ($display) ? $display->get_content_for_display() : $this->user->lang('PAGE_NOT_AVAILABLE', $route),
+			'PAGE_CONTENT'	=> $page->get_content_for_display(),
 		));
 
 		// Create breadcrumbs
-		if ($display)
-		{
-			$this->template->assign_block_vars('navlinks', array(
-				'FORUM_NAME'	=> $page_title,
-				'U_VIEW_FORUM'	=> $this->helper->route('phpbb_pages_main_controller', array('route' => $route)),
-			));
-		}
-
-		// Set the page template to use
-		$page_template = ($display) ? $display->get_template() : 'pages_default.html';
+		$this->template->assign_block_vars('navlinks', array(
+			'FORUM_NAME'	=> $page_title,
+			'U_VIEW_FORUM'	=> $this->helper->route('phpbb_pages_main_controller', array('route' => $route)),
+		));
 
 		// Send all data to the template file
-		return $this->helper->render($page_template, $page_title);
+		return $this->helper->render($page->get_template(), $page_title);
 	}
 
 	/**
 	* Load the page data
 	*
 	* @param string $route The route name for a page
-	* @return object $entity The entity object, or false if page can't be displayed
+	* @return object $entity The entity object
+	* @throws http_exception
 	* @access public
 	*/
 	protected function load_page_data($route)
@@ -110,19 +107,19 @@ protected function load_page_data($route)
 		}
 		catch (\phpbb\pages\exception\base $e)
 		{
-			return false;
+			throw new http_exception(404, 'PAGE_NOT_AVAILABLE', array($route));
 		}
 
-		// Return false if page display to guests is disabled
+		// Throw 404 error if page display to guests is disabled
 		if ($this->user->data['user_id'] == ANONYMOUS && !$entity->get_page_display_to_guests())
 		{
-			return false;
+			throw new http_exception(404, 'PAGE_NOT_AVAILABLE', array($route));
 		}
 
-		// Return false if page display is disabled and user is not an admin
+		// Throw 404 error if page display is disabled and user is not an admin
 		if (!$this->auth->acl_get('a_') && !$entity->get_page_display())
 		{
-			return false;
+			throw new http_exception(404, 'PAGE_NOT_AVAILABLE', array($route));
 		}
 
 		return $entity;

ext.php

@@ -0,0 +1,33 @@
+<?php
+/**
+*
+* Pages extension for the phpBB Forum Software package.
+*
+* @copyright (c) 2015 phpBB Limited <https://www.phpbb.com>
+* @license GNU General Public License, version 2 (GPL-2.0)
+*
+*/
+
+namespace phpbb\pages;
+
+/**
+* Extension class for custom enable/disable/purge actions
+*/
+class ext extends \phpbb\extension\base
+{
+	/**
+	* Check whether or not the extension can be enabled.
+	* The current phpBB version should meet or exceed
+	* the minimum version required by this extension:
+	*
+	* Requires phpBB 3.1.3 due to usage of http_exception.
+	*
+	* @return bool
+	* @access public
+	*/
+	public function is_enableable()
+	{
+		$config = $this->container->get('config');
+		return phpbb_version_compare($config['version'], '3.1.3', '>=');
+	}
+}

tests/controller/page_main_controller_test.php

@@ -36,28 +36,10 @@ public function getDataSet()
 		return $this->createXMLDataSet(dirname(__FILE__) . '/../entity/fixtures/page.xml');
 	}
 
-	/**
-	* Test data for the test_display() function
-	*
-	* @return array Array of test data
-	*/
-	public function display_data()
+	public function setUp()
 	{
-		return array(
-			array('page_1', 200, 'pages_default.html', 2), // normal viewable page by member
-			array('page_4', 200, 'pages_default.html', 2), // disabled page, member sees page missing message
-			array('page_4', 200, 'pages_default.html', 1), // disabled page, guests sees page missing message
-			array('page_foo', 200, 'pages_default.html', 2), // non-existent page, loads page missing message
-		);
-	}
+		parent::setUp();
 
-	/**
-	* Test controller display
-	*
-	* @dataProvider display_data
-	*/
-	public function test_display($route, $status_code, $page_content, $user_id)
-	{
 		global $cache, $config, $phpbb_extension_manager, $phpbb_dispatcher, $user, $phpbb_root_path;
 
 		// Load/Mock classes required by the controller class
@@ -80,7 +62,6 @@ public function test_display($route, $status_code, $page_content, $user_id)
 		;
 
 		$this->user = new \phpbb\user('\phpbb\datetime');
-		$this->user->data['user_id'] = $user_id;
 
 		$this->controller_helper = $this->getMockBuilder('\phpbb\controller\helper')
 			->disableOriginalConstructor()
@@ -96,18 +77,82 @@ public function test_display($route, $status_code, $page_content, $user_id)
 		$cache = new \phpbb_mock_cache();
 		$user = $this->getMock('\phpbb\user', array(), array('\phpbb\datetime'));
 		$phpbb_extension_manager = new \phpbb_mock_extension_manager($phpbb_root_path);
+	}
 
-		$controller = new \phpbb\pages\controller\main_controller(
+	public function get_controller()
+	{
+		return  new \phpbb\pages\controller\main_controller(
 			$this->auth,
 			$this->container,
 			$this->controller_helper,
 			$this->template,
 			$this->user
 		);
+	}
+
+	/**
+	* Test data for the test_display() function
+	*
+	* @return array Array of test data
+	*/
+	public function display_data()
+	{
+		return array(
+			array('page_1', 200, 'pages_default.html', 2), // normal viewable page by member
+		);
+	}
+
+	/**
+	* Test controller display
+	*
+	* @dataProvider display_data
+	*/
+	public function test_display($route, $status_code, $page_content, $user_id)
+	{
+		$this->user->data['user_id'] = $user_id;
+
+		$controller = $this->get_controller();
 
 		$response = $controller->display($route);
 		$this->assertInstanceOf('\Symfony\Component\HttpFoundation\Response', $response);
 		$this->assertEquals($status_code, $response->getStatusCode());
 		$this->assertEquals($page_content, $response->getContent());
 	}
+
+	/**
+	 * Test data for the test_display_fails() function
+	 *
+	 * @return array Array of test data
+	 */
+	public function display_fails_data()
+	{
+		return array(
+			array('page_4', 404, 'PAGE_NOT_AVAILABLE', 2), // disabled page, member sees page missing message
+			array('page_4', 404, 'PAGE_NOT_AVAILABLE', 1), // disabled page, guests sees page missing message
+			array('page_foo', 404, 'PAGE_NOT_AVAILABLE', 2), // non-existent page, loads page missing message
+		);
+	}
+
+	/**
+	 * Test controller display throws 404 exceptions
+	 *
+	 * @dataProvider display_fails_data
+	 */
+	public function test_display_fails($route, $status_code, $page_content, $user_id)
+	{
+		$this->user->data['user_id'] = $user_id;
+
+		$controller = $this->get_controller();
+
+		try
+		{
+			$controller->display($route);
+			$this->fail('The expected \phpbb\exception\http_exception was not thrown');
+		}
+		catch (\phpbb\exception\http_exception $exception)
+		{
+			$this->assertEquals($status_code, $exception->getStatusCode());
+			$this->assertEquals($page_content, $exception->getMessage());
+		}
+	}
 }