jenkins-bot has submitted this change and it was merged.
Change subject: Don't make broken wikitext links for sister project links
......................................................................
Don't make broken wikitext links for sister project links
It's better not to link sister projects in client recent changes
and watchlist, vs. making broken wikitext links.
I think it is somewhat non-trivial to add a decent mechanism
to build interwiki wikitext links for sister projects, and
this is a short term solution that at least makes things
less broken.
The ideal solution would be to have such decent mechanism
for making interwiki links for sister projects, with the
correct site group prefix + subdomain for applicable projects
and handle special cases like wikidata itself, with 'd' prefix
and no subdomain.
Also, added test case for this and while touching the tests,
changed protected -> private and factored out some of the code
that had been duplicated in numerous places in the test file.
and removed the wikitext link formatting in the
RecentChangeFactoryTest, since the SiteStore mock does
not actually know about any sites and thus the links
would be unlinked in this case.
Bug: T116459
Change-Id: Iabc4ea9a82c90ec9db899139cb7b5eed862e4cc6
---
M client/includes/SiteLinkCommentCreator.php
M client/tests/phpunit/includes/SiteLinkCommentCreatorTest.php
M client/tests/phpunit/includes/recentchanges/RecentChangeFactoryTest.php
3 files changed, 46 insertions(+), 43 deletions(-)
Approvals:
Hoo man: Looks good to me, approved
jenkins-bot: Verified
diff --git a/client/includes/SiteLinkCommentCreator.php
b/client/includes/SiteLinkCommentCreator.php
index 3e8ba59..5f4a54b 100644
--- a/client/includes/SiteLinkCommentCreator.php
+++ b/client/includes/SiteLinkCommentCreator.php
@@ -247,18 +247,22 @@
* @return string wikitext interwiki link
*/
private function getSitelinkWikitext( $siteId, $pageTitle ) {
- $interwikiId = $siteId;
-
// Try getting the interwiki id from the Site object of the
link target
$site = $this->siteStore->getSite( $siteId );
+
if ( $site ) {
- $iw_ids = $site->getInterwikiIds();
- if ( isset( $iw_ids[0] ) ) {
- $interwikiId = $iw_ids[0];
+ $interwikiIds = $site->getInterwikiIds();
+
+ if ( isset( $interwikiIds[0] ) ) {
+ $interwikiId = $interwikiIds[0];
+
+ return "[[:$interwikiId:$pageTitle]]";
}
}
- return "[[:$interwikiId:$pageTitle]]";
+ // @todo: provide a mechanism to get the interwiki id for a
sister project wiki.
+ // e.g. get "voy:it" for Italian Wikivoyage from English
Wikipedia. (see T117738)
+ return "$siteId:$pageTitle";
}
/**
diff --git a/client/tests/phpunit/includes/SiteLinkCommentCreatorTest.php
b/client/tests/phpunit/includes/SiteLinkCommentCreatorTest.php
index 3ffc0de..91b1f19 100644
--- a/client/tests/phpunit/includes/SiteLinkCommentCreatorTest.php
+++ b/client/tests/phpunit/includes/SiteLinkCommentCreatorTest.php
@@ -72,10 +72,7 @@
$item2 = $this->getNewItem();
$item2->getSiteLinkList()->addNewSiteLink( 'enwiki', 'Japan' );
- $changeFactory = TestChanges::getEntityChangeFactory();
- $change = $changeFactory->newFromUpdate( ItemChange::UPDATE,
$item, $item2 );
-
- return $change->getSiteLinkDiff();
+ return $this->getSiteLinkDiffForUpdate( $item, $item2 );
}
protected function getUnlinkDiff() {
@@ -85,10 +82,7 @@
$item2 = $this->getNewItem();
$item2->getSiteLinkList()->removeLinkWithSiteId( 'enwiki' );
- $changeFactory = TestChanges::getEntityChangeFactory();
- $change = $changeFactory->newFromUpdate( ItemChange::UPDATE,
$item, $item2 );
-
- return $change->getSiteLinkDiff();
+ return $this->getSiteLinkDiffForUpdate( $item, $item2 );
}
protected function getLinkChangeDiff() {
@@ -98,10 +92,7 @@
$item2 = $this->getNewItem();
$item2->getSiteLinkList()->addNewSiteLink( 'enwiki', 'Tokyo' );
- $changeFactory = TestChanges::getEntityChangeFactory();
- $change = $changeFactory->newFromUpdate( ItemChange::UPDATE,
$item, $item2 );
-
- return $change->getSiteLinkDiff();
+ return $this->getSiteLinkDiffForUpdate( $item, $item2 );
}
protected function getOldLinkChangeDiff() {
@@ -119,10 +110,7 @@
$item2 = $this->getNewItem();
$item2->getSiteLinkList()->addNewSiteLink( 'enwiki', 'Japan',
array( new ItemId( 'Q17' ) ) );
- $changeFactory = TestChanges::getEntityChangeFactory();
- $change = $changeFactory->newFromUpdate( ItemChange::UPDATE,
$item, $item2 );
-
- return $change->getSiteLinkDiff();
+ return $this->getSiteLinkDiffForUpdate( $item, $item2 );
}
protected function getAddLinkDiff() {
@@ -133,13 +121,21 @@
$item2->getSiteLinkList()->addNewSiteLink( 'enwiki', 'Japan' );
$item2->getSiteLinkList()->addNewSiteLink( 'dewiki', 'Japan' );
- $changeFactory = TestChanges::getEntityChangeFactory();
- $change = $changeFactory->newFromUpdate( ItemChange::UPDATE,
$item, $item2 );
-
- return $change->getSiteLinkDiff();
+ return $this->getSiteLinkDiffForUpdate( $item, $item2 );
}
- protected function getAddMultipleLinksDiff() {
+ private function getAddSisterProjectLinkDiff() {
+ $item = $this->getNewItem();
+ $item->getSiteLinkList()->addNewSiteLink( 'enwiki', 'Japan' );
+
+ $item2 = $this->getNewItem();
+ $item2->getSiteLinkList()->addNewSiteLink( 'enwiki', 'Japan' );
+ $item2->getSiteLinkList()->addNewSiteLink( 'enwiktionary',
'Japan' );
+
+ return $this->getSiteLinkDiffForUpdate( $item, $item2 );
+ }
+
+ private function getAddMultipleLinksDiff() {
$item = $this->getNewItem();
$item->getSiteLinkList()->addNewSiteLink( 'enwiki', 'Japan' );
@@ -148,13 +144,10 @@
$item2->getSiteLinkList()->addNewSiteLink( 'dewiki', 'Japan' );
$item2->getSiteLinkList()->addNewSiteLink( 'frwiki', 'Japan' );
- $changeFactory = TestChanges::getEntityChangeFactory();
- $change = $changeFactory->newFromUpdate( ItemChange::UPDATE,
$item, $item2 );
-
- return $change->getSiteLinkDiff();
+ return $this->getSiteLinkDiffForUpdate( $item, $item2 );
}
- protected function getRemoveLinkDiff() {
+ private function getRemoveLinkDiff() {
$item = $this->getNewItem();
$item->getSiteLinkList()->addNewSiteLink( 'enwiki', 'Japan' );
$item->getSiteLinkList()->addNewSiteLink( 'dewiki', 'Japan' );
@@ -162,13 +155,10 @@
$item2 = $this->getNewItem();
$item2->getSiteLinkList()->addNewSiteLink( 'enwiki', 'Japan' );
- $changeFactory = TestChanges::getEntityChangeFactory();
- $change = $changeFactory->newFromUpdate( ItemChange::UPDATE,
$item, $item2 );
-
- return $change->getSiteLinkDiff();
+ return $this->getSiteLinkDiffForUpdate( $item, $item2 );
}
- protected function getChangeLinkDiff() {
+ private function getChangeLinkDiff() {
$item = $this->getNewItem();
$item->getSiteLinkList()->addNewSiteLink( 'enwiki', 'Japan' );
$item->getSiteLinkList()->addNewSiteLink( 'dewiki', 'Japan' );
@@ -177,13 +167,17 @@
$item2->getSiteLinkList()->addNewSiteLink( 'enwiki', 'Japan' );
$item2->getSiteLinkList()->addNewSiteLink( 'dewiki', 'Tokyo' );
+ return $this->getSiteLinkDiffForUpdate( $item, $item2 );
+ }
+
+ private function getSiteLinkDiffForUpdate( Item $item, Item $item2 ) {
$changeFactory = TestChanges::getEntityChangeFactory();
$change = $changeFactory->newFromUpdate( ItemChange::UPDATE,
$item, $item2 );
return $change->getSiteLinkDiff();
}
- protected function getDeleteDiff() {
+ private function getDeleteDiff() {
$item = $this->getNewItem();
$item->getSiteLinkList()->addNewSiteLink( 'enwiki', 'Japan' );
@@ -193,7 +187,7 @@
return $change->getSiteLinkDiff();
}
- protected function getRestoreDiff() {
+ private function getRestoreDiff() {
$item = $this->getNewItem();
$item->getSiteLinkList()->addNewSiteLink( 'enwiki', 'Japan' );
@@ -203,7 +197,7 @@
return $change->getSiteLinkDiff();
}
- protected function getUpdates() {
+ private function getUpdates() {
$updates = array();
$updates[] = array(
@@ -237,6 +231,11 @@
);
$updates[] = array(
+ $this->getAddSisterProjectLinkDiff(),
+ '(wikibase-comment-sitelink-add: enwiktionary:Japan)',
+ );
+
+ $updates[] = array(
$this->getAddMultipleLinksDiff(),
null, // currently multi-link diffs are not supported
);
diff --git
a/client/tests/phpunit/includes/recentchanges/RecentChangeFactoryTest.php
b/client/tests/phpunit/includes/recentchanges/RecentChangeFactoryTest.php
index c87f840..2bbbc68 100644
--- a/client/tests/phpunit/includes/recentchanges/RecentChangeFactoryTest.php
+++ b/client/tests/phpunit/includes/recentchanges/RecentChangeFactoryTest.php
@@ -336,7 +336,7 @@
)
),
'sitelink update' => array(
- '(wikibase-comment-sitelink-change:
[[:dewiki:Dummy]], [[:dewiki:Bummy]])',
+ '(wikibase-comment-sitelink-change:
dewiki:Dummy, dewiki:Bummy)',
'change',
$this->makeItemDiff( $linksDewikiDummy,
$linksDewikiBummy ),
array(),
@@ -345,7 +345,7 @@
)
),
'sitelink added' => array(
- '(wikibase-comment-sitelink-add:
[[:dewiki:Bummy]])',
+ '(wikibase-comment-sitelink-add: dewiki:Bummy)',
'change',
$this->makeItemDiff( $linksEmpty,
$linksDewikiBummy ),
array(),
@@ -354,7 +354,7 @@
)
),
'sitelink removed' => array(
- '(wikibase-comment-sitelink-remove:
[[:dewiki:Dummy]])',
+ '(wikibase-comment-sitelink-remove:
dewiki:Dummy)',
'change',
$this->makeItemDiff( $linksDewikiDummy,
$linksEmpty ),
array(),
--
To view, visit https://gerrit.wikimedia.org/r/251239
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iabc4ea9a82c90ec9db899139cb7b5eed862e4cc6
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: wmf/1.27.0-wmf.5
Gerrit-Owner: Hoo man <h...@online.de>
Gerrit-Reviewer: Aude <aude.w...@gmail.com>
Gerrit-Reviewer: Hoo man <h...@online.de>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
