r102132 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r102131‎ | r102132 | r102133 >
Date:00:17, 6 November 2011
Author:ashley
Status:deferred (Comments)
Tags:socialprofile 
Comment:
SocialProfile: as per Markus' in-depth review:
*move hook registration to the top of the file
*documentation updates
*i18n
*moved inline CSS to the CSS file
*removed some duplicate code
*return an array when we're supposed to return one instead of returning a string
Modified paths:
  • /trunk/extensions/SocialProfile/UserStats/EditCount.php (modified) (history)
  • /trunk/extensions/SocialProfile/UserStats/GenerateTopUsersReport.php (modified) (history)
  • /trunk/extensions/SocialProfile/UserStats/SpecialUpdateEditCounts.php (modified) (history)
  • /trunk/extensions/SocialProfile/UserStats/TopFansByStat.php (modified) (history)
  • /trunk/extensions/SocialProfile/UserStats/TopFansRecent.php (modified) (history)
  • /trunk/extensions/SocialProfile/UserStats/TopList.css (modified) (history)
  • /trunk/extensions/SocialProfile/UserStats/TopUsers.php (modified) (history)
  • /trunk/extensions/SocialProfile/UserStats/UserStats.i18n.php (modified) (history)
  • /trunk/extensions/SocialProfile/UserStats/UserStatsClass.php (modified) (history)

Diff [purge]

Index: trunk/extensions/SocialProfile/UserStats/GenerateTopUsersReport.php
@@ -79,6 +79,8 @@
8080 // Add CSS
8181 $wgOut->addExtensionStyle( $wgScriptPath . '/extensions/SocialProfile/UserStats/TopList.css' );
8282
 83+ // Used as the LIMIT for SQL queries; basically, show this many users
 84+ // in the generated reports.
8385 $user_count = $wgRequest->getInt( 'user_count', 10 );
8486
8587 if( $period == 'weekly' ) {
@@ -101,7 +103,11 @@
102104 );
103105
104106 $last_rank = 0;
 107+ $last_total = 0;
 108+ $x = 1;
105109
 110+ $users = array();
 111+
