* Revert r66878, completely misses the point of factoring out doEdit() in the first...
authorTim Starling <tstarling@users.mediawiki.org>
Mon, 9 Aug 2010 08:33:42 +0000 (08:33 +0000)
committerTim Starling <tstarling@users.mediawiki.org>
Mon, 9 Aug 2010 08:33:42 +0000 (08:33 +0000)
* Reverted followup 66880.
* Reverted dependent changes r67752, r68606, r68608, r68609. The point of deprecating insertArticle()/updateArticle() was to allow the UI code to be moved to EditPage. If you move that exact EditPage-specific functionality back into Article::doEdit(), and call it from all sorts of non-EditPage places, then we'll hit the same sorts of bugs we had before r14834.

docs/hooks.txt
includes/Article.php
includes/EditPage.php
includes/Title.php
includes/api/ApiEditPage.php
maintenance/addwiki.php
maintenance/parserTests.inc

index 7f9eddf..9dafb95 100644 (file)
@@ -660,7 +660,7 @@ pages
 $editPage: EditPage    object
 
 'EditPage::attemptSave': called before an article is
-saved, that is before Article::doEdit() is called
+saved, that is before insertNewArticle() is called
 $editpage_Obj: the current EditPage object
 
 'EditPage::importFormData': allow extensions to read additional data
index dc2d901..d32dcc0 100644 (file)
@@ -1945,33 +1945,71 @@ class Article {
        }
 
        /**
-        * @deprecated use Article::doEdit()
+        * This function is not deprecated until somebody fixes the core not to use
+        * it. Nevertheless, use Article::doEdit() instead.
         */
        function insertNewArticle( $text, $summary, $isminor, $watchthis, $suppressRC = false, $comment = false, $bot = false ) {
-               wfDeprecated( __METHOD__ );
                $flags = EDIT_NEW | EDIT_DEFER_UPDATES | EDIT_AUTOSUMMARY |
                        ( $isminor ? EDIT_MINOR : 0 ) |
                        ( $suppressRC ? EDIT_SUPPRESS_RC : 0 ) |
                        ( $bot ? EDIT_FORCE_BOT : 0 );
 
-               $this->doEdit( $text, $summary, $flags, false, null, $watchthis, $comment, '', true );
+               # If this is a comment, add the summary as headline
+               if ( $comment && $summary != "" ) {
+                       $text = wfMsgForContent( 'newsectionheaderdefaultlevel', $summary ) . "\n\n" . $text;
+               }
+               $this->doEdit( $text, $summary, $flags );
+
+               $dbw = wfGetDB( DB_MASTER );
+               if ( $watchthis ) {
+                       if ( !$this->mTitle->userIsWatching() ) {
+                               $dbw->begin();
+                               $this->doWatch();
+                               $dbw->commit();
+                       }
+               } else {
+                       if ( $this->mTitle->userIsWatching() ) {
+                               $dbw->begin();
+                               $this->doUnwatch();
+                               $dbw->commit();
+                       }
+               }
+               $this->doRedirect( $this->isRedirect( $text ) );
        }
 
        /**
         * @deprecated use Article::doEdit()
         */
        function updateArticle( $text, $summary, $minor, $watchthis, $forceBot = false, $sectionanchor = '' ) {
-               wfDeprecated( __METHOD__ );
                $flags = EDIT_UPDATE | EDIT_DEFER_UPDATES | EDIT_AUTOSUMMARY |
                        ( $minor ? EDIT_MINOR : 0 ) |
                        ( $forceBot ? EDIT_FORCE_BOT : 0 );
 
-               $status = $this->doEdit( $text, $summary, $flags, false, null, $watchthis, false, $sectionanchor, true );
+               $status = $this->doEdit( $text, $summary, $flags );
 
                if ( !$status->isOK() ) {
                        return false;
                }
 
+               $dbw = wfGetDB( DB_MASTER );
+               if ( $watchthis ) {
+                       if ( !$this->mTitle->userIsWatching() ) {
+                               $dbw->begin();
+                               $this->doWatch();
+                               $dbw->commit();
+                       }
+               } else {
+                       if ( $this->mTitle->userIsWatching() ) {
+                               $dbw->begin();
+                               $this->doUnwatch();
+                               $dbw->commit();
+                       }
+               }
+
+               $extraQuery = ''; // Give extensions a chance to modify URL query on update
+               wfRunHooks( 'ArticleUpdateBeforeRedirect', array( $this, &$sectionanchor, &$extraQuery ) );
+
+               $this->doRedirect( $this->isRedirect( $text ), $sectionanchor, $extraQuery );
                return true;
        }
 
