Refactor buildPrevNextNavigation
authorclarakosi <candrewwani@gmail.com>
Wed, 29 May 2019 23:08:11 +0000 (19:08 -0400)
committerclarakosi <candrewwani@gmail.com>
Wed, 5 Jun 2019 23:32:27 +0000 (19:32 -0400)
Refactored buildPrevNextNavigation() into standalone helper class,
PrevNextNavigationRenderer, to be used by both SpecialPages and Pagers.

Bug:T207977
Change-Id: Ic49837a451f795ec203e867961ec1c69075cc91a

RELEASE-NOTES-1.34
autoload.php
includes/Navigation/PrevNextNavigationRenderer.php [new file with mode: 0644]
includes/pager/IndexPager.php
includes/specialpage/SpecialPage.php
includes/specials/SpecialSearch.php
languages/Language.php
tests/phpunit/includes/Navigation/PrevNextNavigationRendererTest.php [new file with mode: 0644]
tests/phpunit/includes/specialpage/SpecialPageTest.php

index d372db8..068575f 100644 (file)
@@ -237,6 +237,8 @@ because of Phabricator reports.
   MovePage::moveSubpagesIfAllowed.
 * The MWNamespace class is deprecated. Use MediaWikiServices::getNamespaceInfo.
 * (T62260) Hard deprecate Language::getExtraUserToggles() method.
+* Language::viewPrevNext function is deprecated, use
+  PrevNextNavigationRenderer::buildPrevNextNavigation instead
 
 === Other changes in 1.34 ===
 * …
