[MediaWiki-commits] [Gerrit] Show raw badge id instead of failing in DiffView - change (mediawiki...Wikibase)
jenkins-bot has submitted this change and it was merged. Change subject: Show raw badge id instead of failing in DiffView .. Show raw badge id instead of failing in DiffView I found this while playing around with the DiffView and badges. I don't think it's useful if the DiffView crashes hard with an exception if there is such an easy, obvious fallback. Just show the item id of the badge. Done. Change-Id: I68ca200a912b36d9f531dd433f1dab06984556fb --- M repo/includes/Diff/DiffView.php M repo/tests/phpunit/includes/Diff/DiffViewTest.php 2 files changed, 33 insertions(+), 8 deletions(-) Approvals: Adrian Lang: Looks good to me, approved jenkins-bot: Verified diff --git a/repo/includes/Diff/DiffView.php b/repo/includes/Diff/DiffView.php index fdf069d..d2538f1 100644 --- a/repo/includes/Diff/DiffView.php +++ b/repo/includes/Diff/DiffView.php @@ -10,6 +10,7 @@ use Diff\DiffOp\DiffOpRemove; use Html; use IContextSource; +use InvalidArgumentException; use MWException; use SiteStore; use Wikibase\DataModel\Entity\EntityId; @@ -224,22 +225,28 @@ } /** -* @param string $badgeId +* @param string $idString * -* @return string +* @return string HTML */ - private function getBadgeLinkElement( $badgeId ) { + private function getBadgeLinkElement( $idString ) { try { - $title = $this->entityTitleLookup->getTitleForId( new ItemId( $badgeId ) ); - } catch ( MWException $ex ) { - wfWarn( "Couldn't get Title for badge $badgeId" ); - return $badgeId; + $itemId = new ItemId( $idString ); + } catch ( InvalidArgumentException $ex ) { + return $idString; + } + + try { + $title = $this->entityTitleLookup->getTitleForId( $itemId ); + } catch ( MwException $ex ) { + wfWarn( "Couldn't get Title for badge $idString" ); + return $idString; } return Html::element( 'a', array( 'href' => $title->getLinkURL(), 'dir' => 'auto', - ), $this->getLabelForBadge( new ItemId( $badgeId ) ) ); + ), $this->getLabelForBadge( $itemId ) ); } /** diff --git a/repo/tests/phpunit/includes/Diff/DiffViewTest.php b/repo/tests/phpunit/includes/Diff/DiffViewTest.php index d3c6386..d2b2a34 100644 --- a/repo/tests/phpunit/includes/Diff/DiffViewTest.php +++ b/repo/tests/phpunit/includes/Diff/DiffViewTest.php @@ -138,4 +138,22 @@ $this->assertRegExp( $pattern, $html, 'Diff table content line' ); } + public function testGivenInvalidBadgeId_getHtmlDoesNotThrowException() { + $path = array( + wfMessage( 'wikibase-diffview-link' )->text(), + 'enwiki', + 'badges' + ); + $diff = new Diff( array( new DiffOpAdd( 'invalidBadgeId' ) ) ); + + $siteStore = new MockSiteStore(); + $entityTitleLookup = WikibaseRepo::getDefaultInstance()->getEntityTitleLookup(); + $entityRevisionLookup = new MockRepository(); + $diffView = new DiffView( $path, $diff, $siteStore, $entityTitleLookup, $entityRevisionLookup ); + + $html = $diffView->getHtml(); + + $this->assertContains( 'invalidBadgeId', $html ); + } + } -- To view, visit https://gerrit.wikimedia.org/r/192352 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I68ca200a912b36d9f531dd433f1dab06984556fb Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Wikibase Gerrit-Branch: master Gerrit-Owner: Thiemo Mättig (WMDE) Gerrit-Reviewer: Adrian Lang Gerrit-Reviewer: jenkins-bot <> ___ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
[MediaWiki-commits] [Gerrit] Show raw badge id instead of failing in DiffView - change (mediawiki...Wikibase)
Thiemo Mättig (WMDE) has uploaded a new change for review. https://gerrit.wikimedia.org/r/192352 Change subject: Show raw badge id instead of failing in DiffView .. Show raw badge id instead of failing in DiffView I found this while playing around with the DiffView and badges. I don't think it's useful if the DiffView crashes hard with an exception if there is such an easy, obvious fallback. Just show the item id of the badge. Done. Change-Id: I68ca200a912b36d9f531dd433f1dab06984556fb --- M repo/includes/Diff/DiffView.php M repo/tests/phpunit/includes/Diff/DiffViewTest.php 2 files changed, 33 insertions(+), 8 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase refs/changes/52/192352/1 diff --git a/repo/includes/Diff/DiffView.php b/repo/includes/Diff/DiffView.php index fdf069d..d2538f1 100644 --- a/repo/includes/Diff/DiffView.php +++ b/repo/includes/Diff/DiffView.php @@ -10,6 +10,7 @@ use Diff\DiffOp\DiffOpRemove; use Html; use IContextSource; +use InvalidArgumentException; use MWException; use SiteStore; use Wikibase\DataModel\Entity\EntityId; @@ -224,22 +225,28 @@ } /** -* @param string $badgeId +* @param string $idString * -* @return string +* @return string HTML */ - private function getBadgeLinkElement( $badgeId ) { + private function getBadgeLinkElement( $idString ) { try { - $title = $this->entityTitleLookup->getTitleForId( new ItemId( $badgeId ) ); - } catch ( MWException $ex ) { - wfWarn( "Couldn't get Title for badge $badgeId" ); - return $badgeId; + $itemId = new ItemId( $idString ); + } catch ( InvalidArgumentException $ex ) { + return $idString; + } + + try { + $title = $this->entityTitleLookup->getTitleForId( $itemId ); + } catch ( MwException $ex ) { + wfWarn( "Couldn't get Title for badge $idString" ); + return $idString; } return Html::element( 'a', array( 'href' => $title->getLinkURL(), 'dir' => 'auto', - ), $this->getLabelForBadge( new ItemId( $badgeId ) ) ); + ), $this->getLabelForBadge( $itemId ) ); } /** diff --git a/repo/tests/phpunit/includes/Diff/DiffViewTest.php b/repo/tests/phpunit/includes/Diff/DiffViewTest.php index d3c6386..d2b2a34 100644 --- a/repo/tests/phpunit/includes/Diff/DiffViewTest.php +++ b/repo/tests/phpunit/includes/Diff/DiffViewTest.php @@ -138,4 +138,22 @@ $this->assertRegExp( $pattern, $html, 'Diff table content line' ); } + public function testGivenInvalidBadgeId_getHtmlDoesNotThrowException() { + $path = array( + wfMessage( 'wikibase-diffview-link' )->text(), + 'enwiki', + 'badges' + ); + $diff = new Diff( array( new DiffOpAdd( 'invalidBadgeId' ) ) ); + + $siteStore = new MockSiteStore(); + $entityTitleLookup = WikibaseRepo::getDefaultInstance()->getEntityTitleLookup(); + $entityRevisionLookup = new MockRepository(); + $diffView = new DiffView( $path, $diff, $siteStore, $entityTitleLookup, $entityRevisionLookup ); + + $html = $diffView->getHtml(); + + $this->assertContains( 'invalidBadgeId', $html ); + } + } -- To view, visit https://gerrit.wikimedia.org/r/192352 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I68ca200a912b36d9f531dd433f1dab06984556fb Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Wikibase Gerrit-Branch: master Gerrit-Owner: Thiemo Mättig (WMDE) ___ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits