jenkins-bot has submitted this change and it was merged.

Change subject: Fix handling of the specialSiteLinkGroups setting
......................................................................


Fix handling of the specialSiteLinkGroups setting

Also add some documentation.

Change-Id: I5004ee8e823ed0cd245793c61d54246a4805e618
---
M docs/options.wiki
M lib/includes/modules/SitesModule.php
M lib/resources/wikibase.ui.SiteLinksEditTool.js
M repo/i18n/en.json
M repo/i18n/qqq.json
M repo/includes/View/SiteLinksView.php
M repo/includes/api/GetEntities.php
M repo/includes/api/LinkTitles.php
M repo/includes/api/ModifyEntity.php
M repo/includes/api/SiteLinkTargetProvider.php
M repo/tests/phpunit/includes/View/SiteLinksViewTest.php
M repo/tests/phpunit/includes/api/SiteLinkTargetProviderTest.php
12 files changed, 157 insertions(+), 63 deletions(-)

Approvals:
  Hoo man: Looks good to me, approved
  WikidataJenkins: Verified
  jenkins-bot: Verified



diff --git a/docs/options.wiki b/docs/options.wiki
index e839424..4621249 100644
--- a/docs/options.wiki
+++ b/docs/options.wiki
@@ -10,6 +10,7 @@
 
 ;changesDatabase: The database that changes are recorded to for processing by 
clients. This must be set to a symbolic database identifier that MediaWiki's 
LBFactory class understands; <code>false</code> means that the wiki's own 
database shall be used. '''Note''' that on the client, this setting should 
usually be the same as the <code>repoDatabase</code> setting.
 ;siteLinkGroups: The site groups to use in sitelinks. Must correspond to a 
value used to give the site group in the MediaWiki <code>sites</code> table. 
Default is array( "wikipedia" ). This defines which groups of sites can be 
linked to Wikibase items. '''Note''' that this setting replaces the old 
''siteLinkGroup'' setting, which only allowed for a single group.
+;specialSiteLinkGroups: This maps one or more site groups into a single 
"special" group. This is useful if sites from multiple site groups should be 
shown in a single "special" section on item pages, instead of one section per 
site group.
 ;localClientDatabases: An array of locally accessible client databases, for 
use by the <code>dispatchChanges.php</code> script. This setting determines 
which wikis changes are pushed to directly. It must be given either as an 
associative array, mapping global site IDs to logical database names, or, of 
the database names are the same as the site IDs, as a list of databases. The 
default is an empty array, indicating no local client databases.
 
 === Expert Settings ===
