[MediaWiki-commits] [Gerrit] Log events using mw#track - change (mediawiki...MobileFrontend)

2015-09-10 Thread Phuedx (Code Review)
Phuedx has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/237360

Change subject: Log events using mw#track
..

Log events using mw#track

MobileFrontend or, more specifically, anything that invokes the
Schema#log method, shouldn't explicitly depend on EventLogging as
currently as it's a soft dependency.

The EventLoggingHooks::onResourceLoaderRegisterModules documentation
recommends that extensions with a soft dependency on EventLogging use
mw#track.

Change-Id: I9f9195b26e9d99efd2a45b4febbfbdf509c7d15f
---
M resources/mobile.startup/Schema.js
M tests/qunit/mobile.startup/test_Schema.js
2 files changed, 45 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/MobileFrontend 
refs/changes/60/237360/1

diff --git a/resources/mobile.startup/Schema.js 
b/resources/mobile.startup/Schema.js
index 106981f..4da91ac 100644
--- a/resources/mobile.startup/Schema.js
+++ b/resources/mobile.startup/Schema.js
@@ -110,23 +110,28 @@
this.defaults = defaults;
Class.prototype.initialize.apply( this, arguments );
},
+
/**
-* Actually log event via EventLogging
+* Actually log event via the EventLogging subscriber.
+*
+* Since we have a soft dependency on the EventLogging 
extension, we use the
+* `mw#track` method to log events to reduce coupling between 
the two extensions.
+*
 * @method
 * @param {Object} data to log
 * @return {jQuery.Deferred}
 */
