jenkins-bot has submitted this change and it was merged.
Change subject: Do not pass a whole list of claims to ClaimSummaryBuilder
......................................................................
Do not pass a whole list of claims to ClaimSummaryBuilder
Also removed some not needed internal complexity
Change-Id: I354063978be5c16bbc6b0eae46063100de44a18c
---
M repo/includes/ClaimSummaryBuilder.php
M repo/includes/api/SetClaim.php
M repo/tests/phpunit/includes/ClaimSummaryBuilderTest.php
3 files changed, 20 insertions(+), 28 deletions(-)
Approvals:
Thiemo Mättig (WMDE): Looks good to me, approved
jenkins-bot: Verified
diff --git a/repo/includes/ClaimSummaryBuilder.php
b/repo/includes/ClaimSummaryBuilder.php
index 162a801..87bf980 100644
--- a/repo/includes/ClaimSummaryBuilder.php
+++ b/repo/includes/ClaimSummaryBuilder.php
@@ -4,7 +4,6 @@
use InvalidArgumentException;
use Wikibase\DataModel\Claim\Claim;
-use Wikibase\DataModel\Claim\Claims;
use Wikibase\Repo\Diff\ClaimDiffer;
/**
@@ -52,20 +51,19 @@
* constructs an edit-summary based upon that information and returns
* a Summary object holding this edit-summary
*
- * @param Claims $existingClaims
+ * @param Claim|null $oldClaim
* @param Claim $newClaim
*
* @return Summary
*/
- public function buildClaimSummary( Claims $existingClaims, Claim
$newClaim ) {
+ public function buildClaimSummary( Claim $oldClaim = null, Claim
$newClaim ) {
$guid = $newClaim->getGuid();
- $oldClaim = $existingClaims->getClaimWithGuid( $guid );
$summary = new Summary( $this->apiModuleName );
$summary->addAutoCommentArgs( 1 ); // only one claim touched,
so we're always having singular here
$summaryArgs = $this->buildSummaryArgs(
- new Claims( array( $newClaim ) ),
- array( $guid )
+ $newClaim,
+ $guid
);
$summary->addAutoSummaryArgs( $summaryArgs );
@@ -105,27 +103,23 @@
* Builds an associative array that can be used as summary arguments.
It uses property IDs as
* array keys and builds arrays of the main Snaks of all Claims given
by the GUIDs.
*
- * @param Claims $claims
- * @param string[] $guids
+ * @param Claim $newClaim
+ * @param string $guid
*
* @return array[] Associative array that contains property ID => array
of main Snaks
*/
- private function buildSummaryArgs( Claims $claims, array $guids ) {
+ private function buildSummaryArgs( Claim $newClaim, $guid ) {
$pairs = array();
- foreach ( $guids as $guid ) {
- $claim = $claims->getClaimWithGuid( $guid );
+ if ( $newClaim->getGuid() === $guid ) {
+ $snak = $newClaim->getMainSnak();
+ $key = $snak->getPropertyId()->getSerialization();
- if ( $claim !== null ) {
- $snak = $claim->getMainSnak();
- $key =
$snak->getPropertyId()->getSerialization();
-
- if ( !array_key_exists( $key, $pairs ) ) {
- $pairs[$key] = array();
- }
-
- $pairs[$key][] = $snak;
+ if ( !array_key_exists( $key, $pairs ) ) {
+ $pairs[$key] = array();
}
+
+ $pairs[$key][] = $snak;
}
return array( $pairs );
diff --git a/repo/includes/api/SetClaim.php b/repo/includes/api/SetClaim.php
index a04b397..45bd7cb 100644
--- a/repo/includes/api/SetClaim.php
+++ b/repo/includes/api/SetClaim.php
@@ -104,8 +104,11 @@
$this->getModuleName(),
new ClaimDiffer( new OrderedListDiffer( new
ComparableComparer() ) )
);
+
+ $claims = new Claims( $entity->getClaims() );
+
$summary = $claimSummaryBuilder->buildClaimSummary(
- new Claims( $entity->getClaims() ),
+ $claims->getClaimWithGuid( $claim->getGuid() ),
$claim
);
diff --git a/repo/tests/phpunit/includes/ClaimSummaryBuilderTest.php
b/repo/tests/phpunit/includes/ClaimSummaryBuilderTest.php
index d32b29f..ffa6791 100644
--- a/repo/tests/phpunit/includes/ClaimSummaryBuilderTest.php
+++ b/repo/tests/phpunit/includes/ClaimSummaryBuilderTest.php
@@ -7,7 +7,6 @@
use Diff\Differ\OrderedListDiffer;
use Wikibase\ClaimSummaryBuilder;
use Wikibase\DataModel\Claim\Claim;
-use Wikibase\DataModel\Claim\Claims;
use Wikibase\DataModel\Reference;
use Wikibase\DataModel\Snak\PropertyNoValueSnak;
use Wikibase\DataModel\Snak\PropertySomeValueSnak;
@@ -134,11 +133,10 @@
new ClaimDiffer( new OrderedListDiffer( new
ComparableComparer() ) )
);
- $claims = new Claims();
$newClaims = $this->claimProvider();
foreach ( $newClaims as $newClaim ) {
- $summary = $claimSummaryBuilder->buildClaimSummary(
$claims, $newClaim );
+ $summary = $claimSummaryBuilder->buildClaimSummary(
null, $newClaim );
$this->assertInstanceOf( 'Wikibase\Summary', $summary,
"this should return a Summary object" );
$this->assertEquals( 'wbsetclaim',
$summary->getModuleName() );
$this->assertEquals( 'create',
$summary->getActionName() );
@@ -158,10 +156,7 @@
new ClaimDiffer( new OrderedListDiffer( new
ComparableComparer() ) )
);
- $claims = new Claims();
- $claims->addClaim( $originalClaim );
-
- $summary = $claimSummaryBuilder->buildClaimSummary( $claims,
$modifiedClaim );
+ $summary = $claimSummaryBuilder->buildClaimSummary(
$originalClaim, $modifiedClaim );
$this->assertInstanceOf( 'Wikibase\Summary', $summary, "this
should return a Summary object" );
$this->assertEquals( 'wbsetclaim', $summary->getModuleName() );
$this->assertEquals( $action, $summary->getActionName() );
--
To view, visit https://gerrit.wikimedia.org/r/208095
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I354063978be5c16bbc6b0eae46063100de44a18c
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Jeroen De Dauw <jeroended...@gmail.com>
Gerrit-Reviewer: Addshore <addshorew...@gmail.com>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <thiemo.maet...@wikimedia.de>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
