jenkins-bot has submitted this change and it was merged. Change subject: Don't display broken, unparsed comments in client recent changes ......................................................................
Don't display broken, unparsed comments in client recent changes If $comment is a string, then it is probably the new comments format that we will eventually parse and display nicely. This is not implemented yet and we shouldn't pass these yet to the ExternalComment objects. afaik, other comment strings are not used anymore and just defaulting to the generic 'wikibase-comment-update' is okayish in this case and is what we want for now. Added details on how this all works in code comments. Also added a lot more test cases to ExternalChangeFactoryTtest This catches this bug and covers other scenarios. Bug: T110823 Change-Id: I343acd5c755f9fb9480d2280982b54df9695a137 (cherry picked from commit 25a164ca610c2e04046773c5d3c534a2b3cf1f02) --- M client/includes/recentchanges/ExternalChangeFactory.php M client/tests/phpunit/includes/recentchanges/ExternalChangeFactoryTest.php 2 files changed, 207 insertions(+), 40 deletions(-) Approvals: Aude: Looks good to me, approved jenkins-bot: Verified diff --git a/client/includes/recentchanges/ExternalChangeFactory.php b/client/includes/recentchanges/ExternalChangeFactory.php index fbfd5b1..cddd1f9 100644 --- a/client/includes/recentchanges/ExternalChangeFactory.php +++ b/client/includes/recentchanges/ExternalChangeFactory.php @@ -141,7 +141,27 @@ } /** - * @fixme refactor comments handling! + * This method transforms the comments field into rc_params into an appropriate + * comment value for ExternalChange. + * + * $comment can be a string or an array with some additional data. + * + * String comments are either 'wikibase-comment-update' (legacy) or have + * comments from the repo, such as '/ wbsetclaim-update:2||1 / [[Property:P213]]: [[Q850]]'. + * + * We don't yet parse repo comments in the client, so for now, we use the + * generic 'wikibase-comment-update' for these. + * + * Comment arrays may contain a message key that provide autocomments for stuff + * like log actions (item deletion) or edits that have no meaningful summary + * to use in the client. + * + * - 'wikibase-comment-unlinked' (when the sitelink to the given page is removed on the repo) + * - 'wikibase-comment-add' (when the item is created, with sitelink to the given page) + * - 'wikibase-comment-remove' (when the item is deleted, the page becomes unconnected) + * - 'wikibase-comment-restore' (when the item is undeleted and reconnected to the page) + * - 'wikibase-comment-sitelink-add' (and other sitelink messages, unused) + * - 'wikibase-comment-update' (legacy, generic, item updated commment) * * @param array|string $comment * @param string $type @@ -163,10 +183,11 @@ } else { $newComment['key'] = $comment['message']; } - } elseif ( is_string( $comment ) ) { - $newComment['key'] = $comment; } + // @todo handle $comment values that are strings or whatever format + // that we use to transfer autocomments from repo to client. + return $newComment; } diff --git a/client/tests/phpunit/includes/recentchanges/ExternalChangeFactoryTest.php b/client/tests/phpunit/includes/recentchanges/ExternalChangeFactoryTest.php index 9912e3f..3b4957d 100644 --- a/client/tests/phpunit/includes/recentchanges/ExternalChangeFactoryTest.php +++ b/client/tests/phpunit/includes/recentchanges/ExternalChangeFactoryTest.php @@ -21,41 +21,194 @@ */ class ExternalChangeFactoryTest extends \MediaWikiTestCase { - /** - * @dataProvider newFromRecentChangeProvider - */ - public function testNewFromRecentChange( $expected, RecentChange $recentChange ) { + public function testNewFromRecentChange_itemUpdated() { + $commentData = 'wikibase-comment-update'; + + $recentChange = $this->makeRecentChange( $commentData, 'wikibase-item~update', false ); + $externalChangeFactory = new ExternalChangeFactory( 'testrepo' ); - $externalChange = $externalChangeFactory->newFromRecentChange( $recentChange ); - $this->assertEquals( $expected, $externalChange ); - } - - public function newFromRecentChangeProvider() { - $rev = new RevisionData( 'Cat', 5, 92, 90, '20130819111741', - array( 'key' => 'wikibase-comment-update' ), 'testrepo' - ); - - $externalChange = new ExternalChange( new ItemId( 'Q4' ), $rev, 'update' ); - - return array( - array( $externalChange, $this->getEditRecentChange( true ), 'bot edit' ), - array( $externalChange, $this->getEditRecentChange( false ), 'non bot edit' ) + $this->assertEquals( + $this->makeExpectedExternalChange( 'wikibase-comment-update', 'update' ), + $externalChangeFactory->newFromRecentChange( $recentChange ) ); } - /** - * @param boolean $bot - * @return RecentChange - */ - protected function getEditRecentChange( $bot ) { + public function testNewFromRecentChange_siteLinkChange() { + // at the moment, we don't do anything with this info :( and just say + // 'wikibase-comment-update' for these changes. + $commentData = array( + 'message' => 'wikibase-comment-sitelink-add', + 'sitelink' => array( + 'newlink' => array( 'site' => 'dewiki', 'page' => 'Kanada' ) + ) + ); + + $recentChange = $this->makeRecentChange( $commentData, 'wikibase-item~update', false ); + + $externalChangeFactory = new ExternalChangeFactory( 'testrepo' ); + + $this->assertEquals( + $this->makeExpectedExternalChange( 'wikibase-comment-update', 'update' ), + $externalChangeFactory->newFromRecentChange( $recentChange ) + ); + } + + public function testNewFromRecentChange_pageLinkedOnRepo() { + $commentData = array( + 'message' => 'wikibase-comment-linked' + ); + + $recentChange = $this->makeRecentChange( $commentData, 'wikibase-item~add', false ); + + $externalChangeFactory = new ExternalChangeFactory( 'testrepo' ); + + $this->assertEquals( + $this->makeExpectedExternalChange( 'wikibase-comment-linked', 'add' ), + $externalChangeFactory->newFromRecentChange( $recentChange ) + ); + } + + public function testNewFromRecentChange_withRepoComment() { + $commentData = '/* wbsetclaim-update:2||1 */ [[Property:P213]]: [[Q850]]'; + + $recentChange = $this->makeRecentChange( $commentData, 'wikibase-item~update', false ); + + $externalChangeFactory = new ExternalChangeFactory( 'testrepo' ); + + $this->assertEquals( + $this->makeExpectedExternalChange( 'wikibase-comment-update', 'update' ), + $externalChangeFactory->newFromRecentChange( $recentChange ) + ); + } + + public function testNewFromRecentChange_compositeComment() { + $commentData = 'wikibase-comment-update'; + $recentChange = new RecentChange(); $recentChange->counter = 2; - $params = array( + $rcParams = $this->makeRCParams( $commentData, 'wikibase-item~update', false ); + + $rcParams['wikibase-repo-change']['composite-comment'] = array( + 'wikibase-comment-update', + 'wikibase-comment-update' + ); + + $recentChange->setAttribs( $this->makeAttribs( $rcParams, false ) ); + + $expected = new ExternalChange( + new ItemId( 'Q4' ), + $this->makeRevisionData( array( + 'key' => 'wikibase-comment-multi', + 'numparams' => 2 + ) ), + 'update' + ); + + $externalChangeFactory = new ExternalChangeFactory( 'testrepo' ); + + $this->assertEquals( + $expected, + $externalChangeFactory->newFromRecentChange( $recentChange ) + ); + } + + public function testNewFromRecentChange_botEdit() { + $commentData = 'wikibase-comment-update'; + + $recentChange = $this->makeRecentChange( $commentData, 'wikibase-item~update', true ); + + $expected = new ExternalChange( + new ItemId( 'Q4' ), + $this->makeRevisionData( array( + 'key' => 'wikibase-comment-update' + ) ), + 'update' + ); + + $externalChangeFactory = new ExternalChangeFactory( 'testrepo' ); + + $this->assertEquals( + $this->makeExpectedExternalChange( 'wikibase-comment-update', 'update' ), + $externalChangeFactory->newFromRecentChange( $recentChange ) + ); + } + + public function testNewFromRecentChange_nonBotEdit() { + $commentData = 'wikibase-comment-update'; + + $recentChange = $this->makeRecentChange( $commentData, 'wikibase-item~update', false ); + + $externalChangeFactory = new ExternalChangeFactory( 'testrepo' ); + + $this->assertEquals( + $this->makeExpectedExternalChange( 'wikibase-comment-update', 'update' ), + $externalChangeFactory->newFromRecentChange( $recentChange ) + ); + } + + /** + * @param string $expectedComment + * @param string $expectedType + * + * @return ExternalChange + */ + private function makeExpectedExternalChange( $expectedComment, $expectedType ) { + return new ExternalChange( + new ItemId( 'Q4' ), + $this->makeRevisionData( array( + 'key' => $expectedComment + ) ), + $expectedType + ); + } + + private function makeRevisionData( array $comment ) { + return new RevisionData( + 'Cat', + 5, + 92, + 90, + '20130819111741', + $comment, + 'testrepo' + ); + } + + /** + * @param array|string $commentData + * @param string $changeType + * @param bool $isBot + * + * @return RecentChange + */ + private function makeRecentChange( $commentData, $changeType, $isBot ) { + $recentChange = new RecentChange(); + $recentChange->counter = 2; + + $attribs = $this->makeAttribs( + $this->makeRCParams( $commentData, $changeType, $isBot ), + $isBot + ); + + $recentChange->setAttribs( $attribs ); + + return $recentChange; + } + + /** + * @param array|string $commentData + * @param string $changeType + * @param boolean $bot + * + * @return array + */ + private function makeRCParams( $commentData, $changeType, $bot ) { + return array( 'wikibase-repo-change' => array( 'id' => 4, - 'type' => 'wikibase-item~update', + 'type' => $changeType, 'time' => '20130819111741', 'object_id' => 'q4', 'user_id' => 1, @@ -66,16 +219,13 @@ 'page_id' => 5, 'rev_id' => 92, 'parent_id' => 90, - 'comment' => array( - 'message' => 'wikibase-comment-sitelink-add', - 'sitelink' => array( - 'newlink' => array( 'site' => 'dewiki', 'page' => 'Kanada' ) - ) - ) + 'comment' => $commentData ) ); + } - $attribs = array( + private function makeAttribs( array $rcParams, $bot ) { + return array( 'rc_id' => 315, 'rc_timestamp' => '20130819111741', 'rc_user' => 0, @@ -98,12 +248,8 @@ 'rc_logid' => 0, 'rc_log_type' => null, 'rc_log_action' => '', - 'rc_params' => serialize( $params ) + 'rc_params' => serialize( $rcParams ) ); - - $recentChange->setAttribs( $attribs ); - - return $recentChange; } } -- To view, visit https://gerrit.wikimedia.org/r/234990 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I343acd5c755f9fb9480d2280982b54df9695a137 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Wikibase Gerrit-Branch: wmf/1.26wmf20 Gerrit-Owner: Aude <aude.w...@gmail.com> Gerrit-Reviewer: Aude <aude.w...@gmail.com> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits