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 <g...@samsmith.io>
Gerrit-Reviewer: Bmansurov <bmansu...@wikimedia.org>
Gerrit-Reviewer: Jdlrobson <jrob...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