diff --git a/lib/includes/modules/SitesModule.php 
b/lib/includes/modules/SitesModule.php
index 1747a1c..4abc461 100644
--- a/lib/includes/modules/SitesModule.php
+++ b/lib/includes/modules/SitesModule.php
@@ -41,44 +41,70 @@
                foreach ( SiteSQLStore::newInstance()->getSites() as $site ) {
                        $group = $site->getGroup();
 
-                       if ( $site->getType() === Site::TYPE_MEDIAWIKI && 
in_array( $group, $groups ) ) {
-                               // FIXME: quickfix to allow a custom site-name 
/ handling for groups defined in $wgSpecialSiteLinkGroups
-                               if ( in_array( $group, $specialGroups ) ) {
-                                       $languageNameMsg = wfMessage( 
'wikibase-sitelinks-sitename-' . $site->getGlobalId() );
-                                       $languageName = 
$languageNameMsg->exists() ? $languageNameMsg->parse() : $site->getGlobalId();
-                                       $groupName = 'special';
-                               } else {
-                                       $languageName = 
Utils::fetchLanguageName( $site->getLanguageCode() );
-                                       $groupName = $group;
-                               }
-                               $globalId = $site->getGlobalId();
-
-                               // Use protocol relative URIs, as it's safe to 
assume that all wikis support the same protocol
-                               list( $pageUrl, $apiUrl ) = preg_replace(
-                                       "/^https?:/i",
-                                       '',
-                                       array(
-                                               $site->getPageUrl(),
-                                               $site->getFileUrl( 'api.php' )
-                                       )
-                               );
-
-                               //TODO: figure out which name ist best
-                               //$localIds = $site->getLocalIds();
-                               //$name = empty( $localIds['equivalent'] ) ? 
$site->getGlobalId() : $localIds['equivalent'][0];
-
-                               $sites[ $globalId ] = array(
-                                       'shortName' => $languageName,
-                                       'name' => $languageName, // use short 
name for both, for now
-                                       'id' => $globalId,
-                                       'pageUrl' => $pageUrl,
-                                       'apiUrl' => $apiUrl,
-                                       'languageCode' => 
$site->getLanguageCode(),
-                                       'group' => $groupName
-                               );
+                       if ( !$this->shouldSiteBeIncluded( $site, $groups, 
$specialGroups ) ) {
+                               continue;
                        }
+
+                       // FIXME: quickfix to allow a custom site-name / 
handling for the site groups which are
+                       // special according to the specialSiteLinkGroups 
setting
+                       if ( in_array( $group, $specialGroups ) ) {
+                               $languageNameMsg = wfMessage( 
'wikibase-sitelinks-sitename-' . $site->getGlobalId() );
+                               $languageName = $languageNameMsg->exists() ? 
$languageNameMsg->parse() : $site->getGlobalId();
+                               $groupName = 'special';
+                       } else {
+                               $languageName = Utils::fetchLanguageName( 
$site->getLanguageCode() );
+                               $groupName = $group;
+                       }
+                       $globalId = $site->getGlobalId();
+
+                       // Use protocol relative URIs, as it's safe to assume 
that all wikis support the same protocol
+                       list( $pageUrl, $apiUrl ) = preg_replace(
+                               "/^https?:/i",
+                               '',
+                               array(
+                                       $site->getPageUrl(),
+                                       $site->getFileUrl( 'api.php' )
+                               )
+                       );
+
+                       //TODO: figure out which name ist best
+                       //$localIds = $site->getLocalIds();
+                       //$name = empty( $localIds['equivalent'] ) ? 
$site->getGlobalId() : $localIds['equivalent'][0];
+
+                       $sites[ $globalId ] = array(
+                               'shortName' => $languageName,
+                               'name' => $languageName, // use short name for 
both, for now
+                               'id' => $globalId,
+                               'pageUrl' => $pageUrl,
+                               'apiUrl' => $apiUrl,
+                               'languageCode' => $site->getLanguageCode(),
+                               'group' => $groupName
+                       );
                }
 
                return 'mediaWiki.config.set( "wbSiteDetails", ' . 
\FormatJson::encode( $sites ) . ' );';
        }
+
+       /**
+        * Whether it's needed to add a Site to the JS variable.
+        *
+        * @param Site $site
+        * @param array $groups
+        * @param array $specialGroups
+        *
+        * @return bool
+        */
+       private function shouldSiteBeIncluded( Site $site, array $groups, array 
$specialGroups ) {
+               if ( in_array( 'special', $groups ) ) {
+                       // The "special" group actually maps to multiple groups
+                       $groups = array_diff( $groups, array( 'special' ) );
+                       $groups = array_merge( $groups, $specialGroups );
+               }
+
+               if ( $site->getType() === Site::TYPE_MEDIAWIKI && in_array( 
$site->getGroup(), $groups ) ) {
+                       return true;
+               }
+
+               return false;
+       }
 }
