# r90749 MediaWiki - Code Review archive

 Repository: MediaWiki Revision: < r90748‎ | r90749 | r90750 > Date: 02:52, 25 June 2011 Author: aaron Status: resolved (Comments) Tags: Comment: Added one-time promote support via Autopromote::autopromoteOnceHook function. This is still a bit rough on the edges. This uses a hook since extension may want to control when it's called for performance reasons. Patch by lampak. (for bug 24948) Modified paths: /trunk/phase3/includes/Autopromote.php (modified) (history) /trunk/phase3/includes/DefaultSettings.php (modified) (history) /trunk/phase3/includes/User.php (modified) (history) /trunk/phase3/includes/installer/MysqlUpdater.php (modified) (history) /trunk/phase3/includes/installer/SqliteUpdater.php (modified) (history) /trunk/phase3/maintenance/archives/patch-user_former_groups.sql (added) (history) /trunk/phase3/maintenance/postgres/archives/patch-user_former_groups.sql (added) (history) /trunk/phase3/maintenance/postgres/tables.sql (modified) (history) /trunk/phase3/maintenance/tables.sql (modified) (history)

## Diff [purge]

 Index: trunk/phase3/maintenance/archives/patch-user_former_groups.sql — — @@ -0,0 +1,10 @@ 2 +-- Stores the groups the user has once belonged to. 3 +-- The user may still belong these groups. Check user_groups. 4 +CREATE TABLE /*_*/user_former_groups ( 5 + -- Key to user_id 6 + ufg_user int unsigned NOT NULL default 0, 7 + ufg_group varbinary(16) NOT NULL default '', 8 + 9 + PRIMARY KEY (ufg_user,ufg_group), 10 + KEY (ufg_group) 11 +) /*$wgDBTableOptions*/; Property changes on: trunk/phase3/maintenance/archives/patch-user_former_groups.sql ___________________________________________________________________ Added: svn:eol-style 1 12 + native Index: trunk/phase3/maintenance/postgres/archives/patch-user_former_groups.sql — — @@ -0,0 +1,5 @@ 2 +CREATE TABLE user_former_groups ( 3 + ufg_user INTEGER NULL REFERENCES mwuser(user_id) ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED, 4 + ufg_group TEXT NOT NULL 5 +); 6 +CREATE UNIQUE INDEX user_former_groups_unique ON user_former_groups (ufg_user, ufg_group); Property changes on: trunk/phase3/maintenance/postgres/archives/patch-user_former_groups.sql ___________________________________________________________________ Added: svn:eol-style 1 7 + native Index: trunk/phase3/maintenance/postgres/tables.sql — — @@ -54,6 +54,12 @@ 55 55 ); 56 56 CREATE UNIQUE INDEX user_groups_unique ON user_groups (ug_user, ug_group); 57 57 58 +CREATE TABLE user_former_groups ( 59 + ufg_user INTEGER NULL REFERENCES mwuser(user_id) ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED, 60 + ufg_group TEXT NOT NULL 61 +); 62 +CREATE UNIQUE INDEX user_former_groups_unique ON user_former_groups (ufg_user, ufg_group); 63 + 58 64 CREATE TABLE user_newtalk ( 59 65 user_id INTEGER NOT NULL REFERENCES mwuser(user_id) ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED, 60 66 user_ip TEXT NULL, Index: trunk/phase3/maintenance/tables.sql — — @@ -164,7 +164,17 @@ 165 165 CREATE UNIQUE INDEX /*i*/ug_user_group ON /*_*/user_groups (ug_user,ug_group); 166 166 CREATE INDEX /*i*/ug_group ON /*_*/user_groups (ug_group); 167 167 168 +-- Stores the groups the user has once belonged to. 169 +-- The user may still belong these groups. Check user_groups. 170 +CREATE TABLE /*_*/user_former_groups ( 171 + -- Key to user_id 172 + ufg_user int unsigned NOT NULL default 0, 173 + ufg_group varbinary(16) NOT NULL default '' 174 +) /*$wgDBTableOptions*/; 168 175 176 +CREATE UNIQUE INDEX /*i*/ufg_user_group ON /*_*/user_former_groups (ufg_user,ufg_group); 177 +CREATE INDEX /*i*/ufg_group ON /*_*/user_former_groups (ufg_group); 178 + 169 179 -- 170 180 -- Stores notifications of user talk page changes, for the display 171 181 -- of the "you have new messages" box Index: trunk/phase3/includes/User.php — — @@ -71,7 +71,7 @@ 72 72 'mEmailTokenExpires', 73 73 'mRegistration', 74 74 'mEditCount', 75 - // user_group table 75 + // user_groups table 76 76 'mGroups', 77 77 // user_properties table 78 78 'mOptionOverrides', — — @@ -182,7 +182,7 @@ 183 183 * Lazy-initialized variables, invalidated with clearInstanceCache 184 184 */ 185 185 var $mNewtalk,$mDatePreference, $mBlockedby,$mHash, $mRights, 186 -$mBlockreason, $mEffectiveGroups,$mBlockedGlobally, 186 + $mBlockreason,$mEffectiveGroups, $mFormerGroups,$mBlockedGlobally, 187 187 $mLocked,$mHideName, $mOptions; 188 188 189 189 /** — — @@ -1103,6 +1103,33 @@ 1104 1104 } 1105 1105 1106 1106 /** 1107 + * Add the user to the group if he/she meets given criteria. 1108 + * 1109 + * Contrary to autopromotion by \ref$wgAutopromote, the group will be 1110 + * possible to remove manually via Special:UserRights. In such case it 1111 + * will not be re-added autmoatically. The user will also not lose the 1112 + * group if they no longer meet the criteria. 1113 + * 1114 + * @param $criteria array Groups and conditions the user must meet in order 1115 + * to be promoted to these groups. Array of the same format as 1116 + * \ref$wgAutopromote. 1117 + * 1118 + * @return array Array of groups the user has been promoted to. 1119 + * 1120 + * @see $wgAutopromote 1121 + * @see Autopromote::autopromoteOnceHook() 1122 + */ 1123 + public function autopromoteOnce($criteria ) { 1124 + if ($this->getId()) { 1125 +$toPromote = Autopromote::getAutopromoteOnceGroups($this,$criteria); 1126 + foreach($toPromote as$group) 1127 + $this->addGroup($group); 1128 + return $toPromote; 1129 + } 1130 + return array(); 1131 + } 1132 + 1133 + /** 1107 1134 * Clear various cached data stored in this object. 1108 1135 * @param$reloadFrom String Reload user and user_groups table data from a 1109 1136 * given source. May be "name", "id", "defaults", "session", or false for — — @@ -1116,7 +1143,7 @@ 1117 1144 $this->mSkin = null; 1118 1145$this->mRights = null; 1119 1146 $this->mEffectiveGroups = null; 1120 -$this->mOptions = null; 1147 + $this->mOptions = null; 1121 1148 1122 1149 if ($reloadFrom ) { 1123 1150 $this->mLoadedItems = array(); — — @@ -2240,8 +2267,32 @@ 2241 2268 } 2242 2269 return$this->mEffectiveGroups; 2243 2270 } 2244 - 2271 + 2245 2272 /** 2273 + * Returns the groups the user has belonged to. 2274 + * 2275 + * The user may still belong to the returned groups. Compare with getGroups(). 2276 + * 2277 + * The function will not return groups the user had belonged to before MW 1.17 2278 + * 2279 + * @return array Names of the groups the user has belonged to. 2280 + */ 2281 + function getFormerGroups() { 2282 + if(is_null($this->mFormerGroups)) { 2283 +$dbr = wfGetDB( DB_MASTER ); 2284 + $res =$dbr->select( 'user_former_groups', 2285 + array( 'ufg_group' ), 2286 + array( 'ufg_user' => $this->mId ), 2287 + __METHOD__ ); 2288 +$this->mFormerGroups = array(); 2289 + while( $row =$dbr->fetchObject( $res ) ) { 2290 +$this->mFormerGroups[] = $row->ufg_group; 2291 + } 2292 + } 2293 + return$this->mFormerGroups; 2294 + } 2295 + 2296 + /** 2246 2297 * Get the user's edit count. 2247 2298 * @return Int 2248 2299 */ — — @@ -2297,6 +2348,14 @@ 2298 2349 'ug_user' => $this->getID(), 2299 2350 'ug_group' =>$group, 2300 2351 ), __METHOD__ ); 2352 + //remember that the user has had this group 2353 + $dbw->insert( 'user_former_groups', 2354 + array( 2355 + 'ufg_user' =>$this->getID(), 2356 + 'ufg_group' => $group, 2357 + ), 2358 + __METHOD__, 2359 + array( 'IGNORE' ) ); 2301 2360 } 2302 2361$this->loadGroups(); 2303 2362 $this->mGroups = array_diff($this->mGroups, array( $group ) ); Index: trunk/phase3/includes/installer/MysqlUpdater.php — — @@ -162,6 +162,7 @@ 163 163 164 164 // 1.17 165 165 array( 'addTable', 'iwlinks', 'patch-iwlinks.sql' ), 166 + array( 'addTable', 'user_former_groups', 'patch-user_former_groups.sql'), 166 167 array( 'addIndex', 'iwlinks', 'iwl_prefix_title_from', 'patch-rename-iwl_prefix.sql' ), 167 168 array( 'addField', 'updatelog', 'ul_value', 'patch-ul_value.sql' ), 168 169 array( 'addField', 'interwiki', 'iw_api', 'patch-iw_api_and_wikiid.sql' ), Index: trunk/phase3/includes/installer/SqliteUpdater.php — — @@ -43,6 +43,7 @@ 44 44 45 45 // 1.17 46 46 array( 'addTable', 'iwlinks', 'patch-iwlinks.sql' ), 47 + array( 'addTable', 'user_former_groups', 'patch-user_former_groups.sql'), 47 48 array( 'addIndex', 'iwlinks', 'iwl_prefix_title_from', 'patch-rename-iwl_prefix.sql' ), 48 49 array( 'addField', 'updatelog', 'ul_value', 'patch-ul_value.sql' ), 49 50 array( 'addField', 'interwiki', 'iw_api', 'patch-iw_api_and_wikiid.sql' ), Index: trunk/phase3/includes/DefaultSettings.php — — @@ -3513,6 +3513,12 @@ 3514 3514 * 3515 3515 * If$wgEmailAuthentication is off, APCOND_EMAILCONFIRMED will be true for any 3516 3516 * user who has provided an e-mail address. 3517 + * 3518 + * If the groups should be removable, consider using 3519 + * Autopromote::autopromoteOnceHook() instead. 3520 + * 3521 + * @see Autopromote::autopromoteOnceHook() 3522 + * @see User::autopromoteOnce() 3517 3523 */ 3518 3524 $wgAutopromote = array( 3519 3525 'autoconfirmed' => array( '&', Index: trunk/phase3/includes/Autopromote.php — — @@ -6,6 +6,41 @@ 7 7 8 8 class Autopromote { 9 9 /** 10 + * A function which may be assigned to a hook in order to check 11 + * autopromotion of the current user (\ref$wgUser) to the specified 12 + * group. 13 + * 14 + * Contrary to autopromotion by \ref $wgAutopromote, the group will be 15 + * possible to remove manually via Special:UserRights. In such case it 16 + * will not be re-added autmoatically. The user will also not lose the 17 + * group if they no longer meet the criteria. 18 + * 19 + * Example configuration: 20 + * \code$wgHooks['ArticleSaveComplete'][] = array ( 21 + * 'Autopromote::autopromoteOnceHook', 22 + * array( 'somegroup' => array(APCOND_EDITCOUNT, 200) ) 23 + * ); \endcode 24 + * 25 + * The second array should be of the same format as \ref $wgAutopromote. 26 + * 27 + * This funciton simply runs User::autopromoteOnce() on$wgUser. You may 28 + * run this method from your custom function if you wish. 29 + * 30 + * @param $criteria array Groups and conditions which must be met in order to 31 + * aquire these groups. Array of the same format as \ref$wgAutopromote. 32 + * 33 + * @return Always true. 34 + * 35 + * @see User::autopromoteOnce() 36 + * @see $wgAutopromote 37 + */ 38 + public static function autopromoteOnceHook($criteria) { 39 + global $wgUser; 40 +$wgUser->autopromoteOnce($criteria); 41 + return true; 42 + } 43 + 44 + /** 10 45 * Get the groups for the given user based on$wgAutopromote. 11 46 * 12 47 * @param $user User The user to get the groups for — — @@ -26,6 +61,41 @@ 27 62 28 63 return$promote; 29 64 } 65 + 66 + /** 67 + * Get the groups for the given user based on the given criteria. 68 + * 69 + * Does not return groups the user already belongs to or has once belonged. 70 + * 71 + * @param $user The user to get the groups for 72 + * @param$criteria array Groups and conditions the user must meet in order 73 + * to be promoted to these groups. Array of the same format as 74 + * \ref $wgAutopromote. 75 + * 76 + * @return array Groups the user should be promoted to. 77 + */ 78 + public static function getAutopromoteOnceGroups( User$user, $criteria ) { 79 +$promote = array(); 80 + 81 + //get the current groups 82 + $currentGroups =$user->getGroups(); 83 + 84 + foreach( $criteria as$group => $cond ) { 85 + //do not check if the user's already a member 86 + if ( in_array($group, $currentGroups)) 87 + continue; 88 + 89 + //do not autopromote if the user has belonged to the group 90 +$formerGroups = $user->getFormerGroups(); 91 + if ( in_array($group, $formerGroups) ) 92 + continue; 93 + 94 + //finally - check the conditions 95 + if ( self::recCheckCondition($cond, $user) ) 96 +$promote[] = $group; 97 + } 98 + return$promote; 99 + } 30 100 31 101 /** 32 102 * Recursively check a condition. Conditions are in the form

## Follow-up revisions

RevisionCommit summaryAuthorDate
r90753Follow-up r90749:...aaron04:11, 25 June 2011
r90755Follow-up r90749:...aaron04:58, 25 June 2011
r90756(bug 25390) autoreview privilege should be removeable like editor (uses r90749)aaron05:05, 25 June 2011
r90757Follow-up r90749: pushed down accessing of \$wgAutopromoteOnce to getAutopromo...aaron05:21, 25 June 2011
r90776* Follow-up r90749...aaron17:59, 25 June 2011
r90816* Core:...aaron04:12, 26 June 2011
r90855Follow-up r90749: use a new 'autopromote' action for the autopromote rights l...aaron21:44, 26 June 2011
r97625Followup r90749, beat Aaron...reedy13:36, 20 September 2011

#Comment by Aaron Schulz (talk | contribs)   03:14, 25 June 2011

autopromoteOnceHook() could probably be renamed and the User should be passed in. I see a few doc typos too.

#Comment by Aaron Schulz (talk | contribs)   03:30, 25 June 2011

Gah, disregard the first bit.

#Comment by Bawolff (talk | contribs)   03:54, 25 June 2011

[21:50] <codurr> Something broke. See <http://ci.tesla.usability.wikimedia.org/cruisecontrol/>. Possible culprits: aaron/r90749 Reedy/r90750

The failing test was:

#Comment by Aaron Schulz (talk | contribs)   04:47, 25 June 2011
#Comment by Reedy (talk | contribs)   16:53, 18 July 2011

Created the barebones of this documentation at Manual:User_former_groups_table

#Comment by Happy-melon (talk | contribs)   21:46, 4 August 2011

Although it seems to be fashionable to add endless extra tables for things like this, adding a ug_current field to the user_groups table would be nicer, IMO. It's not a particularly massive table, shouldn't be too painful to alter it even on enwiki.

#Comment by 😂 (talk | contribs)   16:02, 16 August 2011

I agree. I'm not a huge fan of adding a new table for this.

#Comment by Aaron Schulz (talk | contribs)   20:09, 20 August 2011

Why is this fixme?

#Comment by 😂 (talk | contribs)   20:18, 20 August 2011

I'm guessing Sam marked it fixme for the objections I and Happy-Melon raised above. I'm not a fan of adding an additional table here.

#Comment by Aaron Schulz (talk | contribs)   21:09, 20 August 2011

I'm not sure how well ug_current would work. Any existing queries would need to be updated for that work. The PK would need to change and the new field would probably need to be part of an indexed. I don't really think it would have a good separation of concern either, which is why I ended up using the new table as the patch had.

#Comment by 😂 (talk | contribs)   17:14, 6 September 2011

You're right, marking resolved.

#Comment by MZMcBride (talk | contribs)   23:16, 9 February 2012

Adding a database table is really something that ought to be mentioned in a commit summary.

I don't really like this table as I don't think it will ever be accurate for nearly any user.

## Status & tagging log

• 19:19, 26 January 2012 Reedy (talk | contribs) changed the tags for r90749 [removed: scaptrap]
• 17:14, 6 September 2011 😂 (talk | contribs) changed the tags for r90749 [removed: chad]
• 17:14, 6 September 2011 😂 (talk | contribs) changed the status of r90749 [removed: fixme added: resolved]
• 16:23, 19 August 2011 Reedy (talk | contribs) changed the status of r90749 [removed: new added: fixme]
• 21:58, 26 June 2011 Aaron Schulz (talk | contribs) changed the tags for r90749 [added: chad]
• 05:01, 25 June 2011 Aaron Schulz (talk | contribs) changed the tags for r90749 [added: scaptrap]
• 04:47, 25 June 2011 Aaron Schulz (talk | contribs) changed the status of r90749 [removed: fixme added: new]
• 03:54, 25 June 2011 Bawolff (talk | contribs) changed the status of r90749 [removed: new added: fixme]