diff --git a/composer.json b/composer.json index 17285bf..ea4b43b 100644 --- a/composer.json +++ b/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", diff --git a/controller/main_controller.php b/controller/main_controller.php index 9c34712..f0bfd97 100644 --- a/controller/main_controller.php +++ b/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; diff --git a/ext.php b/ext.php new file mode 100644 index 0000000..52b2f06 --- /dev/null +++ b/ext.php @@ -0,0 +1,33 @@ + +* @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', '>='); + } +} diff --git a/tests/controller/page_main_controller_test.php b/tests/controller/page_main_controller_test.php index 2b0188d..0c6ea61 100644 --- a/tests/controller/page_main_controller_test.php +++ b/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()); + } + } }