106112 // Initial run is a special case
107113 if ( $dbw->numRows( $res ) <= 0 ) {
108114 // For the initial run, everybody's a winner!
@@ -122,11 +128,6 @@
123129
124130 $out = '<div class="top-users">';
125131
126 - $last_total = 0;
127 - $x = 1;
128 -
129 - $users = array();
130 -
131132 foreach( $res as $row ) {
132133 if( $row->stats_total_points == $last_total ) {
133134 $rank = $last_rank;
@@ -146,11 +147,6 @@
147148 } else {
148149 $out = '<div class="top-users">';
149150
150 - $last_total = 0;
151 - $x = 1;
152 -
153 - $users = array();
154 -
155151 foreach( $res as $row ) {
156152 if( $row->up_points == $last_total ) {
157153 $rank = $last_rank;
Index: trunk/extensions/SocialProfile/UserStats/TopFansRecent.php
@@ -95,7 +95,7 @@
9696 $message = wfMsgForContent( 'topfans-by-category' );
9797
9898 if ( !wfEmptyMsg( 'topfans-by-category', $message ) ) {
99 - $out .= '<h1 style="margin-top:15px !important;">' .
 99+ $out .= '<h1 class="top-title">' .
100100 wfMsg( 'top-fans-by-category-nav-header' ) . '</h1>';
101101
102102 $lines = explode( "\n", $message );
Index: trunk/extensions/SocialProfile/UserStats/UserStats.i18n.php
@@ -72,6 +72,10 @@
7373
7474 Click $3
7575 and change your settings to disable e-mail notifications.',
 76+ // Special:UpdateEditCounts
 77+ 'updateeditcounts' => 'Update Edit Counts',
 78+ 'updateeditcounts-updated' => "Updated stats for '''$1''' {{PLURAL:$1|user|users}}",
 79+ 'updateeditcounts-updating' => 'Updating $1 with $2 {{PLURAL:$2|edit|edits}}',
7680 // Special:GenerateTopUsersReport
7781 'generatetopusersreport' => 'Generate Top Users Report',
7882 'user-stats-weekly-winners' => 'Weekly {{PLURAL:$1|Winner|Winners}}',
Index: trunk/extensions/SocialProfile/UserStats/EditCount.php
@@ -7,8 +7,19 @@
88 die( "This is not a valid entry point.\n" );
99 }
1010
 11+/**
 12+ * For the UserLevels (points) functionality to work, you will need to
 13+ * define $wgUserLevels and require_once() this file in your wiki's
 14+ * LocalSettings.php file.
 15+ */
1116 $wgHooks['NewRevisionFromEditComplete'][] = 'incEditCount';
 17+$wgHooks['ArticleDelete'][] = 'removeDeletedEdits';
 18+$wgHooks['ArticleUndelete'][] = 'restoreDeletedEdits';
1219
 20+/**
 21+ * Updates user's points after they've made an edit in a namespace that is
 22+ * listed in the $wgNamespacesForEditPoints array.
 23+ */
1324 function incEditCount( $article, $revision, $baseRevId ) {
1425 global $wgUser, $wgNamespacesForEditPoints;
1526
@@ -24,8 +35,10 @@
2536 return true;
2637 }
2738
28 -$wgHooks['ArticleDelete'][] = 'removeDeletedEdits';
29 -
 39+/**
 40+ * Updates user's points after a page in a namespace that is listed in the
 41+ * $wgNamespacesForEditPoints array that they've edited has been deleted.
 42+ */
3043 function removeDeletedEdits( &$article, &$user, &$reason ) {
3144 global $wgNamespacesForEditPoints;
3245
@@ -51,8 +64,11 @@
5265 return true;
5366 }
5467
55 -$wgHooks['ArticleUndelete'][] = 'restoreDeletedEdits';
56 -
 68+/**
 69+ * Updates user's points after a page in a namespace that is listed in the
 70+ * $wgNamespacesForEditPoints array that they've edited has been restored after
 71+ * it was originally deleted.
 72+ */
5773 function restoreDeletedEdits( &$title, $new ) {
5874 global $wgNamespacesForEditPoints;
5975
Index: trunk/extensions/SocialProfile/UserStats/TopList.css
@@ -73,7 +73,13 @@
7474 width: 200px;
7575 padding: 5px;
7676 }
 77+
7778 .top-fan-nav a {
7879 font-weight: bold;
7980 text-decoration: none;
8081 }
 82+
 83+/* A "Top users by category" <h1> on Special:TopFansByStatistics and Special:TopUsersRecent */
 84+.top-title {
 85+ margin-top: 15px !important;
 86+}
\ No newline at end of file
Index: trunk/extensions/SocialProfile/UserStats/TopFansByStat.php
@@ -56,7 +56,6 @@
5757 $params['ORDER BY'] = "{$column} DESC";
5858 $params['LIMIT'] = $count;
5959
60 - $dbr = wfGetDB( DB_SLAVE );
6160 $res = $dbr->select(
6261 'user_stats',
6362 array( 'stats_user_id', 'stats_user_name', $column ),
@@ -96,7 +95,7 @@
9796 $message = wfMsgForContent( 'topfans-by-category' );
9897
9998 if ( !wfEmptyMsg( 'topfans-by-category', $message ) ) {
100 - $out .= '<h1 style="margin-top:15px !important;">' .
 99+ $out .= '<h1 class="top-title">' .
101100 wfMsg( 'top-fans-by-category-nav-header' ) . '</h1>';
102101
103102 $lines = explode( "\n", $message );
Index: trunk/extensions/SocialProfile/UserStats/UserStatsClass.php
@@ -581,7 +581,7 @@
582582 global $wgEnableFacebook, $wgUserLevels;
583583
584584 if ( $this->user_id == 0 ) {
585 - return '';
 585+ return array();
586586 }
587587
588588 $stats_data = array();
@@ -775,7 +775,7 @@
776776 * amount of points the user has
777777 */
778778 static function getTopFansList( $limit = 10 ) {
779 - $dbr = wfGetDB( DB_MASTER );
 779+ $dbr = wfGetDB( DB_SLAVE );
780780
781781 $res = $dbr->select(
782782 'user_stats',
Index: trunk/extensions/SocialProfile/UserStats/TopUsers.php
@@ -57,7 +57,7 @@
5858 );
5959 $loop++;
6060 }
61 - if ( $loop >= 50 ) {
 61+ if ( $loop >= $realcount ) {
6262 break;
6363 }
6464 }
Index: trunk/extensions/SocialProfile/UserStats/SpecialUpdateEditCounts.php
@@ -73,7 +73,14 @@
7474 __METHOD__
7575 );
7676 }
77 - $wgOut->addHTML( "<p>Updating {$row->rev_user_text} with {$editCount} edits</p>" );
 77+ $wgOut->addHTML(
 78+ wfMsgExt(
 79+ 'updateeditcounts-updating',
 80+ 'parsemag',
 81+ $row->rev_user_text,
 82+ $editCount
 83+ )
 84+ );
7885
7986 $dbw->update(
8087 'user_stats',
@@ -97,8 +104,6 @@
98105 public function execute( $par ) {
99106 global $wgOut, $wgUser;
100107
101 - $wgOut->setPageTitle( 'Update Edit Counts' );
102 -
103108 // Check permissions -- we must be allowed to access this special page
104109 // before we can run any database queries
105110 if ( !$wgUser->isAllowed( 'updatepoints' ) ) {
@@ -112,6 +117,9 @@
113118 return;
114119 }
115120
 121+ // Set the page title, robot policies, etc.
 122+ $this->setHeaders();
 123+
116124 $dbw = wfGetDB( DB_MASTER );
117125 $this->updateMainEditsCount();
118126
@@ -125,7 +133,7 @@
126134 __METHOD__,
127135 array( 'ORDER BY' => 'stats_user_name' )
128136 );
129 - $out = '';
 137+
130138 $x = 0;
131139 foreach ( $res as $row ) {
132140 $x++;
@@ -135,7 +143,7 @@
136144 );
137145 $stats->updateTotalPoints();
138146 }
139 - $out = "Updated stats for <b>{$x}</b> users";
140 - $wgOut->addHTML( $out );
 147+
 148+ $wgOut->addHTML( wfMsgExt( 'updateeditcounts-updated', 'parsemag', $x ) );
141149 }
142150 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r102292SocialProfile: follow-up to r102132 as per Nikerabbit's code review: use Outp...ashley15:50, 7 November 2011

Comments

#Comment by Nikerabbit (talk | contribs)   09:43, 6 November 2011

For updateeditcounts-updating and updateeditcounts-updated I suggest $wgOut->addWikiMsg( 'name', 'param1' ); The other messages should be escaped in way or another.

Status & tagging log