jenkins-bot has submitted this change and it was merged.

Change subject: Refactor moderation logging
......................................................................


Refactor moderation logging

* Move PostModerationLogger to Listener namespace/directory (since it
  is one) and rename.
* Rename Logger to ModerationLogger to reflect that's what it's for.
  Before this round, it already had a comment indicating it was
  specific to suppression:

  
https://git.wikimedia.org/blob/mediawiki%2Fextensions%2FFlow.git/253544059db40123b3629a95744350ba40f2addb/includes%2FLog%2FLogger.php#L48

  After https://gerrit.wikimedia.org/r/#/c/191543/ is merged, it
  will also explicitly use $post->getModerationTimestamp()
* Fix logger to log post ID in postId field, not revision ID.
  From Matthias's change, https://gerrit.wikimedia.org/r/#/c/192554/

Change-Id: Ib7b9177522a0e238c1b15c8e8e13ff27f6cf89af
---
M FlowActions.php
M autoload.php
M container.php
R includes/Data/Listener/ModerationLoggingListener.php
R includes/Log/ModerationLogger.php
M maintenance/FlowAddMissingModerationLogs.php
6 files changed, 43 insertions(+), 39 deletions(-)

Approvals:
  EBernhardson: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/FlowActions.php b/FlowActions.php
index 28d5dce..bf6adc5 100644
--- a/FlowActions.php
+++ b/FlowActions.php
@@ -5,7 +5,7 @@
 use Flow\Model\PostSummary;
 use Flow\Model\Header;
 use Flow\RevisionActionPermissions;