@@ -2026,11 +2064,6 @@ class Article {
         *
         * @param $baseRevId the revision ID this edit was based off, if any
         * @param $user Optional user object, $wgUser will be used if not passed
-        * @param $watchthis Watch the page if true, unwatch the page if false, do nothing if null
-        * @param $comment Boolean: whether the edit is a new section
-        * @param $sectionanchor The section anchor for the page; used for redirecting the user back to the page
-        *              after the edit is successfully committed
-        * @param $redirect If true, redirect the user back to the page after the edit is successfully committed
         *
         * @return Status object. Possible errors:
         *     edit-hook-aborted:       The ArticleSave hook aborted the edit but didn't set the fatal flag of $status
@@ -2047,8 +2080,7 @@ class Article {
         *
         *  Compatibility note: this function previously returned a boolean value indicating success/failure
         */
-       public function doEdit( $text, $summary, $flags = 0, $baseRevId = false, $user = null , $watchthis = null,
-                       $comment = false, $sectionanchor = '', $redirect = false) {
+       public function doEdit( $text, $summary, $flags = 0, $baseRevId = false, $user = null ) {
                global $wgUser, $wgDBtransactions, $wgUseAutomaticEditSummaries;
 
                # Low-level sanity check
@@ -2066,13 +2098,8 @@ class Article {
 
                $flags = $this->checkFlags( $flags );
 
-               # If this is a comment, add the summary as headline
-               if ( $comment && $summary != "" ) {
-                       $text = wfMsgForContent( 'newsectionheaderdefaultlevel', $summary ) . "\n\n" . $text;
-               }
-
                if ( !wfRunHooks( 'ArticleSave', array( &$this, &$user, &$text, &$summary,
-                       $flags & EDIT_MINOR, &$watchthis, null, &$flags, &$status) ) )
+                       $flags & EDIT_MINOR, null, null, &$flags, &$status ) ) )
                {
                        wfDebug( __METHOD__ . ": ArticleSave hook aborted save!\n" );
                        wfProfileOut( __METHOD__ );
@@ -2277,7 +2304,7 @@ class Article {
                        Article::onArticleCreate( $this->mTitle );
 
                        wfRunHooks( 'ArticleInsertComplete', array( &$this, &$user, $text, $summary,
-                               $flags & EDIT_MINOR, &$watchthis, null, &$flags, $revision ) );
+                               $flags & EDIT_MINOR, null, null, &$flags, $revision ) );
                }
 
                # Do updates right now unless deferral was requested
@@ -2289,39 +2316,9 @@ class Article {
                $status->value['revision'] = $revision;
 
                wfRunHooks( 'ArticleSaveComplete', array( &$this, &$user, $text, $summary,
-                       $flags & EDIT_MINOR, &$watchthis, null, &$flags, $revision, &$status, $baseRevId,
-                       &$redirect) );
-
-               # Watch or unwatch the page
-               if ( $watchthis === true ) {
-                       if ( !$this->mTitle->userIsWatching() ) {
-                               $dbw->begin();
-                               $this->doWatch();
-                               $dbw->commit();
-                       }
-               } elseif ( $watchthis === false ) {
-                       if ( $this->mTitle->userIsWatching() ) {
-                               $dbw->begin();
-                               $this->doUnwatch();
-                               $dbw->commit();
-                       }
-               }
-
-               # Give extensions a chance to modify URL query on update
-               $extraQuery = '';
-
-               wfRunHooks( 'ArticleUpdateBeforeRedirect', array( $this, &$sectionanchor, &$extraQuery ) );
-
-               if ( $redirect ) {
-                       if ( $sectionanchor || $extraQuery ) {
-                               $this->doRedirect( $this->isRedirect( $text ), $sectionanchor, $extraQuery );
-                       } else {
-                               $this->doRedirect( $this->isRedirect( $text ) );
-                       }
-               }
+                       $flags & EDIT_MINOR, null, null, &$flags, $revision, &$status, $baseRevId ) );
 
                wfProfileOut( __METHOD__ );
-
                return $status;
        }
 
