Add a Status and a $reason parameter to TitleMove hook

So that using this hook it's possible to prevent the move, also
providing some more context.
Also, clean error message: instead of going with "you do not have
permission blah blah" for *every* kind of error, use it only when the
error is actually about permissions, and use a generic message
otherwise.

Bug: T208907
Change-Id: I4733724075b7514e9db59e7be772d9409aa9da87

diff --git a/docs/hooks.txt b/docs/hooks.txt
index bd06d52..2f800a4 100644 (file)
--- a/docs/hooks.txt
+++ b/docs/hooks.txt
@@ -3418,6 +3418,9 @@ $title: Title object that is being checked
 $old: old title
 $nt: new title
 $user: user who does the move
+$reason: string of the reason provided by the user
+&$status: Status object. To abort the move, add a fatal error to this object
+       (i.e. call $status->fatal()).
 
 'TitleMoveStarting': Before moving an article (title), but just after the atomic
 DB section starts.

diff --git a/includes/MovePage.php b/includes/MovePage.php
index 5213fc1..0fd697b 100644 (file)
--- a/includes/MovePage.php
+++ b/includes/MovePage.php
@@ -240,7 +240,12 @@ class MovePage {
        public function move( User $user, $reason, $createRedirect, array $changeTags = [] ) {
                global $wgCategoryCollation;
 
-               Hooks::run( 'TitleMove', [ $this->oldTitle, $this->newTitle, $user ] );
+               $status = Status::newGood();
+               Hooks::run( 'TitleMove', [ $this->oldTitle, $this->newTitle, $user, $reason, &$status ] );
+               if ( !$status->isOK() ) {
+                       // Move was aborted by the hook
+                       return $status;
+               }
 
                // If it is a file, move it first.
                // It is done before all other moving stuff is done because it's hard to revert.

diff --git a/includes/specials/SpecialMovepage.php b/includes/specials/SpecialMovepage.php
index 5cbad8a..3ce786e 100644 (file)
--- a/includes/specials/SpecialMovepage.php
+++ b/includes/specials/SpecialMovepage.php
@@ -137,8 +137,9 @@ class MovePageForm extends UnlistedSpecialPage {
         * @param array $err Error messages. Each item is an error message.
         *    It may either be a string message name or array message name and
         *    parameters, like the second argument to OutputPage::wrapWikiMsg().
+        * @param bool $isPermError Whether the error message is about user permissions.
         */
-       function showForm( $err ) {
+       function showForm( $err, $isPermError = false ) {
                $this->getSkin()->setRelevantTitle( $this->oldTitle );
 
                $out = $this->getOutput();
@@ -235,9 +236,13 @@ class MovePageForm extends UnlistedSpecialPage {
                }
 
                if ( count( $err ) ) {
-                       $action_desc = $this->msg( 'action-move' )->plain();
-                       $errMsgHtml = $this->msg( 'permissionserrorstext-withaction',
-                               count( $err ), $action_desc )->parseAsBlock();
+                       if ( $isPermError ) {
+                               $action_desc = $this->msg( 'action-move' )->plain();
+                               $errMsgHtml = $this->msg( 'permissionserrorstext-withaction',
+                                       count( $err ), $action_desc )->parseAsBlock();
+                       } else {
+                               $errMsgHtml = $this->msg( 'cannotmove', count( $err ) )->parseAsBlock();
+                       }
 
                        if ( count( $err ) == 1 ) {
                                $errMsg = $err[0];
@@ -542,7 +547,7 @@ class MovePageForm extends UnlistedSpecialPage {
                        $permErrors = $nt->getUserPermissionsErrors( 'delete', $user );
                        if ( count( $permErrors ) ) {
                                # Only show the first error
-                               $this->showForm( $permErrors );
+                               $this->showForm( $permErrors, true );
 
                                return;
                        }
@@ -596,7 +601,7 @@ class MovePageForm extends UnlistedSpecialPage {
 
                $permStatus = $mp->checkPermissions( $user, $this->reason );
                if ( !$permStatus->isOK() ) {
-                       $this->showForm( $permStatus->getErrorsArray() );
+                       $this->showForm( $permStatus->getErrorsArray(), true );
                        return;
                }
 

diff --git a/languages/i18n/en.json b/languages/i18n/en.json
index 39ba360..bae4dee 100644 (file)
--- a/languages/i18n/en.json
+++ b/languages/i18n/en.json
@@ -2734,6 +2734,7 @@
        "move-watch": "Watch source page and target page",
        "movepagebtn": "Move page",
        "pagemovedsub": "Move succeeded",
+       "cannotmove": "The page could not be moved, for the following {{PLURAL:$1|reason|reasons}}:",
        "movepage-moved": "<strong>\"$1\" has been moved to \"$2\"</strong>",
        "movepage-moved-redirect": "A redirect has been created.",
        "movepage-moved-noredirect": "The creation of a redirect has been suppressed.",

diff --git a/languages/i18n/qqq.json b/languages/i18n/qqq.json
index d02dabe..6af7150 100644 (file)
--- a/languages/i18n/qqq.json
+++ b/languages/i18n/qqq.json
@@ -2937,6 +2937,7 @@
        "move-watch": "The text of the checkbox to watch the pages you are moving from and to. If checked, both the destination page and the original page will be added to the watchlist, even if you decide not to leave a redirect behind.\n\nSee also:\n* {{msg-mw|Move-page-legend|legend for the form}}\n* {{msg-mw|newtitle|label for new title}}\n* {{msg-mw|Movereason|label for textarea}}\n* {{msg-mw|Movetalk|label for checkbox}}\n* {{msg-mw|Move-leave-redirect|label for checkbox}}\n* {{msg-mw|Fix-double-redirects|label for checkbox}}\n* {{msg-mw|Move-subpages|label for checkbox}}\n* {{msg-mw|Move-talk-subpages|label for checkbox}}",
        "movepagebtn": "Button label on the special 'Move page'.\n\n{{Identical|Move page}}",
        "pagemovedsub": "Message displayed as aheader of the body, after successfully moving a page from source to target name.",
+       "cannotmove": "Error message for a generic failure while moving a page, to be used together with a specific error message.\n\nParameters:\n* $1 - the number of reasons that were found why the action cannot be performed.",
        "movepage-moved": "Message displayed after successfully moving a page from source to target name.\n\nParameters:\n* $1 - the source page as a link with display name\n* $2 - the target page as a link with display name\n* $3 - (optional) the source page name without a link\n* $4 - (optional) the target page name without a link\nSee also:\n* {{msg-mw|Movepage-moved-redirect}}\n* {{msg-mw|Movepage-moved-noredirect}}",
        "movepage-moved-redirect": "See also:\n* {{msg-mw|Movepage-moved}}\n* {{msg-mw|Movepage-moved-noredirect}}",
        "movepage-moved-noredirect": "The message is shown after pagemove if checkbox \"{{int:move-leave-redirect}}\" was unselected before moving.\n\nSee also:\n* {{msg-mw|Movepage-moved}}\n* {{msg-mw|Movepage-moved-redirect}}",

diff --git a/tests/phpunit/includes/MovePageTest.php b/tests/phpunit/includes/MovePageTest.php
index 583b751..607f4f7 100644 (file)
--- a/tests/phpunit/includes/MovePageTest.php
+++ b/tests/phpunit/includes/MovePageTest.php
@@ -62,4 +62,24 @@ class MovePageTest extends MediaWikiTestCase {
                        WikiPage::factory( $newTitle )->getRevision()
                );
        }
+
+       /**
+        * Test for the move operation being aborted via the TitleMove hook
+        */
+       public function testMoveAbortedByTitleMoveHook() {
+               $error = 'Preventing move operation with TitleMove hook.';
+               $this->setTemporaryHook( 'TitleMove',
+                       function ( $old, $new, $user, $reason, $status ) use ( $error ) {
+                               $status->fatal( $error );
+                       }
+               );
+
+               $oldTitle = Title::newFromText( 'Some old title' );
+               WikiPage::factory( $oldTitle )->doEditContent( new WikitextContent( 'foo' ), 'bar' );
+               $newTitle = Title::newFromText( 'A brand new title' );
+               $mp = new MovePage( $oldTitle, $newTitle );
+               $user = User::newFromName( 'TitleMove tester' );
+               $status = $mp->move( $user, 'Reason', true );
+               $this->assertTrue( $status->hasMessage( $error ) );
+       }
 }