r84805 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r84804‎ | r84805 | r84806 >
Date:16:40, 26 March 2011
Author:happy-melon
Status:reverted (Comments)
Tags:
Comment:
Convert SpecialListusers to subclass SpecialPage. Only two left now!
Modified paths:
  • /trunk/phase3/includes/AutoLoader.php (modified) (history)
  • /trunk/phase3/includes/SpecialPage.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialListusers.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/AutoLoader.php
——@@ -660,6 +660,7 @@
661661 'SpecialFilepath' => 'includes/specials/SpecialFilepath.php',
662662 'SpecialImport' => 'includes/specials/SpecialImport.php',
663663 'SpecialListGroupRights' => 'includes/specials/SpecialListgrouprights.php',
 664+ 'SpecialListusers' => 'includes/specials/SpecialListusers.php',
664665 'SpecialLockdb' => 'includes/specials/SpecialLockdb.php',
665666 'SpecialLog' => 'includes/specials/SpecialLog.php',
666667 'SpecialMergeHistory' => 'includes/specials/SpecialMergeHistory.php',
——@@ -695,7 +696,6 @@
696697 'UploadForm' => 'includes/specials/SpecialUpload.php',
697698 'UploadSourceField' => 'includes/specials/SpecialUpload.php',
698699 'UserrightsPage' => 'includes/specials/SpecialUserrights.php',
699 - 'UsersPager' => 'includes/specials/SpecialListusers.php',
700700 'WantedCategoriesPage' => 'includes/specials/SpecialWantedcategories.php',
701701 'WantedFilesPage' => 'includes/specials/SpecialWantedfiles.php',
702702 'WantedPagesPage' => 'includes/specials/SpecialWantedpages.php',
Index: trunk/phase3/includes/specials/SpecialListusers.php
——@@ -32,7 +32,7 @@
3333 *
3434 * @ingroup SpecialPage
3535 */
36 -class UsersPager extends AlphabeticPager {
 36+class SpecialListUsers extends QueryPage {
3737
3838 function __construct( $par=null ) {
3939 global $wgRequest;
——@@ -61,30 +61,62 @@
6262 $this->requestedUser = $username->getText();
6363 }
6464 }
65 - parent::__construct();
 65+
 66+ $this->limit = $wgRequest->getVal( 'limit' );
 67+ $this->offset = $wgRequest->getVal( 'offset' );
 68+
 69+ parent::__construct( 'Listusers' );
6670 }
6771
 72+ function isExpensive() { return false; }
 73+ function isCacheable() { return false; }
 74+ function isSyndicated() { return false; }
 75+ function sortDescending() { return false; }
