jenkins-bot has submitted this change and it was merged.
Change subject: (bug 50478) Filter langlinks by site group.
......................................................................
(bug 50478) Filter langlinks by site group.
LangLinkHandler should not put links to other site groups
into the sidebar.
Change-Id: Icfd832ac4a36c463eecee021fce4ce59bf664c8b
---
M client/includes/LangLinkHandler.php
M client/tests/phpunit/includes/LangLinkHandlerTest.php
M lib/tests/phpunit/MockSiteStore.php
3 files changed, 199 insertions(+), 19 deletions(-)
Approvals:
Hoo man: Looks good to me, approved
jenkins-bot: Verified
diff --git a/client/includes/LangLinkHandler.php
b/client/includes/LangLinkHandler.php
index a293038..96deb10 100644
--- a/client/includes/LangLinkHandler.php
+++ b/client/includes/LangLinkHandler.php
@@ -1,6 +1,7 @@
<?php
namespace Wikibase;
+use MWException;
use SiteStore;
use Sites;
use Site;
@@ -156,7 +157,7 @@
* @param ParserOutput $out
* @param array $repoLinks An array that uses global site IDs as keys.
*
- * @return array A filtered copy of $repoLinks, which any inappropriate
+ * @return array A filtered copy of $repoLinks, with any inappropriate
* entries removed.
*/
public function suppressRepoLinks( ParserOutput $out, $repoLinks ) {
@@ -165,6 +166,11 @@
$nel = $this->getNoExternalLangLinks( $out );
foreach ( $nel as $code ) {
+ if ( $code === '*' ) {
+ // all are suppressed
+ return array();
+ }
+
$site = $this->getSiteByNavigationId( $code );
if ( $site === false ) {
@@ -176,6 +182,42 @@
}
unset( $repoLinks[$this->siteId] ); // remove self-link
+
+ wfProfileOut( __METHOD__ );
+ return $repoLinks;
+ }
+
+ /**
+ * Filters the given list of links by site group:
+ * Any links pointing to a site that is not in $allowedGroups will be
removed.
+ *
+ * @since 0.4
+ *
+ * @param array $repoLinks An array that uses global site IDs as keys.
+ * @param array $allowedGroups A list of allowed site groups
+ *
+ * @return array A filtered copy of $repoLinks, retaining only the links
+ * pointing to a site in an allowed group.
+ */
+ public function filterRepoLinksByGroup( array $repoLinks, array
$allowedGroups ) {
+ wfProfileIn( __METHOD__ );
+
+ foreach ( $repoLinks as $wiki => $page ) {
+ $site = $this->sites->getSite( $wiki );
+
+ if ( $site === null ) {
+ wfDebugLog( __CLASS__, __FUNCTION__ . ':
skipping link to unknown site ' . $wiki );
+
+ unset( $repoLinks[$wiki] );
+ continue;
+ }
+
+ if ( !in_array( $site->getGroup(), $allowedGroups ) ) {
+ wfDebugLog( __CLASS__, __FUNCTION__ . ':
skipping link to other group: ' . $wiki . ' belongs to ' . $site->getGroup() );
+ unset( $repoLinks[$wiki] );
+ continue;
+ }
+ }
wfProfileOut( __METHOD__ );
return $repoLinks;
@@ -331,7 +373,7 @@
* {{#noexternallanglinks}} function on the page.
*
* The result is an associative array of links that should be added to
the
- * current page, excluding any target languages for which there already
is a
+ * current page, excluding any target sites for which there already is a
* link on the page.
*
* @since 0.4
@@ -350,12 +392,16 @@
return array();
}
+ //TODO: make $allowedGroups configurable
+ $allowedGroups = array( $this->getSiteGroup() );
+
$onPageLinks = $out->getLanguageLinks();
$onPageLinks = $this->localLinksToArray( $onPageLinks );
$repoLinks = $this->getEntityLinks( $title );
-
$repoLinks = $this->repoLinksToArray( $repoLinks );
+
+ $repoLinks = $this->filterRepoLinksByGroup( $repoLinks,
$allowedGroups );
$repoLinks = $this->suppressRepoLinks( $out, $repoLinks );
$repoLinks = array_diff_key( $repoLinks, $onPageLinks ); //
remove local links
@@ -384,7 +430,7 @@
foreach ( $repoLinks as $wiki => $page ) {
$targetSite = $this->sites->getSite( $wiki );
if ( !$targetSite ) {
- trigger_error( "Unknown wiki '$wiki' used as
sitelink target", E_USER_WARNING );
+ wfLogWarning( "Unknown wiki '$wiki' used as
sitelink target" );
continue;
}
@@ -405,6 +451,22 @@
}
/**
+ * Returns the local wiki's site group.
+ * This is based on the siteId provided to the constructor.
+ *
+ * @return string
+ * @throws \MWException
+ */
+ public function getSiteGroup() {
+ $thisSite = $this->sites->getSite( $this->siteId );
+ if ( !$thisSite ) {
+ throw new MWException( "Unable to resolve site ID
'{$this->siteId}'!" );
+ }
+
+ return $thisSite->getGroup();
+ }
+
+ /**
* Add wikibase_item parser output property
*
* @since 0.4
diff --git a/client/tests/phpunit/includes/LangLinkHandlerTest.php
b/client/tests/phpunit/includes/LangLinkHandlerTest.php
index ed07fd2..a18c1eb 100644
--- a/client/tests/phpunit/includes/LangLinkHandlerTest.php
+++ b/client/tests/phpunit/includes/LangLinkHandlerTest.php
@@ -1,8 +1,7 @@
<?php
namespace Wikibase\Test;
-use \Wikibase\LangLinkHandler;
-use \Wikibase\SiteLink;
+use Wikibase\LangLinkHandler;
/**
* Tests for the LangLinkHandler class.
@@ -44,16 +43,18 @@
protected $langLinkHandler;
static $itemData = array(
- array( // matching item
+ 1 => array( // matching item
'id' => 1,
'label' => array( 'en' => 'Foo' ),
'links' => array(
'dewiki' => 'Foo de',
'enwiki' => 'Foo en',
'srwiki' => 'Foo sr',
+ 'dewiktionary' => 'Foo de word',
+ 'enwiktionary' => 'Foo en word',
)
),
- array( // matches, but not in a namespace with external
langlinks enabled
+ 2 => array( // matches, but not in a namespace with external
langlinks enabled
'id' => 2,
'label' => array( 'en' => 'Talk:Foo' ),
'links' => array(
@@ -80,12 +81,14 @@
$this->mockRepo->putEntity( $item );
}
+ $sites = MockSiteStore::newFromTestSites();
+
$this->langLinkHandler = new LangLinkHandler(
'srwiki',
array(),
array( NS_TALK ),
$this->mockRepo,
- MockSiteStore::newFromTestSites()
+ $sites
);
}
@@ -101,6 +104,8 @@
'dewiki' => 'Foo de',
'enwiki' => 'Foo en',
'srwiki' => 'Foo sr',
+ 'dewiktionary' => 'Foo de word',
+ 'enwiktionary' => 'Foo en word',
)
),
);
@@ -367,4 +372,124 @@
return $links;
}
+
+ public static function provideGetSiteGroup() {
+ return array(
+ array( 'dewiki', 'wikipedia' ),
+ array( 'enwiktionary', 'wiktionary' ),
+ );
+ }
+
+ /**
+ * @dataProvider provideGetSiteGroup
+ */
+ public function testGetSiteGroup( $siteId, $group ) {
+ $handler = new LangLinkHandler(
+ $siteId,
+ array(),
+ array( NS_TALK ),
+ $this->mockRepo,
+ MockSiteStore::newFromTestSites()
+ );
+
+ $this->assertEquals( $group, $handler->getSiteGroup() );
+ }
+
+ public static function provideFilterRepoLinksByGroup() {
+ return array(
+ array( // #0: nothing
+ array(), array(), array()
+ ),
+ array( // #1: nothing allowed
+ array(
+ 'dewiki' => 'Foo de',
+ 'enwiki' => 'Foo en',
+ 'srwiki' => 'Foo sr',
+ 'dewiktionary' => 'Foo de word',
+ 'enwiktionary' => 'Foo en word',
+ ),
+ array(),
+ array()
+ ),
+ array( // #2: nothing there
+ array(),
+ array( 'wikipedia' ),
+ array()
+ ),
+ array( // #3: wikipedia only
+ array(
+ 'dewiki' => 'Foo de',
+ 'enwiki' => 'Foo en',
+ 'srwiki' => 'Foo sr',
+ 'dewiktionary' => 'Foo de word',
+ 'enwiktionary' => 'Foo en word',
+ ),
+ array( 'wikipedia' ),
+ array(
+ 'dewiki' => 'Foo de',
+ 'enwiki' => 'Foo en',
+ 'srwiki' => 'Foo sr',
+ ),
+ ),
+ );
+ }
+
+ /**
+ * @dataProvider provideFilterRepoLinksByGroup
+ */
+ public function testFilterRepoLinksByGroup( $repoLinks, $allowedGroups,
$expectedLinks ) {
+ $actualLinks = $this->langLinkHandler->filterRepoLinksByGroup(
$repoLinks, $allowedGroups );
+
+ $this->assertEquals( $expectedLinks, $actualLinks );
+ }
+
+ public static function provideSuppressRepoLinks() {
+ return array(
+ array( // #0: nothing
+ array(), array(), array()
+ ),
+ array( // #1: nothing allowed
+ array(
+ 'dewiki' => 'Foo de',
+ 'enwiki' => 'Foo en',
+ 'srwiki' => 'Foo sr',
+ 'dewiktionary' => 'Foo de word',
+ 'enwiktionary' => 'Foo en word',
+ ),
+ array( '*' ),
+ array()
+ ),
+ array( // #2: nothing there
+ array(),
+ array( 'de' ),
+ array()
+ ),
+ array( // #3: no de
+ array(
+ 'dewiki' => 'Foo de',
+ 'enwiki' => 'Foo en',
+ 'srwiki' => 'Foo sr',
+ 'enwiktionary' => 'Foo en word',
+ ),
+ array( 'de' ),
+ array(
+ 'enwiki' => 'Foo en',
+ //NOTE: srwiki is removed because
that's a self-link
+ 'enwiktionary' => 'Foo en word',
+ ),
+ ),
+ );
+ }
+
+ /**
+ * @dataProvider provideSuppressRepoLinks
+ */
+ public function testSuppressRepoLinks( $repoLinks, $nel, $expectedLinks
) {
+ $out = new \ParserOutput();
+ $out->setProperty( 'noexternallanglinks', serialize( $nel ) );
+
+ $actualLinks = $this->langLinkHandler->suppressRepoLinks( $out,
$repoLinks );
+
+ $this->assertEquals( $expectedLinks, $actualLinks );
+ }
}
diff --git a/lib/tests/phpunit/MockSiteStore.php
b/lib/tests/phpunit/MockSiteStore.php
index c5c0d01..0d08fd9 100644
--- a/lib/tests/phpunit/MockSiteStore.php
+++ b/lib/tests/phpunit/MockSiteStore.php
@@ -39,7 +39,7 @@
/**
* @var Site[]
*/
- private $sites;
+ private $sites = array();
/**
* Returns a SiteStore object that contains TestSites::getSites().
@@ -48,14 +48,7 @@
* @return SiteStore
*/
public static function newFromTestSites() {
- $testSites = TestSites::getSites();
-
- $sites = array();
- foreach( $testSites as $site ) {
- $sites[$site->getGlobalId()] = $site;
- }
-
- $store = new MockSiteStore( $sites );
+ $store = new MockSiteStore( TestSites::getSites() );
return $store;
}
@@ -63,7 +56,7 @@
* @param array $sites
*/
public function __construct( $sites = array() ) {
- $this->sites = $sites;
+ $this->saveSites( $sites );
}
/**
--
To view, visit https://gerrit.wikimedia.org/r/71302
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Icfd832ac4a36c463eecee021fce4ce59bf664c8b
Gerrit-PatchSet: 8
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Aude <[email protected]>
Gerrit-Reviewer: Denny Vrandecic <[email protected]>
Gerrit-Reviewer: Hoo man <[email protected]>
Gerrit-Reviewer: Jeroen De Dauw <[email protected]>
Gerrit-Reviewer: Tobias Gritschacher <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
