(editchange) Add sanity check: disallow editchange of page moves, etc.

edwardspec · Aug 6, 2018 · 303770b · 303770b
1 parent 9312132
commit 303770b
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 7 deletions.
diff --git a/action/review/ModerationActionEditChange.php b/action/review/ModerationActionEditChange.php
@@ -35,21 +35,30 @@ public function execute() {
 			throw new ModerationError( 'moderation-unknown-modaction' );
 		}
 
+		$fields = [
+			'mod_namespace AS namespace',
+			'mod_title AS title',
+			'mod_text AS text',
+			'mod_comment AS comment'
+		];
+		if ( ModerationVersionCheck::hasModType() ) {
+			$fields[] = 'mod_type AS type';
+		}
+
 		$dbw = wfGetDB( DB_MASTER );
 		$row = $dbw->selectRow( 'moderation',
-			[
-				'mod_namespace AS namespace',
-				'mod_title AS title',
-				'mod_text AS text',
-				'mod_comment AS comment'
-			],
+			$fields,
 			[ 'mod_id' => $this->id ],
 			__METHOD__
 		);
 		if ( !$row ) {
 			throw new ModerationError( 'moderation-edit-not-found' );
 		}
 
+		if ( isset( $row->type ) && $row->type != ModerationNewChange::MOD_TYPE_EDIT ) {
+			throw new ModerationError( 'moderation-editchange-not-edit' );
+		}
+
 		return [
 			'id' => $this->id,
 			'namespace' => $row->namespace,

diff --git a/action/review/ModerationActionEditChangeSubmit.php b/action/review/ModerationActionEditChangeSubmit.php
@@ -31,6 +31,12 @@ public function execute() {
 			throw new ModerationError( 'moderation-unknown-modaction' );
 		}
 
+		$where = [ 'mod_id' => $this->id ];
+		if ( ModerationVersionCheck::hasModType() ) {
+			// Disallow modification of non-edits, e.g. pending page moves.
+			$where['mod_type'] = ModerationNewChange::MOD_TYPE_EDIT;
+		}
+
 		$dbw = wfGetDB( DB_MASTER );
 		$row = $dbw->selectRow( 'moderation',
 			[
@@ -39,7 +45,7 @@ public function execute() {
 				'mod_user AS user',
 				'mod_user_text AS user_text'
 			],
-			[ 'mod_id' => $this->id ],
+			$where,
 			__METHOD__
 		);
 		if ( !$row ) {

diff --git a/i18n/en.json b/i18n/en.json
@@ -48,6 +48,7 @@
 	"moderation-edit-not-found": "Edit not found. Probably already approved.",
 	"moderation-edit-queued": "Success: your edit has been sent to moderation.<br>You can continue editing <a href='$1'>your version of this page</a>.",
 	"moderation-editchange": "edit",
+	"moderation-editchange-not-edit": "This is not an edit. Only edits can be modified.",
 	"moderation-editchange-ok": "Pending revision changed.",
 	"moderation-editchange-title": "Pending revision of \"$1\"",
 	"moderation-folder-merged": "Merged",

diff --git a/i18n/qqq.json b/i18n/qqq.json
@@ -48,6 +48,7 @@
 	"moderation-edit-not-found": "Error message when Special:Moderation can't find the change (in 99.9% situations - because the change is already approved).",
 	"moderation-edit-queued": "Success message shown to non-automoderated user after the edit. Informs the user about moderation and that he can continue editing his version of the page. Parameters: $1 - link to the edit form.",
 	"moderation-editchange": "Edit button on Special:Moderation (disabled by default)",
+	"moderation-editchange-not-edit": "Error message when moderator tries to Edit a non-text change (e.g. proposal to rename a page).",
 	"moderation-editchange-ok": "Success message after modifying the pending change.",
 	"moderation-editchange-title": "Title of the editform page where moderator edits a pending change on Special:Moderation.",
 	"moderation-folder-merged": "Folder on Special:Moderation for manually merged changes.",

diff --git a/tests/phpunit/decoupled/30_action/ModerationActionTest.php b/tests/phpunit/decoupled/30_action/ModerationActionTest.php
@@ -157,6 +157,20 @@ public function dataProvider() {
 				'expectedError' => '(moderation-unknown-modaction)'
 			] ],
 
+			// editchange{,submit} shouldn't be applicable to non-text changes (e.g. page moves)
+			[ [
+				'modaction' => 'editchange',
+				'enableEditChange' => true,
+				'mod_type' => 'move',
+				'expectedError' => '(moderation-editchange-not-edit)'
+			] ],
+			[ [
+				'modaction' => 'editchangesubmit',
+				'enableEditChange' => true,
+				'mod_type' => 'move',
+				'expectedError' => '(moderation-edit-not-found)'
+			] ],
+
 			// Actions that don't modify anything shouldn't throw ReadOnlyError
 			[ [ 'modaction' => 'show', 'readonly' => true ] ],
 			// TODO: showimg