Hoo man has uploaded a new change for review. https://gerrit.wikimedia.org/r/150195
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(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase refs/changes/95/150195/1 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: newchange Gerrit-Change-Id: I5004ee8e823ed0cd245793c61d54246a4805e618 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Wikibase Gerrit-Branch: mw1.24-wmf14 Gerrit-Owner: Hoo man <h...@online.de> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits