Page MenuHomePhabricator

Stop autopatrolling changes when rollback fails
Open, Needs TriagePublic

Description

RollbackPage has one more interesting "secret" feature: the method that possibly marks reverted edits as patrolled and/or "done by a bot" can run even if no new revision has been saved (reference: T64157). This typically happens when you rollback a vandal who undid their own changes.

This task concerns the following piece of code:

includes/page/RollbackPage.php
// This is done even on edit failure to have patrolling in that case (T64157).
$this->updateRecentChange( $dbw, $currentRevision, $targetRevision );

if ( !$updater->wasSuccessful() ) {
	return $updater->getStatus();
}

updateRecentChange sets rc_patrolled of all reverted changes to 2. Thanks to this, the reverted changes disappear from the patrolling backlog.
Notice, though, that the method is called unconditionally even if the rollback failed (no revision was saved to the database). I have observed at least two (rather annoying) consequences of this:

  1. If any of the reverted changes has a deferred entry in EditResultCache, no RevertedTagUpdateJob gets ever scheduled (and the RevertedTagUpdate is lost). In other words, if you attempt to rollback a user who undid their own changes, and therefore their last contribution is tagged as mw-manual-revert, the other changes are not eventually tagged as mw-reverted, which would otherwise happen if you manually patrolled the change.
  2. On Wikidata, the rollback may fail if the change would bring the database to an inconsistent state. For example, if a user removes some data from an item including a sitelink (connection to a wikipage) which is then added to another item, the rollback would violate the 1:1 item-to-wikipage rule, and therefore it fails. However, the changes unconditionally become (auto)patrolled even though they still might require some treatment. In case the reverting user is not aware of this functionality, the changes fall out of the backlog and are less likely to be noticed.

We could probably fix 1) by calling RevertedTagUpdateManager::approveRevertedTagForRevision for every reverted revision in case the rollback fails. But not 2).

Therefore, I propose this "feature" is removed for unsuccessful rollbacks.

See also: