Last modified: 2008-01-15 19:28:09 UTC

Wikimedia Bugzilla is closed!

Wikimedia has migrated from Bugzilla to Phabricator. Bug reports should be created and updated in Wikimedia Phabricator instead. Please create an account in Phabricator and add your Bugzilla email address to it.
Wikimedia Bugzilla is read-only. If you try to edit or create any bug report in Bugzilla you will be shown an intentional error message.
In order to access the Phabricator task corresponding to a Bugzilla report, just remove "static-" from its URL.
You could still run searches in Bugzilla or access your list of votes but bug reports will obviously not be up-to-date in Bugzilla.
Bug 12615 - Clean up Article::doRollback
Clean up Article::doRollback
Product: MediaWiki
Classification: Unclassified
Page editing (Other open bugs)
All All
: Normal enhancement (vote)
: ---
Assigned To: Roan Kattouw
Depends on:
Blocks: code_quality
  Show dependency treegraph
Reported: 2008-01-13 22:22 UTC by Aryeh Gregor (not reading bugmail, please e-mail directly)
Modified: 2008-01-15 19:28 UTC (History)
1 user (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---

Nonfunctional beginnings of a patch (2.88 KB, patch)
2008-01-13 22:22 UTC, Aryeh Gregor (not reading bugmail, please e-mail directly)

Description Aryeh Gregor (not reading bugmail, please e-mail directly) 2008-01-13 22:22:11 UTC
Created attachment 4542 [details]
Nonfunctional beginnings of a patch

Article::doRollback is very messy right now, with several problems:

1) It provides no way for scripts to override permissions checks.

2) It uses nonstandard permissions checks and inadequate error return codes, which a) duplicate code in Title::getPermissionsErrors (probably, in some respect, incorrectly), and b) force Article::rollback to check permissions anyway using Title::getPermissionsErrors so it can display a proper error.  Combined, this code duplication probably results in more than one scenario in which you can rollback through the API but not the standard UI, and vice versa.

Proposed fix:

* Break into two functions, doRollback() and commitRollback() (or whatever -- name suggested by BrokenArrow).  doRollback() should perform all permissions checks and then call commitRollback() to do the actual work.  commitRollback() should only ensure that the database is not read-only.

* Remove the pass-by-reference $resultDetails (this isn't C(++) here), and change the return value to an array of errors, like some of the editing logic already does.

The very beginnings of a (not yet functional) patch are attached.
Comment 1 Roan Kattouw 2008-01-13 22:23:42 UTC

I'll probably get around to this by Friday.
Comment 2 Roan Kattouw 2008-01-15 19:28:09 UTC
Done in r28903. The API still has to be updated, and other functions have to be refactored similarly. I'm on that.

Note You need to log in before you can comment on or make changes to this bug.