Apologies in advance if this would be more appropriate for mediawiki-
l, but since I discuss a possible modification to trunk, I thought
this would be the correct list.
In working on my UserRightsList extension, I encountered changes in
includes/SpecialUserrights.php that have me wondering if I'm missing
something really basic about oo programming and how I should have made
the extension. I originally wrote the Special page extension by
extending class SpecialPage and writing methods that largely
duplicated things in SpecialUserrights. For 0.5, I extended class
UserRightsPage (which didn't exist prior to 1.12) to be able to reuse
UserRightsPage::saveUserGroups( $username, $removegroup, $addgroup,
$reason = ''). This works in 1.12...
but no sooner did I announce it than I got a message that it didin't
work in 1.13a.
It turns out that in 1.13a, that UserRightsPage::saveUserGroups() no
longer passes the $removegroup and $addgroup arrays, it takes them
directly from $wgRequest.
Since I'm looping over a list of users, I patched the extension on
0.51 to modify $wgRequest->data for each iteration over the array of
users sent on execution. This strikes me as ugly, as it changes a
global that could contain information expected by something further
downstream (although I think it should be OK for the Userrights
hook). But I'm stumped about how else I could do this. It seems that
it isn't any better to have class UserRightsList extend SpecialPage
instead of UserrightsPage if all it means is instead of $this-
>saveUserGroups(), I use UserrightsPage::saveUserGroups(). That has
the same problem with the method params and that is actually worse,
since I have to load up the code with a bunch of things like:
function fetchUser( $username ) {
return UserrightsPage::fetchUser( $username );
}
just to get it to work. Would it be reasonable to suggest something
like changing SpecialUserrights.php to:
function saveUserGroups( $username, $removegroup = '', $addgroup =
'', $reason = '') {
...
if (is_array($removegroup) && is_array($addgroup) ){
foreach ($allgroups as $group) {
...
}
}
...
This would add a little overhead for the is_array checks, but would
ensure backward compatibility with anything else that calls the method
(perhaps that would only be me), and allow classes to extend class
UserrightsPage without modifying $wgRequest.
The larger questions:
- Should I be avoiding this approach (extending classes in includes)
altogether?
- If not, is there a way to tell whether the parameters/behavior are
likely to be stable?
- If yes, Is there a way I should be doing this to make it less likely
to break in future versions of MW?
- Was Userrights just an outlier because it needed refactoring and now
it should be stable?
Thanks,
JH
=====================================
Jim Hu
Associate Professor
Dept. of Biochemistry and Biophysics
2128 TAMU
Texas A&M Univ.
College Station, TX 77843-2128
979-862-4054