Make change tagging of edits in RecentChange::notifyNew/Edit

Change tags to apply to an edit can now be passed directly to the
WikiPage::doEditContent function. They are then passed to the
RecentChange::notifyNew/Edit functions where tagging is made
after the recent change is saved. This ensures that other callers
of doEditContent will not run into the same issue as T100248.
ApiRollback is fixed in this way.
In addition, we'll have to pass tags in this way for core tagging
of edits (I2e48bd458fc8d7c289f04dc276f9287516e0b987), and this makes
it possible to merge the arrays of tags and call ChangeTags::addTags
only once.

Change-Id: I829960c7a33b70464065839d7504d7529dfd0b72

diff --git a/includes/EditPage.php b/includes/EditPage.php
index 914bad4..caeb57c 100644 (file)
--- a/includes/EditPage.php
+++ b/includes/EditPage.php
@@ -366,7 +366,7 @@ class EditPage {
        public $contentFormat = null;
 
        /** @var null|array */
-       public $changeTags = null;
+       private $changeTags = null;
 
        # Placeholders for text injection by hooks (must be HTML)
        # extensions should take care to _append_ to the present value
@@ -2013,7 +2013,8 @@ class EditPage {
                        $flags,
                        false,
                        $wgUser,
-                       $content->getDefaultFormat()
+                       $content->getDefaultFormat(),
+                       $this->changeTags
                );
 
                if ( !$doEditStatus->isOK() ) {
@@ -2040,17 +2041,6 @@ class EditPage {
 
                $this->updateWatchlist();
 
-               if ( $this->changeTags && isset( $doEditStatus->value['revision'] ) ) {
-                       // If a revision was created, apply any change tags that were requested
-                       $addTags = $this->changeTags;
-                       $revId = $doEditStatus->value['revision']->getId();
-                       // Defer this both for performance and so that addTags() sees the rc_id
-                       // since the recentchange entry addition is deferred first (bug T100248)
-                       DeferredUpdates::addCallableUpdate( function() use ( $addTags, $revId ) {
-                               ChangeTags::addTags( $addTags, null, $revId );
-                       } );
-               }
-
                // If the content model changed, add a log entry
                if ( $changingContentModel ) {
                        $this->addContentModelChangeLogEntry(

diff --git a/includes/api/ApiRollback.php b/includes/api/ApiRollback.php
index 055c7fe..ab2c4ef 100644 (file)
--- a/includes/api/ApiRollback.php
+++ b/includes/api/ApiRollback.php
@@ -75,7 +75,8 @@ class ApiRollback extends ApiBase {
                        $token,
                        $params['markbot'],
                        $details,
-                       $user
+                       $user,
+                       $params['tags']
                );
 
                if ( $retval ) {
@@ -91,10 +92,6 @@ class ApiRollback extends ApiBase {
                // Watch pages
                $this->setWatch( $watch, $titleObj, 'watchrollback' );
 
-               if ( count( $params['tags'] ) ) {
-                       ChangeTags::addTags( $params['tags'], null, intval( $details['newid'] ), null, null );
-               }
-
                $info = array(
                        'title' => $titleObj->getPrefixedText(),
                        'pageid' => intval( $details['current']->getPage() ),

diff --git a/includes/changes/RecentChange.php b/includes/changes/RecentChange.php
index 9ae30c0..c33a94b 100644 (file)
--- a/includes/changes/RecentChange.php
+++ b/includes/changes/RecentChange.php
@@ -538,11 +538,13 @@ class RecentChange {
         * @param int $newSize
         * @param int $newId
         * @param int $patrol
+        * @param array $tags
         * @return RecentChange
         */
        public static function notifyEdit(
                $timestamp, &$title, $minor, &$user, $comment, $oldId, $lastTimestamp,
-               $bot, $ip = '', $oldSize = 0, $newSize = 0, $newId = 0, $patrol = 0
+               $bot, $ip = '', $oldSize = 0, $newSize = 0, $newId = 0, $patrol = 0,
+               $tags = array()
        ) {
                $rc = new RecentChange;
                $rc->mTitle = $title;
@@ -581,11 +583,15 @@ class RecentChange {
                        'pageStatus' => 'changed'
                );
 
-               DeferredUpdates::addCallableUpdate( function() use ( $rc ) {
+               DeferredUpdates::addCallableUpdate( function() use ( $rc, $tags ) {
                        $rc->save();
                        if ( $rc->mAttribs['rc_patrolled'] ) {
                                PatrolLog::record( $rc, true, $rc->getPerformer() );
                        }
+                       if ( count( $tags ) ) {
+                               ChangeTags::addTags( $tags, $rc->mAttribs['rc_id'],
+                                       $rc->mAttribs['rc_this_oldid'], null, null );
+                       }
                } );
 
                return $rc;
@@ -605,11 +611,12 @@ class RecentChange {
         * @param int $size
         * @param int $newId
         * @param int $patrol
+        * @param array $tags
         * @return RecentChange
         */
        public static function notifyNew(
                $timestamp, &$title, $minor, &$user, $comment, $bot,
-               $ip = '', $size = 0, $newId = 0, $patrol = 0
+               $ip = '', $size = 0, $newId = 0, $patrol = 0, $tags = array()
        ) {
                $rc = new RecentChange;
                $rc->mTitle = $title;
@@ -648,11 +655,15 @@ class RecentChange {
                        'pageStatus' => 'created'
                );
 
-               DeferredUpdates::addCallableUpdate( function() use ( $rc ) {
+               DeferredUpdates::addCallableUpdate( function() use ( $rc, $tags ) {
                        $rc->save();
                        if ( $rc->mAttribs['rc_patrolled'] ) {
                                PatrolLog::record( $rc, true, $rc->getPerformer() );
                        }
+                       if ( count( $tags ) ) {
+                               ChangeTags::addTags( $tags, $rc->mAttribs['rc_id'],
+                                       $rc->mAttribs['rc_this_oldid'], null, null );
+                       }
                } );
 
                return $rc;

diff --git a/includes/page/WikiPage.php b/includes/page/WikiPage.php
index 598d956..4e491e8 100644 (file)
--- a/includes/page/WikiPage.php
+++ b/includes/page/WikiPage.php
@@ -1636,6 +1636,9 @@ class WikiPage implements Page, IDBAccessObject {
         * @param User $user The user doing the edit
         * @param string $serialFormat Format for storing the content in the
         *   database.
+        * @param array|null $tags Change tags to apply to this edit
+        * Callers are responsible for permission checks
+        * (with ChangeTags::canAddTagsAccompanyingChange)
         *
         * @throws MWException
         * @return Status Possible errors:
@@ -1657,7 +1660,7 @@ class WikiPage implements Page, IDBAccessObject {
         */
        public function doEditContent(
                Content $content, $summary, $flags = 0, $baseRevId = false,
-               User $user = null, $serialFormat = null
+               User $user = null, $serialFormat = null, $tags = null
        ) {
                global $wgUser, $wgUseAutomaticEditSummaries;
 
@@ -1719,7 +1722,8 @@ class WikiPage implements Page, IDBAccessObject {
                        'oldContent' => $old_content,
                        'oldId' => $this->getLatest(),
                        'oldIsRedirect' => $this->isRedirect(),
-                       'oldCountable' => $this->isCountable()
+                       'oldCountable' => $this->isCountable(),
+                       'tags' => ( $tags !== null ) ? (array)$tags : array()
                );
 
                // Actually create the revision and create/update the page
@@ -1850,7 +1854,8 @@ class WikiPage implements Page, IDBAccessObject {
                                        $oldContent ? $oldContent->getSize() : 0,
                                        $newsize,
                                        $revisionId,
-                                       $patrolled
+                                       $patrolled,
+                                       $meta['tags']
                                );
                        }
 
@@ -1989,7 +1994,8 @@ class WikiPage implements Page, IDBAccessObject {
                                '',
                                $newsize,
                                $revisionId,
-                               $patrolled
+                               $patrolled,
+                               $meta['tags']
                        );
                }
 
@@ -3059,13 +3065,17 @@ class WikiPage implements Page, IDBAccessObject {
         *    success        : 'summary' (str), 'current' (rev), 'target' (rev)
         *
         * @param User $user The user performing the rollback
+        * @param array|null $tags Change tags to apply to the rollback
+        * Callers are responsible for permission checks
+        * (with ChangeTags::canAddTagsAccompanyingChange)
+        *
         * @return array Array of errors, each error formatted as
         *   array(messagekey, param1, param2, ...).
         * On success, the array is empty.  This array can also be passed to
         * OutputPage::showPermissionsErrorPage().
         */
        public function doRollback(
-               $fromP, $summary, $token, $bot, &$resultDetails, User $user
+               $fromP, $summary, $token, $bot, &$resultDetails, User $user, $tags = null
        ) {
                $resultDetails = null;
 
@@ -3087,7 +3097,7 @@ class WikiPage implements Page, IDBAccessObject {
                        return $errors;
                }
 
-               return $this->commitRollback( $fromP, $summary, $bot, $resultDetails, $user );
+               return $this->commitRollback( $fromP, $summary, $bot, $resultDetails, $user, $tags );
        }
 
        /**
@@ -3104,9 +3114,15 @@ class WikiPage implements Page, IDBAccessObject {
         *
         * @param array $resultDetails Contains result-specific array of additional values
         * @param User $guser The user performing the rollback
+        * @param array|null $tags Change tags to apply to the rollback
+        * Callers are responsible for permission checks
+        * (with ChangeTags::canAddTagsAccompanyingChange)
+        *
         * @return array
         */
-       public function commitRollback( $fromP, $summary, $bot, &$resultDetails, User $guser ) {
+       public function commitRollback( $fromP, $summary, $bot,
+               &$resultDetails, User $guser, $tags = null
+       ) {
                global $wgUseRCPatrol, $wgContLang;
 
                $dbw = wfGetDB( DB_MASTER );
@@ -3201,7 +3217,9 @@ class WikiPage implements Page, IDBAccessObject {
                        $summary,
                        $flags,
                        $target->getId(),
-                       $guser
+                       $guser,
+                       null,
+                       $tags
                );
 
                // Set patrolling and bot flag on the edits, which gets rollbacked.