r52118 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r52117‎ | r52118 | r52119 >
Date:14:47, 18 June 2009
Author:skizzerz
Status:ok (Comments)
Tags:
Comment:
* Remove the two hooks introduced in r52082
* Remove the unused UserrightsChangeableGroups hook introduced in r39368 (1.14)
* Fix typo in Special:ListGroupRights introduced in r52083
* Prevent duplicate key display in Special:ListGroupRights (new behavior: if a permission is both assigned and revoked from a group, it only displays as revoked).
* Fix $wgRevokePermissions handling, it now runs after every group permission has been assigned in order to revoke the permission properly.
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/docs/hooks.txt (modified) (history)
  • /trunk/phase3/includes/User.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialListgrouprights.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialUserrights.php (modified) (history)

Diff [purge]

Index: trunk/phase3/docs/hooks.txt
@@ -1538,36 +1538,6 @@
15391539 $add : Array of strings corresponding to groups added
15401540 $remove: Array of strings corresponding to groups removed
15411541
1542 -'UserrightsChangeableGroups': allows modification of the groups a user
1543 -may add or remove via Special:UserRights
1544 -$userrights : UserrightsPage object
1545 -$user : User object of the current user
1546 -$addergroups : Array of groups that the user is in
1547 -&$groups : Array of groups that can be added or removed. In format of
1548 - array(
1549 - 'add' => array( addablegroups ),
1550 - 'remove' => array( removablegroups ),
1551 - 'add-self' => array( addablegroups to self ),
1552 - 'remove-self' => array( removable groups from self )
1553 - )
1554 -
1555 -'UserrightsGroupCheckboxes': allows modification of the display of
1556 -checkboxes in the Special:UserRights interface.
1557 -$usergroups : Array of groups that the target user belongs to
1558 -&$columns : Array of checkboxes, in the form of
1559 - $columns['column name']['group name'] = array(
1560 - 'set' => is this checkbox checked by default?
1561 - 'disabled' => is this checkbox disabled?
1562 - 'irreversible' => can this action not be reversed?
1563 - );
1564 -
1565 -'UserrightsSaveUserGroups': allow extensions to modify the added/removed groups
1566 -&$user : User object of the user being altered
1567 -$oldGroups : Array of groups that the user is currently in
1568 -&$add : Array of groups to add
1569 -&$remove : Array of groups to remove
1570 -$reason : Summary provided by user on the form
1571 -
15721542 'UserRetrieveNewTalks': called when retrieving "You have new messages!"
15731543 message(s)
15741544 $user: user retrieving new talks messages
Index: trunk/phase3/includes/User.php
@@ -3067,12 +3067,16 @@
30683068 static function getGroupPermissions( $groups ) {
30693069 global $wgGroupPermissions, $wgRevokePermissions;
30703070 $rights = array();
 3071+ // grant every granted permission first
30713072 foreach( $groups as $group ) {
30723073 if( isset( $wgGroupPermissions[$group] ) ) {
30733074 $rights = array_merge( $rights,
30743075 // array_filter removes empty items
30753076 array_keys( array_filter( $wgGroupPermissions[$group] ) ) );
30763077 }
 3078+ }
 3079+ // now revoke the revoked permissions
 3080+ foreach( $groups as $group ) {
30773081 if( isset( $wgRevokePermissions[$group] ) ) {
30783082 $rights = array_diff( $rights,
30793083 array_keys( array_filter( $wgRevokePermissions[$group] ) ) );
Index: trunk/phase3/includes/specials/SpecialUserrights.php
@@ -196,9 +196,6 @@
197197 $oldGroups = $user->getGroups();
198198 $newGroups = $oldGroups;
199199
200 - // Run a hook beforehand to allow extensions to modify the added/removed groups
201 - wfRunHooks( 'UserrightsSaveUserGroups', array( &$user, $oldGroups, &$add, &$remove, $reason ) );
202 -
203200 // remove then add groups
204201 if( $remove ) {
205202 $newGroups = array_diff($newGroups, $remove);
@@ -486,9 +483,6 @@
487484 }
488485 }
489486
490 - # Run a hook to allow extensions to modify the column listing
491 - wfRunHooks( 'UserrightsGroupCheckboxes', array( $usergroups, &$columns ) );
492 -
493487 # Build the HTML table
494488 $ret .= Xml::openElement( 'table', array( 'border' => '0', 'class' => 'mw-userrights-groups' ) ) .
495489 "<tr>\n";
@@ -548,11 +542,7 @@
549543 */
550544 function changeableGroups() {
551545 global $wgUser;
552 - $groups = $wgUser->changeableGroups();
553 - // Run a hook because we can
554 - wfRunHooks( 'UserrightsChangeableGroups', array( $this,
555 - $wgUser, $wgUser->getEffectiveGroups(), &$groups ) );
556 - return $groups;
 546+ return $wgUser->changeableGroups();
557547 }
558548
559549 /**
Index: trunk/phase3/includes/specials/SpecialListgrouprights.php
@@ -127,7 +127,8 @@
128128 global $wgLang;
129129 $r = array();
130130 foreach( $permissions as $permission => $granted ) {
131 - if( $granted ) {
 131+ //show as granted only if it isn't revoked to prevent duplicate display of permissions
 132+ if( $granted && ( !isset( $revoke[$permission] ) || !$revoke[$permission] ) ) {
132133 $description = wfMsgExt( 'listgrouprights-right-display', array( 'parseinline' ),
133134 User::getRightDescription( $permission ),
134135 $permission
@@ -137,7 +138,7 @@
138139 }
139140 foreach( $revoke as $permission => $revoked ) {
140141 if( $revoked ) {
141 - $description = wfMsgExt( 'lisgrouprights-right-revoked', array( 'parseinline' ),
 142+ $description = wfMsgExt( 'listgrouprights-right-revoked', array( 'parseinline' ),
142143 User::getRightDescription( $permission ),
143144 $permission
144145 );
Index: trunk/phase3/RELEASE-NOTES
@@ -40,6 +40,7 @@
4141 * Oracle: maintenance/ora/user.sql script for creating DB user on oracle with
4242 appropriate privileges. Creating this user with web-install page requires
4343 oci8.privileged_connect set to On in php.ini.
 44+* Removed UserrightsChangeableGroups hook introduced in 1.14
4445
4546 === New features in 1.16 ===
4647
@@ -83,10 +84,6 @@
8485 * DISPLAYTITLE now accepts a limited amount of wiki markup (the single-quote items)
8586 * Special:Search now could search terms in all variant-forms. ONLY apply on
8687 wikis with LanguageConverter
87 -* Add hook 'UserrightsGetCheckboxes' to give extensions the ability to modify
88 - the arrangement of checkboxes on the Special:UserRights form
89 -* Add hook 'UserrightsSaveUserGroups' to give extensions the ability to modify
90 - the groups being added and removed last-minute.
9188 * Add autopromote condition APCOND_BLOCKED to autopromote blocked users to various
9289 user groups.
9390 * Add $wgRevokePermissions as a means of restricting a group's rights. The syntax is

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r39368* $wgGroupsAddToSelf and $wgGroupsRemoveFromSelf now work more like...skizzerz21:55, 14 August 2008
r52082* (bug 17014) Blocked users can no longer use Special:UserRights if they do...skizzerz02:13, 18 June 2009
r52083* Add autopromote condition APCOND_BLOCKED to autopromote blocked users to va...skizzerz02:50, 18 June 2009

Comments

#Comment by Bryan (talk | contribs)   12:43, 20 June 2009

What is the rationale of breaking backwards compatibility by removing UserrightsChangeableGroups ?

#Comment by Skizzerz (talk | contribs)   14:24, 20 June 2009

See Tim's comment on r52082 for the reason why the hook isn't all that good. Plus, I ran a grep on the entire extensions directory and nothing was using this hook, so it doesn't really break backwards compatibility in that regard anyway.

Status & tagging log