From c8fcdf5d5f4883b0940ef1a4b0daf7a5d8ce4a4a Mon Sep 17 00:00:00 2001 From: Anomie Date: Mon, 19 Aug 2019 14:49:30 +0000 Subject: [PATCH] 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 --- includes/api/ApiEditPage.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 ] ); } -- 2.20.1