From 275910d8b0fd8d5edeee07eb05ad82da48cc72a3 Mon Sep 17 00:00:00 2001
From: Joas Schilling <nickvergessen@gmx.de>
Date: Sat, 8 Mar 2014 15:59:40 +0100
Subject: [PATCH] [ticket/12090] Fix pagination for routes

No clickable "jump to" at the moment, as we can not get the route url
by the route name in js yet. Need to find another solution later.

PHPBB3-12090
---
 phpBB/config/services.yml                     |  1 +
 phpBB/phpbb/pagination.php                    | 28 ++++++-
 phpBB/styles/prosilver/template/forum_fn.js   | 11 +--
 .../styles/prosilver/template/pagination.html |  6 +-
 .../subsilver2/template/pagination.html       |  2 +-
 tests/pagination/config/routing.yml           |  6 ++
 tests/pagination/pagination_test.php          | 84 ++++++++++++-------
 7 files changed, 93 insertions(+), 45 deletions(-)
 create mode 100644 tests/pagination/config/routing.yml

diff --git a/phpBB/config/services.yml b/phpBB/config/services.yml
index 0699cfcc6d..3427f95cc1 100644
--- a/phpBB/config/services.yml
+++ b/phpBB/config/services.yml
@@ -263,6 +263,7 @@ services:
         arguments:
             - @template
             - @user
+            - @controller.helper
 
     path_helper:
         class: phpbb\path_helper
diff --git a/phpBB/phpbb/pagination.php b/phpBB/phpbb/pagination.php
index 57e7932341..6a7631c89d 100644
--- a/phpBB/phpbb/pagination.php
+++ b/phpBB/phpbb/pagination.php
@@ -22,11 +22,13 @@ class pagination
 	*
 	* @param	\phpbb\template\template	$template
 	* @param	\phpbb\user					$user
+	* @param	\phpbb\controller\helper	$helper
 	*/
