Aude has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/250999

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.

Bug: T116459
Change-Id: Iabc4ea9a82c90ec9db899139cb7b5eed862e4cc6
---
M client/includes/SiteLinkCommentCreator.php
M client/tests/phpunit/includes/SiteLinkCommentCreatorTest.php
2 files changed, 43 insertions(+), 40 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase 
refs/changes/99/250999/1

diff --git a/client/includes/SiteLinkCommentCreator.php 
b/client/includes/SiteLinkCommentCreator.php
index 3e8ba59..2d69ce7 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.
+               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
                );

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iabc4ea9a82c90ec9db899139cb7b5eed862e4cc6
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Aude <aude.w...@gmail.com>

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

Reply via email to