Bene has submitted this change and it was merged.
Change subject: Fix link to badge items pointing to the wrong site
......................................................................
Fix link to badge items pointing to the wrong site
Bug: 69758
Change-Id: Ia3ece23cfbe1813b441fedfcd5bc9d967a434fc6
---
M repo/includes/DiffView.php
M repo/includes/EntityContentDiffView.php
M repo/includes/EntityDiffVisualizer.php
M repo/includes/actions/EditEntityAction.php
M repo/tests/phpunit/includes/DiffViewTest.php
M repo/tests/phpunit/includes/EntityDiffVisualizerTest.php
6 files changed, 91 insertions(+), 20 deletions(-)
Approvals:
Bene: Looks good to me, approved
WikidataJenkins: Verified
jenkins-bot: Checked
diff --git a/repo/includes/DiffView.php b/repo/includes/DiffView.php
index 0586f2f..e3461b1 100644
--- a/repo/includes/DiffView.php
+++ b/repo/includes/DiffView.php
@@ -8,6 +8,7 @@
use Diff\DiffOp\DiffOpAdd;
use Diff\DiffOp\DiffOpChange;
use Diff\DiffOp\DiffOpRemove;
+use Wikibase\DataModel\Entity\ItemId;
use Html;
use IContextSource;
use MWException;
@@ -47,6 +48,11 @@
private $diff;
/**
+ * @var EntityTitleLookup
+ */
+ private $entityTitleLookup;
+
+ /**
* Constructor.
*
* @since 0.1
@@ -54,12 +60,20 @@
* @param string[] $path
* @param Diff $diff
* @param SiteStore $siteStore
+ * @param EntityTitleLookup $entityTitleLookup
* @param IContextSource|null $contextSource
*/
- public function __construct( array $path, Diff $diff, SiteStore
$siteStore, IContextSource $contextSource = null ) {
+ public function __construct(
+ array $path,
+ Diff $diff,
+ SiteStore $siteStore,
+ EntityTitleLookup $entityTitleLookup,
+ IContextSource $contextSource = null
+ ) {
$this->path = $path;
$this->diff = $diff;
$this->siteStore = $siteStore;
+ $this->entityTitleLookup = $entityTitleLookup;
if ( !is_null( $contextSource ) ) {
$this->setContext( $contextSource );
@@ -159,14 +173,7 @@
* @return string
*/
private function getDeletedLine( $value, array $path ) {
- // @todo: inject a formatter instead of doing special cases
based on the path here!
- if ( $path[0] === $this->getLanguage()->getMessage(
'wikibase-diffview-link' ) ) {
- return Html::rawElement( 'del', array( 'class' =>
'diffchange diffchange-inline' ),
- $this->getSiteLinkElement( $path[1], $value )
- );
- } else {
- return Html::element( 'del', array( 'class' =>
'diffchange diffchange-inline' ), $value );
- }
+ return $this->getChangedLine( 'del', $value, $path );;
}
/**
@@ -175,14 +182,26 @@
* @return string
*/
private function getAddedLine( $value, array $path ) {
+ return $this->getChangedLine( 'ins', $value, $path );
+ }
+
+ /**
+ * @param string $tag
+ * @param string $value
+ * @param string[] $path
+ * @return string
+ */
+ private function getChangedLine( $tag, $value, array $path ) {
// @todo: inject a formatter instead of doing special cases
based on the path here!
if ( $path[0] === $this->getLanguage()->getMessage(
'wikibase-diffview-link' ) ) {
- return Html::rawElement( 'ins', array( 'class' =>
'diffchange diffchange-inline' ),
- $this->getSiteLinkElement( $path[1], $value )
- );
- } else {
- return Html::element( 'ins', array( 'class' =>
'diffchange diffchange-inline' ), $value );
+ if ( $path[2] === 'badges' ) {
+ $value = $this->getBadgeLinkElement( $value );
+ } else {
+ $value = $this->getSiteLinkElement( $path[1],
$value );
+ }
+ return Html::rawElement( $tag, array( 'class' =>
'diffchange diffchange-inline' ), $value );
}
+ return Html::element( $tag, array( 'class' => 'diffchange
diffchange-inline' ), $value );
}
/**
@@ -202,6 +221,25 @@
}
/**
+ * @param string $badgeId
+ *
+ * @return string
+ */
+ private function getBadgeLinkElement( $badgeId ) {
+ try {
+ $title = $this->entityTitleLookup->getTitleForId( new
ItemId( $badgeId ) );
+ } catch ( MWException $ex ) {
+ wfWarn( "Couldn't get Title for badge $badgeId" );
+ return $badgeId;
+ }
+
+ return Html::element( 'a', array(
+ 'href' => $title->getLinkURL(),
+ 'dir' => 'auto',
+ ), $badgeId );
+ }
+
+ /**
* Generates HTML for the header of the diff operation
*
* @since 0.4
diff --git a/repo/includes/EntityContentDiffView.php
b/repo/includes/EntityContentDiffView.php
index f4c8e12..ea465ac 100644
--- a/repo/includes/EntityContentDiffView.php
+++ b/repo/includes/EntityContentDiffView.php
@@ -85,7 +85,8 @@
$this->getContext(),
new ClaimDiffer( new OrderedListDiffer( new
ComparableComparer() ) ),
new ClaimDifferenceVisualizer(
$this->propertyNameFormatter, $this->detailedSnakFormatter,
$this->terseSnakFormatter, $langCode ),
- $wikibaseRepo->getSiteStore()
+ $wikibaseRepo->getSiteStore(),
+ $wikibaseRepo->getEntityTitleLookup()
);
}
diff --git a/repo/includes/EntityDiffVisualizer.php
b/repo/includes/EntityDiffVisualizer.php
index 6af6ec8..b6303e0 100644
--- a/repo/includes/EntityDiffVisualizer.php
+++ b/repo/includes/EntityDiffVisualizer.php
@@ -51,6 +51,11 @@
private $siteStore;
/**
+ * @var EntityTitleLookup
+ */
+ private $entityTitleLookup;
+
+ /**
* Constructor.
*
* @since 0.4
@@ -63,12 +68,14 @@
public function __construct( IContextSource $contextSource,
ClaimDiffer $claimDiffer,
ClaimDifferenceVisualizer $claimDiffView,
- SiteStore $siteStore
+ SiteStore $siteStore,
+ EntityTitleLookup $entityTitleLookup
) {
$this->context = $contextSource;
$this->claimDiffer = $claimDiffer;
$this->claimDiffVisualizer = $claimDiffView;
$this->siteStore = $siteStore;
+ $this->entityTitleLookup = $entityTitleLookup;
}
/**
@@ -114,6 +121,7 @@
true
),
$this->siteStore,
+ $this->entityTitleLookup,
$this->context
);
@@ -134,6 +142,7 @@
true
),
$this->siteStore,
+ $this->entityTitleLookup,
$this->context
);
@@ -164,6 +173,7 @@
array(),
$diff,
$this->siteStore,
+ $this->entityTitleLookup,
$this->context
);
diff --git a/repo/includes/actions/EditEntityAction.php
b/repo/includes/actions/EditEntityAction.php
index 9a85dd8..862e74d 100644
--- a/repo/includes/actions/EditEntityAction.php
+++ b/repo/includes/actions/EditEntityAction.php
@@ -76,7 +76,8 @@
$this->getContext(),
new ClaimDiffer( new OrderedListDiffer( new
ComparableComparer() ) ),
new ClaimDifferenceVisualizer(
$this->propertyNameFormatter, $this->detailedSnakFormatter,
$this->terseSnakFormatter, $langCode ),
- $wikibaseRepo->getSiteStore()
+ $wikibaseRepo->getSiteStore(),
+ $wikibaseRepo->getEntityTitleLookup()
);
}
diff --git a/repo/tests/phpunit/includes/DiffViewTest.php
b/repo/tests/phpunit/includes/DiffViewTest.php
index fcbae5a..de3f104 100644
--- a/repo/tests/phpunit/includes/DiffViewTest.php
+++ b/repo/tests/phpunit/includes/DiffViewTest.php
@@ -7,6 +7,8 @@
use Diff\DiffOp\DiffOpChange;
use Diff\DiffOp\DiffOpRemove;
use Wikibase\DiffView;
+use Wikibase\Repo\WikibaseRepo;
+use Wikibase\DataModel\Entity\ItemId;
/**
* @covers Wikibase\DiffView
@@ -21,6 +23,9 @@
public function diffOpProvider() {
$linkPath = wfMessage( 'wikibase-diffview-link' )->text();
+ $itemId = new ItemId( 'Q123' );
+ $itemTitle =
WikibaseRepo::getDefaultInstance()->getEntityTitleLookup()->getTitleForId(
$itemId );
+ $itemLink = $itemTitle->getLinkURL();
return array(
'Empty' => array(
@@ -47,10 +52,22 @@
$linkPath . '/enwiki'
),
'Link has direction' => array(
- '@<a\b[^>]* hreflang="en" dir="auto"@',
+ '@<a\b[^>]* dir="auto"@',
null,
'NEW',
$linkPath . '/enwiki'
+ ),
+ 'Link has hreflang' => array(
+ '@<a\b[^>]* hreflang="en"@',
+ null,
+ 'NEW',
+ $linkPath . '/enwiki'
+ ),
+ 'Badge is linked correctly' => array(
+ '@<a\b[^>]* href="' . preg_quote( $itemLink,
'@' ) . '"@',
+ null,
+ 'Q123',
+ $linkPath . '/enwiki/badges'
),
);
}
@@ -81,7 +98,8 @@
}
$diff = new Diff( $this->getDiffOps( $oldValue, $newValue ) );
$siteStore = MockSiteStore::newFromTestSites();
- $diffView = new DiffView( $path, $diff, $siteStore );
+ $entityTitleLookup =
WikibaseRepo::getDefaultInstance()->getEntityTitleLookup();
+ $diffView = new DiffView( $path, $diff, $siteStore,
$entityTitleLookup );
$html = $diffView->getHtml();
diff --git a/repo/tests/phpunit/includes/EntityDiffVisualizerTest.php
b/repo/tests/phpunit/includes/EntityDiffVisualizerTest.php
index fb6120f..705ac17 100644
--- a/repo/tests/phpunit/includes/EntityDiffVisualizerTest.php
+++ b/repo/tests/phpunit/includes/EntityDiffVisualizerTest.php
@@ -13,6 +13,7 @@
use Wikibase\Repo\Content\EntityContentDiff;
use Wikibase\DataModel\Entity\EntityDiff;
use Wikibase\EntityDiffVisualizer;
+use Wikibase\Repo\WikibaseRepo;
/**
* @covers Wikibase\EntityDiffVisualizer
@@ -119,12 +120,14 @@
protected function getVisualizer() {
$enwiki = new Site();
$enwiki->setGlobalId( 'enwiki' );
+ $entityTitleLookup =
WikibaseRepo::getDefaultInstance()->getEntityTitleLookup();
return new EntityDiffVisualizer(
$this->getMockContext(),
$this->getMockClaimDiffer(),
$this->getMockClaimDiffVisualizer(),
- new MockSiteStore( array( $enwiki ) )
+ new MockSiteStore( array( $enwiki ) ),
+ $entityTitleLookup
);
}
--
To view, visit https://gerrit.wikimedia.org/r/155180
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia3ece23cfbe1813b441fedfcd5bc9d967a434fc6
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Hoo man <[email protected]>
Gerrit-Reviewer: Aude <[email protected]>
Gerrit-Reviewer: Bene <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Hoo man <[email protected]>
Gerrit-Reviewer: Jeroen De Dauw <[email protected]>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: WikidataJenkins <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