6876
69 - function getIndexField() {
70 - return $this->creationSort ? 'user_id' : 'user_name';
 77+ function openList( $offset ) {
 78+ return "\n<ul class='special'>\n";
7179 }
7280
 81+ function closeList() {
 82+ return "</ul>\n";
 83+ }
 84+
 85+ function linkParameters() {
 86+ return array(
 87+ 'group' => $this->requestedGroup,
 88+ 'editsOnly' => $this->editsOnly,
 89+ 'creationSort' => $this->creationSort,
 90+ 'requestedUser' => $this->requestedUser,
 91+ );
 92+ }
 93+
7394 function getQueryInfo() {
 95+
7496 global $wgUser;
7597 $dbr = wfGetDB( DB_SLAVE );
76 - $conds = array();
 98+ $conds = $jconds = array();
 99+ $tables = array( 'user' );
 100+
77101 // Don't show hidden names
78102 if( !$wgUser->isAllowed('hideuser') ) {
 103+ $tables[] = 'ipblocks';
79104 $conds[] = 'ipb_deleted IS NULL';
 105+ $jconds['ipblocks'] = array( 'LEFT JOIN', array(
 106+ # Unique index on (ipb_address,ipb_user,ipb_auto)
 107+ 'ipb_address = user_name',
 108+ 'ipb_user = user_id',
 109+ 'ipb_auto = 0',
 110+ 'ipb_deleted = 1',
 111+ ) );
80112 }
81113
82 - $options = array();
83 -
84114 if( $this->requestedGroup != '' ) {
 115+ $tables[] = 'user_groups';
85116 $conds['ug_group'] = $this->requestedGroup;
86 - } else {
87 - //$options['USE INDEX'] = $this->creationSort ? 'PRIMARY' : 'user_name';
 117+ # Unique index on (ug_user,ug_group)
 118+ $jconds['user_groups'] = array( 'LEFT JOIN', 'user_id = ug_user' );
88119 }
 120+
89121 if( $this->requestedUser != '' ) {
90122 # Sorted either by account creation or name
91123 if( $this->creationSort ) {
——@@ -93,28 +125,15 @@
94126 $conds[] = 'user_name >= ' . $dbr->addQuotes( $this->requestedUser );
95127 }
96128 }
 129+
97130 if( $this->editsOnly ) {
98131 $conds[] = 'user_editcount > 0';
99132 }
100133
101 - $options['GROUP BY'] = $this->creationSort ? 'user_id' : 'user_name';
102 -
103134 $query = array(
104 - 'tables' => array( 'user', 'user_groups', 'ipblocks'),
105 - 'fields' => array(
106 - $this->creationSort ? 'MAX(user_name) AS user_name' : 'user_name',
107 - $this->creationSort ? 'user_id' : 'MAX(user_id) AS user_id',
108 - 'MAX(user_editcount) AS edits',
109 - 'COUNT(ug_group) AS numgroups',
110 - 'MAX(ug_group) AS singlegroup', // the usergroup if there is only one
111 - 'MIN(user_registration) AS creation',
112 - 'MAX(ipb_deleted) AS ipb_deleted' // block/hide status
113 - ),
114 - 'options' => $options,
115 - 'join_conds' => array(
116 - 'user_groups' => array( 'LEFT JOIN', 'user_id=ug_user' ),
117 - 'ipblocks' => array( 'LEFT JOIN', 'user_id=ipb_user AND ipb_deleted=1 AND ipb_auto=0' ),
118 - ),
 135+ 'tables' => $tables,
 136+ 'fields' => '*',
 137+ 'join_conds' => $jconds,
119138 'conds' => $conds
120139 );
121140
——@@ -122,16 +141,21 @@
123142 return $query;
124143 }
125144
126 - function formatRow( $row ) {
 145+ function formatResult( $skin, $row ) {
127146 global $wgLang;
128147
129 - if ($row->user_id == 0) #Bug 16487
130 - return '';
 148+ if ( $row->user_id == 0 ){
 149+ #Bug 16487
 150+ return false;
 151+ }
131152
132 - $userPage = Title::makeTitle( NS_USER, $row->user_name );
133 - $name = $this->getSkin()->link( $userPage, htmlspecialchars( $userPage->getText() ) );
 153+ $user = User::newFromId( $row->user_id );
 154+ $name = $skin->link(
 155+ $user->getUserpage(),
 156+ $user->getName()
 157+ );
134158
135 - $groups_list = self::getGroups( $row->user_id );
 159+ $groups_list = array_diff( $user->getEffectiveGroups(), $user->getImplicitGroups() );
136160 if( count( $groups_list ) > 0 ) {
137161 $list = array();
138162 foreach( $groups_list as $group )
——@@ -142,7 +166,7 @@
143167 }
144168
145169 $item = wfSpecialList( $name, $groups );
146 - if( $row->ipb_deleted ) {
 170+ if( isset( $row->ipb_deleted ) ) {
147171 $item = "<span class=\"deleted\">$item</span>";
148172 }
149173
——@@ -156,29 +180,36 @@
157181
158182 $created = '';
159183 # Some rows may be NULL
160 - if( $row->creation ) {
161 - $d = $wgLang->date( wfTimestamp( TS_MW, $row->creation ), true );
162 - $t = $wgLang->time( wfTimestamp( TS_MW, $row->creation ), true );
163 - $created = ' (' . wfMsg( 'usercreated', $d, $t ) . ')';
164 - $created = htmlspecialchars( $created );
 184+ if( $row->user_registration ) {
 185+ $d = $wgLang->date( wfTimestamp( TS_MW, $row->user_registration ), true );
 186+ $t = $wgLang->time( wfTimestamp( TS_MW, $row->user_registration ), true );
 187+ $created = ' (' . wfMessage( 'usercreated', $d, $t ) . ')';
165188 }
166189
167190 wfRunHooks( 'SpecialListusersFormatRow', array( &$item, $row ) );
168 - return "<li>{$item}{$edits}{$created}</li>";
 191+ return "{$item}{$edits}{$created}";
169192 }
170193
171 - function getBody() {
172 - if( !$this->mQueryDone ) {
173 - $this->doQuery();
174 - }
175 - $this->mResult->rewind();
 194+ function getOrderFields() {
 195+ return $this->creationSort ? array( 'user_id' ) : array( 'user_name' );
 196+ }
 197+
 198+ /**
 199+ * Cache page existence for performance
 200+ */
 201+ function preprocessResults( $db, $res ) {
176202 $batch = new LinkBatch;
177 - foreach ( $this->mResult as $row ) {
178 - $batch->addObj( Title::makeTitleSafe( NS_USER, $row->user_name ) );
 203+ foreach ( $res as $row ) {
 204+ $batch->add( NS_USER, $row->user_name );
 205+ $batch->add( NS_USER_TALK, $row->user_name );
179206 }
180207 $batch->execute();
181 - $this->mResult->rewind();
182 - return parent::getBody();
 208+
 209+ // Back to start for display
 210+ if ( $db->numRows( $res ) > 0 ) {
 211+ // If there are no rows we get an error seeking.
 212+ $db->dataSeek( $res, 0 );
 213+ }
183214 }
184215
185216 function getPageHeader( ) {
——@@ -198,8 +229,16 @@
199230 $out .= Xml::label( wfMsg( 'group' ), 'group' ) . ' ' .
200231 Xml::openElement('select', array( 'name' => 'group', 'id' => 'group' ) ) .
201232 Xml::option( wfMsg( 'group-all' ), '' );
202 - foreach( $this->getAllGroups() as $group => $groupText )
203 - $out .= Xml::option( $groupText, $group, $group == $this->requestedGroup );
 233+
 234+ $groups = array_unique( array_diff( User::getAllGroups(), array( '*', 'user' ) ) );
 235+ foreach( $groups as $group ){
 236+ $out .= Xml::option(
 237+ User::getGroupName( $group ),
 238+ $group,
 239+ $group == $this->requestedGroup
 240+ );
 241+ }
 242+
204243 $out .= Xml::closeElement( 'select' ) . '<br />';
205244 $out .= Xml::checkLabel( wfMsg('listusers-editsonly'), 'editsOnly', 'editsOnly', $this->editsOnly );
206245 $out .= '&#160;';
——@@ -209,7 +248,7 @@
210249 wfRunHooks( 'SpecialListusersHeaderForm', array( $this, &$out ) );
211250
212251 # Submit button and form bottom
213 - $out .= Html::hidden( 'limit', $this->mLimit );
 252+ $out .= Html::hidden( 'limit', $this->limit );
214253 $out .= Xml::submitButton( wfMsg( 'allpagessubmit' ) );
215254 wfRunHooks( 'SpecialListusersHeader', array( $this, &$out ) );
216255 $out .= Xml::closeElement( 'fieldset' ) .
——@@ -219,45 +258,6 @@
220259 }
221260
222261 /**
223 - * Get a list of all explicit groups
224 - * @return array
225 - */
226 - function getAllGroups() {
227 - $result = array();
228 - foreach( User::getAllGroups() as $group ) {
229 - $result[$group] = User::getGroupName( $group );
230 - }
231 - asort( $result );
232 - return $result;
233 - }
234 -
235 - /**
236 - * Preserve group and username offset parameters when paging
237 - * @return array
238 - */
239 - function getDefaultQuery() {
240 - $query = parent::getDefaultQuery();
241 - if( $this->requestedGroup != '' )
242 - $query['group'] = $this->requestedGroup;
243 - if( $this->requestedUser != '' )
244 - $query['username'] = $this->requestedUser;
245 - wfRunHooks( 'SpecialListusersDefaultQuery', array( $this, &$query ) );
246 - return $query;
247 - }
248 -
249 - /**
250 - * Get a list of groups the specified user belongs to
251 - *
252 - * @param $uid Integer: user id
253 - * @return array
254 - */
255 - protected static function getGroups( $uid ) {
256 - $user = User::newFromId( $uid );
257 - $groups = array_diff( $user->getEffectiveGroups(), $user->getImplicitGroups() );
258 - return $groups;
259 - }
260 -
261 - /**
262262 * Format a link to a group description page
263263 *
264264 * @param $group String: group name
——@@ -269,28 +269,4 @@
270270 $cache[$group] = User::makeGroupLinkHtml( $group, htmlspecialchars( User::getGroupMember( $group ) ) );
271271 return $cache[$group];
272272 }
273 -}
274 -
275 -/**
276 - * constructor
277 - * $par string (optional) A group to list users from
278 - */
279 -function wfSpecialListusers( $par = null ) {
280 - global $wgOut;
281 -
282 - $up = new UsersPager($par);
283 -
284 - # getBody() first to check, if empty
285 - $usersbody = $up->getBody();
286 - $s = Xml::openElement( 'div', array('class' => 'mw-spcontent') );
287 - $s .= $up->getPageHeader();
288 - if( $usersbody ) {
289 - $s .= $up->getNavigationBar();
290 - $s .= '<ul>' . $usersbody . '</ul>';
291 - $s .= $up->getNavigationBar() ;
292 - } else {
293 - $s .= '<p>' . wfMsgHTML('listusers-noresult') . '</p>';
294 - };
295 - $s .= Xml::closeElement( 'div' );
296 - $wgOut->addHTML( $s );
297 -}
 273+}
\ No newline at end of file
Index: trunk/phase3/includes/SpecialPage.php
——@@ -144,7 +144,7 @@
145145 'Preferences' => 'SpecialPreferences',
146146 'Contributions' => 'SpecialContributions',
147147 'Listgrouprights' => 'SpecialListGroupRights',
148 - 'Listusers' => array( 'SpecialPage', 'Listusers' ),
 148+ 'Listusers' => 'SpecialListusers',
149149 'Listadmins' => array( 'SpecialRedirectToSpecial', 'Listadmins', 'Listusers', 'sysop' ),
150150 'Listbots' => array( 'SpecialRedirectToSpecial', 'Listbots', 'Listusers', 'bot' ),
151151 'Activeusers' => 'SpecialActiveUsers',

Follow-up revisions

RevisionCommit summaryAuthorDate
r84856Follow-up r84805: convert SpecialActiveusers to not use the now-absent UsersP...happy-melon16:49, 27 March 2011
r85889Revert r84805: broke CentralAuth by removing base UsersPager class used in ot...brion17:04, 12 April 2011
r86048Recommit r84805, but without removing UsersPager, which is actually a Good Th...happy-melon13:27, 14 April 2011

Comments

#Comment by IAlex (talk | contribs)   19:26, 26 March 2011

This broke Special:Activeusers:

Fatal error: Class 'UsersPager' not found in includes/specials/SpecialActiveusers.php on line 33

#Comment by Happy-melon (talk | contribs)   16:49, 27 March 2011

Should be fixed in r84856.

#Comment by Brion VIBBER (talk | contribs)   17:10, 12 April 2011

That's also now reverted, since the updated version broke when this was reverted.

#Comment by Brion VIBBER (talk | contribs)   18:02, 6 April 2011

This broke Special:Specialpages with CentralAuth enabled:

PHP Fatal error: Class 'UsersPager' not found in /var/www/trunk/extensions/CentralAuth/SpecialGlobalUsers.php on line 3

#Comment by Brion VIBBER (talk | contribs)   17:05, 12 April 2011

I've worked around that by reverting this change for now in r85889.

#Comment by Happy-melon (talk | contribs)   17:16, 12 April 2011

Thanks Brion. I'm actually no longer convinced that disposing of UsersPager was productive anyway; it's actually better suited to the job in hand here than QueryPage is.

Status & tagging log