Aude has uploaded a new change for review.
https://gerrit.wikimedia.org/r/53543
Change subject: (bug 45037) show edit link only if we have repo links
......................................................................
(bug 45037) show edit link only if we have repo links
This also fixes bug 44536 to have Q### links instead of link via
Special:ItemByTitle
This patch adds parser output and output page properties that indicate:
1) userepolinks: if a page uses repo links... e.g. they've been suppressed or
not.
if not, it considers if the page is associated with the repo and has links.
2) wikibase_item: the associated prefixed item id, added regardless if
userepolinks
is true or not.
Then it's easier and more efficient for the code to determine whether or not to
add "edit links" link and build the link to the associated item.
Change-Id: I97cc9378477953bddebd04840871964f31ef1b7f
---
M client/WikibaseClient.hooks.php
M client/WikibaseClient.php
M client/includes/ClientUtils.php
M client/includes/LangLinkHandler.php
M client/includes/recentchanges/ExternalChangesLine.php
M client/tests/phpunit/MockRepository.php
M client/tests/phpunit/includes/LangLinkHandlerTest.php
7 files changed, 175 insertions(+), 70 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/43/53543/1
diff --git a/client/WikibaseClient.hooks.php b/client/WikibaseClient.hooks.php
index 659dec8..f5b4756 100644
--- a/client/WikibaseClient.hooks.php
+++ b/client/WikibaseClient.hooks.php
@@ -602,12 +602,43 @@
}
/**
+ * Add output page property if repo links are suppressed, and property
for item id
+ *
+ * @since 0.4
+ *
+ * @param \OutputPage &$out
+ * @param \ParserOutput $pout
+ *
+ * @return bool
+ */
+ public static function onOutputPageParserOutput( \OutputPage &$out,
\ParserOutput $pout ) {
+ $langLinkHandler = new LangLinkHandler(
+ Settings::get( 'siteGlobalID' ),
+ Settings::get( 'namespaces' ),
+ Settings::get( 'excludeNamespaces' ),
+ ClientStoreFactory::getStore()->newSiteLinkTable(),
+ \Sites::singleton() );
+
+ $noExternalLangLinks =
$langLinkHandler->getNoExternalLangLinks( $pout );
+ if ( $noExternalLangLinks !== array() ) {
+ $out->setProperty( 'noexternallanglinks',
$noExternalLangLinks );
+ }
+
+ $itemId = $pout->getProperty( 'wikibase_item' );
+ if ( $itemId !== false ) {
+ $out->setProperty( 'wikibase_item', $itemId );
+ }
+
+ return true;
+ }
+
+ /**
* Displays a list of links to pages on the central wiki at the end of
the language box.
+ *
+ * @since 0.1
*
* @param \Skin $skin
* @param \QuickTemplate $template
- *
- * @since 0.1
*
* @return bool
*/
@@ -615,34 +646,38 @@
wfProfileIn( __METHOD__ );
$title = $skin->getContext()->getTitle();
- if ( !in_array( $title->getNamespace(), Settings::get(
'excludeNamespaces' ) ) && $title->exists() ) {
+ $namespaceChecker = new NamespaceChecker(
+ Settings::get( 'excludeNamespaces' ),
+ Settings::get( 'namespaces' )
+ );
+ if ( $title->exists() && $namespaceChecker->isWikibaseEnabled(
$title->getNamespace() ) ) {
if ( empty( $template->data['language_urls'] ) &&
\Action::getActionName( $skin->getContext() ) === 'view' ) {
- // Placeholder in case the page doesn't have
any langlinks yet
- // self::onBeforePageDisplay adds the
JavaScript module which will overwrite this with a link
- $template->data['language_urls'][] = array(
- 'text' => '',
- 'id' => 'wbc-linkToItem',
- 'class' => 'wbc-editpage
wbc-nolanglinks',
- );
+ $noExternalLangLinks =
$skin->getOutput()->getProperty( 'noexternallanglinks' );
+
+ if ( $noExternalLangLinks !== null &&
!in_array( '*', $noExternalLangLinks ) ) {
+ // Placeholder in case the page doesn't
have any langlinks yet
+ // self::onBeforePageDisplay adds the
JavaScript module which will overwrite this with a link
+ $template->data['language_urls'][] =
array(
+ 'text' => '',
+ 'id' => 'wbc-linkToItem',
+ 'class' => 'wbc-editpage
wbc-nolanglinks',
+ );
+ }
wfProfileOut( __METHOD__ );
return true;
}
- $title = $skin->getContext()->getTitle();
+ $itemId = $skin->getOutput()->getProperty(
'wikibase_item' );
- // gets the main part of the title, no underscores used
in this db table
- // TODO: use the item id from the page props when they
are available
- $titleText = $title->getPrefixedText();
- $siteId = Settings::get( 'siteGlobalID' );
+ if ( $itemId !== null ) {
+ $itemNamespace = ClientUtils::getNamespace(
Item::ENTITY_TYPE, true );
- $itemId =
ClientStoreFactory::getStore()->newSiteLinkTable()->getItemIdForLink( $siteId,
$titleText );
-
- if ( $itemId ) {
// links to the special page
$template->data['language_urls'][] = array(
- 'href' => ClientUtils::repoArticleUrl(
"Special:ItemByTitle/$siteId/" . wfUrlencode( $title->getPrefixedDBkey() ) ),
+ // todo: make nicer without strtoupper
here
+ 'href' => ClientUtils::repoArticleUrl(
$itemNamespace . strtoupper( $itemId ) ),
'text' => wfMessage(
'wikibase-editlinks' )->text(),
'title' => wfMessage(
'wikibase-editlinkstitle' )->text(),
'class' => 'wbc-editpage',
diff --git a/client/WikibaseClient.php b/client/WikibaseClient.php
index 671d677..53dcf7f 100644
--- a/client/WikibaseClient.php
+++ b/client/WikibaseClient.php
@@ -98,6 +98,7 @@
$wgHooks['UnitTestsList'][] =
'\Wikibase\ClientHooks::registerUnitTests';
$wgHooks['LoadExtensionSchemaUpdates'][] =
'\Wikibase\ClientHooks::onSchemaUpdate';
$wgHooks['OldChangesListRecentChangesLine'][] =
'\Wikibase\ClientHooks::onOldChangesListRecentChangesLine';
+$wgHooks['OutputPageParserOutput'][] =
'\Wikibase\ClientHooks::onOutputPageParserOutput';
$wgHooks['ParserAfterParse'][] =
'\Wikibase\ClientHooks::onParserAfterParse';
$wgHooks['ParserFirstCallInit'][] =
'\Wikibase\ClientHooks::onParserFirstCallInit';
$wgHooks['MagicWordwgVariableIDs'][] =
'\Wikibase\ClientHooks::onMagicWordwgVariableIDs';
diff --git a/client/includes/ClientUtils.php b/client/includes/ClientUtils.php
index 3fe1f73..034f4e8 100644
--- a/client/includes/ClientUtils.php
+++ b/client/includes/ClientUtils.php
@@ -51,6 +51,38 @@
}
/**
+ * TODO: returning a string as namespace like this is odd.
+ * Returning the namespace ID would make more sense.
+ * If the result of this is not handled to a Title object
+ * we miss out on proper localization and stuff.
+ *
+ * @since 0.2
+ *
+ * @param array $entityData
+ * @param bool $formatted formats and adds colon separator
+ *
+ * @return string
+ */
+ public static function getNamespace( $entityType, $formatted = false ) {
+ $nsList = Settings::get( 'repoNamespaces' );
+ $ns = null;
+
+ $contentType = 'wikibase-' . $entityType;
+ if ( is_array( $nsList ) && array_key_exists( $contentType,
$nsList ) ) {
+ $ns = $nsList[$contentType];
+ } else {
+ // todo: support queries and better error handling here
+ return false;
+ }
+
+ if ( $formatted === true && ! empty( $ns ) ) {
+ $ns = $ns . ':';
+ }
+
+ return $ns;
+ }
+
+ /**
* @since 0.3
*
* @param string $target
diff --git a/client/includes/LangLinkHandler.php
b/client/includes/LangLinkHandler.php
index 6226f75..41f6830 100644
--- a/client/includes/LangLinkHandler.php
+++ b/client/includes/LangLinkHandler.php
@@ -2,6 +2,7 @@
namespace Wikibase;
use SiteStore;
+use Sites;
use Site;
use Title;
use ParserOutput;
@@ -80,18 +81,24 @@
wfProfileIn( __METHOD__ );
wfDebugLog( __CLASS__, __FUNCTION__ . ": Looking for sitelinks
defined by the corresponding item on the wikibase repo." );
- $itemId = $this->siteLinksLookup->getItemIdForLink(
- $this->siteId,
- $title->getFullText()
- );
+ $links = array();
+
+ $site = $this->sites->getSite( $this->siteId );
+
+ if ( $site === null ) {
+ wfWarn( 'Site not found for ' . $this->siteId );
+ return $links;
+ }
+
+ $siteLink = new SiteLink( $site, $title->getFullText() );
+
+ $itemId = $this->siteLinksLookup->getEntityIdForSiteLink(
$siteLink );
$links = array();
- if ( $itemId !== false ) {
- wfDebugLog( __CLASS__, __FUNCTION__ . ": Item ID for "
. $title->getFullText() . " is " . $itemId );
-
- $links = $this->siteLinksLookup->getSiteLinksForItem(
- new EntityId( Item::ENTITY_TYPE, $itemId ) );
+ if ( $itemId instanceof EntityId ) {
+ wfDebugLog( __CLASS__, __FUNCTION__ . ": Item ID for "
. $title->getFullText() . " is " . $itemId->getPrefixedId() );
+ $links = $this->siteLinksLookup->getSiteLinksForItem(
$itemId );
} else {
wfDebugLog( __CLASS__, __FUNCTION__ . ": No
corresponding item found for " . $title->getFullText() );
}
@@ -124,7 +131,7 @@
);
// use repoLinks in only the namespaces specified in settings
- if ( $namespaceChecker->isWikibaseEnabled(
$title->getNamespace() ) ) {
+ if ( $namespaceChecker->isWikibaseEnabled(
$title->getNamespace() ) === true ) {
$nel = self::getNoExternalLangLinks( $out );
if( in_array( '*', $nel ) ) {
@@ -203,7 +210,7 @@
*/
public function getNoExternalLangLinks( ParserOutput $out ) {
wfProfileIn( __METHOD__ );
- $nel = $out->getProperty( 'noexternallanglinks' );
+ $nel = unserialize( $out->getProperty( 'noexternallanglinks' )
);
if( empty( $nel ) ) {
$nel = array();
@@ -224,7 +231,7 @@
*/
public function setNoExternalLangLinks( ParserOutput $out, array
$noexternallanglinks ) {
wfProfileIn( __METHOD__ );
- $out->setProperty( 'noexternallanglinks', $noexternallanglinks
);
+ $out->setProperty( 'noexternallanglinks', serialize(
$noexternallanglinks ) );
wfProfileOut( __METHOD__ );
}
@@ -355,6 +362,7 @@
$onPageLinks = $this->localLinksToArray( $onPageLinks );
$repoLinks = $this->getEntityLinks( $title );
+
$repoLinks = $this->repoLinksToArray( $repoLinks );
$repoLinks = $this->suppressRepoLinks( $out, $repoLinks );
@@ -362,6 +370,36 @@
wfProfileOut( __METHOD__ );
return $repoLinks;
+ }
+
+ /**
+ * Set parser output property with item id
+ *
+ * @since 0.4
+ *
+ * @param ParserOutput $out
+ * @param SiteLink $siteLink
+ */
+ protected function setItemIdProperty( ParserOutput $out, SiteLink
$siteLink ) {
+ // todo: do we really want to fetch item id twice during
parsing?
+ $itemId = $this->siteLinksLookup->getEntityIdForSiteLink(
$siteLink );
+
+ if ( $itemId instanceof EntityId ) {
+ // @todo get prefixed id in nicer way, or maybe we want
it to be numeric id
+ $out->setProperty( 'wikibase_item',
$itemId->getPrefixedId() );
+ } else {
+ // unset property, if it was set
+ $properties = $out->getProperties();
+
+ if ( array_key_exists( 'wikibase_item', $properties ) )
{
+ unset( $properties['wikibase_item'] );
+ $out->mProperties = $properties;
+ }
+
+ wfDebugLog( __CLASS__, __FUNCTION__ . 'Trying to set
wikibase_item property for '
+ . $siteLink->getSite()->getGlobalId() . ':' .
$siteLink->getPage()
+ . ' but $itemId is not an EntityId object.' );
+ }
}
/**
@@ -378,6 +416,17 @@
*/
public function addLinksFromRepository( Title $title, ParserOutput $out
) {
wfProfileIn( __METHOD__ );
+
+ $site = $this->sites->getSite( $this->siteId );
+
+ if ( $site === null ) {
+ wfWarn( 'Site not found for ' . $this->siteId . '.
Cannot add links from repository.' );
+ return;
+ }
+
+ $siteLink = new SiteLink( $site, $title->getFullText() );
+
+ $this->setItemIdProperty( $out, $siteLink );
$repoLinks = $this->getEffectiveRepoLinks( $title, $out );
@@ -402,6 +451,10 @@
}
}
+ // @todo: perhaps a better place to do this
+ $linksUpdate = new \LinksUpdate( $title, $out );
+ $linksUpdate->doUpdate();
+
wfProfileOut( __METHOD__ );
}
}
diff --git a/client/includes/recentchanges/ExternalChangesLine.php
b/client/includes/recentchanges/ExternalChangesLine.php
index 617b272..5687511 100644
--- a/client/includes/recentchanges/ExternalChangesLine.php
+++ b/client/includes/recentchanges/ExternalChangesLine.php
@@ -370,40 +370,6 @@
}
/**
- * TODO: returning a string as namespace like this is odd.
- * Returning the namespace ID would make more sense.
- * If the result of this is not handled to a Title object
- * we miss out on proper localization and stuff.
- *
- * @since 0.2
- *
- * @param array $entityData
- *
- * @return string
- */
- protected static function getNamespace( $entityData ) {
- $nsList = Settings::get( 'repoNamespaces' );
- $ns = null;
-
- switch( $entityData['entity_type'] ) {
- case 'item':
- $ns = $nsList['wikibase-item'];
- break;
- case 'property':
- $ns = $nsList['wikibase-property'];
- break;
- default:
- // invalid entity type
- // todo: query data type
- return false;
- }
- if ( ! empty( $ns ) ) {
- $ns = $ns . ':';
- }
- return $ns;
- }
-
- /**
* @since 0.2
*
* @param array $entityData
@@ -430,7 +396,7 @@
$titleText = $entityId ? strtoupper(
$entityId->getPrefixedId() ) : $id;
if ( $includeNamespace ) {
- $ns = self::getNamespace( $entityData );
+ $ns = ClientUtils::getNamespace(
$entityData['entity_type'], true );
$titleText = $ns . $titleText;
}
diff --git a/client/tests/phpunit/MockRepository.php
b/client/tests/phpunit/MockRepository.php
index 9f36357..4fc49e2 100644
--- a/client/tests/phpunit/MockRepository.php
+++ b/client/tests/phpunit/MockRepository.php
@@ -143,6 +143,23 @@
}
/**
+ * Gets an EntityId for a SiteLink
+ *
+ * @since 0.4
+ *
+ * @param SiteLink $siteLink
+ *
+ * @return EntityId
+ */
+ public function getEntityIdForSiteLink( SiteLink $siteLink ) {
+ $globalSiteId = $siteLink->getSite()->getGlobalId();
+ $pageTitle = $siteLink->getPage();
+
+ $numericItemId = $this->getItemIdForLink( $globalSiteId,
$pageTitle );
+ return is_int( $numericItemId ) ? new EntityId(
Item::ENTITY_TYPE, $numericItemId ) : null;
+ }
+
+ /**
* Registers the sitelinsk of the given Item so they can later be found
with getLinks, etc
*
* @param \Wikibase\Item $item
diff --git a/client/tests/phpunit/includes/LangLinkHandlerTest.php
b/client/tests/phpunit/includes/LangLinkHandlerTest.php
index e7e945c..f662fc5 100644
--- a/client/tests/phpunit/includes/LangLinkHandlerTest.php
+++ b/client/tests/phpunit/includes/LangLinkHandlerTest.php
@@ -49,7 +49,7 @@
'id' => 1,
'label' => array( 'en' => 'Foo' ),
'links' => array(
- 'testwiki' => 'Foo',
+ 'srwiki' => 'Foo',
'dewiki' => 'Foo_de',
'enwiki' => 'Foo_en',
)
@@ -58,7 +58,7 @@
'id' => 2,
'label' => array( 'en' => 'Talk:Foo' ),
'links' => array(
- 'testwiki' => 'Talk:Foo',
+ 'srwiki' => 'Talk:Foo',
'dewiki' => 'Talk:Foo_de',
'enwiki' => 'Talk:Foo_en',
)
@@ -84,7 +84,7 @@
}
$this->langLinkHandler = new \Wikibase\LangLinkHandler(
- 'testwiki',
+ 'srwiki',
array( NS_MAIN ),
array(),
$this->mockRepo,
@@ -101,7 +101,7 @@
array( // #1
'Foo', // page
array( // expected links
- 'testwiki' => 'Foo',
+ 'srwiki' => 'Foo',
'dewiki' => 'Foo_de',
'enwiki' => 'Foo_en',
)
@@ -229,6 +229,7 @@
public function testUseRepoLinks( $title, $noexternallanglinks,
$expected ) {
if ( is_string( $title ) ) {
$title = \Title::newFromText( $title );
+ $title->resetArticleID( 1 );
}
$out = $this->makeParserOutput( array(), $noexternallanglinks );
--
To view, visit https://gerrit.wikimedia.org/r/53543
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I97cc9378477953bddebd04840871964f31ef1b7f
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Aude <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits