r111001 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r111000‎ | r111001 | r111002 >
Date:01:05, 9 February 2012
Author:werdna
Status:resolved (Comments)
Tags:acw-deploy 
Comment:
ClickTracking: Refactor into a single method, allow overriding the namespace
Modified paths:
  • /trunk/extensions/ClickTracking/modules/jquery.clickTracking.js (modified) (history)

Diff [purge]

Index: trunk/extensions/ClickTracking/modules/jquery.clickTracking.js
——@@ -25,33 +25,50 @@
2626 * @param {string} id event identifier
2727 */
2828 $.trackAction = function( id ) {
29 - $.post(
30 - mw.config.get( 'wgScriptPath' ) + '/api.php', {
31 - 'action': 'clicktracking',
32 - 'format' : 'json',
33 - 'namespacenumber': mw.config.get( 'wgNamespaceNumber' ),
34 - 'eventid': id,
35 - 'token': $.cookie( 'clicktracking-session' )
36 - }
37 - );
 29+ $.trackActionWithOptions( { 'id' : id });
3830 };
3931 /**
4032 * Performs click tracking API call
41 - *
 33+ *
4234 * @param {string} id event identifier
4335 * @param {string} info additional information to be stored with the click
4436 */
4537 $.trackActionWithInfo = function( id, info ) {
46 - $.post(
47 - mw.config.get( 'wgScriptPath' ) + '/api.php', {
 38+ $.trackActionWithOptions( { 'id' : id, 'info' : info });
 39+ };
 40+
 41+ /**
 42+ * Performs click tracking API call
 43+ *
 44+ * @param {map} options Data to submit. Valid keys: id, namespace, info, token
 45+ */
 46+ $.trackActionWithOptions = function( options ) {
 47+ options = $.extend( {
 48+ 'namespace' : mw.config.get( 'wgNamespaceNumber' ),
 49+ 'token' : $.cookie( 'clicktracking-session' )
 50+ }, options);
 51+
 52+ if ( ! options.id ) {
 53+ $.error("You must specify an event ID");
 54+ return;
 55+ }
 56+
 57+ var data = {
4858 'action': 'clicktracking',
4959 'format' : 'json',
50 - 'eventid': id,
51 - 'namespacenumber': mw.config.get( 'wgNamespaceNumber' ),
52 - 'token': $.cookie( 'clicktracking-session' ),
53 - 'additional': info
54 - }
55 - );
 60+ 'eventid': options.id,
 61+ 'token': options.token
 62+ };
 63+
 64+ if ( typeof options.namespace != 'undefined' ) {
 65+ data.namespacenumber = options.namespace;
 66+ }
 67+
 68+ if ( typeof options.info != 'undefined' ) {
 69+ data.additional = options.info;
 70+ }
 71+
 72+ $.post( mw.config.get( 'wgScriptPath' ) + '/api.php', data);
5673 };
5774
5875 /**

Follow-up revisions

RevisionCommit summaryAuthorDate
r111013Apply changes per CR r111001: direct comparisons with undefined, Error instea...werdna01:24, 9 February 2012

Comments

#Comment by Catrope (talk | contribs)   01:22, 9 February 2012 
+			$.error("You must specify an event ID");

$.error( 'foo' ) just does throw foo; . You're better off doing throw new Error( 'foo' ); because that generates a proper error, and Firebug shows a line number and a link to the code in that case. 

+		if ( typeof options.namespace != 'undefined' ) {

One of Timo's pet peeves is that people should use foo === undefined rather than typeof foo == 'undefined' , because the latter is implemented with an actual string comparison. 

+		$.post( mw.config.get( 'wgScriptPath' ) + '/api.php', data);

You should use mw.util.wikiScript( 'api' ) to get the path to api.php (or api.php5!)

OK otherwise.

Status & tagging log