index a9b65fa..9cde066 100644 (file)
@@ -908,6 +908,7 @@ $wgAutoloadLocalClasses = [
        'MediaWiki\\Logger\\NullSpi' => __DIR__ . '/includes/debug/logger/NullSpi.php',
        'MediaWiki\\Logger\\Spi' => __DIR__ . '/includes/debug/logger/Spi.php',
        'MediaWiki\\MediaWikiServices' => __DIR__ . '/includes/MediaWikiServices.php',
+       'MediaWiki\\Navigation\\PrevNextNavigationRenderer' => __DIR__ . '/includes/Navigation/PrevNextNavigationRenderer.php',
        'MediaWiki\\OutputHandler' => __DIR__ . '/includes/OutputHandler.php',
        'MediaWiki\\ProcOpenError' => __DIR__ . '/includes/exception/ProcOpenError.php',
        'MediaWiki\\Search\\ParserOutputSearchDataExtractor' => __DIR__ . '/includes/search/ParserOutputSearchDataExtractor.php',
diff --git a/includes/Navigation/PrevNextNavigationRenderer.php b/includes/Navigation/PrevNextNavigationRenderer.php
new file mode 100644 (file)
index 0000000..c60b8c6
--- /dev/null
@@ -0,0 +1,111 @@
+<?php
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+namespace MediaWiki\Navigation;
+
+use MediaWiki\Linker\LinkTarget;
+use MessageLocalizer;
+use Html;
+
+/**
+ * Helper class for generating prev/next links for paging.
+ *
+ * @since 1.34
+ */
+class PrevNextNavigationRenderer {
+
+       /**
+        * @var MessageLocalizer
+        */
+       private $messageLocalizer;
+
+       public function __construct( MessageLocalizer $messageLocalizer ) {
+               $this->messageLocalizer = $messageLocalizer;
+       }
+
+       /**
+        * Generate (prev x| next x) (20|50|100...) type links for paging
+        *
+        * @param LinkTarget $title LinkTarget object to link
+        * @param int $offset
+        * @param int $limit
+        * @param array $query Optional URL query parameter string
+        * @param bool $atend Optional param for specified if this is the last page
+        * @return string
+        */
+       public function buildPrevNextNavigation( LinkTarget $title, $offset, $limit,
+                                                                                        array $query = [], $atend = false
+       ) {
+               # Make 'previous' link
+               $prev = $this->messageLocalizer->msg( 'prevn' )->title( $title )
+                       ->numParams( $limit )->text();
+
+               if ( $offset > 0 ) {
+                       $plink = $this->numLink( $title, max( $offset - $limit, 0 ), $limit, $query,
+                               $prev, 'prevn-title', 'mw-prevlink' );
+               } else {
+                       $plink = htmlspecialchars( $prev );
+               }
+
+               # Make 'next' link
+               $next = $this->messageLocalizer->msg( 'nextn' )->title( $title )
+                       ->numParams( $limit )->text();
+               if ( $atend ) {
+                       $nlink = htmlspecialchars( $next );
+               } else {
+                       $nlink = $this->numLink( $title, $offset + $limit, $limit,
+                               $query, $next, 'nextn-title', 'mw-nextlink' );
+               }
+
+               # Make links to set number of items per page
+               $numLinks = [];
+               $lang = $this->messageLocalizer->getLanguage();
+               foreach ( [ 20, 50, 100, 250, 500 ] as $num ) {
+                       $numLinks[] = $this->numLink( $title, $offset, $num, $query,
+                               $lang->formatNum( $num ), 'shown-title', 'mw-numlink' );
+               }
+
+               return $this->messageLocalizer->msg( 'viewprevnext' )->title( $title
+               )->rawParams( $plink, $nlink, $lang->pipeList( $numLinks ) )->escaped();
+       }
+
+       /**
+        * Helper function for buildPrevNextNavigation() that generates links
+        *
+        * @param LinkTarget $title LinkTarget object to link
+        * @param int $offset
+        * @param int $limit
+        * @param array $query Extra query parameters
+        * @param string $link Text to use for the link; will be escaped
+        * @param string $tooltipMsg Name of the message to use as tooltip
+        * @param string $class Value of the "class" attribute of the link
+        * @return string HTML fragment
+        */
+       private function numLink( LinkTarget $title, $offset, $limit, array $query, $link,
+                                                         $tooltipMsg, $class
+       ) {
+               $query = [ 'limit' => $limit, 'offset' => $offset ] + $query;
+               $tooltip = $this->messageLocalizer->msg( $tooltipMsg )->title( $title )
+                       ->numParams( $limit )->text();
+               return Html::element( 'a', [ 'href' => $title->getLocalURL( $query ),
+                       'title' => $tooltip, 'class' => $class ], $link );
+       }
+
+}
index 64dbb22..04021cc 100644 (file)
@@ -23,6 +23,8 @@
 
 use Wikimedia\Rdbms\IResultWrapper;
 use Wikimedia\Rdbms\IDatabase;
+use MediaWiki\Linker\LinkTarget;
+use MediaWiki\Navigation\PrevNextNavigationRenderer;
 
 /**
  * IndexPager is an efficient pager which uses a (roughly unique) index in the
@@ -784,4 +786,22 @@ abstract class IndexPager extends ContextSource implements Pager {
        protected function getDefaultDirections() {
                return self::DIR_ASCENDING;
        }
+
+       /**
+        * Generate (prev x| next x) (20|50|100...) type links for paging
+        *
+        * @param LinkTarget $title
+        * @param int $offset
+        * @param int $limit
+        * @param array $query Optional URL query parameter string
+        * @param bool $atend Optional param for specified if this is the last page
+        * @return string
+        */
+       protected function buildPrevNextNavigation( LinkTarget $title, $offset, $limit,
+                                                                                               array $query = [], $atend = false
+       ) {
+               $prevNext = new PrevNextNavigationRenderer( $this );
+
+               return $prevNext->buildPrevNextNavigation( $title, $offset, $limit, $query,  $atend );
+       }
 }
index 670a0b8..b4e244c 100644 (file)
@@ -24,6 +24,7 @@
 use MediaWiki\Auth\AuthManager;
 use MediaWiki\Linker\LinkRenderer;
 use MediaWiki\MediaWikiServices;
