jenkins-bot has submitted this change and it was merged.
Change subject: Use ChangeOps to generate edit summaries in API modules
......................................................................
Use ChangeOps to generate edit summaries in API modules
* wbsetlabel, wbsetdescription, wbsetaliases and wbsetsitelink
are now using ChangeOps
* edit summaries are updated when ChangeOps get applied
Handling of summaries of multiple ChangeOps in one API request
will be done in a followup.
Change-Id: I44c407a708b5322fdaa7adf14ff20d5561ab67c0
---
M docs/summaries.txt
M repo/Wikibase.i18n.php
M repo/includes/api/SetAliases.php
M repo/includes/api/SetDescription.php
M repo/includes/api/SetLabel.php
M repo/includes/api/SetSiteLink.php
M repo/includes/changeop/ChangeOp.php
M repo/includes/changeop/ChangeOpAliases.php
M repo/includes/changeop/ChangeOpDescription.php
M repo/includes/changeop/ChangeOpLabel.php
M repo/includes/changeop/ChangeOpSiteLink.php
M repo/includes/changeop/ChangeOps.php
M repo/tests/phpunit/includes/changeop/ChangeOpDescriptionTest.php
M repo/tests/phpunit/includes/changeop/ChangeOpLabelTest.php
14 files changed, 323 insertions(+), 145 deletions(-)
Approvals:
Addshore: Looks good to me, approved
jenkins-bot: Verified
diff --git a/docs/summaries.txt b/docs/summaries.txt
index aad6402..f6888ed 100644
--- a/docs/summaries.txt
+++ b/docs/summaries.txt
@@ -11,9 +11,11 @@
/* wbsetsitelink-set:1|site */ linktitle
/* wbsetsitelink-remove:1|site */
+/* wbsetlabel-add:1|lang */ value
/* wbsetlabel-set:1|lang */ value
/* wbsetlabel-remove:1|lang */ value
+/* wbsetdescription-add:1|lang */ value
/* wbsetdescription-set:1|lang */ value
/* wbsetdescription-remove:1|lang */ value
diff --git a/repo/Wikibase.i18n.php b/repo/Wikibase.i18n.php
index 6246a4f..019bdd9 100644
--- a/repo/Wikibase.i18n.php
+++ b/repo/Wikibase.i18n.php
@@ -293,8 +293,10 @@
'wikibase-item-summary-wbeditentity-update' => 'Updated an item',
'wikibase-item-summary-wbeditentity-override' => 'Cleared an item',
'wikibase-item-summary-wbsetreference' => 'Set a reference',
+ 'wikibase-item-summary-wbsetlabel-add' => 'Added [$2] label',
'wikibase-item-summary-wbsetlabel-set' => 'Changed [$2] label',
'wikibase-item-summary-wbsetlabel-remove' => 'Removed [$2] label',
+ 'wikibase-item-summary-wbsetdescription-add' => 'Added [$2]
description',
'wikibase-item-summary-wbsetdescription-set' => 'Changed [$2]
description',
'wikibase-item-summary-wbsetdescription-remove' => 'Removed [$2]
description',
'wikibase-item-summary-wbsetaliases-set' => 'Setting [$2]
{{PLURAL:$1|alias|aliases}}',
@@ -960,9 +962,11 @@
'wikibase-item-summary-wbeditentity-update' => '{{wikibase summary
messages|item|Automatic edit summary generated when updating an existing
item.}}',
'wikibase-item-summary-wbeditentity-override' => '{{wikibase summary
messages|item|Automatic edit summary generated when overriding an existing
item.}}',
'wikibase-item-summary-wbsetreference' => '{{wikibase summary
messages|item|Automatic edit summary generated when creating a reference or
setting the value of an existing one.}}',
+ 'wikibase-item-summary-wbsetlabel-add' => "{{wikibase summary
messages|item|Automatic edit summary generated when creating an item label,
which appears in the ''h1'' heading at top of the item-page. Example of a full
edit summary is \"Added [en] label: Egypt\".}}",
'wikibase-item-summary-wbsetlabel-set' => "{{wikibase summary
messages|item|Automatic edit summary generated when editing an item label,
which appears in the ''h1'' heading at top of the item-page. Example of a full
edit summary is \"Updated [en] label: Egypt\".}}",
'wikibase-item-summary-wbsetlabel-remove' => '{{wikibase summary
messages|item|Automatic edit summary generated when removing an item label.
Example edit summary is "Removed [en] label".}}',
- 'wikibase-item-summary-wbsetdescription-set' => '{{wikibase summary
messages|item|Automatic edit summary when creating or editing an item
description. Example edit summary is "Updated [en] description: a country in
Africa".}}',
+ 'wikibase-item-summary-wbsetdescription-add' => '{{wikibase summary
messages|item|Automatic edit summary when creating an item description. Example
edit summary is "Added [en] description: a country in Africa".}}',
+ 'wikibase-item-summary-wbsetdescription-set' => '{{wikibase summary
messages|item|Automatic edit summary when editing an item description. Example
edit summary is "Updated [en] description: a country in Africa".}}',
'wikibase-item-summary-wbsetdescription-remove' => '{{wikibase summary
messages|item|Automatic edit summary when removing an item description. Example
edit summary is "Removed [en] description".}}',
'wikibase-item-summary-wbsetaliases-set' => '{{wikibase summary
messages|item|Automatic edit summary when adding or editing item aliases.
Example for adding aliases on the English item page for Italy: "Setting [en]
aliases: Italian Republic, Repubblica italiana".}}',
'wikibase-item-summary-wbsetaliases-add-remove' => '{{wikibase summary
messages|item|Automatic edit summary when adding and removing item aliases. A
user can be in edit mode for aliases and can remove one or more aliases, while
adding others in the same edit.}}',
diff --git a/repo/includes/api/SetAliases.php b/repo/includes/api/SetAliases.php
index 622e1b9..9f6e934 100644
--- a/repo/includes/api/SetAliases.php
+++ b/repo/includes/api/SetAliases.php
@@ -2,12 +2,14 @@
namespace Wikibase\Api;
-use ApiBase, User, Language;
+use Wikibase\ChangeOps;
+use ApiBase, User, Language;
use Wikibase\Entity;
use Wikibase\EntityContent;
use Wikibase\Autocomment;
use Wikibase\Utils;
+use Wikibase\ChangeOpAliases;
/**
* API module to set the aliases for a Wikibase entity.
@@ -23,6 +25,7 @@
* @author Jeroen De Dauw < [email protected] >
* @author John Erling Blad < [email protected] >
* @author Marius Hoch < [email protected] >
+ * @author Tobias Gritschacher < [email protected] >
*/
class SetAliases extends ModifyEntity {
@@ -49,7 +52,7 @@
parent::validateParameters( $params );
if ( !( ( !empty( $params['add'] ) || !empty( $params['remove']
) ) xor isset( $params['set'] ) ) ) {
- $this->dieUsage( 'Invalid-list' , 'invalid-list' );
+ $this->dieUsage( "Parameters 'add' and 'remove' are not
allowed to be set when parameter 'set' is provided" , 'invalid-list' );
}
}
@@ -64,67 +67,32 @@
* @see \Wikibase\Api\ModifyEntity::modifyEntity()
*/
protected function modifyEntity( EntityContent &$entityContent, array
$params ) {
- $stringNormalizer = $this->stringNormalizer; // hack for PHP
fail.
-
wfProfileIn( __METHOD__ );
$summary = $this->createSummary( $params );
- $summary->setLanguage( $params['language'] );
+ $entity = $entityContent->getEntity();
+ $language = $params['language'];
- // Set the list of aliases to a user given one OR add/ remove
certain entries
- if ( isset( $params['set'] ) ) {
- $summary->setAction( 'set' );
- $summary->addAutoSummaryArgs( $params['set'] );
- $entityContent->getEntity()->setAliases(
- $params['language'],
- array_map(
- function( $str ) use (
$stringNormalizer ) { return $stringNormalizer->trimToNFC( $str ); },
- $params['set']
- )
- );
+ $aliasesChangeOps = $this->getChangeOps( $params );
+ if ( count( $aliasesChangeOps ) == 1 ) {
+ $aliasesChangeOps[0]->apply( $entity, $summary );
} else {
+ $changeOps = new ChangeOps();
+ $changeOps->add( $aliasesChangeOps );
+ $changeOps->apply( $entity );
- if ( !empty( $params['add'] ) ) {
- $entityContent->getEntity()->addAliases(
- $params['language'],
- array_map(
- function( $str ) use (
$stringNormalizer ) { return $stringNormalizer->trimToNFC( $str ); },
- $params['add']
- )
- );
- }
+ // Set the action to 'set' in case we add and remove
aliases in a single edit
+ $summary->setAction( 'set' );
+ $summary->setLanguage( $language );
- if ( !empty( $params['remove'] ) ) {
- $entityContent->getEntity()->removeAliases(
- $params['language'],
- array_map(
- function( $str ) use (
$stringNormalizer ) { return $stringNormalizer->trimToNFC( $str ); },
- $params['remove']
- )
- );
- }
-
- // Set the action to set in case we add and remove
entries in a single edit.
- if ( !empty( $params['add'] ) && !empty(
$params['remove'] ) ) {
- $summary->setAction( 'set' );
- // Get the full list of current aliases
- $summary->addAutoSummaryArgs(
-
$entityContent->getEntity()->getAliases( $params['language'] )
- );
- } elseif ( !empty( $params['add'] ) ) {
- $summary->setAction( 'add' );
- $summary->addAutoSummaryArgs( $params['add'] );
- } elseif ( !empty( $params['remove'] ) ) {
- $summary->setAction( 'remove' );
- $summary->addAutoSummaryArgs( $params['remove']
);
- }
-
+ // Get the full list of current aliases
+ $summary->addAutoSummaryArgs( $entity->getAliases(
$language ) );
}
- $aliases = $entityContent->getEntity()->getAliases(
$params['language'] );
+ $aliases = $entity->getAliases( $language );
if ( count( $aliases ) ) {
- $this->addAliasesToResult( array( $params['language']
=> $aliases ), 'entity' );
+ $this->addAliasesToResult( array( $language => $aliases
), 'entity' );
}
wfProfileOut( __METHOD__ );
@@ -132,6 +100,72 @@
}
/**
+ * @since 0.4
+ *
+ * @param array $params
+ * @return ChangeOpAliases
+ */
+ protected function getChangeOps( array $params ) {
+ $stringNormalizer = $this->stringNormalizer; // hack for PHP
fail.
+
+ wfProfileIn( __METHOD__ );
+ $changeOps = array();
+ $language = $params['language'];
+
+ // Set the list of aliases to a user given one OR add/ remove
certain entries
+ if ( isset( $params['set'] ) ) {
+ $changeOps[] =
+ new ChangeOpAliases(
+ $language,
+ array_map(
+ function( $str ) use (
$stringNormalizer ) {
+ return
$stringNormalizer->trimToNFC( $str );
+ },
+ $params['set']
+ ),
+ 'set',
+ $this->createSummary( $params )
+ );
+ } else {
+ // FIXME: if we have ADD and REMOVE operations in the
same call,
+ // we will also have two ChangeOps updating the same
edit summary.
+ // This will cause the edit summary to be overwritten
by the last ChangeOp beeing applied.
+ if ( !empty( $params['add'] ) ) {
+ $changeOps[] =
+ new ChangeOpAliases(
+ $language,
+ array_map(
+ function( $str ) use (
$stringNormalizer ) {
+ return
$stringNormalizer->trimToNFC( $str );
+ },
+ $params['add']
+ ),
+ 'add',
+ $this->createSummary( $params )
+ );
+ }
+
+ if ( !empty( $params['remove'] ) ) {
+ $changeOps[] =
+ new ChangeOpAliases(
+ $language,
+ array_map(
+ function( $str ) use (
$stringNormalizer ) {
+ return
$stringNormalizer->trimToNFC( $str );
+ },
+ $params['remove']
+ ),
+ 'remove',
+ $this->createSummary( $params )
+ );
+ }
+ }
+
+ wfProfileOut( __METHOD__ );
+ return $changeOps;
+ }
+
+ /**
* @see ApiBase::getPossibleErrors()
*/
public function getPossibleErrors() {
diff --git a/repo/includes/api/SetDescription.php
b/repo/includes/api/SetDescription.php
index 31b3b9f..61e1f4d 100644
--- a/repo/includes/api/SetDescription.php
+++ b/repo/includes/api/SetDescription.php
@@ -7,6 +7,7 @@
use Wikibase\Entity;
use Wikibase\EntityContent;
use Wikibase\Utils;
+use Wikibase\ChangeOpDescription;
/**
* API module for the language attributes for a Wikibase entity.
@@ -21,6 +22,7 @@
* @licence GNU GPL v2+
* @author Jeroen De Dauw < [email protected] >
* @author John Erling Blad < [email protected] >
+ * @author Tobias Gritschacher < [email protected] >
*/
class SetDescription extends ModifyLangAttribute {
@@ -42,32 +44,42 @@
protected function modifyEntity( EntityContent &$entityContent, array
$params ) {
wfProfileIn( __METHOD__ );
$summary = $this->createSummary( $params );
+ $entity = $entityContent->getEntity();
+ $language = $params['language'];
- if ( isset( $params['value'] ) ) {
-
- $description = $this->stringNormalizer->trimToNFC(
$params['value'] );
- $language = $params['language'];
-
- if ( 0 < strlen( $description ) ) {
- $summary->addAutoSummaryArgs( $description );
- $descriptions = array( $language =>
$entityContent->getEntity()->setDescription( $language, $description ) );
- }
- else {
- $old =
$entityContent->getEntity()->getDescription( $language );
- $summary->addAutoSummaryArgs( $old );
-
- $entityContent->getEntity()->removeDescription(
$language );
- $descriptions = array( $language => '' );
- }
-
- $this->addDescriptionsToResult( $descriptions, 'entity'
);
- }
+ $this->getChangeOp( $params )->apply( $entity, $summary );
+ $descriptions = array( $language => $entity->getDescription(
$language ) ? $entity->getDescription( $language ) : "" );
+ $this->addDescriptionsToResult( $descriptions, 'entity' );
wfProfileOut( __METHOD__ );
return $summary;
}
/**
+ * @since 0.4
+ *
+ * @param array $params
+ * @return ChangeOpDescription
+ */
+ protected function getChangeOp( array $params ) {
+ wfProfileIn( __METHOD__ );
+ $description = "";
+ $language = $params['language'];
+
+ if ( isset( $params['value'] ) ) {
+ $description = $this->stringNormalizer->trimToNFC(
$params['value'] );
+ }
+
+ if ( $description === "" ) {
+ wfProfileOut( __METHOD__ );
+ return new ChangeOpDescription( $language, null );
+ } else {
+ wfProfileOut( __METHOD__ );
+ return new ChangeOpDescription( $language, $description
);
+ }
+ }
+
+ /**
* @see \ApiBase::getParamDescription()
*/
public function getParamDescription() {
diff --git a/repo/includes/api/SetLabel.php b/repo/includes/api/SetLabel.php
index 92e5324..b2a00a6 100644
--- a/repo/includes/api/SetLabel.php
+++ b/repo/includes/api/SetLabel.php
@@ -3,10 +3,10 @@
namespace Wikibase\Api;
use ApiBase;
-
use Wikibase\Entity;
use Wikibase\EntityContent;
use Wikibase\Utils;
+use Wikibase\ChangeOpLabel;
/**
* API module to set the label for a Wikibase entity.
@@ -21,6 +21,7 @@
* @licence GNU GPL v2+
* @author Jeroen De Dauw < [email protected] >
* @author John Erling Blad < [email protected] >
+ * @author Tobias Gritschacher < [email protected] >
*/
class SetLabel extends ModifyLangAttribute {
@@ -30,7 +31,7 @@
protected function getRequiredPermissions( Entity $entity, array
$params ) {
$permissions = parent::getRequiredPermissions( $entity, $params
);
- $permissions[] = ( isset( $params['value'] ) && 0<strlen(
$params['value'] ) )
+ $permissions[] = ( isset( $params['value'] ) && 0 < strlen(
$params['value'] ) )
? 'label-update'
: 'label-remove';
return $permissions;
@@ -42,31 +43,43 @@
protected function modifyEntity( EntityContent &$entityContent, array
$params ) {
wfProfileIn( __METHOD__ );
$summary = $this->createSummary( $params );
+ $entity = $entityContent->getEntity();
+ $language = $params['language'];
- if ( isset( $params['value'] ) ) {
-
- $label = $this->stringNormalizer->trimToNFC(
$params['value'] );
- $language = $params['language'];
- if ( 0 < strlen( $label ) ) {
- $summary->addAutoSummaryArgs( $label );
- $labels = array( $language =>
$entityContent->getEntity()->setLabel( $language, $label ) );
- }
- else {
- $old = $entityContent->getEntity()->getLabel(
$language );
- $summary->addAutoSummaryArgs( $old );
-
- $entityContent->getEntity()->removeLabel(
$language );
- $labels = array( $language => '' );
- }
-
- $this->addLabelsToResult( $labels, 'entity' );
- }
+ $this->getChangeOp( $params )->apply( $entity, $summary );
+ $labels = array( $language => $entity->getLabel( $language ) ?
$entity->getLabel( $language ) : "" );
+ $this->addLabelsToResult( $labels, 'entity' );
wfProfileOut( __METHOD__ );
return $summary;
}
/**
+ * @since 0.4
+ *
+ * @param array $params
+ * @return ChangeOpLabel
+ */
+ protected function getChangeOp( array $params ) {
+ wfProfileIn( __METHOD__ );
+ $changeOps = array();
+ $label = "";
+ $language = $params['language'];
+
+ if ( isset( $params['value'] ) ) {
+ $label = $this->stringNormalizer->trimToNFC(
$params['value'] );
+ }
+
+ if ( $label === "" ) {
+ wfProfileOut( __METHOD__ );
+ return new ChangeOpLabel( $language, null );
+ } else {
+ wfProfileOut( __METHOD__ );
+ return new ChangeOpLabel( $language, $label );
+ }
+ }
+
+ /**
* @see \ApiBase::getParamDescription()
*/
public function getParamDescription() {
diff --git a/repo/includes/api/SetSiteLink.php
b/repo/includes/api/SetSiteLink.php
index eacc13e..cb3beee 100644
--- a/repo/includes/api/SetSiteLink.php
+++ b/repo/includes/api/SetSiteLink.php
@@ -2,8 +2,8 @@
namespace Wikibase\Api;
+use Wikibase\ChangeOpSiteLink;
use ApiBase, User;
-
use Wikibase\DataModel\SimpleSiteLink;
use Wikibase\Entity;
use Wikibase\EntityContent;
@@ -26,6 +26,7 @@
* @author John Erling Blad < [email protected] >
* @author Jeroen De Dauw < [email protected] >
* @author Daniel Kinzler
+ * @author Tobias Gritschacher < [email protected] >
*/
class SetSiteLink extends ModifyEntity {
@@ -72,36 +73,48 @@
*/
protected function modifyEntity( EntityContent &$entityContent, array
$params ) {
wfProfileIn( __METHOD__ );
- $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-item" );
}
- /* @var Item $item */
+ $summary = $this->createSummary( $params );
$item = $entityContent->getItem();
+ $linksite = $this->stringNormalizer->trimToNFC(
$params['linksite'] );
+ if ( isset( $params['linksite'] ) && $params['linktitle'] ===
'' ) {
+ if ( $item->hasLinkToSite( $linksite ) ) {
+ $link = $item->getSimpleSiteLink( $linksite );
+ $this->getChangeOp( $params )->apply( $item,
$summary );
+ $this->addSiteLinksToResult( array( $link ),
'entity', 'sitelinks', 'sitelink', array( 'removed' ) );
+ }
+ } else {
+ $this->getChangeOp( $params )->apply( $item, $summary );
+ $link = $item->getSimpleSiteLink( $linksite );
+ $this->addSiteLinksToResult( array( $link ), 'entity',
'sitelinks', 'sitelink', array( 'url' ) );
+ }
+
+ wfProfileOut( __METHOD__ );
+ return $summary;
+ }
+
+ /**
+ * @since 0.4
+ *
+ * @param array $params
+ * @return ChangeOpSiteLink
+ */
+ protected function getChangeOp( array $params ) {
+ wfProfileIn( __METHOD__ );
if ( isset( $params['linksite'] ) && ( $params['linktitle'] ===
'' ) ) {
$linksite = $this->stringNormalizer->trimToNFC(
$params['linksite'] );
-
- try {
- $link = $item->getSimpleSiteLink( $linksite );
- $item->removeSiteLink( $params['linksite'] );
- $this->addSiteLinksToResult( array( $link ),
'entity', 'sitelinks', 'sitelink', array( 'removed' ) );
-
- $summary->setAction( 'remove' );
- } catch ( \OutOfBoundsException $exception ) {
- // never mind then
- }
-
wfProfileOut( __METHOD__ );
- return $summary; // would be nice to signal "nothing to
do" somehow
- }
- else {
+ return new ChangeOpSiteLink( $linksite, null );
+ } else {
+ $linksite = $this->stringNormalizer->trimToNFC(
$params['linksite'] );
$sites = $this->getSiteLinkTargetSites();
- $site = $sites->getSite( $params['linksite'] );
+ $site = $sites->getSite( $linksite );
if ( $site === false ) {
wfProfileOut( __METHOD__ );
@@ -115,17 +128,8 @@
$this->dieUsage( 'The external client site did
not provide page information' , 'no-external-page' );
}
- $link = new SimpleSiteLink( $site->getGlobalId(), $page
);
-
- $item->addSimpleSiteLink( $link );
-
- $this->addSiteLinksToResult( array( $link ), 'entity',
'sitelinks', 'sitelink', array( 'url' ) );
-
- $summary->setAction( 'set' );
- $summary->addAutoSummaryArgs( $page );
-
wfProfileOut( __METHOD__ );
- return $summary;
+ return new ChangeOpSiteLink( $linksite, $page );
}
}
diff --git a/repo/includes/changeop/ChangeOp.php
b/repo/includes/changeop/ChangeOp.php
index 2a2b15b..354209e 100644
--- a/repo/includes/changeop/ChangeOp.php
+++ b/repo/includes/changeop/ChangeOp.php
@@ -28,15 +28,38 @@
* @licence GNU GPL v2+
* @author Tobias Gritschacher < [email protected] >
*/
-interface ChangeOp {
+abstract class ChangeOp {
/**
* @since 0.4
*
* @param Entity $entity
+ * @param Summary|null $summary
*
* @return bool
*/
- public function apply( Entity $entity );
+ abstract public function apply( Entity $entity, Summary $summary = null
);
+
+ /**
+ * @since 0.4
+ *
+ * @param Summary $summary
+ * @param string $action
+ * @param string $language
+ * @param string|array $args
+ *
+ * @throws InvalidArgumentException
+ */
+ protected function updateSummary( $summary, $action, $language, $args )
{
+ if ( $summary !== null && !$summary instanceof Summary ) {
+ throw new InvalidArgumentException( '$summary needs to
be an instance of Summary or null' );
+ }
+
+ if ( $summary !== null ) {
+ $summary->setAction( $action );
+ $summary->setLanguage( $language );
+ $summary->addAutoSummaryArgs( $args );
+ }
+ }
}
diff --git a/repo/includes/changeop/ChangeOpAliases.php
b/repo/includes/changeop/ChangeOpAliases.php
index d6d9854..867738f 100644
--- a/repo/includes/changeop/ChangeOpAliases.php
+++ b/repo/includes/changeop/ChangeOpAliases.php
@@ -30,7 +30,7 @@
* @licence GNU GPL v2+
* @author Tobias Gritschacher < [email protected] >
*/
-class ChangeOpAliases implements ChangeOp {
+class ChangeOpAliases extends ChangeOp {
/**
* @since 0.4
@@ -58,7 +58,7 @@
*
* @param string $language
* @param string[] $aliases
- * @param string $action
+ * @param string $action should be set|add|remove
*
* @throws InvalidArgumentException
*/
@@ -82,22 +82,25 @@
* @since 0.4
*
* @param Entity $entity
+ * @param Summary|null $summary
*
* @return bool
*
* @throws MWException
*/
- public function apply( Entity $entity ) {
+ public function apply( Entity $entity, Summary $summary = null ) {
if ( $this->action === "" || $this->action === "set" ) {
+ $this->updateSummary( $summary, 'set', $this->language,
$this->aliases );
$entity->setAliases( $this->language, $this->aliases );
} elseif ( $this->action === "add" ) {
+ $this->updateSummary( $summary, 'add', $this->language,
$this->aliases );
$entity->addAliases( $this->language, $this->aliases );
} elseif ( $this->action === "remove" ) {
+ $this->updateSummary( $summary, 'remove',
$this->language, $this->aliases );
$entity->removeAliases( $this->language, $this->aliases
);
} else {
throw new \MWException( "Unknown action: $this->action"
);
}
return true;
}
-
}
diff --git a/repo/includes/changeop/ChangeOpDescription.php
b/repo/includes/changeop/ChangeOpDescription.php
index fe86075..c249180 100644
--- a/repo/includes/changeop/ChangeOpDescription.php
+++ b/repo/includes/changeop/ChangeOpDescription.php
@@ -29,7 +29,7 @@
* @licence GNU GPL v2+
* @author Tobias Gritschacher < [email protected] >
*/
-class ChangeOpDescription implements ChangeOp {
+class ChangeOpDescription extends ChangeOp {
/**
* @since 0.4
@@ -68,16 +68,19 @@
* @since 0.4
*
* @param Entity $entity
+ * @param Summary|null $summary
*
* @return bool
*/
- public function apply( Entity $entity ) {
+ public function apply( Entity $entity, Summary $summary = null ) {
if ( $this->description === null ) {
+ $this->updateSummary( $summary, 'remove',
$this->language, $entity->getDescription( $this->language ) );
$entity->removeDescription( $this->language );
} else {
+ $entity->getDescription( $this->language ) === false ?
$action = 'add' : $action = 'set';
+ $this->updateSummary( $summary, $action,
$this->language, $this->description );
$entity->setDescription( $this->language,
$this->description );
}
return true;
}
-
}
diff --git a/repo/includes/changeop/ChangeOpLabel.php
b/repo/includes/changeop/ChangeOpLabel.php
index 0a19a44..9f825ee 100644
--- a/repo/includes/changeop/ChangeOpLabel.php
+++ b/repo/includes/changeop/ChangeOpLabel.php
@@ -29,7 +29,7 @@
* @licence GNU GPL v2+
* @author Tobias Gritschacher < [email protected] >
*/
-class ChangeOpLabel implements ChangeOp {
+class ChangeOpLabel extends ChangeOp {
/**
* @since 0.4
@@ -68,16 +68,19 @@
* @since 0.4
*
* @param Entity $entity
+ * @param Summary|null $summary
*
* @return bool
*/
- public function apply( Entity $entity ) {
+ public function apply( Entity $entity, Summary $summary = null ) {
if ( $this->label === null ) {
+ $this->updateSummary( $summary, 'remove',
$this->language, $entity->getLabel( $this->language ) );
$entity->removeLabel( $this->language );
} else {
+ $entity->getLabel( $this->language ) === false ?
$action = 'add' : $action = 'set';
+ $this->updateSummary( $summary, $action,
$this->language, $this->label );
$entity->setLabel( $this->language, $this->label );
}
return true;
}
-
}
diff --git a/repo/includes/changeop/ChangeOpSiteLink.php
b/repo/includes/changeop/ChangeOpSiteLink.php
index 6e9165a..8e1c5c0 100644
--- a/repo/includes/changeop/ChangeOpSiteLink.php
+++ b/repo/includes/changeop/ChangeOpSiteLink.php
@@ -31,7 +31,7 @@
* @licence GNU GPL v2+
* @author Tobias Gritschacher < [email protected] >
*/
-class ChangeOpSiteLink implements ChangeOp {
+class ChangeOpSiteLink extends ChangeOp {
/**
* @since 0.4
@@ -74,20 +74,25 @@
* @since 0.4
*
* @param Entity $entity
+ * @param Summary|null $summary
*
* @throws InvalidArgumentException
*/
- public function apply( Entity $entity ) {
+ public function apply( Entity $entity, Summary $summary = null ) {
if ( !( $entity instanceof Item ) ) {
throw new InvalidArgumentException( 'ChangeOpSiteLink
can only be applied to Item instances' );
}
if ( $this->pageName === null ) {
- $entity->removeSiteLink( $this->siteId );
- }
- else {
+ if ( $entity->hasLinkToSite( $this->siteId ) ) {
+ $this->updateSummary( $summary, 'remove',
$this->siteId, $entity->getSimpleSiteLink( $this->siteId )->getPageName() );
+ $entity->removeSiteLink( $this->siteId );
+ }
+ } else {
+ $this->updateSummary( $summary, 'set', $this->siteId,
$this->pageName );
$entity->addSimpleSiteLink( new SimpleSiteLink(
$this->siteId, $this->pageName ) );
}
- }
+ return true;
+ }
}
diff --git a/repo/includes/changeop/ChangeOps.php
b/repo/includes/changeop/ChangeOps.php
index 68091b9..f286cd8 100644
--- a/repo/includes/changeop/ChangeOps.php
+++ b/repo/includes/changeop/ChangeOps.php
@@ -86,15 +86,17 @@
/**
* Applies all changes to the given entity
+ *
* @since 0.4
*
* @param Entity $entity
+ * @param Summary|null $summary
*
* @return bool
*/
- public function apply( Entity $entity ) {
+ public function apply( Entity $entity, Summary $summary = null ) {
foreach ( $this->ops as $op ) {
- if ( $op->apply( $entity ) === false ) {
+ if ( $op->apply( $entity, $summary ) === false ) {
return false;
}
}
diff --git a/repo/tests/phpunit/includes/changeop/ChangeOpDescriptionTest.php
b/repo/tests/phpunit/includes/changeop/ChangeOpDescriptionTest.php
index add402c..b12d2d1 100644
--- a/repo/tests/phpunit/includes/changeop/ChangeOpDescriptionTest.php
+++ b/repo/tests/phpunit/includes/changeop/ChangeOpDescriptionTest.php
@@ -5,6 +5,7 @@
use Wikibase\ChangeOpDescription;
use Wikibase\ItemContent;
use InvalidArgumentException;
+use Wikibase\Summary;
/**
* @covers Wikibase\ChangeOpDescription
@@ -60,8 +61,7 @@
* @param string $expectedDescription
*/
public function testApply( $changeOpDescription, $expectedDescription )
{
- $item = ItemContent::newEmpty();
- $entity = $item->getEntity();
+ $entity = $this->provideNewEntity();
$entity->setDescription( 'en', 'test' );
$changeOpDescription->apply( $entity );
@@ -69,4 +69,39 @@
$this->assertEquals( $expectedDescription,
$entity->getDescription( 'en' ) );
}
+ protected function provideNewEntity() {
+ $item = ItemContent::newEmpty();
+ return $item->getEntity();
+ }
+
+ public function changeOpSummaryProvider() {
+ $args = array();
+
+ $entity = $this->provideNewEntity();
+ $entity->setDescription( 'de', 'Test' );
+ $args[] = array ( $entity, new ChangeOpDescription( 'de',
'Zusammenfassung' ), 'set', 'de' );
+
+ $entity = $this->provideNewEntity();
+ $entity->setDescription( 'de', 'Test' );
+ $args[] = array ( $entity, new ChangeOpDescription( 'de', null
), 'remove', 'de' );
+
+ $entity = $this->provideNewEntity();
+ $entity->removeDescription( 'de' );
+ $args[] = array ( $entity, new ChangeOpDescription( 'de',
'Zusammenfassung' ), 'add', 'de' );
+
+ return $args;
+ }
+
+ /**
+ * @dataProvider changeOpSummaryProvider
+ */
+ public function testUpdateSummary( $entity, $changeOp,
$summaryExpectedAction, $summaryExpectedLanguage ) {
+ $summary = new Summary();
+
+ $changeOp->apply( $entity, $summary );
+
+ $this->assertEquals( $summaryExpectedAction,
$summary->getActionName() );
+ $this->assertEquals( $summaryExpectedLanguage,
$summary->getLanguageCode() );
+ }
+
}
diff --git a/repo/tests/phpunit/includes/changeop/ChangeOpLabelTest.php
b/repo/tests/phpunit/includes/changeop/ChangeOpLabelTest.php
index 27bb205..1a1ff31 100644
--- a/repo/tests/phpunit/includes/changeop/ChangeOpLabelTest.php
+++ b/repo/tests/phpunit/includes/changeop/ChangeOpLabelTest.php
@@ -2,6 +2,7 @@
namespace Wikibase\Test;
+use Wikibase\Summary;
use Wikibase\ChangeOpLabel;
use Wikibase\ItemContent;
use InvalidArgumentException;
@@ -60,8 +61,7 @@
* @param string $expectedLabel
*/
public function testApply( $changeOpLabel, $expectedLabel ) {
- $item = ItemContent::newEmpty();
- $entity = $item->getEntity();
+ $entity = $this->provideNewEntity();
$entity->setLabel( 'en', 'test' );
$changeOpLabel->apply( $entity );
@@ -69,4 +69,39 @@
$this->assertEquals( $expectedLabel, $entity->getLabel( 'en' )
);
}
+ protected function provideNewEntity() {
+ $item = ItemContent::newEmpty();
+ return $item->getEntity();
+ }
+
+ public function changeOpSummaryProvider() {
+ $args = array();
+
+ $entity = $this->provideNewEntity();
+ $entity->setLabel( 'de', 'Test' );
+ $args[] = array ( $entity, new ChangeOpLabel( 'de',
'Zusammenfassung' ), 'set', 'de' );
+
+ $entity = $this->provideNewEntity();
+ $entity->setLabel( 'de', 'Test' );
+ $args[] = array ( $entity, new ChangeOpLabel( 'de', null ),
'remove', 'de' );
+
+ $entity = $this->provideNewEntity();
+ $entity->removeLabel( 'de' );
+ $args[] = array ( $entity, new ChangeOpLabel( 'de',
'Zusammenfassung' ), 'add', 'de' );
+
+ return $args;
+ }
+
+ /**
+ * @dataProvider changeOpSummaryProvider
+ */
+ public function testUpdateSummary( $entity, $changeOp,
$summaryExpectedAction, $summaryExpectedLanguage ) {
+ $summary = new Summary();
+
+ $changeOp->apply( $entity, $summary );
+
+ $this->assertEquals( $summaryExpectedAction,
$summary->getActionName() );
+ $this->assertEquals( $summaryExpectedLanguage,
$summary->getLanguageCode() );
+ }
+
}
--
To view, visit https://gerrit.wikimedia.org/r/74171
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I44c407a708b5322fdaa7adf14ff20d5561ab67c0
Gerrit-PatchSet: 12
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Tobias Gritschacher <[email protected]>
Gerrit-Reviewer: Addshore <[email protected]>
Gerrit-Reviewer: Aude <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Daniel Werner <[email protected]>
Gerrit-Reviewer: Henning Snater <[email protected]>
Gerrit-Reviewer: Jeroen De Dauw <[email protected]>
Gerrit-Reviewer: Tobias Gritschacher <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