-	public function __construct(\phpbb\template\template $template, \phpbb\user $user)
+	public function __construct(\phpbb\template\template $template, \phpbb\user $user, \phpbb\controller\helper $helper)
 	{
 		$this->template = $template;
 		$this->user = $user;
+		$this->helper = $helper;
 	}
 
 	/**
@@ -44,9 +46,26 @@ class pagination
 	*/
 	protected function generate_page_link($base_url, $on_page, $start_name, $per_page)
 	{
-		if (strpos($start_name, '%d') !== false)
+		if (!is_string($base_url))
 		{
-			return ($on_page > 1) ? sprintf($base_url, (int) $on_page) : str_replace($start_name, '', $base_url);
+			if (is_array($base_url['routes']))
+			{
+				$route = ($on_page > 1) ? $base_url['routes'][1] : $base_url['routes'][0];
+			}
+			else
+			{
+				$route = $base_url['routes'];
+			}
+			$params = (isset($base_url['params'])) ? $base_url['params'] : array();
+			$is_amp = (isset($base_url['is_amp'])) ? $base_url['is_amp'] : true;
+			$session_id = (isset($base_url['session_id'])) ? $base_url['session_id'] : false;
+
+			if ($on_page > 1 || !is_array($base_url['routes']))
+			{
+				$params[$start_name] = (int) $on_page;
+			}
+
+			return $this->helper->route($route, $params, $is_amp, $session_id);
 		}
 		else
 		{
@@ -194,7 +213,8 @@ class pagination
 		$tpl_prefix = ($tpl_prefix == 'PAGINATION') ? '' : $tpl_prefix . '_';
 
 		$template_array = array(
-			$tpl_prefix . 'BASE_URL'		=> $base_url,
+			$tpl_prefix . 'BASE_URL'		=> is_string($base_url) ? $base_url : '',//@todo: Fix this for routes
+			$tpl_prefix . 'START_NAME'		=> $start_name,
 			$tpl_prefix . 'PER_PAGE'		=> $per_page,
 			'U_' . $tpl_prefix . 'PREVIOUS_PAGE'	=> ($on_page != 1) ? $u_previous_page : '',
 			'U_' . $tpl_prefix . 'NEXT_PAGE'		=> ($on_page != $total_pages) ? $u_next_page : '',
diff --git a/phpBB/styles/prosilver/template/forum_fn.js b/phpBB/styles/prosilver/template/forum_fn.js
index 408c9b9b8c..de51b54e9b 100644
--- a/phpBB/styles/prosilver/template/forum_fn.js
+++ b/phpBB/styles/prosilver/template/forum_fn.js
@@ -37,17 +37,14 @@ function jumpto(item) {
 		on_page = item.attr('data-on-page'),
 		per_page = item.attr('data-per-page'),
 		base_url = item.attr('data-base-url'),
+		start_name = item.attr('data-start-name'),
 		page = prompt(jump_page, on_page);
 
 	if (page !== null && !isNaN(page) && page == Math.floor(page) && page > 0) {
-		if (base_url.indexOf('%d') === -1) {
-			if (base_url.indexOf('?') === -1) {
-				document.location.href = base_url + '?start=' + ((page - 1) * per_page);
-			} else {
-				document.location.href = base_url.replace(/&amp;/g, '&') + '&start=' + ((page - 1) * per_page);
-			}
+		if (base_url.indexOf('?') === -1) {
+			document.location.href = base_url + '?' + start_name + '=' + ((page - 1) * per_page);
 		} else {
-			document.location.href = base_url.replace('%d', page);
+			document.location.href = base_url.replace(/&amp;/g, '&') + '&' + start_name + '=' + ((page - 1) * per_page);
 		}
 	}
 }
diff --git a/phpBB/styles/prosilver/template/pagination.html b/phpBB/styles/prosilver/template/pagination.html
index cb54193c3f..e27a90900a 100644
--- a/phpBB/styles/prosilver/template/pagination.html
+++ b/phpBB/styles/prosilver/template/pagination.html
@@ -1,4 +1,8 @@
-					<a href="#" class="pagination-trigger" title="{L_JUMP_TO_PAGE}" data-lang-jump-page="{L_JUMP_PAGE|e('html_attr')}{L_COLON}" data-on-page="{CURRENT_PAGE}" data-per-page="{PER_PAGE}" data-base-url="{BASE_URL|e('html_attr')}">{PAGE_NUMBER}</a> &bull;
+					<!-- IF BASE_URL -->
+					<a href="#" class="pagination-trigger" title="{L_JUMP_TO_PAGE}" data-lang-jump-page="{L_JUMP_PAGE|e('html_attr')}{L_COLON}" data-on-page="{CURRENT_PAGE}" data-per-page="{PER_PAGE}" data-base-url="{BASE_URL|e('html_attr')}" data-base-is-route="{BASE_IS_ROUTE}" data-start-name="{START_NAME}">{PAGE_NUMBER}</a> &bull;
+					<!-- ELSE -->
+					{PAGE_NUMBER} &bull;
+					<!-- ENDIF -->
 					<ul>
 					<!-- BEGIN pagination -->
 						<!-- IF pagination.S_IS_PREV -->
diff --git a/phpBB/styles/subsilver2/template/pagination.html b/phpBB/styles/subsilver2/template/pagination.html
index a2e023ac22..550b28d305 100644
--- a/phpBB/styles/subsilver2/template/pagination.html
+++ b/phpBB/styles/subsilver2/template/pagination.html
@@ -1,5 +1,5 @@
 <!-- IF .pagination -->
-	<b><a href="#" onclick="jumpto(); return false;" title="{L_JUMP_TO_PAGE}">{L_GOTO_PAGE}</a> 
+	<!-- IF BASE_URL --><b><a href="#" onclick="jumpto(); return false;" title="{L_JUMP_TO_PAGE}">{L_GOTO_PAGE}</a></b><!-- ENDIF -->
 	<!-- BEGIN pagination -->
 		<!-- IF pagination.S_IS_PREV --><a href="{pagination.PAGE_URL}">{L_PREVIOUS}</a>
 		<!-- ELSEIF pagination.S_IS_CURRENT --><strong>{pagination.PAGE_NUMBER}</strong>
diff --git a/tests/pagination/config/routing.yml b/tests/pagination/config/routing.yml
new file mode 100644
index 0000000000..dd667274cd
--- /dev/null
+++ b/tests/pagination/config/routing.yml
@@ -0,0 +1,6 @@
+core_controller:
+    pattern: /test
+    defaults: { _controller: core_foo.controller:bar, page: 1}
+core_page_controller:
+    pattern: /test/page/{page}
+    defaults: { _controller: core_foo.controller:bar}
diff --git a/tests/pagination/pagination_test.php b/tests/pagination/pagination_test.php
index b7a4f101aa..38ab8e62a4 100644
--- a/tests/pagination/pagination_test.php
+++ b/tests/pagination/pagination_test.php
@@ -21,11 +21,25 @@ class phpbb_pagination_pagination_test extends phpbb_template_template_test_case
 	public function setUp()
 	{
 		parent::setUp();
-		$user = $this->getMock('\phpbb\user');
-		$user->expects($this->any())
+
+		global $phpbb_dispatcher;
+
+		$phpbb_dispatcher = new phpbb_mock_event_dispatcher;
+		$this->user = $this->getMock('\phpbb\user');
+		$this->user->expects($this->any())
 			->method('lang')
 			->will($this->returnCallback(array($this, 'return_callback_implode')));
-		$this->pagination = new \phpbb\pagination($this->template, $user);
+
+		$this->finder = new \phpbb\extension\finder(
+			new phpbb_mock_extension_manager(dirname(__FILE__) . '/', array()),
+			new \phpbb\filesystem(),
+			dirname(__FILE__) . '/',
+			new phpbb_mock_cache()
+		);
+
+		$this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '1'));
+		$this->helper = new \phpbb\controller\helper($this->finder, $this->template, $this->user, $this->config, dirname(__FILE__) . '/', 'php');
+		$this->pagination = new \phpbb\pagination($this->template, $this->user, $this->helper);
 	}
 
 	public function generate_template_pagination_data()
@@ -77,49 +91,55 @@ class phpbb_pagination_pagination_test extends phpbb_template_template_test_case
 				:u_next:page.php?start=30',
 			),
 			array(
-				'test/page/%d',
-				'/page/%d',
+				array('routes' => array(
+					'core_controller',
+					'core_page_controller',
+				)),
+				'page',
 				95,
 				10,
 				10,
 				'pagination
 				:per_page:10
 				:current_page:2
-				:base_url:test/page/%d
-				:previous::test
-				:else:1:test
-				:current:2:test/page/2
-				:else:3:test/page/3
-				:else:4:test/page/4
-				:else:5:test/page/5
-				:ellipsis:9:test/page/9
-				:else:10:test/page/10
-				:next::test/page/3
-				:u_prev:test
-				:u_next:test/page/3',
+				:base_url:
+				:previous::' . dirname(__FILE__) . '/' . 'test
+				:else:1:' . dirname(__FILE__) . '/' . 'test
+				:current:2:' . dirname(__FILE__) . '/' . 'test/page/2
+				:else:3:' . dirname(__FILE__) . '/' . 'test/page/3
+				:else:4:' . dirname(__FILE__) . '/' . 'test/page/4
+				:else:5:' . dirname(__FILE__) . '/' . 'test/page/5
+				:ellipsis:9:' . dirname(__FILE__) . '/' . 'test/page/9
+				:else:10:' . dirname(__FILE__) . '/' . 'test/page/10
+				:next::' . dirname(__FILE__) . '/' . 'test/page/3
+				:u_prev:' . dirname(__FILE__) . '/' . 'test
+				:u_next:' . dirname(__FILE__) . '/' . 'test/page/3',
 			),
 			array(
-				'test/page/%d',
-				'/page/%d',
+				array('routes' => array(
+					'core_controller',
+					'core_page_controller',
+				)),
+				'page',
 				95,
 				10,
 				20,
 				'pagination
 				:per_page:10
 				:current_page:3
-				:base_url:test/page/%d
-				:previous::test/page/2
-				:else:1:test
-				:else:2:test/page/2
-				:current:3:test/page/3
-				:else:4:test/page/4
-				:else:5:test/page/5
-				:else:6:test/page/6
-				:ellipsis:9:test/page/9
-				:else:10:test/page/10
-				:next::test/page/4
-				:u_prev:test/page/2
-				:u_next:test/page/4',
+				:base_url:
+				:previous::' . dirname(__FILE__) . '/' . 'test/page/2
+				:else:1:' . dirname(__FILE__) . '/' . 'test
+				:else:2:' . dirname(__FILE__) . '/' . 'test/page/2
+				:current:3:' . dirname(__FILE__) . '/' . 'test/page/3
+				:else:4:' . dirname(__FILE__) . '/' . 'test/page/4
+				:else:5:' . dirname(__FILE__) . '/' . 'test/page/5
+				:else:6:' . dirname(__FILE__) . '/' . 'test/page/6
+				:ellipsis:9:' . dirname(__FILE__) . '/' . 'test/page/9
+				:else:10:' . dirname(__FILE__) . '/' . 'test/page/10
+				:next::' . dirname(__FILE__) . '/' . 'test/page/4
+				:u_prev:' . dirname(__FILE__) . '/' . 'test/page/2
+				:u_next:' . dirname(__FILE__) . '/' . 'test/page/4',
 			),
 		);
 	}