-use Flow\Log\Logger;
+use Flow\Log\ModerationLogger;
 use Flow\Data\Listener\RecentChangesListener;
 
 /**
@@ -399,7 +399,7 @@
 
        'restore-post' => array(
                'performs-writes' => true,
-               'log_type' => function( PostRevision $revision, Logger $logger 
) {
+               'log_type' => function( PostRevision $revision, 
ModerationLogger $logger ) {
                        $post = $revision->getCollection();
                        $previousRevision = $post->getPrevRevision( $revision );
                        if ( $previousRevision ) {
@@ -452,7 +452,7 @@
 
        'restore-topic' => array(
                'performs-writes' => true,
-               'log_type' => function( PostRevision $revision, Logger $logger 
) {
+               'log_type' => function( PostRevision $revision, 
ModerationLogger $logger ) {
                        $post = $revision->getCollection();
                        $previousRevision = $post->getPrevRevision( $revision );
                        if ( $previousRevision ) {
diff --git a/autoload.php b/autoload.php
index 593efa1..76716b0 100644
--- a/autoload.php
+++ b/autoload.php
@@ -83,6 +83,7 @@
        'Flow\\Data\\Listener\\DeferredInsertLifecycleHandler' => __DIR__ . 
'/includes/Data/Listener/DeferredInsertLifecycleHandler.php',
        'Flow\\Data\\Listener\\EditCountListener' => __DIR__ . 
'/includes/Data/Listener/EditCountListener.php',
        'Flow\\Data\\Listener\\ImmediateWatchTopicListener' => __DIR__ . 
'/includes/Data/Listener/WatchTopicListener.php',
+       'Flow\\Data\\Listener\\ModerationLoggingListener' => __DIR__ . 
'/includes/Data/Listener/ModerationLoggingListener.php',
        'Flow\\Data\\Listener\\NotificationListener' => __DIR__ . 
'/includes/Data/Listener/NotificationListener.php',
        'Flow\\Data\\Listener\\OccupationListener' => __DIR__ . 
'/includes/Data/Listener/OccupationListener.php',
        'Flow\\Data\\Listener\\RecentChangesListener' => __DIR__ . 
'/includes/Data/Listener/RecentChangesListener.php',
@@ -224,9 +225,8 @@
        'Flow\\Import\\Wikitext\\ImportSource' => __DIR__ . 
'/includes/Import/Wikitext/ImportSource.php',
        'Flow\\LinksTableUpdater' => __DIR__ . 
'/includes/LinksTableUpdater.php',
        'Flow\\Log\\ActionFormatter' => __DIR__ . 
'/includes/Log/ActionFormatter.php',
-       'Flow\\Log\\Logger' => __DIR__ . '/includes/Log/Logger.php',
        'Flow\\Log\\LqtImportFormatter' => __DIR__ . 
'/includes/Log/LqtImportFormatter.php',
-       'Flow\\Log\\PostModerationLogger' => __DIR__ . 
'/includes/Log/PostModerationLogger.php',
+       'Flow\\Log\\ModerationLogger' => __DIR__ . 
'/includes/Log/ModerationLogger.php',
        'Flow\\Model\\AbstractRevision' => __DIR__ . 
'/includes/Model/AbstractRevision.php',
        'Flow\\Model\\AbstractSummary' => __DIR__ . 
'/includes/Model/AbstractSummary.php',
        'Flow\\Model\\Anchor' => __DIR__ . '/includes/Model/Anchor.php',
diff --git a/container.php b/container.php
index 3ba5919..858cc61 100644
--- a/container.php
+++ b/container.php
@@ -524,9 +524,9 @@
                $c['repository.tree']
        );
 } );
-$c['storage.post.listeners.moderation_logger'] = $c->share( function( $c ) {
-       return new Flow\Log\PostModerationLogger(
-               $c['logger']
+$c['storage.post.listeners.moderation_logging'] = $c->share( function( $c ) {
+       return new Flow\Data\Listener\ModerationLoggingListener(
+               $c['logger.moderation']
        );
 } );
 $c['storage.post.listeners.username'] = $c->share( function( $c ) {
@@ -564,7 +564,7 @@
                $c['storage.post.listeners.username'],
                $c['storage.post.listeners.watch_topic'],
                $c['storage.post.listeners.notification'],
-               $c['storage.post.listeners.moderation_logger'],
+               $c['storage.post.listeners.moderation_logging'],
                $c['listener.recentchanges'],
                $c['listener.editcount'],
                // topic history -- to keep a history by topic we have to know 
what topic every post
@@ -973,8 +973,8 @@
                $c['formatter.revision']
        );
 } );
-$c['logger'] = $c->share( function( $c ) {
-       return new Flow\Log\Logger(
+$c['logger.moderation'] = $c->share( function( $c ) {
+       return new Flow\Log\ModerationLogger(
                $c['flow_actions'],
                $c['user']
        );
diff --git a/includes/Log/PostModerationLogger.php 
b/includes/Data/Listener/ModerationLoggingListener.php
similarity index 65%
rename from includes/Log/PostModerationLogger.php
rename to includes/Data/Listener/ModerationLoggingListener.php
index d096424..a243a96 100644
--- a/includes/Log/PostModerationLogger.php
+++ b/includes/Data/Listener/ModerationLoggingListener.php
@@ -1,30 +1,32 @@
 <?php
 
-namespace Flow\Log;
+namespace Flow\Data\Listener;
 
 use Flow\Data\LifecycleHandler;
+use Flow\Log\ModerationLogger;
 use Flow\Model\AbstractRevision;
 use Flow\Model\PostRevision;
+use Flow\Model\Workflow;
 
-class PostModerationLogger implements LifecycleHandler {
+class ModerationLoggingListener implements LifecycleHandler {
 
        /**
-        * @var Logger
+        * @var ModerationLogger
         */
-       protected $logger;
+       protected $moderationLogger;
 
-       function __construct( Logger $logger ) {
-               $this->logger = $logger;
+       function __construct( ModerationLogger $moderationLogger ) {
+               $this->moderationLogger = $moderationLogger;
        }
 
        /**
         * @param PostRevision $object
         * @param array $row
-        * @param array $metadata
+        * @param array $metadata (must contain 'workflow' key with a Workflow 
object)
         */
        function onAfterInsert( $object, array $row, array $metadata ) {
                if ( $object instanceof PostRevision ) {
-                       $this->log( $object );
+                       $this->log( $object, $metadata['workflow'] );
                }
        }
 
@@ -40,29 +42,21 @@
                // Move along
        }
 
