From: Anomie <bjorsch@wikimedia.org>
Date: Mon, 19 Aug 2019 14:49:30 +0000 (+0000)
Subject: Revert "Clean up ApiEditPage content handler undo logic"
X-Git-Tag: 1.34.0-rc.0~681^2
X-Git-Url: http://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/operations/?a=commitdiff_plain;h=c8fcdf5d5f4883b0940ef1a4b0daf7a5d8ce4a4a;p=lhc%2Fweb%2Fwiklou.git

Revert "Clean up ApiEditPage content handler undo logic"

This reverts commit 01564dcbf76cd279c88320f96f0844cfadf4f7c9.

Reason for revert: I find the empty case far clearer as to intention than this change, and I doubt this is such a hot path that optimization should take precedence over clarity. If we really want to get rid of the empty case, I'd have structured it somewhat clearly as

 if (
     // always allow undo via api, T230702
     !( $params['undo'] > 0 ) &&
     // [something else here to explain this clause]
     $contentHandler->supportsDirectApiEditing() === false
 ) {

But even that is IMO less clear. At the very least this seems to deserve more discussion.

Bug: T230702
Change-Id: Icc16250faad31abf7308c4af9be2e7658540346c
---

diff --git a/includes/api/ApiEditPage.php b/includes/api/ApiEditPage.php
index 554df61e9c..3f63a0085a 100644
--- a/includes/api/ApiEditPage.php
+++ b/includes/api/ApiEditPage.php
@@ -92,8 +92,9 @@ class ApiEditPage extends ApiBase {
 		$name = $titleObj->getPrefixedDBkey();
 		$model = $contentHandler->getModelID();
 
-		// always allow undo via api, T230702
-		if ( !( $params['undo'] > 0 ) && $contentHandler->supportsDirectApiEditing() === false ) {
+		if ( $params['undo'] > 0 ) {
+			// allow undo via api
+		} elseif ( $contentHandler->supportsDirectApiEditing() === false ) {
 			$this->dieWithError( [ 'apierror-no-direct-editing', $model, $name ] );
 		}