From cc9f81ed945cc5ec57e27a6e5b42e80a12aa4ce8 Mon Sep 17 00:00:00 2001 From: Tim Starling Date: Mon, 9 Aug 2010 08:33:42 +0000 Subject: [PATCH] * Revert r66878, completely misses the point of factoring out doEdit() in the first place, which was to separate the UI layer from the backend layer, to support callers with alternate UIs or no UIs. * 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 | 2 +- includes/Article.php | 97 +++++++++++++++++------------------- includes/EditPage.php | 27 +++------- includes/Title.php | 4 ++ includes/api/ApiEditPage.php | 11 ++-- maintenance/addwiki.php | 3 +- maintenance/parserTests.inc | 3 +- 7 files changed, 65 insertions(+), 82 deletions(-) diff --git a/docs/hooks.txt b/docs/hooks.txt index 7f9eddf1f1..9dafb95a9f 100644 --- a/docs/hooks.txt +++ b/docs/hooks.txt @@ -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 diff --git a/includes/Article.php b/includes/Article.php index dc2d901ed1..d32dcc0ed8 100644 --- a/includes/Article.php +++ b/includes/Article.php @@ -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; } diff --git a/includes/EditPage.php b/includes/EditPage.php index 77627cbc5b..224b151b99 100644 --- a/includes/EditPage.php +++ b/includes/EditPage.php @@ -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; diff --git a/includes/Title.php b/includes/Title.php index b47e8cc2fb..ff0721f37b 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -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 ) { diff --git a/includes/api/ApiEditPage.php b/includes/api/ApiEditPage.php index c2812e440a..6e55b0a078 100644 --- a/includes/api/ApiEditPage.php +++ b/includes/api/ApiEditPage.php @@ -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 ) ); } diff --git a/maintenance/addwiki.php b/maintenance/addwiki.php index 63e5c0c49a..80a2b1481f 100644 --- a/maintenance/addwiki.php +++ b/maintenance/addwiki.php @@ -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" ); diff --git a/maintenance/parserTests.inc b/maintenance/parserTests.inc index 5803ba743e..6a5f657e96 100644 --- a/maintenance/parserTests.inc +++ b/maintenance/parserTests.inc @@ -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(); } -- 2.20.1