diff --git a/lib/resources/wikibase.ui.SiteLinksEditTool.js 
b/lib/resources/wikibase.ui.SiteLinksEditTool.js
index 9eeeea6..92c892b 100644
--- a/lib/resources/wikibase.ui.SiteLinksEditTool.js
+++ b/lib/resources/wikibase.ui.SiteLinksEditTool.js
@@ -179,7 +179,8 @@
                if ( this._editableValues.length === 0 ) {
                        var siteNameMessageKey = 
'wikibase-sitelinks-sitename-columnheading';
 
-                       // FIXME: quickfix to allow a custom site-name / 
handling for groups defined in $wgSpecialSiteLinkGroups
+                       // FIXME: quickfix to allow a custom site-name / 
handling for the site groups which are
+                       // special according to the specialSiteLinkGroups 
setting
                        if ( $siteLinksTable.data( 'wb-sitelinks-group' ) === 
'special' ) {
                                siteNameMessageKey += '-special';
                        }
diff --git a/repo/i18n/en.json b/repo/i18n/en.json
index 766354c..c57e432 100644
--- a/repo/i18n/en.json
+++ b/repo/i18n/en.json
@@ -38,6 +38,7 @@
        "wikibase-terms": "In other languages",
        "wikibase-sitelinks-empty": "No page is linked to this item yet.",
        "wikibase-sitelinks-input-help-message": "Set a link to a page related 
to this item.",
+       "wikibase-sitelinks-special": "Pages on other sites linked to this 
item",
        "wikibase-remove": "remove",
        "wikibase-move-up": "move up",
        "wikibase-move-down": "move down",
diff --git a/repo/i18n/qqq.json b/repo/i18n/qqq.json
index 9570116..899039c 100644
--- a/repo/i18n/qqq.json
+++ b/repo/i18n/qqq.json
@@ -61,6 +61,7 @@
        "wikibase-terms": "Heading for the table with the labels and 
descriptions in other languages.\n{{Identical|Otherlanguages}}",
        "wikibase-sitelinks-empty": "There are no site links for any of the 
language specific pages on the given cluster.  See also Wikidatas glossary for 
[[d:Wikidata:Glossary#sitelinks|sitelinks]] and 
[[d:Wikidata:Glossary#sitelinks-title|title]].",
        "wikibase-sitelinks-input-help-message": "[[File:Screenshot 
WikidataRepo 2012-05-13 D.png|right|0x150px]]\nBubble help message to set a 
sitelink to a language specific page on a given cluster. See also Wikidatas 
glossary for [[d:Wikidata:Glossary#sitelinks|site links]] and 
[[d:Wikidata:Glossary#sitelinks-title|title]].\n\nParameters:\n* $1 - (Unused) 
language name\n{{Related|Wikibase-input-help-message}}",
+       "wikibase-sitelinks-special": "Section header for a section containing 
links to various sites being arbitrary grouped.",
        "wikibase-remove": "[[File:Screenshot WikidataRepo 2012-05-13 
A.png|right|0x150px]]\nThis is a generic text used for a link (fig. 3 on 
[[m:Wikidata/Notes/JavaScript ui implementation]]) that removes an element of 
some kind, without the the user interface being put in edit mode.",
        "wikibase-move-up": "Label of a link to move a list item one step up 
within an ordered list. The label should be a rather generic expression since 
it is used for lists featuring various content.\n\nSee also:\n* 
{{msg-mw|Wikibase-move-down}}\n{{Identical|Move up}}",
        "wikibase-move-down": "Label of a link to move a list item one step 
down within an ordered list. The label should be a rather generic expression 
since it is used for lists featuring various content.\n\nSee also:\n* 
{{msg-mw|Wikibase-move-up}}\n{{Identical|Move down}}",
diff --git a/repo/includes/View/SiteLinksView.php 
b/repo/includes/View/SiteLinksView.php
index 4e61753..9a2d11e 100644
--- a/repo/includes/View/SiteLinksView.php
+++ b/repo/includes/View/SiteLinksView.php
@@ -71,9 +71,9 @@
         * @return string
         */
        private function getHtmlForSiteLinkGroup( array $siteLinks, $itemId, 
$group ) {
-               $isSpecialGroup = $this->siteLinkGroupIsSpecial( $group );
+               $isSpecialGroup = $group === 'special';
 
-               $sites = $this->sites->getGroup( $group );
+               $sites = $this->getSitesForGroup( $group );
                $siteLinksForTable = $this->getSiteLinksForTable( $sites, 
$siteLinks );
 
                $html = $thead = $tbody = $tfoot = '';
@@ -97,19 +97,40 @@
                $isFull = count( $siteLinksForTable ) >= count( $sites );
                $tfoot = $this->getTableFootHtml( $itemId, $isFull );
 
-               $groupName = $isSpecialGroup ? 'special' : $group;
-
                return $html . wfTemplate(
                        'wb-sitelinks-table',
                        $thead,
                        $tbody,
                        $tfoot,
-                       htmlspecialchars( $groupName )
+                       htmlspecialchars( $group )
                );
        }
 
-       private function siteLinkGroupIsSpecial( $groupName ) {
-               return in_array( $groupName, $this->specialSiteLinkGroups );
+       /**
+        * Get all sites for a given site group, with special handling for the
+        * "special" site group.
+        *
+        * @param string $group
+        *
+        * @return SiteList
+        */
+       private function getSitesForGroup( $group ) {
+               $siteList = new SiteList();
+
+               if ( $group === 'special' ) {
+                       $groups = $this->specialSiteLinkGroups;
+               } else {
+                       $groups = array( $group );
+               }
+
+               foreach ( $groups as $group ) {
+                       $sites = $this->sites->getGroup( $group );
+                       foreach ( $sites as $site ) {
+                               $siteList->setSite( $site );
+                       }
+               }
+
+               return $siteList;
        }
 
        /**
@@ -157,7 +178,8 @@
         * @return string
         */
        private function getTableHeadHtml( $isSpecialGroup ) {
-               // FIXME: quickfix to allow a custom site-name / handling for 
groups defined in $wgSpecialSiteLinkGroups
+               // FIXME: quickfix to allow a custom site-name / handling for 
the site groups which are
+               // special according to the specialSiteLinkGroups setting
                $siteNameMessageKey = 
'wikibase-sitelinks-sitename-columnheading';
                if ( $isSpecialGroup ) {
                        $siteNameMessageKey .= '-special';
@@ -231,7 +253,8 @@
                $escapedPageName = htmlspecialchars( $pageName );
                $escapedSiteId = htmlspecialchars( $siteId );
 
-               // FIXME: this is a quickfix to allow a custom site-name for 
groups defined in $wgSpecialSiteLinkGroups instead of showing the language-name
+               // FIXME: this is a quickfix to allow a custom site-name for 
the site groups which are
+               // special according to the specialSiteLinkGroups setting
                if ( $isSpecialGroup ) {
                        // FIXME: not escaped?
                        $siteNameMsg = wfMessage( 
'wikibase-sitelinks-sitename-' . $siteId );
diff --git a/repo/includes/api/GetEntities.php 
b/repo/includes/api/GetEntities.php
index b9c9415..3328a60 100644
--- a/repo/includes/api/GetEntities.php
+++ b/repo/includes/api/GetEntities.php
@@ -62,7 +62,12 @@
 
                $this->stringNormalizer = $wikibaseRepo->getStringNormalizer();
                $this->languageFallbackChainFactory = 
$wikibaseRepo->getLanguageFallbackChainFactory();
-               $this->siteLinkTargetProvider = new SiteLinkTargetProvider( 
$wikibaseRepo->getSiteStore() );
+
+               $this->siteLinkTargetProvider = new SiteLinkTargetProvider(
+                       $wikibaseRepo->getSiteStore(),
+                       $wikibaseRepo->getSettings()->getSetting( 
'specialSiteLinkGroups' )
+               );
+
                $this->siteLinkGroups = 
$wikibaseRepo->getSettings()->getSetting( 'siteLinkGroups' );
        }
 
diff --git a/repo/includes/api/LinkTitles.php b/repo/includes/api/LinkTitles.php
index 47caf29..3167a8b 100644
--- a/repo/includes/api/LinkTitles.php
+++ b/repo/includes/api/LinkTitles.php
@@ -44,7 +44,11 @@
                parent::__construct( $mainModule, $moduleName, $modulePrefix );
                $wikibaseRepo = WikibaseRepo::getDefaultInstance();
 
-               $this->siteLinkTargetProvider = new SiteLinkTargetProvider( 
$wikibaseRepo->getSiteStore() );
+               $this->siteLinkTargetProvider = new SiteLinkTargetProvider(
+                       $wikibaseRepo->getSiteStore(),
+                       $wikibaseRepo->getSettings()->getSetting( 
'specialSiteLinkGroups' )
+               );
+
                $this->siteLinkGroups = 
$wikibaseRepo->getSettings()->getSetting( 'siteLinkGroups' );
        }
 
diff --git a/repo/includes/api/ModifyEntity.php 
b/repo/includes/api/ModifyEntity.php
index 0157d19..cbdaa1d 100644
--- a/repo/includes/api/ModifyEntity.php
+++ b/repo/includes/api/ModifyEntity.php
@@ -74,7 +74,11 @@
 
                //TODO: provide a mechanism to override the services
                $this->stringNormalizer = $repo->getStringNormalizer();
-               $this->siteLinkTargetProvider = new SiteLinkTargetProvider( 
$repo->getSiteStore() );
+
+               $this->siteLinkTargetProvider = new SiteLinkTargetProvider(
+                       $repo->getSiteStore(),
+                       $repo->getSettings()->getSetting( 
'specialSiteLinkGroups' )
+               );
 
                $this->siteLinkGroups = $repo->getSettings()->getSetting( 
'siteLinkGroups' );
                $this->siteLinkLookup = $repo->getStore()->newSiteLinkCache();
diff --git a/repo/includes/api/SiteLinkTargetProvider.php 
b/repo/includes/api/SiteLinkTargetProvider.php
index 2d61dfc..59a652a 100644
--- a/repo/includes/api/SiteLinkTargetProvider.php
+++ b/repo/includes/api/SiteLinkTargetProvider.php
@@ -13,6 +13,7 @@
  *
  * @author Daniel K
  * @author Adam Shorland
+ * @author Marius Hoch < h...@online.de >
  */
 class SiteLinkTargetProvider {
 
@@ -22,10 +23,17 @@
        protected $siteStore;
 
        /**
-        * @param SiteStore $siteStore
+        * @var array
         */
-       public function __construct( SiteStore $siteStore ) {
+       private $specialSiteGroups;
+
+       /**
+        * @param SiteStore $siteStore
+        * @param array $specialSiteGroups
+        */
+       public function __construct( SiteStore $siteStore, array 
$specialSiteGroups ) {
                $this->siteStore = $siteStore;
+               $this->specialSiteGroups = $specialSiteGroups;
        }
 
        /**
@@ -36,6 +44,10 @@
         * @return SiteList
         */
        public function getSiteList( array $groups ) {
+               // As the special sitelink group actually just wraps multiple 
groups
+               // into one we have to replace it with the actual groups
+               $this->substituteSpecialSiteGroups( $groups );
+
                $sites = new SiteList();
                $allSites = $this->siteStore->getSites();
 
@@ -49,4 +61,15 @@
                return $sites;
        }
 
-}
\ No newline at end of file
+       /**
+        * @param array &$groups
+        */
+       private function substituteSpecialSiteGroups( &$groups ) {
+               if ( !in_array( 'special', $groups ) ) {
+                       return;
+               }
+
+               $groups = array_diff( $groups, array( 'special' ) );
+               $groups = array_merge( $groups, $this->specialSiteGroups );
+       }
+}
diff --git a/repo/tests/phpunit/includes/View/SiteLinksViewTest.php 
b/repo/tests/phpunit/includes/View/SiteLinksViewTest.php
index f01c09f..80a615b 100644
--- a/repo/tests/phpunit/includes/View/SiteLinksViewTest.php
+++ b/repo/tests/phpunit/includes/View/SiteLinksViewTest.php
@@ -95,7 +95,7 @@
 
                $testCases[] = array(
                        $item,
-                       array( 'special group' ),
+                       array( 'special' ),
                        true,
                        array(
                                'tag' => 'table',
@@ -116,7 +116,7 @@
 
                $testCases[] = array(
                        $item,
-                       array( 'wikipedia', 'special group' ),
+                       array( 'wikipedia', 'special' ),
                        true,
                        array(
                                'tag' => 'table',
diff --git a/repo/tests/phpunit/includes/api/SiteLinkTargetProviderTest.php 
b/repo/tests/phpunit/includes/api/SiteLinkTargetProviderTest.php
index a673b61..e0a1a01 100644
--- a/repo/tests/phpunit/includes/api/SiteLinkTargetProviderTest.php
+++ b/repo/tests/phpunit/includes/api/SiteLinkTargetProviderTest.php
@@ -14,14 +14,15 @@
  *
  * @licence GNU GPL v2+
  * @author Adam Shorland
+ * @author Marius Hoch < h...@online.de >
  */
 class SiteLinkTargetProviderTest extends \PHPUnit_Framework_TestCase {
 
        /**
         * @dataProvider provideExpected
         */
-       public function testGetSiteList( $groups, $expectedGlobalIds ) {
-               $provider = new SiteLinkTargetProvider( 
$this->getMockSiteStore() );
+       public function testGetSiteList( $groups, $specialGroups, 
$expectedGlobalIds ) {
+               $provider = new SiteLinkTargetProvider( 
$this->getMockSiteStore(), $specialGroups );
 
                $siteList = $provider->getSiteList( $groups );
 
@@ -33,14 +34,18 @@
 
        public static function provideExpected() {
                return array(
-                       //groupsToGet, siteIdsExpected
-                       array( array( 'foo' ), array( 'site1', 'site2' ) ),
-                       array( array( 'bar' ), array( 'site3' ) ),
-                       array( array( 'baz' ), array( 'site4' ) ),
-                       array( array( 'qwerty' ), array() ),
-                       array( array( 'foo', 'bar' ), array( 'site1', 'site2', 
'site3' ) ),
-                       array( array( 'foo', 'baz' ), array( 'site1', 'site2', 
'site4' ) ),
-                       array( array(), array() ),
+                       //groupsToGet, specialGroups, siteIdsExpected
+                       array( array( 'foo' ), array(), array( 'site1', 'site2' 
) ),
+                       array( array( 'bar' ), array(), array( 'site3' ) ),
+                       array( array( 'baz' ), array(), array( 'site4' ) ),
+                       array( array( 'qwerty' ), array(), array() ),
+                       array( array( 'foo', 'bar' ), array(), array( 'site1', 
'site2', 'site3' ) ),
+                       array( array( 'foo', 'baz' ), array(), array( 'site1', 
'site2', 'site4' ) ),
+                       array( array( 'special' ), array( 'bar' ), array( 
'site3' ) ),
+                       array( array( 'foo' ), array( 'bar' ), array( 'site1', 
'site2' ) ),
+                       array( array( 'special', 'foo' ), array( 'bar', 'baz' 
), array( 'site1', 'site2', 'site3', 'site4' ) ),
+                       array( array(), array( 'foo' ), array() ),
+                       array( array(), array(), array() ),
                );
        }
 

-- 
To view, visit https://gerrit.wikimedia.org/r/150195
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I5004ee8e823ed0cd245793c61d54246a4805e618
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: mw1.24-wmf14
Gerrit-Owner: Hoo man <h...@online.de>
Gerrit-Reviewer: Addshore <addshorew...@gmail.com>
Gerrit-Reviewer: Hoo man <h...@online.de>
Gerrit-Reviewer: Siebrand <siebr...@kitano.nl>
Gerrit-Reviewer: WikidataJenkins <wikidata-servi...@wikimedia.de>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to