jenkins-bot has submitted this change and it was merged.
Change subject: allow removing non-existing sitelinks.
......................................................................
allow removing non-existing sitelinks.
wbsitelinks should not report an error when trying to remove a link that isn't
there.
Change-Id: I4d11fd89345191baf8ea4874d9fbf32e173bcf32
---
M repo/includes/api/ModifyEntity.php
M repo/includes/api/SetSiteLink.php
M repo/tests/phpunit/includes/api/SetSiteLinkTest.php
3 files changed, 35 insertions(+), 19 deletions(-)
Approvals:
Aude: Looks good to me, approved
jenkins-bot: Verified
diff --git a/repo/includes/api/ModifyEntity.php
b/repo/includes/api/ModifyEntity.php
index fe9528b..2719811 100644
--- a/repo/includes/api/ModifyEntity.php
+++ b/repo/includes/api/ModifyEntity.php
@@ -191,6 +191,8 @@
$summary = $this->modifyEntity( $entityContent, $params );
if ( !$summary ) {
+ //XXX: This could rather be used for "silent" failure,
i.e. in cases where
+ // there was simply nothing to do.
wfProfileOut( __METHOD__ );
$this->dieUsage( $this->msg(
'wikibase-api-modify-failed' )->text(), 'modify-failed' );
}
diff --git a/repo/includes/api/SetSiteLink.php
b/repo/includes/api/SetSiteLink.php
index 6d5392b..ca4be9a 100644
--- a/repo/includes/api/SetSiteLink.php
+++ b/repo/includes/api/SetSiteLink.php
@@ -75,22 +75,29 @@
$summary = $this->createSummary( $params );
$summary->setLanguage( $params['linksite'] ); //XXX: not really
a language!
+ if ( !( $entityContent instanceof ItemContent ) ) {
+ wfProfileOut( __METHOD__ );
+ $this->dieUsage( "The given entity is not an item",
"not-and-item" );
+ }
+
+ /* @var Item $item */
+ $item = $entityContent->getItem();
+
if ( isset( $params['linksite'] ) && ( $params['linktitle'] ===
'' ) ) {
+ $linksite = Utils::trimToNFC( $params['linksite'] );
+
try {
- $link =
$entityContent->getItem()->getSimpleSiteLink( Utils::trimToNFC(
$params['linksite'] ) );
- }
- catch ( \OutOfBoundsException $exception ) {
- wfProfileOut( __METHOD__ );
- $this->dieUsage( $this->msg(
'wikibase-api-remove-sitelink-failed' )->text(), 'remove-sitelink-failed' );
- }
+ $link = $item->getSimpleSiteLink( $linksite );
+ $item->removeSiteLink( $params['linksite'] );
+ $this->addSiteLinksToResult( array( $link ),
'entity', 'sitelinks', 'sitelink', array( 'removed' ) );
- $entityContent->getItem()->removeSiteLink(
$params['linksite'] );
- $this->addSiteLinksToResult( array( $link ), 'entity',
'sitelinks', 'sitelink', array( 'removed' ) );
-
- $summary->setAction( 'remove' );
+ $summary->setAction( 'remove' );
+ } catch ( \OutOfBoundsException $exception ) {
+ // never mind then
+ }
wfProfileOut( __METHOD__ );
- return $summary;
+ return $summary; // would be nice to signal "nothing to
do" somehow
}
else {
$sites = $this->getSiteLinkTargetSites();
@@ -110,7 +117,7 @@
$link = new SimpleSiteLink( $site->getGlobalId(), $page
);
- $entityContent->getEntity()->addSimpleSiteLink( $link );
+ $item->addSimpleSiteLink( $link );
$this->addSiteLinksToResult( array( $link ), 'entity',
'sitelinks', 'sitelink', array( 'url' ) );
diff --git a/repo/tests/phpunit/includes/api/SetSiteLinkTest.php
b/repo/tests/phpunit/includes/api/SetSiteLinkTest.php
index 579a0c4..082ab62 100644
--- a/repo/tests/phpunit/includes/api/SetSiteLinkTest.php
+++ b/repo/tests/phpunit/includes/api/SetSiteLinkTest.php
@@ -36,6 +36,7 @@
* @group WikibaseAPI
* @group SetSiteLinkTest
* @group BreakingTheSlownessBarrier
+ * @group XXX
*
* The database group has as a side effect that temporal database tables are
created. This makes
* it possible to test without poisoning a production database.
@@ -186,8 +187,7 @@
array( 'site' => 'dewiki', 'title' => 'London'
), // by sitelink
'svwiki', // not set
'', // remove the entry
- false, // will fail
- 'should not be able to remove non-existing
link' //XXX: shouldn't that rather be a warning?!
+ false, // expect nothing
),
);
}
@@ -228,13 +228,20 @@
// check the response -------------------------------
//$this->assertSuccess( $res, 'entity', 'sitelinks', 0
);
- $this->assertEquals( 1, count(
$res['entity']['sitelinks'] ), "expected exactly one sitelinks structure" );
+ if ( $expectedTitle !== false ) {
+ $this->assertEquals( 1, count(
$res['entity']['sitelinks'] ), "expected exactly one sitelinks structure" );
+ }
+
$this->assertArrayHasKey( 'lastrevid', $res['entity'] ,
'entity should contain lastrevid key' );
- $link = array_shift( $res['entity']['sitelinks'] );
- $this->assertEquals( $linksite, $link['site'] );
+ if ( $expectedTitle !== false ) {
+ $link = array_shift(
$res['entity']['sitelinks'] );
+ $this->assertEquals( $linksite, $link['site'] );
+ } else {
+ $link = null;
+ }
- if ( $linktitle === '' ) {
+ if ( $linktitle === '' && $link !== null ) {
$this->assertArrayHasKey( 'removed', $link );
}
@@ -245,7 +252,7 @@
if ( $expectedTitle !== false && $linktitle !== '' ) {
$this->assertArrayHasKey( 'url', $link );
}
- else {
+ elseif ( $link !== null ) {
$this->assertArrayNotHasKey( 'url', $link );
}
} catch ( \UsageException $e ) {
--
To view, visit https://gerrit.wikimedia.org/r/70848
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I4d11fd89345191baf8ea4874d9fbf32e173bcf32
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Aude <[email protected]>
Gerrit-Reviewer: Denny Vrandecic <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
