jenkins-bot has submitted this change and it was merged.
Change subject: [SCHEMA CHANGE] Refactor of moderation logging.
......................................................................
[SCHEMA CHANGE] Refactor of moderation logging.
* Store moderation reason in object.
* Use a LifecycleHandler to do logging rather than doing it directly.
* Includes permalink handling for the logging class. Unfortunately not
backwards compatible.
Bug: 57055
Bug: 57056
Change-Id: Idd5a6570268e480c9cbe2c9620dfbb587db71ee4
---
M Flow.php
M Hooks.php
M Resources.php
M container.php
A db_patches/patch-moderation_reason.sql
M flow.sql
M includes/Block/Topic.php
M includes/Data/RevisionStorage.php
M includes/Log/Formatter.php
A includes/Log/PostModerationLogger.php
M includes/Model/AbstractRevision.php
M modules/moderation/moderation.js
12 files changed, 132 insertions(+), 17 deletions(-)
Approvals:
Matthias Mullie: Looks good to me, approved
jenkins-bot: Verified
diff --git a/Flow.php b/Flow.php
index 60c2431..ffd16e8 100755
--- a/Flow.php
+++ b/Flow.php
@@ -122,6 +122,7 @@
$wgAutoloadClasses['Flow\RecentChanges\Formatter'] = $dir .
'includes/RecentChanges/Formatter.php';
$wgAutoloadClasses['Flow\Log\Logger'] = $dir . 'includes/Log/Logger.php';
$wgAutoloadClasses['Flow\Log\Formatter'] = $dir . 'includes/Log/Formatter.php';
+$wgAutoloadClasses['Flow\Log\PostModerationLogger'] = $dir .
'includes/Log/PostModerationLogger.php';
// database interaction for singular models
$wgAutoloadClasses['Flow\Data\RevisionStorage'] = $dir .
'includes/Data/RevisionStorage.php';
diff --git a/Hooks.php b/Hooks.php
index 8c8599e..b481802 100644
--- a/Hooks.php
+++ b/Hooks.php
@@ -27,6 +27,7 @@
$baseSQLFile = "$dir/flow.sql";
$updater->addExtensionTable( 'flow_revision', $baseSQLFile );
$updater->addExtensionField( 'flow_revision',
'rev_last_edit_id', "$dir/db_patches/patch-revision_last_editor.sql" );
+ $updater->addExtensionField( 'flow_revision', 'rev_mod_reason',
"$dir/db_patches/patch-moderation_reason.sql" );
if ( $updater->getDB()->getType() === 'sqlite' ) {
$updater->modifyExtensionField(
'flow_summary_revision', 'summary_workflow_id',
"$dir/db_patches/patch-summary2header.sqlite.sql" );
$updater->modifyExtensionField( 'flow_revision',
'rev_comment', "$dir/db_patches/patch-rev_change_type.sqlite.sql" );
diff --git a/Resources.php b/Resources.php
index d05785d..51ef8a4 100644
--- a/Resources.php
+++ b/Resources.php
@@ -129,6 +129,7 @@
'ext.flow.base',
'jquery.ui.dialog',
'jquery.spinner',
+ 'jquery.byteLimit',
),
),
'ext.flow.editor' => $flowResourceTemplate + array(
diff --git a/container.php b/container.php
index c7372b1..0a590f1 100644
--- a/container.php
+++ b/container.php
@@ -287,6 +287,7 @@
);
$handlers = array(
+ new Flow\Log\PostModerationLogger( $c['storage'],
$c['repository.tree'], $c['logger'] ),
new Flow\Data\PostRevisionRecentChanges( $c['storage'],
$c['repository.tree'], $wgContLang ),
$c['storage.board_history.index'],
);
diff --git a/db_patches/patch-moderation_reason.sql
b/db_patches/patch-moderation_reason.sql
new file mode 100644
index 0000000..8bf2a24
--- /dev/null
+++ b/db_patches/patch-moderation_reason.sql
@@ -0,0 +1,3 @@
+-- Patch to add the "moderated reason" field to revision table
+
+ALTER TABLE /*_*/flow_revision ADD COLUMN rev_mod_reason varchar(255) binary;
\ No newline at end of file
diff --git a/flow.sql b/flow.sql
index b0c7ac7..e54f618 100644
--- a/flow.sql
+++ b/flow.sql
@@ -122,6 +122,8 @@
rev_mod_user_id bigint unsigned,
rev_mod_user_text varchar(255) binary,
rev_mod_timestamp varchar(14) binary,
+ -- moderated why? (coming soon: how?, where? and what?)
+ rev_mod_reason varchar(255) binary,
-- track who made the most recent content edit
rev_last_edit_id binary(16) null,
diff --git a/includes/Block/Topic.php b/includes/Block/Topic.php
index ae15731..0905ad3 100644
--- a/includes/Block/Topic.php
+++ b/includes/Block/Topic.php
@@ -228,27 +228,16 @@
$this->errors['moderate'] = wfMessage(
'flow-error-invalid-moderation-reason' );
return;
}
+
+ $reason = $this->submitted['reason'];
+
if ( $post->needsModerateHistorical( $newState ) ) {
$this->relatedRevisions = $this->loadHistorical( $post
);
}
- $this->newRevision = $post->moderate( $this->user, $newState,
$action, $this->relatedRevisions );
+ $this->newRevision = $post->moderate( $this->user, $newState,
$action, $reason, $this->relatedRevisions );
if ( !$this->newRevision ) {
$this->errors['moderate'] = wfMessage(
'flow-error-not-allowed' );
- } else {
- // @todo: would be nice to get this logging into a
LifecycleHandler
- $logger = Container::get( 'logger' );
- if ( $logger->canLog( $post, $action ) ) {
- $logger->log(
- $post,
- $action,
- $this->submitted['reason'],
- $this->getWorkflow(),
- array(
- $this->getName() . '[postId]'
=> $post->getPostId()->getHex(),
- )
- );
- }
}
}
diff --git a/includes/Data/RevisionStorage.php
b/includes/Data/RevisionStorage.php
index b8b0547..5d6c202 100644
--- a/includes/Data/RevisionStorage.php
+++ b/includes/Data/RevisionStorage.php
@@ -11,7 +11,11 @@
abstract class RevisionStorage implements WritableObjectStorage {
static protected $allowedUpdateColumns = array(
- 'rev_mod_state', 'rev_mod_user_id', 'rev_mod_user_text',
'rev_mod_timestamp',
+ 'rev_mod_state',
+ 'rev_mod_user_id',
+ 'rev_mod_user_text',
+ 'rev_mod_timestamp',
+ 'rev_mod_reason',
);
protected $dbFactory;
protected $externalStores;
diff --git a/includes/Log/Formatter.php b/includes/Log/Formatter.php
index 659cf28..88dfd8d 100644
--- a/includes/Log/Formatter.php
+++ b/includes/Log/Formatter.php
@@ -2,6 +2,7 @@
namespace Flow\Log;
+use Flow\Model\UUID;
use Message;
class Formatter extends \LogFormatter {
@@ -19,6 +20,29 @@
$skin = $this->plaintext ? null : $this->context->getSkin();
$params = $this->entry->getParameters();
+ // FIXME this is ugly. Why were we treating log parameters as
+ // URL GET parameters in the first place?
+ if ( isset( $params['postId'] ) ) {
+ $title = clone $title;
+ $postId = $params['postId'];
+ if ( $postId instanceof UUID ) {
+ $postId = $postId->getHex();
+ }
+ $title->setFragment( '#flow-post-' . $postId );
+ unset( $params['postId'] );
+ }
+
+ if ( isset( $params['topicId'] ) ) {
+ $title = clone $title;
+ $topicId = $params['topicId'];
+ if ( $topicId instanceof UUID ) {
+ $topicId = $topicId->getHex();
+ }
+
+ $title->setFragment( '#flow-topic-' . $topicId );
+ unset( $params['topicId'] );
+ }
+
// Give grep a chance to find the usages:
// logentry-delete-flow-delete-post,
logentry-delete-flow-restore-post,
// logentry-suppress-flow-restore-post,
logentry-suppress-flow-censor-post,
diff --git a/includes/Log/PostModerationLogger.php
b/includes/Log/PostModerationLogger.php
new file mode 100644
index 0000000..29ffc89
--- /dev/null
+++ b/includes/Log/PostModerationLogger.php
@@ -0,0 +1,77 @@
+<?php
+
+namespace Flow\Log;
+
+use Flow\Data\LifecycleHandler;
+use Flow\Data\ManagerGroup;
+use Flow\Log\Logger;
+use Flow\Model\AbstractRevision;
+use Flow\Repository\TreeRepository;
+
+class PostModerationLogger implements LifecycleHandler {
+ function __construct( ManagerGroup $storage, TreeRepository $treeRepo,
Logger $logger ) {
+ $this->storage = $storage;
+ $this->treeRepo = $treeRepo;
+ $this->logger = $logger;
+ }
+
+ function onAfterInsert( $object, array $row ) {
+ $moderationChangeTypes = self::getModerationChangeTypes();
+ if ( ! in_array( $object->getChangeType(),
$moderationChangeTypes ) ) {
+ // Do nothing for non-moderation actions
+ return;
+ }
+
+ if ( $this->logger->canLog( $object, $object->getChangeType() )
) {
+ // This is awful but it's all I can think of
+ $rootPost = $this->treeRepo->findRoot(
$object->getPostId() );
+ $workflow = $this->storage->get( 'Workflow', $rootPost
);
+ $logParams = array();
+
+ if ( $object->isTopicTitle() ) {
+ $logParams['topicId'] = $workflow->getId();
+ } else {
+ $logParams['postId'] = $object->getRevisionId();
+ }
+
+ $this->logger->log(
+ $object,
+ $object->getChangeType(),
+ $object->getModeratedReason(),
+ $workflow,
+ $logParams
+ );
+ }
+ }
+
+ function onAfterLoad( $object, array $old ) {
+ // You don't need to see my identification
+ }
+
+ function onAfterUpdate( $object, array $old, array $new ) {
+ // These aren't the droids you're looking for
+ }
+
+ function onAfterRemove( $object, array $old ) {
+ // Move along
+ }
+
+ protected static function getModerationChangeTypes() {
+ static $changeTypes = false;
+
+ if ( ! $changeTypes ) {
+ $changeTypes = array();
+ foreach( AbstractRevision::$perms as $perm => $info ) {
+ if ( $perm != '' ) {
+ $changeTypes[] = "{$perm}-topic";
+ $changeTypes[] = "{$perm}-post";
+ }
+ }
+
+ $changeTypes[] = 'restore-topic';
+ $changeTypes[] = 'restore-post';
+ }
+
+ return $changeTypes;
+ }
+}
\ No newline at end of file
diff --git a/includes/Model/AbstractRevision.php
b/includes/Model/AbstractRevision.php
index c8f4268..475a432 100644
--- a/includes/Model/AbstractRevision.php
+++ b/includes/Model/AbstractRevision.php
@@ -77,6 +77,7 @@
protected $moderationTimestamp;
protected $moderatedByUserId;
protected $moderatedByUserText;
+ protected $moderatedReason;
protected $lastEditId;
protected $lastEditUserId;
@@ -103,6 +104,7 @@
$obj->moderatedByUserId = $row['rev_mod_user_id'];
$obj->moderatedByUserText = $row['rev_mod_user_text'];
$obj->moderationTimestamp = $row['rev_mod_timestamp'];
+ $obj->moderatedReason = isset( $row['rev_mod_reason'] ) ?
$row['rev_mod_reason'] : null;
// isset required because there is a possible db migration,
cached data will not have it
$obj->lastEditId = isset( $row['rev_last_edit_id'] ) ?
UUID::create( $row['rev_last_edit_id'] ) : null;
@@ -129,6 +131,7 @@
'rev_mod_user_id' => $obj->moderatedByUserId,
'rev_mod_user_text' => $obj->moderatedByUserText,
'rev_mod_timestamp' => $obj->moderationTimestamp,
+ 'rev_mod_reason' => $obj->moderatedReason,
'rev_last_edit_id' => $obj->lastEditId ?
$obj->lastEditId->getBinary() : null,
'rev_edit_user_id' => $obj->lastEditUserId,
@@ -180,7 +183,7 @@
* $historical revisions must be provided when
self::needsModerateHistorical
* returns true.
*/
- public function moderate( User $user, $state, $changeType, array
$historical = array() ) {
+ public function moderate( User $user, $state, $changeType, $reason,
array $historical = array() ) {
if ( ! $this->isValidModerationState( $state ) ) {
wfWarn( __METHOD__ . ': Provided moderation state does
not exist : ' . $state );
return null;
@@ -209,10 +212,14 @@
$rev->moderatedByUserId = null;
$rev->moderatedByUserText = null;
$rev->moderationTimestamp = null;
+ // This is a bit hacky, but we store the
restore reason
+ // in the "moderated reason" field. Hmmph.
+ $rev->moderatedReason = $reason;
} else {
$rev->moderatedByUserId = $user->getId();
$rev->moderatedByUserText = $user->getName();
$rev->moderationTimestamp = $timestamp;
+ $rev->moderatedReason = $reason;
}
}
@@ -396,6 +403,10 @@
return $this->moderationState;
}
+ public function getModeratedReason() {
+ return $this->moderatedReason;
+ }
+
public function isModerated() {
return $this->moderationState !== self::MODERATED_NONE;
}
diff --git a/modules/moderation/moderation.js b/modules/moderation/moderation.js
index 77d52c2..9968aff 100644
--- a/modules/moderation/moderation.js
+++ b/modules/moderation/moderation.js
@@ -88,6 +88,7 @@
.append(
$( '<textarea/>' )
.attr( 'id',
'flow-moderation-reason' )
+ .byteLimit( 255 )
.attr( 'placeholder', mw.msg(
'flow-moderation-reason-placeholder' ) )
)
.appendTo( $dialog );
--
To view, visit https://gerrit.wikimedia.org/r/95985
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Idd5a6570268e480c9cbe2c9620dfbb587db71ee4
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: Werdna <[email protected]>
Gerrit-Reviewer: EBernhardson <[email protected]>
Gerrit-Reviewer: Matthias Mullie <[email protected]>
Gerrit-Reviewer: Werdna <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
