EBernhardson has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/165682

Change subject: Hygiene: Pass FormatterRow for context if possible
......................................................................

Hygiene: Pass FormatterRow for context if possible

I ran into an issue in a followup patch writing tests for the
RevisionFormatter that it was reaching out into global state via the
Collection class to get the previous revision of the one it was formatting.

This patch allows a FormatterRow instance, which is often available, to
be passed into processParams as well so it can access the already loaded
previous revision rather than reaching out through global state.

Change-Id: I416da501eeb0d41506c0192c6df581d576fe5ffc
---
M includes/Formatter/RevisionFormatter.php
1 file changed, 25 insertions(+), 26 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Flow 
refs/changes/82/165682/1

diff --git a/includes/Formatter/RevisionFormatter.php 
b/includes/Formatter/RevisionFormatter.php
index 454da42..141ef2f 100644
--- a/includes/Formatter/RevisionFormatter.php
+++ b/includes/Formatter/RevisionFormatter.php
@@ -154,6 +154,8 @@
                if ( !$isHistoryAllowed ) {
                        return array();
                }
+               $isPrevHistoryAllowed =  $row->previousRevision &&
+                       $this->permissions->isAllowed( $row->previousRevision, 
'history' );
 
                $moderatedRevision = $this->templating->getModeratedRevision( 
$row->revision );
                $ts = $row->revision->getRevisionId()->getTimestampObj();
@@ -171,18 +173,20 @@
                        // These are write urls
                        'actions' => $this->buildActions( $row ),
                        'size' => array(
-                               'old' => strlen( $row->previousRevision ? 
$row->previousRevision->getContentRaw() : '' ),
-                               'new' => strlen( 
$row->revision->getContentRaw() ),
+                               'old' => $isPrevHistoryAllowed
+                                       ? 
$row->revision->getPreviousContentLength()
+                                       : null,
+                               'new' => $row->revision->getContentLength(),
                        ),
                        'author' => $this->serializeUser(
                                $row->revision->getUserWiki(),
                                $row->revision->getUserId(),
                                $row->revision->getUserIp()
                        ),
+                       'previousRevisionId' => 
$row->revision->isFirstRevision()
+                               ? null
+                               : 
$row->revision->getPrevRevisionId()->getAlphadecimal(),
                );
-
-               $prevRevId = $row->revision->getPrevRevisionId();
-               $res['previousRevisionId'] = $prevRevId ? 
$prevRevId->getAlphadecimal() : null;
 
                if ( $res['isModerated'] ) {
                        $res['moderator'] = $this->serializeUser(
@@ -203,23 +207,11 @@
                        // topic titles are always forced to plain text
                        $contentFormat = $this->decideContentFormat( 
$row->revision );
 
-                       $res += array(
-                               // @todo better name?
-                               'content' => array(
-                                       'content' => 
$this->templating->getContent( $row->revision, $contentFormat ),
-                                       'format' => $contentFormat
-                               ),
-                               'size' => array(
-                                       'old' => null,
-                                       // @todo this isn't really correct
-                                       'new' => strlen( 
$row->revision->getContentRaw() ),
-                               ),
+                       // @todo better name?
+                       $res['content'] = array(
+                               'content' => $this->templating->getContent( 
$row->revision, $contentFormat ),
+                               'format' => $contentFormat
                        );
-                       if ( $row->previousRevision
-                               && $this->permissions->isAllowed( 
$row->previousRevision, 'view' )
-                       ) {
-                               $res['size']['old'] = strlen( 
$row->previousRevision->getContentRaw() );
-                       }
                }
 
                if ( $row instanceof TopicRow ) {
@@ -263,7 +255,8 @@
                                        'topic-of-post',
                                        $row->revision,
                                        $row->workflow->getId(),
-                                       $ctx
+                                       $ctx,
+                                       $row
                                );
                        }
                }
@@ -726,9 +719,10 @@
         * @param UUID $workflowId
         * @param AbstractRevision $revision
         * @param IContextSource $ctx
+        * @param FormatterRow|null $row
         * @return array
         */
-       public function buildProperties( UUID $workflowId, AbstractRevision 
$revision, IContextSource $ctx ) {
+       public function buildProperties( UUID $workflowId, AbstractRevision 
$revision, IContextSource $ctx, FormatterRow $row = null ) {
                if ( $this->includeProperties === false ) {
                        return array();
                }
@@ -746,7 +740,7 @@
 
                $res = array( '_key' => $actions->getValue( $changeType, 
'history', 'i18n-message' ) );
                foreach ( $params as $param ) {
-                       $res[$param] = $this->processParam( $param, $revision, 
$workflowId, $ctx );
+                       $res[$param] = $this->processParam( $param, $revision, 
$workflowId, $ctx, $row );
                }
 
                return $res;
@@ -759,11 +753,12 @@
         * @param AbstractRevision|array $revision The revision to format or an 
array of revisions
         * @param UUID $workflowId The UUID of the workflow $revision belongs 
tow
         * @param IContextSource $ctx
+        * @param FormatterRow|null $row
         * @return mixed A valid parameter for a core Message instance. These 
parameters will be used
         *  with Message::parse
         * @throws FlowException
         */
-       public function processParam( $param, /* AbstractRevision|array */ 
$revision, UUID $workflowId, IContextSource $ctx ) {
+       public function processParam( $param, /* AbstractRevision|array */ 
$revision, UUID $workflowId, IContextSource $ctx, FormatterRow $row = null ) {
                switch ( $param ) {
                case 'creator-text':
                        if ( $revision instanceof PostRevision ) {
@@ -805,7 +800,11 @@
                        if ( $revision->isFirstRevision() ) {
                                return '';
                        }
-                       $previousRevision = 
$revision->getCollection()->getPrevRevision( $revision );
+                       if ( $row === null ) {
+                               $previousRevision = 
$revision->getCollection()->getPrevRevision( $revision );
+                       } else {
+                               $previousRevision = $row->previousRevision;
+                       }
                        if ( !$previousRevision ) {
                                return '';
                        }

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I416da501eeb0d41506c0192c6df581d576fe5ffc
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: EBernhardson <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to