jenkins-bot has submitted this change and it was merged.

Change subject: Adjusted ChangeOpClaim/SetClaim to be able to reorder groups
......................................................................


Adjusted ChangeOpClaim/SetClaim to be able to reorder groups

This enables moving groups of statements/claims according to their common main 
snak property.

Change-Id: I6579a1df79f7e612e5a574cc6065f31c76adea75
---
M repo/includes/ChangeOp/ChangeOpClaim.php
M repo/includes/api/SetClaim.php
M repo/tests/phpunit/includes/ChangeOp/ChangeOpClaimTest.php
3 files changed, 39 insertions(+), 45 deletions(-)

Approvals:
  Tobias Gritschacher: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/repo/includes/ChangeOp/ChangeOpClaim.php 
b/repo/includes/ChangeOp/ChangeOpClaim.php
index acbf9ec..b59c6ee 100644
--- a/repo/includes/ChangeOp/ChangeOpClaim.php
+++ b/repo/includes/ChangeOp/ChangeOpClaim.php
@@ -3,6 +3,7 @@
 namespace Wikibase\ChangeOp;
 
 use InvalidArgumentException;
+use Wikibase\ByPropertyIdArray;
 use Wikibase\Claim;
 use Wikibase\Claims;
 use Wikibase\Entity;
@@ -89,54 +90,35 @@
                $entityClaims = $entity->getClaims();
                $claims = new Claims( $entityClaims );
 
-               $index = $this->index;
-
-               if( $index !== null ) {
-                       $index = $this->getOverallClaimIndex( $entityClaims );
-               }
-
-               if( $claims->hasClaimWithGuid( $this->claim->getGuid() ) ){
-                       if( is_null( $index ) ) {
-                               // Set index to current index to not have the 
claim removed and appended but retain
-                               // its position within the list of claims.
-                               $index = $claims->indexOf( $this->claim );
-                       }
-
-                       $claims->removeClaimWithGuid( $this->claim->getGuid() );
-                       $this->updateSummary( $summary, 'update' );
-               } else {
+               if( !$claims->hasClaimWithGuid( $this->claim->getGuid() ) ) {
+                       // Adding a new claim.
                        $this->updateSummary( $summary, 'create' );
+
+                       $indexedClaimList = new ByPropertyIdArray( 
$entityClaims );
+                       $indexedClaimList->buildIndex();
+
+                       $indexedClaimList->addObjectAtIndex( $this->claim, 
$this->index );
+
+               } else {
+                       // Altering an existing claim.
+                       $this->updateSummary( $summary, 'update' );
+
+                       // Replace claim at its current index:
+                       $currentIndex = $claims->indexOf( $this->claim );
+                       $claims->removeClaimWithGuid( $this->claim->getGuid() );
+                       $claims->addClaim( $this->claim, $currentIndex );
+
+                       // Move claim to its designated index:
+                       $indexedClaimList = new ByPropertyIdArray( $claims );
+                       $indexedClaimList->buildIndex();
+
+                       $index = !is_null( $this->index ) ? $this->index : 
$currentIndex;
+                       $indexedClaimList->moveObjectToIndex( $this->claim, 
$index );
                }
 
-               $claims->addClaim( $this->claim, $index );
+               $claims = new Claims( $indexedClaimList->toFlatArray() );
                $entity->setClaims( $claims );
 
                return true;
        }
-
-       /**
-        * Computes the claim's overall index within the list of claims from 
the index within the subset
-        * of claims whose main snak features the same property id.
-        * @since 0.5
-        *
-        * @param Claim[] $claims
-        * @return int|null
-        */
-       protected function getOverallClaimIndex( $claims ) {
-               $overallIndex = 0;
-               $indexInProperty = 0;
-
-               foreach( $claims as $claim ) {
-                       if( $claim->getPropertyId()->equals( 
$this->claim->getPropertyId() ) ) {
-                               if( $indexInProperty++ === $this->index ) {
-                                       return $overallIndex;
-                               }
-                       }
-                       $overallIndex++;
-               }
-
-               // No claims with the same main snak property exist.
-               return $this->index;
-       }
-
 }
