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

Reply via email to