From 07530dfb63fe24c06ab4508c6cb8436896104e7c Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Fri, 31 Aug 2018 11:22:30 -0400 Subject: [PATCH] ApiComparePages: Clean up handling of slot deletion We can't allow the main slot to be deleted. DifferenceEngine assumes it exits. We also shouldn't allow parameters such as `tosection-{role}` to be used without the corresponing `totext-{role}`. This will help prevent people from being confused into thinking that `tosection-{role}` will do anything in that situation (as opposed to `tosection`, which did). Bug: T203255 Change-Id: I58573bb2c1ee68e6907ef2e88385fe36e5184076 --- includes/api/ApiComparePages.php | 17 +++++++++++++++++ includes/api/i18n/en.json | 2 ++ includes/api/i18n/qqq.json | 2 ++ .../includes/api/ApiComparePagesTest.php | 10 ++++++++++ 4 files changed, 31 insertions(+) diff --git a/includes/api/ApiComparePages.php b/includes/api/ApiComparePages.php index 6bfa35dd6e..02cadbd400 100644 --- a/includes/api/ApiComparePages.php +++ b/includes/api/ApiComparePages.php @@ -412,6 +412,23 @@ class ApiComparePages extends ApiBase { foreach ( $params["{$prefix}slots"] as $role ) { $text = $params["{$prefix}text-{$role}"]; if ( $text === null ) { + // The 'main' role can't be deleted + if ( $role === 'main' ) { + $this->dieWithError( [ 'apierror-compare-maintextrequired', $prefix ] ); + } + + // These parameters make no sense without text. Reject them to avoid + // confusion. + foreach ( [ 'section', 'contentmodel', 'contentformat' ] as $param ) { + if ( isset( $params["{$prefix}{$param}-{$role}"] ) ) { + $this->dieWithError( [ + 'apierror-compare-notext', + wfEscapeWikiText( "{$prefix}{$param}-{$role}" ), + wfEscapeWikiText( "{$prefix}text-{$role}" ), + ] ); + } + } + $newRev->removeSlot( $role ); continue; } diff --git a/includes/api/i18n/en.json b/includes/api/i18n/en.json index ae2ffd3ddb..253380c978 100644 --- a/includes/api/i18n/en.json +++ b/includes/api/i18n/en.json @@ -1717,10 +1717,12 @@ "apierror-changeauth-norequest": "Failed to create change request.", "apierror-chunk-too-small": "Minimum chunk size is $1 {{PLURAL:$1|byte|bytes}} for non-final chunks.", "apierror-cidrtoobroad": "$1 CIDR ranges broader than /$2 are not accepted.", + "apierror-compare-maintextrequired": "Parameter $1text-main is required when $1slots contains main (cannot delete the main slot).", "apierror-compare-no-title": "Cannot pre-save transform without a title. Try specifying fromtitle or totitle.", "apierror-compare-nosuchfromsection": "There is no section $1 in the 'from' content.", "apierror-compare-nosuchtosection": "There is no section $1 in the 'to' content.", "apierror-compare-nofromrevision": "No 'from' revision. Specify fromrev, fromtitle, or fromid.", + "apierror-compare-notext": "Parameter $1 cannot be used without $2.", "apierror-compare-notorevision": "No 'to' revision. Specify torev, totitle, or toid.", "apierror-compare-relative-to-nothing": "No 'from' revision for torelative to be relative to.", "apierror-contentserializationexception": "Content serialization failed: $1", diff --git a/includes/api/i18n/qqq.json b/includes/api/i18n/qqq.json index 33f6613ada..eb3fdef0e4 100644 --- a/includes/api/i18n/qqq.json +++ b/includes/api/i18n/qqq.json @@ -1605,10 +1605,12 @@ "apierror-changeauth-norequest": "{{doc-apierror}}", "apierror-chunk-too-small": "{{doc-apierror}}\n\nParameters:\n* $1 - Minimum size in bytes.", "apierror-cidrtoobroad": "{{doc-apierror}}\n\nParameters:\n* $1 - \"IPv4\" or \"IPv6\"\n* $2 - Minimum CIDR mask length.", + "apierror-compare-maintextrequired": "{{doc-apierror}}\n\nParameters:\n* $1 - Parameter name prefix, 'to' or 'from'.", "apierror-compare-no-title": "{{doc-apierror}}", "apierror-compare-nosuchfromsection": "{{doc-apierror}}\n\nParameters:\n* $1 - Section identifier. Probably a number or \"T-\" followed by a number.", "apierror-compare-nosuchtosection": "{{doc-apierror}}\n\nParameters:\n* $1 - Section identifier. Probably a number or \"T-\" followed by a number.", "apierror-compare-nofromrevision": "{{doc-apierror}}", + "apierror-compare-notext": "{{doc-apierror}}\n\nParameters:\n* $1 - Parameter that is not allowed without totext-{role}/fromtext-{role}.\n* $2 - The specific totext-{role}/fromtext-{role} parameter that must be present.", "apierror-compare-notorevision": "{{doc-apierror}}", "apierror-compare-relative-to-nothing": "{{doc-apierror}}", "apierror-contentserializationexception": "{{doc-apierror}}\n\nParameters:\n* $1 - Exception text, may end with punctuation. Currently this is probably English, hopefully we'll fix that in the future.", diff --git a/tests/phpunit/includes/api/ApiComparePagesTest.php b/tests/phpunit/includes/api/ApiComparePagesTest.php index 30e1d0c618..3709c2195a 100644 --- a/tests/phpunit/includes/api/ApiComparePagesTest.php +++ b/tests/phpunit/includes/api/ApiComparePagesTest.php @@ -936,6 +936,16 @@ class ApiComparePagesTest extends ApiTestCase { [], 'sectionreplacefailed', ], + 'Error, deleting the main slot' => [ + [ + 'fromtitle' => 'ApiComparePagesTest A', + 'totitle' => 'ApiComparePagesTest A', + 'toslots' => 'main', + ], + [], + 'compare-maintextrequired', + ], + // @todo Add a test for using 'tosection-foo' without 'totext-foo' (can't do it with main) ]; // phpcs:enable } -- 2.20.1