Expose the log_id of the deletion log entry in the action=delete API

This entails some refactoring to actually surface the log_id all the way
up:

* Made doDeleteArticleReal() return a Status object rather than a
  constant, and put the log_id in $status->value. This Status object is
  also passed to the ArticleDelete hook.
* Kept doDeleteArticle() the same for extension compatibility.
* Switched all core callers of doDeleteArticle() to
  doDeleteArticleReal() and surfaced the error message from the Status
  if appropriate, rather than hardcoding 'cannotdelete' all over the
  place.
* Exposed the log_id in ApiDelete
* Add 'delete-hook-aborted' message for when a hook aborts the deletion
  but does not provide an error message. Previously this just caused the
  'cannotdelete' message to appear.

Change-Id: Ia6415b390d5d4172ce96667f46ccdba2be02461f

diff --git a/docs/hooks.txt b/docs/hooks.txt
index 33db1c5..c927db4 100644 (file)
--- a/docs/hooks.txt
+++ b/docs/hooks.txt
@@ -422,6 +422,8 @@ $user: the user (object) deleting the article
 $reason: the reason (string) the article is being deleted
 $error: if the deletion was prohibited, the (raw HTML) error message to display
   (added in 1.13)
+$status: Status object, modify this to throw an error. Overridden by $error
+  (added in 1.20)
 
 'ArticleDeleteComplete': after an article is deleted
 $article: the WikiPage that was deleted

