r105239 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r105238‎ | r105239 | r105240 >
Date:22:54, 5 December 2011
Author:krinkle
Status:resolved (Comments)
Tags:
Comment:
[HarvardResearch][CentralNotice] Temporary wgNoticeBanner_Harvard2011 data.
-- Banner display is handled through JavaScript because it's CentralNotice that is also for anonymous users and page output must be unaffected.
-- Banner submission to Harvard/Science PO needs a hash of which the salt must not be in cleartext in the front-end code (also generating hashes in JS is ugly). So we need at least one temporary server component.
-- Instead of requesting things from 2 or three different APIs on all pages + outputting a hash from PHP, might as well do it all in PHP and output an object in mw.config with all the needed data
-- Cached in the session. Not sure whether it should be in memcached or session. Choose session for now since this is going to happen for all logged-in users on en.wiki, not sure wether it's good to have a gazillion memcached entries only used by 1 user only needed for about a week.
-- Also notices that session is somewhat tied into memcached
Modified paths:
  • /trunk/extensions/CentralNotice/CentralNotice.php (modified) (history)

Diff [purge]

Index: trunk/extensions/CentralNotice/CentralNotice.php
@@ -94,6 +94,9 @@
9595 'scripts' => 'bannerstats.js',
9696 );
9797
 98+// Temporary setting to configure salt for Harvard banner protocol
 99+$wgNoticeBanner_Harvard2011_salt = 'default';
 100+
98101 /**
99102 * UnitTestsList hook handler
100103 */
@@ -243,12 +246,90 @@
244247 * MakeGlobalVariablesScript hook handler
245248 */
246249 function efCentralNoticeDefaults( &$vars ) {
247 - global $wgNoticeProject;
 250+ // Using global $wgUser for compatibility with 1.18
 251+ global $wgNoticeProject, $wgUser, $wgRequest;
 252+
248253 // Initialize global Javascript variables. We initialize Geo with empty values so if the geo
249254 // IP lookup fails we don't have any surprises.
250255 $geo = array( 'city' => '', 'country' => '' );
251256 $vars['Geo'] = $geo; // change this to wgGeo as soon as Mark updates on his end
252257 $vars['wgNoticeProject'] = $wgNoticeProject;
 258+
 259+ // XXX: Temporary WMF-specific code for the 2011 Harvard survey invitation banner.
 260+ // Only do this for logged-in users, keeping anonymous user output equal (for Squid-cache).
 261+ // Also, don't run if the UserDailyContribs-extension isn't installed.
 262+ if ( $wgUser->isLoggedIn() && function_exists( 'getUserEditCountSince' ) ) {
 263+
 264+ $cacheKey = wfMemcKey( 'CentralNotice', 'Harvard2011', $wgUser->getId() );
 265+ $data = $wgMemc->get( $cacheKey );
 266+
 267+ // Cached ?
 268+ if ( is_null( $data ) ) {
 269+ /**
 270+ * To be eligible, the user must match all of the following:
 271+ * - have an account
 272+ * - not be a bot (userright=bot)
 273+ * .. and match one of the following:
 274+ * - be an admin (group=sysop)
 275+ * - have an editcount higher than 300, of which 20 within the last 180 days (on the launch date)
 276+ * - have had their account registered for less than 30 days (on to the launch date)
 277+ */
 278+ if ( $wgUser->isAllowed( 'bot' ) ) {
 279+ $data = false;
 280+
 281+ } else {
 282+ global $wgNoticeBanner_Harvard2011_salt;
 283+
 284+ $launchTimestamp = wfTimestamp( TS_UNIX, '2011-12-06 00:00:00' );
 285+ $groups = $wgUser->getGroups();
 286+ $registrationDate = !$wgUser->getRegistration() ? 0 : $wgUser->getRegistration();
 287+ $daysOld = floor( ( $launchTimestamp - wfTimestamp( TS_UNIX, $registrationDate ) ) / ( 60*60*24 ) );
 288+ $salt = $wgNoticeBanner_Harvard2011_salt;
 289+ $metrics = array(
 290+ // "username" the user's username
 291+ 'username' => $wgUser->getName(),
 292+
 293+ // "group" is the group name(s) of the user (comma-separated).
 294+ 'group' => join( ',', $groups ),
 295+
 296+ // "duration" is the number of days since the user registered his (on the launching date).
 297+ // Note: Will be negative if user registered after launch date!
 298+ 'duration' => $daysOld,
 299+
 300+ // "editcounts" is the user's total number of edits
 301+ 'editcounts' => $wgUser->getEditCount() == NULL ? 0 : $wgUser->getEditCount(),
 302+
 303+ // "last6monthseditcount" is the user's total number of edits in the last 180 days (on the launching date)
 304+ 'last6monthseditcount' => getUserEditCountSince(
 305+ $launchTimestamp - ( 180*24*3600 ),
 306+ $wgUser,
 307+ $launchTimestamp
 308+ ),
 309+ );
 310+ $realData = array(
 311+ 'id' => $wgUser->getId(),
 312+ 'metrics' => $metrics,
 313+ 'hash' => md5( $salt . serialize( $metrics ) ),
 314+ );
 315+
 316+ if (
 317+ in_array( 'sysop', $groups )
 318+ || ( $metrics['editcounts'] >= 300 && $metrics['last6monthseditcount'] >= 20 )
 319+ || ( $metrics['duration'] < 30 )
 320+ ) {
 321+ $data = $realData;
 322+ } else {
 323+ $data = false;
 324+ }
 325+ }
 326+
 327+ $wgMemc->set( $cacheKey, $data, strtotime( '+10 days' ) );
 328+ }
 329+
 330+ $vars['wgNoticeBanner_Harvard2011'] = $data;
 331+
 332+ }
 333+
253334 return true;
254335 }
255336

Sign-offs

UserFlagDate
Kaldariinspected00:22, 6 December 2011

Follow-up revisions

RevisionCommit summaryAuthorDate
r105241[HarvardResearch][CentralNotice] Fix wgNoticeBanner_Harvard2011...krinkle23:02, 5 December 2011
r105267[HarvardResearch] Swap ternary operator in registration fallback to zero...krinkle01:02, 6 December 2011
r105450[HarvardResearch] Update output according new protocol and banner-javascript...krinkle19:05, 7 December 2011
r105588CentralNotice MFT r105239, 105241, r105267, r105450, r105452, r105453, r105586reedy20:57, 8 December 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r104622[UserDailyContribs] Add basetimestamp parameter...krinkle23:52, 29 November 2011

Comments

#Comment by Krinkle (talk | contribs)   22:55, 5 December 2011

Requires r104622

#Comment by Kaldari (talk | contribs)   00:22, 6 December 2011

Looks good. My only suggestion is that you probably don't need to refetch the username since it's already available in wgUserName. Marking inspected. I'll let someone else sanity check for DB load before marking OK.

#Comment by Kaldari (talk | contribs)   00:25, 6 December 2011
$registrationDate = !$wgUser->getRegistration() ? 0 : $wgUser->getRegistration();

Wouldn't this be more readable as:

$registrationDate = $wgUser->getRegistration() ? $wgUser->getRegistration() : 0;
#Comment by Krinkle (talk | contribs)   01:02, 6 December 2011

Agreed.

Taken from ApiUserDailyContribs.php (originally like this, broken and fixed).

Fixed in r105267.

Status & tagging log