+use MediaWiki\Navigation\PrevNextNavigationRenderer;
 
 /**
  * Parent class for all special pages.
@@ -933,58 +934,11 @@ class SpecialPage implements MessageLocalizer {
         * @return string
         */
        protected function buildPrevNextNavigation( $offset, $limit,
-               array $query = [], $atend = false, $subpage = false
+                                                                                        array $query = [], $atend = false, $subpage = false
        ) {
-               $lang = $this->getLanguage();
+               $title = $this->getPageTitle( $subpage );
+               $prevNext = new PrevNextNavigationRenderer( $this );
 
-               # Make 'previous' link
-               $prev = $this->msg( 'prevn' )->numParams( $limit )->text();
-               if ( $offset > 0 ) {
-                       $plink = $this->numLink( max( $offset - $limit, 0 ), $limit, $query,
-                               $prev, 'prevn-title', 'mw-prevlink', $subpage );
-               } else {
-                       $plink = htmlspecialchars( $prev );
-               }
-
-               # Make 'next' link
-               $next = $this->msg( 'nextn' )->numParams( $limit )->text();
-               if ( $atend ) {
-                       $nlink = htmlspecialchars( $next );
-               } else {
-                       $nlink = $this->numLink( $offset + $limit, $limit,
-                               $query, $next, 'nextn-title', 'mw-nextlink', $subpage );
-               }
-
-               # Make links to set number of items per page
-               $numLinks = [];
-               foreach ( [ 20, 50, 100, 250, 500 ] as $num ) {
-                       $numLinks[] = $this->numLink( $offset, $num, $query,
-                               $lang->formatNum( $num ), 'shown-title', 'mw-numlink', $subpage );
-               }
-
-               return $this->msg( 'viewprevnext' )->rawParams( $plink, $nlink, $lang->pipeList( $numLinks ) )->
-                       escaped();
-       }
-
-       /**
-        * Helper function for buildPrevNextNavigation() that generates links
-        *
-        * @param int $offset
-        * @param int $limit
-        * @param array $query Extra query parameters
-        * @param string $link Text to use for the link; will be escaped
-        * @param string $tooltipMsg Name of the message to use as tooltip
-        * @param string $class Value of the "class" attribute of the link
-        * @param string|bool $subpage Optional param for specifying subpage
-        * @return string HTML fragment
-        */
-       private function numLink( $offset, $limit, array $query, $link,
-               $tooltipMsg, $class, $subpage = false
-       ) {
-               $query = [ 'limit' => $limit, 'offset' => $offset ] + $query;
-               $tooltip = $this->msg( $tooltipMsg )->numParams( $limit )->text();
-               $href = $this->getPageTitle( $subpage )->getLocalURL( $query );
-               return Html::element( 'a', [ 'href' => $href,
-                       'title' => $tooltip, 'class' => $class ], $link );
+               return $prevNext->buildPrevNextNavigation( $title, $offset, $limit, $query,  $atend );
        }
 }
index 49f26ad..e1fbe6a 100644 (file)
@@ -463,13 +463,13 @@ class SpecialSearch extends SpecialPage {
                                $offset = $this->offset;
                        }
 
-                       $prevnext = $this->buildPrevNextNavigation(
+                       $prevNext = $this->buildPrevNextNavigation(
                                $offset,
                                $this->limit,
                                $this->powerSearchOptions() + [ 'search' => $term ],
                                $this->limit + $this->offset >= $totalRes
                        );
-                       $out->addHTML( "<p class='mw-search-pager-bottom'>{$prevnext}</p>\n" );
+                       $out->addHTML( "<p class='mw-search-pager-bottom'>{$prevNext}</p>\n" );
                }
 
                // Close <div class='searchresults'>
