r86577 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r86576‎ | r86577 | r86578 >
Date:23:58, 20 April 2011
Author:tparscal
Status:resolved (Comments)
Tags:
Comment:
Added generic bucketing feature to the user object, which handles bucketing users, invalidating bucketing on version changes, tracking and and persistence.
Modified paths:
  • /trunk/phase3/resources/mediawiki/mediawiki.js (modified) (history)

Diff [purge]

Index: trunk/phase3/resources/mediawiki/mediawiki.js
@@ -409,6 +409,82 @@
410410 $.cookie( 'mediaWiki.user.id', id, { 'expires': 365, 'path': '/' } );
411411 return id;
412412 };
 413+
 414+ /**
 415+ * Gets the user's bucket, placing them in one at random based on set odds if needed.
 416+ *
 417+ * @param key String: Name of bucket
 418+ * @param options Object: Bucket configuration options
 419+ * @param options.buckets Object: List of bucket-name/relative-probability pairs (required,
 420+ * must have at least one pair)
 421+ * @param options.version Number: Version of bucket test, changing this forces rebucketing
 422+ * (optional, default: 0)
 423+ * @param options.tracked Boolean: Track the event of bucketing through the API module of
 424+ * the ClickTracking extension (optional, default: false)
 425+ * @param options.expires Number: Length of time (in days) until the user gets rebucketed
 426+ * (optional, default: 30)
 427+ * @return String: Bucket name - the randomly chosen key of the options.buckets object
 428+ *
 429+ * @example
 430+ * mw.user.bucket( 'test', {
 431+ * 'buckets': { 'ignored': 50, 'control': 25, 'test': 25 },
 432+ * 'version': 1,
 433+ * 'tracked': true,
 434+ * 'expires': 7
 435+ * } );
 436+ */
 437+ this.bucket = function( key, options ) {
 438+ options = $.extend( {
 439+ 'buckets': {},
 440+ 'version': 0,
 441+ 'tracked': false,
 442+ 'expires': 30
 443+ }, options || {} );
 444+ var cookie = $.cookie( 'mw.user.bucket-' + key );
 445+ var bucket = null;
 446+ var version = 0;
 447+ // Bucket information is stored as 2 integers, together as version:bucket like: "1:2"
 448+ if ( typeof cookie === 'string' && cookie.length > 2 && cookie.indexOf( ':' ) > 0 ) {
 449+ var parts = cookie.split( ':' );
 450+ if ( parts.length > 1 && parts[1] == options.version ) {
 451+ version = Number( parts[0] );
 452+ bucket = Number( parts[1] );
 453+ }
 454+ }
 455+ if ( bucket === null ) {
 456+ if ( !$.isPlainObject( options.buckets ) ) {
 457+ throw 'Invalid buckets error. Object expected for options.buckets .';
 458+ }
 459+ version = Number( options.version );
 460+ // Find range
 461+ var range = 0;
 462+ for ( var k in options.buckets ) {
 463+ range += options.buckets[k];
 464+ }
 465+ // Select random value within range
 466+ var rand = Math.random() * range;
 467+ // Determine which bucket the value landed in
 468+ var total = 0;
 469+ for ( var k in options.buckets ) {
 470+ bucket = k;
 471+ total += options.buckets[k];
 472+ if ( total >= rand ) {
 473+ break;
 474+ }
 475+ }
 476+ if ( options.tracked ) {
 477+ mw.loader.using( 'jquery.clickTracking', function() {
 478+ $.trackAction( 'mw.user.bucket-' + key + '@' + version + ':' + bucket );
 479+ } );
 480+ }
 481+ $.cookie(
 482+ 'mw.userBuckets-' + key,
 483+ version + ':' + bucket,
 484+ { 'path': '/', 'expires': Number( options.expires ) }
 485+ );
 486+ }
 487+ return bucket;
 488+ };
413489 }
414490
415491 /* Public Members */

Follow-up revisions

RevisionCommit summaryAuthorDate
r86579Fixed mismatched cookie names, changed to using ":" instead of "-" between co...tparscal00:02, 21 April 2011
r870201.17wmf1: MFT r86577, r86579, r86657catrope17:30, 27 April 2011

Comments

#Comment by Brion VIBBER (talk | contribs)   18:01, 22 April 2011

Hmm, this revision is marked as resolved but has no follow-ups and no description of the problem or its fix.

#Comment by Trevor Parscal (WMF) (talk | contribs)   18:02, 22 April 2011

Maybe roan ticked the wrong box?

#Comment by Trevor Parscal (WMF) (talk | contribs)   18:04, 22 April 2011

Hah, of course... see r86579 (i've now associated the revisions)

#Comment by Mdale (talk | contribs)   21:43, 9 May 2011

Could we consider putting this User class stuff into a user class file? Maybe something like mediawiki.User for example? The core should ideally be as thin as possible no? This way stand alone usage of resource loading is possible without so much extra stuff. ( also see bug 26799 )

#Comment by Trevor Parscal (WMF) (talk | contribs)   23:22, 12 May 2011

It's certainly possible to break it out for cleanliness, and possibly load it only when needed - we don't really have any clear objectives here other than "make it really small" but there's a certain value to having certain things available at all times.

#Comment by Mdale (talk | contribs)   03:54, 13 May 2011

I don't think anyone is against having it available at all times within mediaWiki interface pages. Just that the user class should have its own file and be loaded similar to $.cookie on every page view.

#Comment by Trevor Parscal (WMF) (talk | contribs)   06:33, 14 May 2011

Seems sane enough to me, I have no problem with breaking these things out. There's also a fair amount of loader and message code - should we break that stuff out too? What should actually be in this file? Where should we draw the line?

#Comment by Mdale (talk | contribs)   09:39, 14 May 2011

Well to "load" subsequent modules we need the "loader code" in there. Basic message code is also kind of core in the sense the when you load modules they have associated messages. Extended message parsing is already a separate module ( presently hosted in upload wizard and MwEmbedSupport extensions, but would be good to move them into a module in core resources but not copy it into mediawiki.js ;). This way interface modules that need extended message parsing can set it as a dependency.

Status & tagging log