diff --git a/includes/Article.php b/includes/Article.php
index 6a3e41c..d5324ec 100644 (file)
--- a/includes/Article.php
+++ b/includes/Article.php
@@ -1479,7 +1479,8 @@ class Article extends Page {
        public function doDelete( $reason, $suppress = false ) {
                $error = '';
                $outputPage = $this->getContext()->getOutput();
-               if ( $this->mPage->doDeleteArticle( $reason, $suppress, 0, true, $error ) ) {
+               $status = $this->mPage->doDeleteArticleReal( $reason, $suppress, 0, true, $error );
+               if ( $status->isGood() ) {
                        $deleted = $this->getTitle()->getPrefixedText();
 
                        $outputPage->setPageTitle( wfMessage( 'actioncomplete' ) );
@@ -1492,8 +1493,9 @@ class Article extends Page {
                } else {
                        $outputPage->setPageTitle( wfMessage( 'cannotdelete-title', $this->getTitle()->getPrefixedText() ) );
                        if ( $error == '' ) {
+                               $errors = $status->getErrorsArray();
                                $outputPage->wrapWikiMsg( "<div class=\"error mw-error-cannotdelete\">\n$1\n</div>",
-                                       array( 'cannotdelete', wfEscapeWikiText( $this->getTitle()->getPrefixedText() ) )
+                                       $errors[0]
                                );
                                $outputPage->addHTML( Xml::element( 'h2', null, LogPage::logName( 'delete' ) ) );
 

diff --git a/includes/FileDeleteForm.php b/includes/FileDeleteForm.php
index d86c8d8..b794770 100644 (file)
--- a/includes/FileDeleteForm.php
+++ b/includes/FileDeleteForm.php
@@ -149,7 +149,10 @@ class FileDeleteForm {
                        try {
                                // delete the associated article first
                                $error = '';
-                               if ( $page->doDeleteArticleReal( $reason, $suppress, 0, false, $error, $user ) >= WikiPage::DELETE_SUCCESS ) {
+                               $deleteStatus = $page->doDeleteArticleReal( $reason, $suppress, 0, false, $error, $user );
+                               // doDeleteArticleReal() returns a non-fatal error status if the page
+                               // or revision is missing, so check for isOK() rather than isGood()
+                               if ( $deleteStatus->isOK() ) {
                                        $status = $file->delete( $reason, $suppress );
                                        if( $status->isOK() ) {
                                                $dbw->commit( __METHOD__ );

diff --git a/includes/Title.php b/includes/Title.php
index 4f0a897..6e3f16c 100644 (file)
--- a/includes/Title.php
+++ b/includes/Title.php
@@ -2866,7 +2866,7 @@ class Title {
         *
         * - This is called from WikiPage::doEdit() and WikiPage::insertOn() to allow
         * loading of the new page_id. It's also called from
-        * WikiPage::doDeleteArticle()
+        * WikiPage::doDeleteArticleReal()
         *
         * @param $newid Int the new Article ID
         */

diff --git a/includes/WikiPage.php b/includes/WikiPage.php
index 8e9b0a0..cd300de 100644 (file)
--- a/includes/WikiPage.php
+++ b/includes/WikiPage.php
@@ -34,30 +34,6 @@ abstract class Page {}
  * @internal documentation reviewed 15 Mar 2010
  */
 class WikiPage extends Page {
-       // doDeleteArticleReal() return values. Values less than zero indicate fatal errors,
-       // values greater than zero indicate that there were problems not resulting in page
-       // not being deleted
-
-       /**
-        * Delete operation aborted by hook
-        */
-       const DELETE_HOOK_ABORTED = -1;
-
-       /**
-        * Deletion successful
-        */
-       const DELETE_SUCCESS = 0;
-
-       /**
-        * Page not found
-        */
-       const DELETE_NO_PAGE = 1;
-
-       /**
-        * No revisions found to delete
-        */
-       const DELETE_NO_REVISIONS = 2;
-
        // Constants for $mDataLoadedFrom and related
 
        /**
@@ -2106,7 +2082,10 @@ class WikiPage extends Page {
        }
 
        /**
-        * Same as doDeleteArticleReal(), but returns more detailed success/failure status
+        * Same as doDeleteArticleReal(), but returns a simple boolean. This is kept around for
+        * backwards compatibility, if you care about error reporting you should use
+        * doDeleteArticleReal() instead.
+        *
         * Deletes the article with database consistency, writes logs, purges caches
         *
         * @param $reason string delete reason for deletion log
@@ -2124,8 +2103,8 @@ class WikiPage extends Page {
        public function doDeleteArticle(
                $reason, $suppress = false, $id = 0, $commit = true, &$error = '', User $user = null
        ) {
-               return $this->doDeleteArticleReal( $reason, $suppress, $id, $commit, $error, $user )
-                       == WikiPage::DELETE_SUCCESS;
+               $status = $this->doDeleteArticleReal( $reason, $suppress, $id, $commit, $error, $user );
+               return $status->isGood();
        }
 
        /**
@@ -2142,7 +2121,9 @@ class WikiPage extends Page {
         * @param $commit boolean defaults to true, triggers transaction end
         * @param &$error Array of errors to append to
         * @param $user User The deleting user
-        * @return int: One of WikiPage::DELETE_* constants
+        * @return Status: Status object; if successful, $status->value is the log_id of the
+        *                 deletion log entry. If the page couldn't be deleted because it wasn't
+        *                 found, $status is a non-fatal 'cannotdelete' error
         */
        public function doDeleteArticleReal(
                $reason, $suppress = false, $id = 0, $commit = true, &$error = '', User $user = null
@@ -2151,20 +2132,28 @@ class WikiPage extends Page {
 
                wfDebug( __METHOD__ . "\n" );
 
+               $status = Status::newGood();
+
                if ( $this->mTitle->getDBkey() === '' ) {
-                       return WikiPage::DELETE_NO_PAGE;
+                       $status->error( 'cannotdelete', wfEscapeWikiText( $this->getTitle()->getPrefixedText() ) );
+                       return $status;
                }
 
                $user = is_null( $user ) ? $wgUser : $user;
-               if ( ! wfRunHooks( 'ArticleDelete', array( &$this, &$user, &$reason, &$error ) ) ) {
-                       return WikiPage::DELETE_HOOK_ABORTED;
+               if ( ! wfRunHooks( 'ArticleDelete', array( &$this, &$user, &$reason, &$error, &$status ) ) ) {
+                       if ( $status->isOK() ) {
+                               // Hook aborted but didn't set a fatal status
+                               $status->fatal( 'delete-hook-aborted' );
+                       }
+                       return $status;
                }
 
                if ( $id == 0 ) {
                        $this->loadPageData( 'forupdate' );
                        $id = $this->getID();
                        if ( $id == 0 ) {
-                               return WikiPage::DELETE_NO_PAGE;
+                               $status->error( 'cannotdelete', wfEscapeWikiText( $this->getTitle()->getPrefixedText() ) );
+                               return $status;
                        }
                }
 
@@ -2222,7 +2211,8 @@ class WikiPage extends Page {
 
                if ( !$ok ) {
                        $dbw->rollback( __METHOD__ );
-                       return WikiPage::DELETE_NO_REVISIONS;
+                       $status->error( 'cannotdelete', wfEscapeWikiText( $this->getTitle()->getPrefixedText() ) );
+                       return $status;
                }
 
                $this->doDeleteUpdates( $id );
@@ -2242,7 +2232,8 @@ class WikiPage extends Page {
                }
 
                wfRunHooks( 'ArticleDeleteComplete', array( &$this, &$user, $reason, $id ) );
-               return WikiPage::DELETE_SUCCESS;
+               $status->value = $logid;
+               return $status;
        }
 
        /**

diff --git a/includes/api/ApiDelete.php b/includes/api/ApiDelete.php
index cefdaac..72ddc37 100644 (file)
--- a/includes/api/ApiDelete.php
+++ b/includes/api/ApiDelete.php
@@ -56,13 +56,14 @@ class ApiDelete extends ApiBase {
                $user = $this->getUser();
 
                if ( $titleObj->getNamespace() == NS_FILE ) {
-                       $retval = self::deleteFile( $pageObj, $user, $params['token'], $params['oldimage'], $reason, false );
+                       $status = self::deleteFile( $pageObj, $user, $params['token'], $params['oldimage'], $reason, false );
                } else {
-                       $retval = self::delete( $pageObj, $user, $params['token'], $reason );
+                       $status = self::delete( $pageObj, $user, $params['token'], $reason );
                }
 
-               if ( count( $retval ) ) {
-                       $this->dieUsageMsg( reset( $retval ) ); // We don't care about multiple errors, just report one of them
+               if ( !$status->isGood() ) {
+                       $errors = $this->getErrorsArray();
+                       $this->dieUsageMsg( $errors[0] ); // We don't care about multiple errors, just report one of them
                }
 
                // Deprecated parameters
@@ -75,7 +76,11 @@ class ApiDelete extends ApiBase {
                }
                $this->setWatch( $watch, $titleObj, 'watchdeletion' );
 
-               $r = array( 'title' => $titleObj->getPrefixedText(), 'reason' => $reason );
+               $r = array(
+                       'title' => $titleObj->getPrefixedText(),
+                       'reason' => $reason,
+                       'logid' => $status->value
+               );
                $this->getResult()->addValue( null, $this->getModuleName(), $r );
        }
 
@@ -97,7 +102,7 @@ class ApiDelete extends ApiBase {
         * @param $user User doing the action
         * @param $token String: delete token (same as edit token)
         * @param $reason String: reason for the deletion. Autogenerated if NULL
-        * @return Title::getUserPermissionsErrors()-like array
+        * @return Status
         */
        public static function delete( Page $page, User $user, $token, &$reason = null ) {
                $title = $page->getTitle();
@@ -119,11 +124,7 @@ class ApiDelete extends ApiBase {
 
                $error = '';
                // Luckily, Article.php provides a reusable delete function that does the hard work for us
-               if ( $page->doDeleteArticle( $reason, false, 0, true, $error ) ) {
-                       return array();
-               } else {
-                       return array( array( 'cannotdelete', $title->getPrefixedText() ) );
-               }
+               return $page->doDeleteArticleReal( $reason, false, 0, true, $error );
        }
 
        /**
@@ -133,7 +134,7 @@ class ApiDelete extends ApiBase {
         * @param $oldimage
         * @param $reason
         * @param $suppress bool
-        * @return array|Title
+        * @return Status
         */
        public static function deleteFile( Page $page, User $user, $token, $oldimage, &$reason = null, $suppress = false ) {
                $title = $page->getTitle();
@@ -162,12 +163,7 @@ class ApiDelete extends ApiBase {
                if ( is_null( $reason ) ) { // Log and RC don't like null reasons
                        $reason = '';
                }
-               $status = FileDeleteForm::doDelete( $title, $file, $oldimage, $reason, $suppress );
-               if ( !$status->isGood() ) {
-                       return array( array( 'cannotdelete', $title->getPrefixedText() ) );
-               }
-
-               return array();
+               return FileDeleteForm::doDelete( $title, $file, $oldimage, $reason, $suppress );
        }
 
        public function mustBePosted() {

diff --git a/includes/specials/SpecialMovepage.php b/includes/specials/SpecialMovepage.php
index 6b817d2..f7d13bb 100644 (file)
--- a/includes/specials/SpecialMovepage.php
+++ b/includes/specials/SpecialMovepage.php
@@ -429,8 +429,9 @@ class MovePageForm extends UnlistedSpecialPage {
 
                        $error = ''; // passed by ref
                        $page = WikiPage::factory( $nt );
-                       if ( !$page->doDeleteArticle( $reason, false, 0, true, $error, $user ) ) {
-                               $this->showForm( array( array( 'cannotdelete', wfEscapeWikiText( $nt->getPrefixedText() ) ) ) );
+                       $deleteStatus = $page->doDeleteArticleReal( $reason, false, 0, true, $error, $user );
+                       if ( !$deleteStatus->isGood() ) {
+                               $this->showForm( $deleteStatus->getErrorsArray() );
                                return;
                        }
                }

diff --git a/languages/messages/MessagesEn.php b/languages/messages/MessagesEn.php
index c13c9da..f1c14d0 100644 (file)
--- a/languages/messages/MessagesEn.php
+++ b/languages/messages/MessagesEn.php
@@ -1005,6 +1005,8 @@ Please report this to an [[Special:ListUsers/sysop|administrator]], making note
 'cannotdelete'         => 'The page or file "$1" could not be deleted.
 It may have already been deleted by someone else.',
 'cannotdelete-title'   => 'Cannot delete page "$1"',
+'delete-hook-aborted'  => 'Deletion aborted by hook.
+It gave no explanation.',
 'badtitle'             => 'Bad title',
 'badtitletext'         => 'The requested page title was invalid, empty, or an incorrectly linked inter-language or inter-wiki title.
 It may contain one or more characters which cannot be used in titles.',

diff --git a/languages/messages/MessagesQqq.php b/languages/messages/MessagesQqq.php
index eb85c53..651343b 100644 (file)
--- a/languages/messages/MessagesQqq.php
+++ b/languages/messages/MessagesQqq.php
@@ -659,6 +659,7 @@ HTML markup cannot be used.
 $1 is a filename, I think.',
 'cannotdelete-title' => 'Title of error page when the user cannot delete a page
 * $1 is the page name',
+'delete-hook-aborted' => 'Error message shown when an extension hook prevents a page deletion, but does not provide an error message.',
 'badtitle' => 'The page title when a user requested a page with invalid page name. The content will be {{msg-mw|badtitletext}}.',
 'badtitletext' => 'The message shown when a user requested a page with invalid page name. The page title will be {{msg-mw|badtitle}}.',
 'perfcached' => 'Like {{msg-mw|perfcachedts}} but used when we do not know how long ago page was cached (unlikely to happen). Parameters:

diff --git a/maintenance/language/messages.inc b/maintenance/language/messages.inc
index c55b83d..2ad558d 100644 (file)
--- a/maintenance/language/messages.inc
+++ b/maintenance/language/messages.inc
@@ -386,6 +386,7 @@ $wgMessageStructure = array(
                'badarticleerror',
                'cannotdelete',
                'cannotdelete-title',
+               'delete-hook-aborted',
                'badtitle',
                'badtitletext',
                'perfcached',