From 4ca72763ecf97f44f45b28f5c37c1db1e5c9fc3e Mon Sep 17 00:00:00 2001 From: clarakosi Date: Wed, 29 May 2019 19:08:11 -0400 Subject: [PATCH] Refactor buildPrevNextNavigation Refactored buildPrevNextNavigation() into standalone helper class, PrevNextNavigationRenderer, to be used by both SpecialPages and Pagers. Bug:T207977 Change-Id: Ic49837a451f795ec203e867961ec1c69075cc91a --- RELEASE-NOTES-1.34 | 2 + autoload.php | 1 + .../Navigation/PrevNextNavigationRenderer.php | 111 ++++++++++++++++++ includes/pager/IndexPager.php | 20 ++++ includes/specialpage/SpecialPage.php | 56 +-------- includes/specials/SpecialSearch.php | 4 +- languages/Language.php | 2 +- .../PrevNextNavigationRendererTest.php | 88 ++++++++++++++ .../includes/specialpage/SpecialPageTest.php | 81 ------------- 9 files changed, 230 insertions(+), 135 deletions(-) create mode 100644 includes/Navigation/PrevNextNavigationRenderer.php create mode 100644 tests/phpunit/includes/Navigation/PrevNextNavigationRendererTest.php diff --git a/RELEASE-NOTES-1.34 b/RELEASE-NOTES-1.34 index d372db8ec2..068575f68e 100644 --- a/RELEASE-NOTES-1.34 +++ b/RELEASE-NOTES-1.34 @@ -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 === * … diff --git a/autoload.php b/autoload.php index a9b65faceb..9cde06652c 100644 --- a/autoload.php +++ b/autoload.php @@ -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 index 0000000000..c60b8c6130 --- /dev/null +++ b/includes/Navigation/PrevNextNavigationRenderer.php @@ -0,0 +1,111 @@ +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 ); + } + +} diff --git a/includes/pager/IndexPager.php b/includes/pager/IndexPager.php index 64dbb228a4..04021cc038 100644 --- a/includes/pager/IndexPager.php +++ b/includes/pager/IndexPager.php @@ -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 ); + } } diff --git a/includes/specialpage/SpecialPage.php b/includes/specialpage/SpecialPage.php index 670a0b8c98..b4e244ceee 100644 --- a/includes/specialpage/SpecialPage.php +++ b/includes/specialpage/SpecialPage.php @@ -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 ); } } diff --git a/includes/specials/SpecialSearch.php b/includes/specials/SpecialSearch.php index 49f26ad20d..e1fbe6a488 100644 --- a/includes/specials/SpecialSearch.php +++ b/includes/specials/SpecialSearch.php @@ -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( "

{$prevnext}

\n" ); + $out->addHTML( "

{$prevNext}

\n" ); } // Close
diff --git a/languages/Language.php b/languages/Language.php index 6c8b19ed80..4e663c250c 100644 --- a/languages/Language.php +++ b/languages/Language.php @@ -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 index 0000000000..be5ab03870 --- /dev/null +++ b/tests/phpunit/includes/Navigation/PrevNextNavigationRendererTest.php @@ -0,0 +1,88 @@ +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( '!!', $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 . '&offset=' . max( 0, $offset - $limit ) . '&', + $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 . '&offset=' . ( $offset + $limit ) . '&', + $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&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&offset=' . $offset, $links[$i] ); + $this->assertContains( 'title="(shown-title: 500)"', $links[$i] ); + $this->assertContains( 'class="mw-numlink"', $links[$i] ); + $this->assertContains( '>500<', $links[$i] ); + } +} diff --git a/tests/phpunit/includes/specialpage/SpecialPageTest.php b/tests/phpunit/includes/specialpage/SpecialPageTest.php index ec4bf0fc43..783adce4da 100644 --- a/tests/phpunit/includes/specialpage/SpecialPageTest.php +++ b/tests/phpunit/includes/specialpage/SpecialPageTest.php @@ -1,7 +1,5 @@ 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( '!!', $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 . '&offset=' . max( 0, $offset - $limit ) . '&', - $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 . '&offset=' . ( $offset + $limit ) . '&', - $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&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&offset=' . $offset, $links[$i] ); - $this->assertContains( 'title="(shown-title: 500)"', $links[$i] ); - $this->assertContains( 'class="mw-numlink"', $links[$i] ); - $this->assertContains( '>500<', $links[$i] ); - } - } -- 2.20.1