Aude has uploaded a new change for review. https://gerrit.wikimedia.org/r/234916
Change subject: Various refactoring of comments code in the client ...................................................................... Various refactoring of comments code in the client Split from https://gerrit.wikimedia.org/r/#/c/221582/ Change-Id: I324984e4537bacc1dff3990cc9bfc3d614f35afd --- M client/includes/Changes/ChangeHandler.php M client/includes/SiteLinkCommentCreator.php M client/includes/recentchanges/ChangeLineFormatter.php M client/includes/recentchanges/ExternalChangeFactory.php M client/includes/recentchanges/ExternalRecentChange.php M client/includes/recentchanges/RevisionData.php M client/tests/phpunit/includes/Changes/ChangeHandlerTest.php M client/tests/phpunit/includes/SiteLinkCommentCreatorTest.php M lib/includes/changes/EntityChange.php M lib/tests/phpunit/changes/TestChanges.php 10 files changed, 61 insertions(+), 63 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase refs/changes/16/234916/1 diff --git a/client/includes/Changes/ChangeHandler.php b/client/includes/Changes/ChangeHandler.php index a5e7d7a..59f2e8e 100644 --- a/client/includes/Changes/ChangeHandler.php +++ b/client/includes/Changes/ChangeHandler.php @@ -327,24 +327,22 @@ $fields = $change->getFields(); $fields['entity_type'] = $change->getEntityId()->getEntityType(); - if ( $change instanceof ItemChange ) { - $rcinfo['comment'] = $this->getEditComment( $change ); + $comment = $this->getEditComment( $change ); + $rcinfo['comment'] = $comment; - if ( isset( $fields['info']['changes'] ) ) { - $rcinfo['composite-comment'] = array(); + if ( isset( $fields['info']['changes'] ) ) { + $rcinfo['composite-comment'] = array(); - foreach ( $fields['info']['changes'] as $part ) { - $rcinfo['composite-comment'][] = $this->getEditComment( $part ); - } + foreach ( $fields['info']['changes'] as $part ) { + $rcinfo['composite-comment'][] = $this->getEditComment( $part ); } } unset( $fields['info'] ); - $rcinfo = array_merge( $fields, $rcinfo ); - return array( - 'wikibase-repo-change' => array_merge( $fields, $rcinfo ) + 'wikibase-repo-change' => array_merge( $fields, $rcinfo ), + 'comment' => $comment, ); } @@ -364,14 +362,25 @@ $siteLinkDiff = $change instanceof ItemChange ? $change->getSiteLinkDiff() : null; + $action = $change->getAction(); - $comment = $change->getComment(); + $editComment = null; - $commentCreator = new SiteLinkCommentCreator( $this->localSiteId ); - $editComment = $commentCreator->getEditComment( $siteLinkDiff, $action, $comment ); + if ( $siteLinkDiff !== null && !$siteLinkDiff->isEmpty() ) { + $commentCreator = new SiteLinkCommentCreator( $this->localSiteId ); + $editComment = $commentCreator->getEditComment( $siteLinkDiff, $action ); + } - if ( is_array( $editComment ) && !isset( $editComment['message'] ) ) { - throw new MWException( 'getEditComment returned an empty comment' ); + if ( $editComment === null ) { + $repoComment = $change->getComment(); + + $editComment = array( + 'repo-comment' => $repoComment + ); + } + + if ( !isset( $editComment['message'] ) ) { + $editComment['message'] = 'wikibase-comment-updated'; } return $editComment; diff --git a/client/includes/SiteLinkCommentCreator.php b/client/includes/SiteLinkCommentCreator.php index 34c5c37..5b1b732 100644 --- a/client/includes/SiteLinkCommentCreator.php +++ b/client/includes/SiteLinkCommentCreator.php @@ -38,38 +38,12 @@ * * @since 0.5 * - * @param Diff|null $siteLinkDiff + * @param Diff $siteLinkDiff * @param string $action e.g. 'remove', see the constants in EntityChange - * @param string $comment * * @return array|string */ - public function getEditComment( Diff $siteLinkDiff = null, $action, $comment ) { - if ( $siteLinkDiff !== null && !$siteLinkDiff->isEmpty() ) { - $siteLinkComment = $this->getSiteLinkComment( $action, $siteLinkDiff ); - - if ( !empty( $siteLinkComment ) ) { - return $siteLinkComment; - } - } - - return $comment; - } - - /** - * Returns an array structure suitable for building an edit summary for the respective - * change to site links. - * - * @param string $action e.g. 'remove', see the constants in EntityChange - * @param Diff $siteLinkDiff The change's site link diff - * - * @return array|null - */ - private function getSiteLinkComment( $action, Diff $siteLinkDiff ) { - if ( $siteLinkDiff->isEmpty() ) { - return null; - } - + public function getEditComment( Diff $siteLinkDiff, $action ) { //TODO: Implement comments specific to the affected page. // Different pages may be affected in different ways by the same change. // Also, merged changes may affect the same page in multiple ways. @@ -97,7 +71,7 @@ if ( $diffOpCount === 1 ) { $params = $this->getSiteLinkChangeParams( $diffOps ); } else { - // @todo report how many changes + // @todo FIXME report at least how many changes $params = array( 'message' => 'wikibase-comment-update' ); @@ -133,7 +107,7 @@ $args ); - // TODO: Handle if there are multiple DiffOp's here. + //@todo FIXME handle if there are multiple diffOps here break; } diff --git a/client/includes/recentchanges/ChangeLineFormatter.php b/client/includes/recentchanges/ChangeLineFormatter.php index 41a5310..73bc91e 100644 --- a/client/includes/recentchanges/ChangeLineFormatter.php +++ b/client/includes/recentchanges/ChangeLineFormatter.php @@ -321,6 +321,7 @@ * @return string */ private function formatComment( array $comment ) { + //FIXME: use repo-comment with AutoCommentFormatter $commentMsg = wfMessage( $comment['key'] ); if ( isset( $comment['numparams'] ) ) { diff --git a/client/includes/recentchanges/ExternalChangeFactory.php b/client/includes/recentchanges/ExternalChangeFactory.php index fbfd5b1..8862f97 100644 --- a/client/includes/recentchanges/ExternalChangeFactory.php +++ b/client/includes/recentchanges/ExternalChangeFactory.php @@ -141,7 +141,7 @@ } /** - * @fixme refactor comments handling! + * @todo: FIXME refactor comments handling! * * @param array|string $comment * @param string $type @@ -153,6 +153,7 @@ 'key' => 'wikibase-comment-update' ); + //TODO: parse $comment['text'] if present, and apply AutoCommentFormatter! if ( is_array( $comment ) ) { if ( $type === 'wikibase-item~add' ) { // @todo: provide a link to the entity @@ -171,13 +172,15 @@ } /** - * @fixme refactor comments handling! + * @todo: FIXME refactor comments handling! * * @param array $changeParams * - * @return string + * @return string|array */ private function extractComment( $changeParams ) { + global $wgContentLang; + $comment = array( 'key' => 'wikibase-comment-update' ); @@ -186,6 +189,7 @@ // Combine all the comments! Up to some max length? if ( array_key_exists( 'composite-comment', $changeParams ) ) { $comment['key'] = 'wikibase-comment-multi'; + //FIXME: combine comments in a better way! Can't we just generate localized messages and concatenate them? $comment['numparams'] = $this->countCompositeComments( $changeParams['composite-comment'] ); } elseif ( array_key_exists( 'comment', $changeParams ) ) { $comment = $this->parseComment( $changeParams['comment'], $changeParams['type'] ); diff --git a/client/includes/recentchanges/ExternalRecentChange.php b/client/includes/recentchanges/ExternalRecentChange.php index e98fea2..4f4f116 100644 --- a/client/includes/recentchanges/ExternalRecentChange.php +++ b/client/includes/recentchanges/ExternalRecentChange.php @@ -70,7 +70,7 @@ 'rc_last_oldid' => $title->getLatestRevID(), 'rc_params' => serialize( $attribs ), 'rc_cur_id' => $title->getArticleID(), - 'rc_comment' => '', + 'rc_comment' => isset( $attribs['comment'] ) ? $attribs['comment'] : '', 'rc_timestamp' => $time, 'rc_log_action' => '', 'rc_source' => 'wb' diff --git a/client/includes/recentchanges/RevisionData.php b/client/includes/recentchanges/RevisionData.php index 73b1d38..a09ce78 100644 --- a/client/includes/recentchanges/RevisionData.php +++ b/client/includes/recentchanges/RevisionData.php @@ -48,7 +48,7 @@ * @param int $revId * @param int $parentId * @param string $timestamp - * @param string $comment + * @param string|array $comment * @param string $siteId */ public function __construct( $userName, $pageId, $revId, $parentId, $timestamp, @@ -99,7 +99,7 @@ } /** - * @return string + * @return string|array */ public function getComment() { return $this->comment; diff --git a/client/tests/phpunit/includes/Changes/ChangeHandlerTest.php b/client/tests/phpunit/includes/Changes/ChangeHandlerTest.php index 9e695b4..7d1722c 100644 --- a/client/tests/phpunit/includes/Changes/ChangeHandlerTest.php +++ b/client/tests/phpunit/includes/Changes/ChangeHandlerTest.php @@ -255,19 +255,28 @@ $changes['set-de-label'], $dummy, array( 'q100' => array( 'Emmy' ) ), - 'set-de-label:1|' + array( + 'message' => 'wikibase-comment-updated', + 'repo-comment' => 'set-de-label:1|', + ), ), array( // #2 $changes['add-claim'], $dummy, array( 'q100' => array( 'Emmy' ) ), - 'add-claim:1|' + array( + 'message' => 'wikibase-comment-updated', + 'repo-comment' => 'add-claim:1|', + ), ), array( // #3 $changes['remove-claim'], $dummy, array( 'q100' => array( 'Emmy' ) ), - 'remove-claim:1|' + array( + 'message' => 'wikibase-comment-updated', + 'repo-comment' => 'remove-claim:1|', + ), ), array( // #4 $changes['set-dewiki-sitelink'], @@ -458,7 +467,7 @@ */ public function testGetEditComment( Change $change, Title $title, array $pageNamesPerItemId, $expected ) { $handler = $this->getChangeHandler( $pageNamesPerItemId ); - $comment = $handler->getEditComment( $change, $title ); + $comment = $handler->getEditComment( $change ); if ( is_array( $comment ) && is_array( $expected ) ) { $this->assertArrayEquals( $expected, $comment, false, true ); diff --git a/client/tests/phpunit/includes/SiteLinkCommentCreatorTest.php b/client/tests/phpunit/includes/SiteLinkCommentCreatorTest.php index fac2798..ec871b0 100644 --- a/client/tests/phpunit/includes/SiteLinkCommentCreatorTest.php +++ b/client/tests/phpunit/includes/SiteLinkCommentCreatorTest.php @@ -253,7 +253,7 @@ $updates[] = array( $this->getBadgeChangeDiff(), - 'wikibase-comment-update', + null, ); $updates[] = array( diff --git a/lib/includes/changes/EntityChange.php b/lib/includes/changes/EntityChange.php index 12f6698..e5cff7b 100644 --- a/lib/includes/changes/EntityChange.php +++ b/lib/includes/changes/EntityChange.php @@ -86,7 +86,7 @@ /** * @param string $cache set to 'cache' to cache the unserialized diff. * - * @return array false if no meta data could be found in the info array + * @return array */ public function getMetadata( $cache = 'no' ) { $info = $this->getInfo( $cache ); @@ -111,7 +111,7 @@ 'rev_id', 'parent_id', 'user_text', - 'comment' + 'comment', ); // strip extra fields from metadata @@ -134,17 +134,17 @@ * @return string */ public function getComment() { - $metadata = $this->getMetadata(); + $meta = $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, + if ( !isset( $meta['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(); + $meta['comment'] = 'wikibase-comment-' . $this->getAction(); } - return $metadata['comment']; + return $meta['comment']; } /** diff --git a/lib/tests/phpunit/changes/TestChanges.php b/lib/tests/phpunit/changes/TestChanges.php index 57bbcdd..56d60cb 100644 --- a/lib/tests/phpunit/changes/TestChanges.php +++ b/lib/tests/phpunit/changes/TestChanges.php @@ -194,6 +194,7 @@ foreach ( $changes as $key => $change ) { $meta = array( + 'comment' => '', 'page_id' => 23, 'bot' => false, 'rev_id' => $rev, -- To view, visit https://gerrit.wikimedia.org/r/234916 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I324984e4537bacc1dff3990cc9bfc3d614f35afd Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Wikibase Gerrit-Branch: master Gerrit-Owner: 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