index 77627cb..224b151 100644 (file)
@@ -904,20 +904,11 @@ class EditPage {
 
                        $isComment = ( $this->section == 'new' );
 
-                       $flags = EDIT_NEW | EDIT_DEFER_UPDATES | EDIT_AUTOSUMMARY |
-                               ( $this->minoredit ? EDIT_MINOR : 0 ) |
-                               ( $bot ? EDIT_FORCE_BOT : 0 );
-                       $status = $this->mArticle->doEdit( $this->textbox1, $this->summary, $flags,
-                               false, null, $this->watchthis, $isComment, '', true );
+                       $this->mArticle->insertNewArticle( $this->textbox1, $this->summary,
+                               $this->minoredit, $this->watchthis, false, $isComment, $bot );
 
-                       if ( $status->isOK() ) {
-                               wfProfileOut( __METHOD__ );
-                               return self::AS_SUCCESS_NEW_ARTICLE;
-                       } else {
-                               $result = $status->getErrorsArray();
-                       }
                        wfProfileOut( __METHOD__ );
-                       return self::AS_END;
+                       return self::AS_SUCCESS_NEW_ARTICLE;
                }
 
                # Article exists. Check for edit conflict.
@@ -1059,20 +1050,14 @@ class EditPage {
                        return self::AS_MAX_ARTICLE_SIZE_EXCEEDED;
                }
 
-               // Update the article here
-               $flags = EDIT_UPDATE | EDIT_DEFER_UPDATES | EDIT_AUTOSUMMARY |
-                       ( $this->minoredit ? EDIT_MINOR : 0 ) |
-                       ( $bot ? EDIT_FORCE_BOT : 0 );
-               $status = $this->mArticle->doEdit( $text, $this->summary, $flags,
-                       false, null, $this->watchthis, false, $sectionanchor, true );
-
-               if ( $status->isOK() )
+               # update the article here
+               if ( $this->mArticle->updateArticle( $text, $this->summary, $this->minoredit,
+                       $this->watchthis, $bot, $sectionanchor ) )
                {
                        wfProfileOut( __METHOD__ );
                        return self::AS_SUCCESS_UPDATE;
                } else {
                        $this->isConflict = true;
-                       $result = $status->getErrorsArray();
                }
                wfProfileOut( __METHOD__ );
                return self::AS_END;
index b47e8cc..ff0721f 100644 (file)
@@ -2413,6 +2413,10 @@ class Title {
         * This clears some fields in this object, and clears any associated
         * keys in the "bad links" section of the link cache.
         *
+        * - This is called from Article::insertNewArticle() to allow
+        * loading of the new page_id. It's also called from
+        * Article::doDeleteArticle()
+        *
         * @param $newid \type{\int} the new Article ID
         */
        public function resetArticleID( $newid ) {
index c2812e4..6e55b0a 100644 (file)
@@ -333,13 +333,12 @@ class ApiEditPage extends ApiBase {
 
                        case EditPage::AS_END:
                                // This usually means some kind of race condition
-                               // or DB weirdness occurred.
-                               if ( is_array( $result ) && count( $result ) > 0 ) {
-                                       $this->dieUsageMsg( array( 'unknownerror', $result[0][0] ) );
-                               }
+                               // or DB weirdness occurred. Fall through to throw an unknown
+                               // error.
 
-                               // Unknown error, but no specific error message
-                               // Fall through
+                               // This needs fixing higher up, as Article::doEdit should be
+                               // used rather than Article::updateArticle, so that specific
+                               // error conditions can be returned
                        default:
                                $this->dieUsageMsg( array( 'unknownerror', $retval ) );
                }
index 63e5c0c..80a2b14 100644 (file)
@@ -132,8 +132,7 @@ class AddWiki extends Maintenance {
                $wgArticle = new Article( $wgTitle );
                $ucsite = ucfirst( $site );
 
-               $wgArticle->doEdit( $this->getFirstArticle( $ucsite, $name ), '', EDIT_NEW | EDIT_DEFER_UPDATES | EDIT_AUTOSUMMARY,
-                       false, null, false, false, '', true );
+               $wgArticle->insertNewArticle( $this->getFirstArticle( $ucsite, $name ), '', false, false );
 
                $this->output( "Adding to dblists\n" );
 
index 5803ba7..6a5f657 100644 (file)
@@ -1050,8 +1050,7 @@ class ParserTest {
                }
 
                $art = new Article( $title );
-               $art->doEdit( $text, '', EDIT_NEW | EDIT_DEFER_UPDATES | EDIT_AUTOSUMMARY,
-                       false, null, false, false, '', true );
+               $art->insertNewArticle( $text, '', false, false );
 
                $this->teardownGlobals();
        }