index 6c8b19e..4e663c2 100644 (file)
@@ -4857,7 +4857,7 @@ class Language {
         * @param array $query Optional URL query parameter string
         * @param bool $atend Optional param for specified if this is the last page
         * @return string
-        * @deprecated since 1.33, use SpecialPage::viewPrevNext()
+        * @deprecated since 1.34, use PrevNextNavigationRenderer::buildPrevNextNavigation()
         *  instead.
         */
        public function viewPrevNext( Title $title, $offset, $limit,
diff --git a/tests/phpunit/includes/Navigation/PrevNextNavigationRendererTest.php b/tests/phpunit/includes/Navigation/PrevNextNavigationRendererTest.php
new file mode 100644 (file)
index 0000000..be5ab03
--- /dev/null
@@ -0,0 +1,88 @@
+<?php
+
+use Wikimedia\TestingAccessWrapper;
+use MediaWiki\Navigation\PrevNextNavigationRenderer;
+
+/**
+ * @covers \MediaWiki\Navigation\PrevNextNavigationRenderer
+ */
+class PrevNextNavigationRendererTest extends MediaWikiTestCase {
+
+       public function provideBuildPrevNextNavigation() {
+               yield [ 0, 20, false, false ];
+               yield [ 17, 20, false, false ];
+               yield [ 0, 17, false, false ];
+               yield [ 0, 20, true, 'Foo' ];
+               yield [ 17, 20, true, 'Föö_Bär' ];
+       }
+
+       /**
+        * @dataProvider provideBuildPrevNextNavigation
+        */
+       public function testBuildPrevNextNavigation( $offset, $limit, $atEnd, $subPage ) {
+               $this->setUserLang( Language::factory( 'qqx' ) ); // disable i18n
+
+               $prevNext = new PrevNextNavigationRenderer( RequestContext::getMain() );
+               $prevNext = TestingAccessWrapper::newFromObject( $prevNext );
+
+               $html = $prevNext->buildPrevNextNavigation(
+                       SpecialPage::getTitleFor( 'Watchlist', $subPage ),
+                       $offset,
+                       $limit,
+                       [ 'x' => 25 ],
+                       $atEnd
+               );
+
+               $this->assertStringStartsWith( '(viewprevnext:', $html );
+
+               preg_match_all( '!<a.*?</a>!', $html, $m, PREG_PATTERN_ORDER );
+               $links = $m[0];
+
+               foreach ( $links as $a ) {
+                       if ( $subPage ) {
+                               $this->assertContains( 'Special:Watchlist/' . wfUrlencode( $subPage ), $a );
+                       } else {
+                               $this->assertContains( 'Special:Watchlist', $a );
+                               $this->assertNotContains( 'Special:Watchlist/', $a );
+                       }
+                       $this->assertContains( 'x=25', $a );
+               }
+
+               $i = 0;
+
+               if ( $offset > 0 ) {
+                       $this->assertContains(
+                               'limit=' . $limit . '&amp;offset=' . max( 0, $offset - $limit ) . '&amp;',
+                               $links[ $i ]
+                       );
+                       $this->assertContains( 'title="(prevn-title: ' . $limit . ')"', $links[$i] );
+                       $this->assertContains( 'class="mw-prevlink"', $links[$i] );
+                       $this->assertContains( '>(prevn: ' . $limit . ')<', $links[$i] );
+                       $i += 1;
+               }
+
+               if ( !$atEnd ) {
+                       $this->assertContains(
+                               'limit=' . $limit . '&amp;offset=' . ( $offset + $limit ) . '&amp;',
+                               $links[ $i ]
+                       );
+                       $this->assertContains( 'title="(nextn-title: ' . $limit . ')"', $links[$i] );
+                       $this->assertContains( 'class="mw-nextlink"', $links[$i] );
+                       $this->assertContains( '>(nextn: ' . $limit . ')<', $links[$i] );
+                       $i += 1;
+               }
+
+               $this->assertCount( 5 + $i, $links );
+
+               $this->assertContains( 'limit=20&amp;offset=' . $offset, $links[$i] );
+               $this->assertContains( 'title="(shown-title: 20)"', $links[$i] );
+               $this->assertContains( 'class="mw-numlink"', $links[$i] );
+               $this->assertContains( '>20<', $links[$i] );
+               $i += 4;
+
+               $this->assertContains( 'limit=500&amp;offset=' . $offset, $links[$i] );
+               $this->assertContains( 'title="(shown-title: 500)"', $links[$i] );
+               $this->assertContains( 'class="mw-numlink"', $links[$i] );
+               $this->assertContains( '>500<', $links[$i] );
+       }
+}
index ec4bf0f..783adce 100644 (file)
@@ -1,7 +1,5 @@
 <?php
 
-use Wikimedia\TestingAccessWrapper;
-
 /**
  * @covers SpecialPage
  *
@@ -102,83 +100,4 @@ class SpecialPageTest extends MediaWikiTestCase {
                // no exception thrown, logged in use can access special page
                $this->assertTrue( true );
        }
-
-       public function provideBuildPrevNextNavigation() {
-               yield [ 0, 20, false, false ];
-               yield [ 17, 20, false, false ];
-               yield [ 0, 17, false, false ];
-               yield [ 0, 20, true, 'Foo' ];
-               yield [ 17, 20, true, 'Föö_Bär' ];
-       }
-
-       /**
-        * @dataProvider provideBuildPrevNextNavigation
-        */
-       public function testBuildPrevNextNavigation( $offset, $limit, $atEnd, $subPage ) {
-               $this->setUserLang( Language::factory( 'qqx' ) ); // disable i18n
-
-               $specialPage = new SpecialPage( 'Watchlist' );
-               $specialPage = TestingAccessWrapper::newFromObject( $specialPage );
-
-               $html = $specialPage->buildPrevNextNavigation(
-                       $offset,
-                       $limit,
-                       [ 'x' => 25 ],
-                       $atEnd,
-                       $subPage
-               );
-
-               $this->assertStringStartsWith( '(viewprevnext:', $html );
-
-               preg_match_all( '!<a.*?</a>!', $html, $m, PREG_PATTERN_ORDER );
-               $links = $m[0];
-
-               foreach ( $links as $a ) {
-                       if ( $subPage ) {
-                               $this->assertContains( 'Special:Watchlist/' . wfUrlencode( $subPage ), $a );
-                       } else {
-                               $this->assertContains( 'Special:Watchlist', $a );
-                               $this->assertNotContains( 'Special:Watchlist/', $a );
-                       }
-                       $this->assertContains( 'x=25', $a );
-               }
-
-               $i = 0;
-
-               if ( $offset > 0 ) {
-                       $this->assertContains(
-                               'limit=' . $limit . '&amp;offset=' . max( 0, $offset - $limit ) . '&amp;',
-                               $links[ $i ]
-                       );
-                       $this->assertContains( 'title="(prevn-title: ' . $limit . ')"', $links[$i] );
-                       $this->assertContains( 'class="mw-prevlink"', $links[$i] );
-                       $this->assertContains( '>(prevn: ' . $limit . ')<', $links[$i] );
-                       $i += 1;
-               }
-
-               if ( !$atEnd ) {
-                       $this->assertContains(
-                               'limit=' . $limit . '&amp;offset=' . ( $offset + $limit ) . '&amp;',
-                               $links[ $i ]
-                       );
-                       $this->assertContains( 'title="(nextn-title: ' . $limit . ')"', $links[$i] );
-                       $this->assertContains( 'class="mw-nextlink"', $links[$i] );
-                       $this->assertContains( '>(nextn: ' . $limit . ')<', $links[$i] );
-                       $i += 1;
-               }
-
-               $this->assertCount( 5 + $i, $links );
-
-               $this->assertContains( 'limit=20&amp;offset=' . $offset, $links[$i] );
-               $this->assertContains( 'title="(shown-title: 20)"', $links[$i] );
-               $this->assertContains( 'class="mw-numlink"', $links[$i] );
-               $this->assertContains( '>20<', $links[$i] );
-               $i += 4;
-
-               $this->assertContains( 'limit=500&amp;offset=' . $offset, $links[$i] );
-               $this->assertContains( 'title="(shown-title: 500)"', $links[$i] );
-               $this->assertContains( 'class="mw-numlink"', $links[$i] );
-               $this->assertContains( '>500<', $links[$i] );
-       }
-
 }