diff --git a/repo/includes/api/SetClaim.php b/repo/includes/api/SetClaim.php
index 551d236..84647ba 100644
--- a/repo/includes/api/SetClaim.php
+++ b/repo/includes/api/SetClaim.php
@@ -162,7 +162,7 @@
                        parent::getParamDescription(),
                        array(
                                'claim' => 'Claim serialization',
-                               'index' => 'The index within the entity\'s list 
of claims/statements featuring the same main snak property where to move the 
claim/statement to. Optional. When not provided, an existing claim/statement 
will stay in place while a new claim/statement will be appended to the last 
claim/statement whose main snak features the same property.',
+                               'index' => 'The index within the entity\'s list 
of claims/statements to move the claim/statement to. Optional. Be aware that 
when setting an index that specifies a position not next to a clam/statement 
whose main snak does not feature the same property, the whole group of 
claims/statements whose main snaks feature the same property is moved. When not 
provided, an existing claim/statement will stay in place while a new 
claim/statement will be appended to the last claim/statement whose main snak 
features the same property.',
                        )
                );
        }
@@ -192,7 +192,7 @@
                        
'api.php?action=wbsetclaim&claim={"id":"Q2$5627445f-43cb-ed6d-3adb-760e85bd17ee","type":"claim","mainsnak":{"snaktype":"value","property":"P1","datavalue":{"value":"City","type":"string"}}}'
                        => 'Set the claim with the given id to property P1 with 
a string value of "City"',
                        
'api.php?action=wbsetclaim&claim={"id":"Q2$5627445f-43cb-ed6d-3adb-760e85bd17ee","type":"claim","mainsnak":{"snaktype":"value","property":"P1","datavalue":{"value":"City","type":"string"}}}&index=0'
-                       => 'Set the claim with the given id to property P1 with 
a string value of "City" and move the claim to the topmost position within the 
entity\'s subgroup of claims that feature the main snak property P1.',
+                       => 'Set the claim with the given id to property P1 with 
a string value of "City" and move the claim to the topmost position within the 
entity\'s subgroup of claims that feature the main snak property P1. In 
addition, move the whole subgroup to the top of all subgroups aggregated by 
property.',
                );
        }
 
diff --git a/repo/tests/phpunit/includes/ChangeOp/ChangeOpClaimTest.php 
b/repo/tests/phpunit/includes/ChangeOp/ChangeOpClaimTest.php
index 9242c51..7b83e87 100644
--- a/repo/tests/phpunit/includes/ChangeOp/ChangeOpClaimTest.php
+++ b/repo/tests/phpunit/includes/ChangeOp/ChangeOpClaimTest.php
@@ -76,6 +76,13 @@
                $claims[6666] = clone $claim666;
                $claims[6666]->setGuid( 
'Q666$D8404CDA-25E4-4334-AF13-A3290BC66666' );
 
+               $claims[11] = new Claim( new PropertyNoValueSnak( 1 ) );
+               $claims[11]->setGuid( null );
+               $claims[12] = new Claim( new PropertySomeValueSnak( 1 ) );
+               $claims[12]->setGuid( null );
+               $claims[13] = clone $claims[12];
+               $claims[13]->setGuid( 
'Q666$D8404CDA-25E4-4334-AF13-A3290BC66613' );
+
                $args = array();
                //test adding claims with guids from other items(these 
shouldn't be added)
                $args[] = array( $itemEmpty, $claims[666], false );
@@ -95,6 +102,11 @@
                $args[] = array( $item777, $claims[0], array( $claims[0], 
$claims[777], $claims[7770], $claims[7777] ), 0 );
                // test moving a claim
                $args[] = array( $item666, $claims[6666], array( $claims[666], 
$claims[6666], $claims[6660] ), 1 );
+               // test adding a claim featuring another property id within the 
boundaries of claims the
+               // same property
+               $args[] = array( $item666, $claims[11], array( $claims[666], 
$claims[6666], $claims[6660], $claims[11] ), 1 );
+               // test moving a subset of claims featuring the same property
+               $args[] = array( $item666, $claims[12], array( $claims[12], 
$claims[11], $claims[666], $claims[6666], $claims[6660] ), 0 );
 
                return $args;
        }

-- 
To view, visit https://gerrit.wikimedia.org/r/91189
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I6579a1df79f7e612e5a574cc6065f31c76adea75
Gerrit-PatchSet: 8
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Henning Snater <henning.sna...@wikimedia.de>
Gerrit-Reviewer: Tobias Gritschacher <tobias.gritschac...@wikimedia.de>
Gerrit-Reviewer: jenkins-bot

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to