log: function ( data ) {
-   if ( mw.eventLog ) {
-   // Log event if logging schema is not sampled 
or if user is in the bucket
-   if ( !this.isSampled || this._isUserInBucket() 
) {
-   return mw.eventLog.logEvent( this.name, 
$.extend( {}, this.defaults, data ) );
-   } else {
-   return $.Deferred().reject( 'User not 
in event sampling bucket.' );
-   }
-   } else {
-   return $.Deferred().reject( 'EventLogging not 
installed.' );
+   var deferred = $.Deferred();
+
+   // Log event if logging schema is not sampled or if 
user is in the bucket
+   if ( !this.isSampled || this._isUserInBucket() ) {
+   mw.track( 'event.' + this.name, $.extend( {}, 
this.defaults, data ) );
+
+   return deferred.resolve();
}
+
+   return deferred.reject();
},
 
/**
diff --git a/tests/qunit/mobile.startup/test_Schema.js 
b/tests/qunit/mobile.startup/test_Schema.js
index 707aa0a..90d8cd3 100644
--- a/tests/qunit/mobile.startup/test_Schema.js
+++ b/tests/qunit/mobile.startup/test_Schema.js
@@ -70,4 +70,34 @@
assert.strictEqual( testSchema._isUserInBucket(), false, 'user 
is not in bucket' );
} );
 
+   QUnit.test( '#log', 2, function ( assert ) {
+   var schema = new TestSchema(),
+   event = {
+   foo: 'bar'
+   };
+
+   // Restore Schema#log as we intend to stub the mw#track method.
+   TestSchema.prototype.log.restore();
+   this.sandbox.stub( mw, 'track' ).returns( undefined );
+
+   schema.log( event ).then( function () {
+   assert.deepEqual(
+   [ 'event.test', event ],
+   mw.track.firstCall.args,
+   '#log invokes mw#track and immediately resolves'
+   );
+   } );
+
+   schema.isSampled = true;
+   this.sandbox.stub( schema, '_isUserInBucket' ).returns( false );
+
+   schema.log( event ).fail( function () {
+   assert.strictEqual(
+   1,
+   mw.track.callCount,
+   '#log doesn\'t invoke mw#track and rejects if 
the user isn\'t in the bucket.'
+   );
+   } );
+   } );
+
 }( jQuery, mw.mobileFrontend ) );

-- 
To view, visit https://gerrit.wikimedia.org/r/237360
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I9f9195b26e9d99efd2a45b4febbfbdf509c7d15f
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: master
Gerrit-Owner: Phuedx 

___
MediaWiki-commits 

[MediaWiki-commits] [Gerrit] Log events using mw#track - change (mediawiki...MobileFrontend)

2015-09-10 Thread jenkins-bot (Code Review)
jenkins-bot has submitted this change and it was merged.

Change subject: Log events using mw#track
..


Log events using mw#track

MobileFrontend or, more specifically, anything that invokes the
Schema#log method, shouldn't explicitly depend on EventLogging as
currently as it's a soft dependency.

The EventLoggingHooks::onResourceLoaderRegisterModules documentation
recommends that extensions with a soft dependency on EventLogging use
mw#track.

Change-Id: I9f9195b26e9d99efd2a45b4febbfbdf509c7d15f
---
M resources/mobile.startup/Schema.js
M tests/qunit/mobile.startup/test_Schema.js
2 files changed, 45 insertions(+), 10 deletions(-)

Approvals:
  Jdlrobson: Looks good to me, but someone else must approve
  Bmansurov: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/resources/mobile.startup/Schema.js 
b/resources/mobile.startup/Schema.js
index 106981f..4da91ac 100644
--- a/resources/mobile.startup/Schema.js
+++ b/resources/mobile.startup/Schema.js
@@ -110,23 +110,28 @@
this.defaults = defaults;
Class.prototype.initialize.apply( this, arguments );
},
+
/**
-* Actually log event via EventLogging
+* Actually log event via the EventLogging subscriber.
+*
+* Since we have a soft dependency on the EventLogging 
extension, we use the
+* `mw#track` method to log events to reduce coupling between 
the two extensions.
+*
 * @method
 * @param {Object} data to log
 * @return {jQuery.Deferred}
 */
log: function ( data ) {
-   if ( mw.eventLog ) {
-   // Log event if logging schema is not sampled 
or if user is in the bucket
-   if ( !this.isSampled || this._isUserInBucket() 
) {
-   return mw.eventLog.logEvent( this.name, 
$.extend( {}, this.defaults, data ) );
-   } else {
-   return $.Deferred().reject( 'User not 
in event sampling bucket.' );
-   }
-   } else {
-   return $.Deferred().reject( 'EventLogging not 
installed.' );
+   var deferred = $.Deferred();
+
+   // Log event if logging schema is not sampled or if 
user is in the bucket
+   if ( !this.isSampled || this._isUserInBucket() ) {
+   mw.track( 'event.' + this.name, $.extend( {}, 
this.defaults, data ) );
+
+   return deferred.resolve();
}
+
+   return deferred.reject();
},
 
/**
diff --git a/tests/qunit/mobile.startup/test_Schema.js 
b/tests/qunit/mobile.startup/test_Schema.js
index 707aa0a..90d8cd3 100644
--- a/tests/qunit/mobile.startup/test_Schema.js
+++ b/tests/qunit/mobile.startup/test_Schema.js
@@ -70,4 +70,34 @@
assert.strictEqual( testSchema._isUserInBucket(), false, 'user 
is not in bucket' );
} );
 
+   QUnit.test( '#log', 2, function ( assert ) {
+   var schema = new TestSchema(),
+   event = {
+   foo: 'bar'
+   };
+
+   // Restore Schema#log as we intend to stub the mw#track method.
+   TestSchema.prototype.log.restore();
+   this.sandbox.stub( mw, 'track' ).returns( undefined );
+
+   schema.log( event ).then( function () {
+   assert.deepEqual(
+   [ 'event.test', event ],
+   mw.track.firstCall.args,
+   '#log invokes mw#track and immediately resolves'
+   );
+   } );
+
+   schema.isSampled = true;
+   this.sandbox.stub( schema, '_isUserInBucket' ).returns( false );
+
+   schema.log( event ).fail( function () {
+   assert.strictEqual(
+   1,
+   mw.track.callCount,
+   '#log doesn\'t invoke mw#track and rejects if 
the user isn\'t in the bucket.'
+   );
+   } );
+   } );
+
 }( jQuery, mw.mobileFrontend ) );

-- 
To view, visit https://gerrit.wikimedia.org/r/237360
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I9f9195b26e9d99efd2a45b4febbfbdf509c7d15f
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: master
Gerrit-Owner: Phuedx 
Gerrit-Reviewer: Bmansurov 
Gerrit-Reviewer: