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

diff --git a/includes/api/ApiComparePages.php b/includes/api/ApiComparePages.php
index 6bfa35d..02cadbd 100644 (file)
--- 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 ae2ffd3..253380c 100644 (file)
--- 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 <var>$1text-main</var> is required when <var>$1slots</var> contains <kbd>main</kbd> (cannot delete the main slot).",
        "apierror-compare-no-title": "Cannot pre-save transform without a title. Try specifying <var>fromtitle</var> or <var>totitle</var>.",
        "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 <var>fromrev</var>, <var>fromtitle</var>, or <var>fromid</var>.",
+       "apierror-compare-notext": "Parameter <var>$1</var> cannot be used without <var>$2</var>.",
        "apierror-compare-notorevision": "No 'to' revision. Specify <var>torev</var>, <var>totitle</var>, or <var>toid</var>.",
        "apierror-compare-relative-to-nothing": "No 'from' revision for <var>torelative</var> 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 33f6613..eb3fdef 100644 (file)
--- 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 <var>totext-&#x7B;role}</var>/<var>fromtext-&#x7B;role}</var>.\n* $2 - The specific <var>totext-&#x7B;role}</var>/<var>fromtext-&#x7B;role}</var> 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 30e1d0c..3709c21 100644 (file)
--- 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
        }