Daniel Kinzler has uploaded a new change for review.
https://gerrit.wikimedia.org/r/232924
Change subject: Record edit summary in Change objects.
......................................................................
Record edit summary in Change objects.
This change improves consistency of change metadata with
the corresponding recentchanges entries.
Bug: T108686
Change-Id: Ie87fe574987c39b15df826fbf1fcc19a2d4ed391
---
M client/includes/Changes/ChangeHandler.php
M lib/includes/changes/EntityChange.php
M lib/tests/phpunit/changes/EntityChangeTest.php
M lib/tests/phpunit/changes/TestChanges.php
M repo/includes/Notifications/ChangeNotifier.php
5 files changed, 189 insertions(+), 91 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/24/232924/1
diff --git a/client/includes/Changes/ChangeHandler.php
b/client/includes/Changes/ChangeHandler.php
index b75d5f0..dc9ea8b 100644
--- a/client/includes/Changes/ChangeHandler.php
+++ b/client/includes/Changes/ChangeHandler.php
@@ -328,7 +328,7 @@
private function getRCAttributes( EntityChange $change ) {
$rcinfo = $change->getMetadata();
- if ( !is_array( $rcinfo ) ) {
+ if ( empty( $rcinfo ) ) {
return false;
}
diff --git a/lib/includes/changes/EntityChange.php
b/lib/includes/changes/EntityChange.php
index d3747e2..12f6698 100644
--- a/lib/includes/changes/EntityChange.php
+++ b/lib/includes/changes/EntityChange.php
@@ -36,14 +36,9 @@
private $entityId = null;
/**
- * @var string|null
- */
- protected $comment = null;
-
- /**
* @see ORMRow::setField
*
- * Overwritten to force lower case object_id
+ * @todo FIXME use uppecase ID, like everywhere else!
*
* @param string $name
* @param mixed $value
@@ -91,7 +86,7 @@
/**
* @param string $cache set to 'cache' to cache the unserialized diff.
*
- * @return array|bool false if no meta data could be found in the info
array
+ * @return array false if no meta data could be found in the info array
*/
public function getMetadata( $cache = 'no' ) {
$info = $this->getInfo( $cache );
@@ -100,13 +95,14 @@
return $info['metadata'];
}
- return false;
+ return array();
}
/**
- * @param array $metadata
+ * Sets metadata fields. Unknown fields are ignored. New metadata is
merged into
+ * the current metadata array.
*
- * @return bool
+ * @param array $metadata
*/
public function setMetadata( array $metadata ) {
$validKeys = array(
@@ -115,50 +111,40 @@
'rev_id',
'parent_id',
'user_text',
+ 'comment'
);
- if ( is_array( $metadata ) ) {
- foreach ( array_keys( $metadata ) as $key ) {
- if ( !in_array( $key, $validKeys ) ) {
- unset( $metadata[$key] );
- }
- }
+ // strip extra fields from metadata
+ $metadata = array_intersect_key( $metadata, array_flip(
$validKeys ) );
+ // merge new metadata into current metadata
+ $metadata = array_merge( $this->getMetadata(), $metadata );
+
+ // make sure the comment field is set
+ if ( !isset( $metadata['comment'] ) ) {
$metadata['comment'] = $this->getComment();
-
- $info = $this->hasField( 'info' ) ? $this->getField(
'info' ) : array();
- $info['metadata'] = $metadata;
- $this->setField( 'info', $info );
-
- return true;
}
- return false;
- }
-
- /**
- * @param string|null $comment
- *
- * @return string
- */
- public function setComment( $comment = null ) {
- if ( $comment !== null ) {
- $this->comment = $comment;
- } else {
- // Messages: wikibase-comment-add,
wikibase-comment-remove, wikibase-comment-linked,
- // wikibase-comment-unlink, wikibase-comment-restore,
wikibase-comment-update
- $this->comment = 'wikibase-comment-' .
$this->getAction();
- }
+ $info = $this->hasField( 'info' ) ? $this->getField( 'info' ) :
array();
+ $info['metadata'] = $metadata;
+ $this->setField( 'info', $info );
}
/**
* @return string
*/
public function getComment() {
- if ( $this->comment === null ) {
- $this->setComment();
+ $metadata = $this->getMetadata();
+
+ // TODO: get rid of this awkward fallback and messages.
Comments and messages
+ // should come from the revision, not be invented here.
+ if ( !isset( $metadata['comment'] ) ) {
+ // Messages: wikibase-comment-add,
wikibase-comment-remove, wikibase-comment-linked,
+ // wikibase-comment-unlink, wikibase-comment-restore,
wikibase-comment-update
+ $metadata['comment'] = 'wikibase-comment-' .
$this->getAction();
}
- return $this->comment;
+
+ return $metadata['comment'];
}
/**
@@ -174,27 +160,44 @@
* @todo rename to setRecentChangeInfo
*/
public function setMetadataFromRC( RecentChange $rc ) {
+ $this->setFields( array(
+ 'revision_id' => $rc->getAttribute( 'rc_this_oldid' ),
+ 'user_id' => $rc->getAttribute( 'rc_user' ),
+ 'time' => $rc->getAttribute( 'rc_timestamp' ),
+ ) );
+
$this->setMetadata( array(
'user_text' => $rc->getAttribute( 'rc_user_text' ),
'bot' => $rc->getAttribute( 'rc_bot' ),
'page_id' => $rc->getAttribute( 'rc_cur_id' ),
'rev_id' => $rc->getAttribute( 'rc_this_oldid' ),
'parent_id' => $rc->getAttribute( 'rc_last_oldid' ),
+ 'comment' => $rc->getAttribute( 'rc_comment' ),
) );
}
/**
* @param User $user
*
- * @todo rename to setUserInfo, set fields too.
+ * @todo rename to setUserInfo
*/
public function setMetadataFromUser( User $user ) {
- $this->setMetadata( array(
- 'user_text' => $user->getName(),
- 'page_id' => 0,
- 'rev_id' => 0,
- 'parent_id' => 0,
+ $this->setFields( array(
+ 'user_id' => $user->getId(),
) );
+
+ // TODO: init page_id etc in getMetadata, not here!
+ $metadata = array_merge( array(
+ 'page_id' => 0,
+ 'rev_id' => 0,
+ 'parent_id' => 0,
+ ),
+ $this->getMetadata()
+ );
+
+ $metadata['user_text'] = $user->getName();
+
+ $this->setMetadata( $metadata );
}
/**
@@ -203,25 +206,29 @@
* @param Revision $revision
*/
public function setRevisionInfo( Revision $revision ) {
- /* @var EntityContent $content */
- $content = $revision->getContent();
- $entityId = $content->getEntityId();
-
$this->setFields( array(
'revision_id' => $revision->getId(),
'user_id' => $revision->getUser(),
- 'object_id' => $entityId->getSerialization(),
'time' => $revision->getTimestamp(),
) );
- }
- /**
- * @param int $userId
- *
- * @todo Merge into future setUserInfo.
- */
- public function setUserId( $userId ) {
- $this->setField( 'user_id', $userId );
+ if ( !$this->hasField( 'object_id' ) ) {
+ /* @var EntityContent $content */
+ $content = $revision->getContent(); // potentially
expensive!
+ $entityId = $content->getEntityId();
+
+ $this->setFields( array(
+ 'object_id' => $entityId->getSerialization(),
+ ) );
+ }
+
+ $this->setMetadata( array(
+ 'user_text' => $revision->getUserText(),
+ 'page_id' => $revision->getPage(),
+ 'parent_id' => $revision->getParentId(),
+ 'comment' => $revision->getComment(),
+ 'rev_id' => $revision->getId(),
+ ) );
}
/**
diff --git a/lib/tests/phpunit/changes/EntityChangeTest.php
b/lib/tests/phpunit/changes/EntityChangeTest.php
index 90db7ff..9189f2c 100644
--- a/lib/tests/phpunit/changes/EntityChangeTest.php
+++ b/lib/tests/phpunit/changes/EntityChangeTest.php
@@ -2,7 +2,9 @@
namespace Wikibase\Test;
+use RecentChange;
use Revision;
+use stdClass;
use Wikibase\DataModel\Entity\Entity;
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\Item;
@@ -127,11 +129,9 @@
$this->assertInternalType( 'string', $entityChange->getType() );
}
- /**
- * @dataProvider changeProvider
- * @since 0.3
- */
- public function testMetadata( EntityChange $entityChange ) {
+ public function testMetadata() {
+ $entityChange = $this->newEntityChange( new ItemId( 'Q13' ) );
+
$entityChange->setMetadata( array(
'kittens' => 3,
'rev_id' => 23,
@@ -141,20 +141,38 @@
array(
'rev_id' => 23,
'user_text' => '171.80.182.208',
- 'comment' => $entityChange->getComment(),
+ 'comment' => $entityChange->getComment(), //
the comment field is magically initialized
+ ),
+ $entityChange->getMetadata()
+ );
+
+ // override some fields, keep others
+ $entityChange->setMetadata( array(
+ 'rev_id' => 25,
+ 'comment' => 'foo',
+ ) );
+ $this->assertEquals(
+ array(
+ 'rev_id' => 25,
+ 'user_text' => '171.80.182.208',
+ 'comment' => 'foo', // the comment field is not
magically initialized
),
$entityChange->getMetadata()
);
}
- /**
- * @dataProvider changeProvider
- * @since 0.3
- */
- public function testGetEmptyMetadata( EntityChange $entityChange ) {
+ public function testGetEmptyMetadata() {
+ $entityChange = $this->newEntityChange( new ItemId( 'Q13' ) );
+
+ $entityChange->setMetadata( array(
+ 'kittens' => 3,
+ 'rev_id' => 23,
+ 'user_text' => '171.80.182.208',
+ ) );
+
$entityChange->setField( 'info', array() );
$this->assertEquals(
- false,
+ array(),
$entityChange->getMetadata()
);
}
@@ -173,38 +191,113 @@
$this->assertTrue( stripos( $s, $type ) !== false, "missing
type $type" );
}
+ public function testGetComment() {
+ $entityChange = $this->newEntityChange( new ItemId( 'Q13' ) );
+
+ $this->assertEquals( 'wikibase-comment-update',
$entityChange->getComment(), 'comment' );
+
+ $entityChange->setMetadata( array(
+ 'comment' => 'Foo!',
+ ) );
+
+ $this->assertEquals( 'Foo!', $entityChange->getComment(),
'comment' );
+ }
+
+ public function testSetMetadataFromRC() {
+ $timestamp = '20140523' . '174422';
+
+ $row = new stdClass();
+ $row->rc_last_oldid = 3;
+ $row->rc_this_oldid = 5;
+ $row->rc_user = 7;
+ $row->rc_user_text = 'Mr. Kittens';
+ $row->rc_timestamp = $timestamp;
+ $row->rc_cur_id = 6;
+ $row->rc_bot = 1;
+ $row->rc_deleted = 0;
+ $row->rc_comment = 'Test!';
+
+ $rc = RecentChange::newFromRow( $row );
+
+ $entityChange = $this->newEntityChange( new ItemId( 'Q7' ) );
+ $entityChange->setMetadataFromRC( $rc );
+
+ $this->assertEquals( 5, $entityChange->getField( 'revision_id'
), 'revision_id' );
+ $this->assertEquals( 7, $entityChange->getField( 'user_id' ),
'user_id' );
+ $this->assertEquals( 'q7', $entityChange->getObjectId(),
'object_id' );
+ $this->assertEquals( $timestamp, $entityChange->getTime(),
'timestamp' );
+ $this->assertEquals( 'Test!', $entityChange->getComment(),
'comment' );
+
+ $metadata = $entityChange->getMetadata();
+ $this->assertEquals( 3, $metadata['parent_id'], 'parent_id' );
+ $this->assertEquals( 6, $metadata['page_id'], 'page_id' );
+ $this->assertEquals( 5, $metadata['rev_id'], 'rev_id' );
+ $this->assertEquals( 1, $metadata['bot'], 'bot' );
+ $this->assertEquals( 'Mr. Kittens', $metadata['user_text'],
'user_text' );
+ }
+
+ public function testSetMetadataFromUser() {
+ $user = $this->getMockBuilder( 'User' )
+ ->disableOriginalConstructor()
+ ->getMock();
+
+ $user->expects( $this->atLeastOnce() )
+ ->method( 'getId' )
+ ->will( $this->returnValue( 7 ) );
+
+ $user->expects( $this->atLeastOnce() )
+ ->method( 'getName' )
+ ->will( $this->returnValue( 'Mr. Kittens' ) );
+
+ $entityChange = $this->newEntityChange( new ItemId( 'Q7' ) );
+
+ $entityChange->setMetadata( array(
+ 'user_text' => 'Dobby', // will be overwritten
+ 'page_id' => 5, // will NOT be overwritten
+ ) );
+
+ $entityChange->setMetadataFromUser( $user );
+
+ $this->assertEquals( 7, $entityChange->getField( 'user_id' ),
'user_id' );
+
+ $metadata = $entityChange->getMetadata();
+ $this->assertEquals( 'Mr. Kittens', $metadata['user_text'],
'user_text' );
+ $this->assertEquals( 5, $metadata['page_id'], 'page_id should
be preserved' );
+ $this->assertArrayHasKey( 'rev_id', $metadata, 'rev_id should
be initialized' );
+ }
+
public function testSetRevisionInfo() {
$id = new ItemId( 'Q7' );
$item = new Item( $id );
- $changeFactory = TestChanges::getEntityChangeFactory();
- $entityChange = $changeFactory->newForEntity(
EntityChange::UPDATE, $id );
+ $entityChange = $this->newEntityChange( $id );
$timestamp = '20140523' . '174422';
$revision = new Revision( array(
'id' => 5,
+ 'page' => 6,
'user' => 7,
+ 'parent_id' => 3,
+ 'user_text' => 'Mr. Kittens',
'timestamp' => $timestamp,
'content' => ItemContent::newFromItem( $item ),
+ 'comment' => 'Test!',
) );
$entityChange->setRevisionInfo( $revision );
- $this->assertEquals( 5, $entityChange->getField( 'revision_id'
) );
- $this->assertEquals( 7, $entityChange->getField( 'user_id' ) );
- $this->assertEquals( 'q7', $entityChange->getObjectId() );
- $this->assertEquals( $timestamp, $entityChange->getTime() );
- }
+ $this->assertEquals( 5, $entityChange->getField( 'revision_id'
), 'revision_id' );
+ $this->assertEquals( 7, $entityChange->getField( 'user_id' ),
'user_id' );
+ $this->assertEquals( 'q7', $entityChange->getObjectId(),
'object_id' );
+ $this->assertEquals( $timestamp, $entityChange->getTime(),
'timestamp' );
+ $this->assertEquals( 'Test!', $entityChange->getComment(),
'comment' );
- public function testSetUserId() {
- $id = new ItemId( 'Q7' );
-
- $changeFactory = TestChanges::getEntityChangeFactory();
- $change = $changeFactory->newForEntity( EntityChange::UPDATE,
$id );
-
- $change->setUserId( 7 );
- $this->assertEquals( 7, $change->getField( 'user_id' ), 7 );
+ $metadata = $entityChange->getMetadata();
+ $this->assertEquals( 3, $metadata['parent_id'], 'parent_id' );
+ $this->assertEquals( 6, $metadata['page_id'], 'page_id' );
+ $this->assertEquals( 5, $metadata['rev_id'], 'rev_id' );
+ $this->assertEquals( 'Mr. Kittens', $metadata['user_text'],
'user_text' );
}
public function testSetTimestamp() {
diff --git a/lib/tests/phpunit/changes/TestChanges.php
b/lib/tests/phpunit/changes/TestChanges.php
index 2ac9183..8ad4259 100644
--- a/lib/tests/phpunit/changes/TestChanges.php
+++ b/lib/tests/phpunit/changes/TestChanges.php
@@ -193,14 +193,13 @@
$rev = 1000;
foreach ( $changes as $key => $change ) {
- $change->setComment( "$key:1|" );
-
$meta = array(
'page_id' => 23,
'bot' => false,
'rev_id' => $rev,
'parent_id' => $rev -1,
'user_text' => 'Some User',
+ 'comment' => "$key:1|",
);
$change->setMetadata( $meta );
diff --git a/repo/includes/Notifications/ChangeNotifier.php
b/repo/includes/Notifications/ChangeNotifier.php
index c0c249b..102f35b 100644
--- a/repo/includes/Notifications/ChangeNotifier.php
+++ b/repo/includes/Notifications/ChangeNotifier.php
@@ -63,7 +63,6 @@
$change = $this->changeFactory->newFromUpdate(
EntityChange::REMOVE, $content->getEntity() );
$change->setTimestamp( $timestamp );
- $change->setUserId( $user->getId() );
$change->setMetadataFromUser( $user );
$this->transmitChange( $change );
--
To view, visit https://gerrit.wikimedia.org/r/232924
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie87fe574987c39b15df826fbf1fcc19a2d4ed391
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Daniel Kinzler <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