-       protected function log( PostRevision $post ) {
+       protected function log( PostRevision $post, Workflow $workflow ) {
                $moderationChangeTypes = self::getModerationChangeTypes();
                if ( ! in_array( $post->getChangeType(), $moderationChangeTypes 
) ) {
                        // Do nothing for non-moderation actions
                        return;
                }
 
-               if ( $this->logger->canLog( $post, $post->getChangeType() ) ) {
-                       $workflowId = $post->getRootPost()->getPostId();
-                       $logParams = array();
+               if ( $this->moderationLogger->canLog( $post, 
$post->getChangeType() ) ) {
+                       $workflowId = $workflow->getId();
 
-                       if ( $post->isTopicTitle() ) {
-                               $logParams['topicId'] = $workflowId;
-                       } else {
-                               $logParams['postId'] = $post->getRevisionId();
-                       }
-
-                       $this->logger->log(
+                       $this->moderationLogger->log(
                                $post,
                                $post->getChangeType(),
                                $post->getModeratedReason(),
-                               $workflowId,
-                               $logParams
+                               $workflowId
                        );
                }
        }
diff --git a/includes/Log/Logger.php b/includes/Log/ModerationLogger.php
similarity index 93%
rename from includes/Log/Logger.php
rename to includes/Log/ModerationLogger.php
index 9c1e5f9..75aace7 100644
--- a/includes/Log/Logger.php
+++ b/includes/Log/ModerationLogger.php
@@ -12,7 +12,7 @@
 use Title;
 use User;
 
-class Logger {
+class ModerationLogger {
 
        /**
         * @var User
@@ -51,14 +51,21 @@
         * @param string $action The action we'll be logging
         * @param string $reason Comment, reason for the moderation
         * @param UUID $workflowId Workflow being worked on
-        * @param array $params Additional parameters to be saved
         * @return int The id of the newly inserted log entry
         */
-       public function log( PostRevision $post, $action, $reason, UUID 
$workflowId, $params = array() ) {
+       public function log( PostRevision $post, $action, $reason, UUID 
$workflowId ) {
                if ( !$this->canLog( $post, $action ) ) {
                        return null;
                }
 
+               $params = array();
+
+               if ( $post->isTopicTitle() ) {
+                       $params['topicId'] = $workflowId;
+               } else {
+                       $params['postId'] = $post->getPostId();
+               }
+
                $logType = $this->getLogType( $post, $action );
 
                // reasonably likely this is already loaded in-process and just 
returns that object
diff --git a/maintenance/FlowAddMissingModerationLogs.php 
b/maintenance/FlowAddMissingModerationLogs.php
index 9508319..cda2776 100644
--- a/maintenance/FlowAddMissingModerationLogs.php
+++ b/maintenance/FlowAddMissingModerationLogs.php
@@ -1,7 +1,7 @@
 <?php
 
 use Flow\Container;
-use Flow\Log\PostModerationLogger;
+use Flow\Data\Listener\ModerationLoggingListener;
 use Flow\Model\UUID;
 
 require_once ( getenv( 'MW_INSTALL_PATH' ) !== false
@@ -37,7 +37,7 @@
 
                $storage = $container['storage'];
 
-               $moderationLogger = 
$container['storage.post.listeners.moderation_logger'];
+               $moderationLoggingListener = 
$container['storage.post.listeners.moderation_logging'];
 
                $rowIterator = new EchoBatchRowIterator(
                        $dbw,
@@ -53,7 +53,7 @@
 
                // Fetch rows that are a moderation action
                $rowIterator->addConditions( array(
-                       'rev_change_type' => 
PostModerationLogger::getModerationChangeTypes(),
+                       'rev_change_type' => 
ModerationLoggingListener::getModerationChangeTypes(),
                ) );
 
                $start = $this->getOption( 'start' );
@@ -82,7 +82,10 @@
                                        continue;
                                }
 
-                               $moderationLogger->onAfterInsert( $obj, 
array(), array() );
+                               $workflow = 
$obj->getCollection()->getWorkflow();
+                               $moderationLoggingListener->onAfterInsert( 
$obj, array(), array(
+                                       'workflow' => $workflow,
+                               ) );
                        }
 
                        $dbw->commit();

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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib7b9177522a0e238c1b15c8e8e13ff27f6cf89af
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: Mattflaschen <[email protected]>
Gerrit-Reviewer: EBernhardson <[email protected]>
Gerrit-Reviewer: Mattflaschen <[email protected]>
Gerrit-Reviewer: Matthias Mullie <[email protected]>
Gerrit-Reviewer: SG <[email protected]>
Gerrit-Reviewer: jenkins-bot <>

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

Reply via email to