Use mw.eventLog.Schema to log EventLogging events

This change is an intermediate step in our transition
to logging a variety of events easily.

Also:
  * Some events may need sendBeacon support and may not be
    logged if the navigator does not support sendBeacon.
  * Do not load schema code if EventLogging is not available.

Bug: T131315
Change-Id: Iff939577f65f1c6c71701dd6967939445385fb70

Popups.hooks.php

@@ -64,14 +64,28 @@ class PopupsHooks {
 			'ext.popups.targets.desktopTarget',
 		);
 
-		// If EventLogging is present, add the schema as a dependency.
-		if ( class_exists( 'ResourceLoaderSchemaModule' ) ) {
-			$moduleDependencies[] = "schema.Popups";
+		// Create a schema module and add it as a dependency of `ext.popups.desktop`.
+		$schemaPopups = [
+			'remoteExtPath' => 'Popups',
+			'localBasePath' => __DIR__,
+			'targets' => [ 'desktop' ],
+		];
+
+		if ( class_exists( 'EventLogging' ) ) {
+			$schemaPopups += [
+				'dependencies' => [
+					'schema.Popups',
+				],
+				'scripts' => [
+					'resources/ext.popups.schemaPopups.js',
+				]
+			];
 		}
+		$rl->register('ext.popups.schemaPopups', $schemaPopups );
+		$moduleDependencies[] = 'ext.popups.schemaPopups';
 
 		$rl->register( "ext.popups.desktop", array(
 			'scripts' => array(
-				'resources/ext.popups.logger.js',
 				'resources/ext.popups.renderer.article.js',
 				'resources/ext.popups.disablenavpop.js',
 				'resources/ext.popups.settings.js',
@@ -206,7 +220,6 @@ class PopupsHooks {
 			'scripts' => array(
 				'tests/qunit/ext.popups.renderer.article.test.js',
 				'tests/qunit/ext.popups.core.test.js',
-				'tests/qunit/ext.popups.logger.test.js',
 				'tests/qunit/ext.popups.settings.test.js',
 			),
 			'dependencies' => array( 'ext.popups.desktop' ),
@@ -222,5 +235,6 @@ class PopupsHooks {
 	public static function onResourceLoaderGetConfigVars( array &$vars ) {
 		$conf = ConfigFactory::getDefaultInstance()->makeConfig( 'popups' );
 		$vars['wgPopupsSurveyLink'] = $conf->get( 'PopupsSurveyLink' );
+		$vars['wgPopupsSchemaPopupsSamplingRate'] = $conf->get( 'SchemaPopupsSamplingRate' );
 	}
 }

extension.json

@@ -47,7 +47,9 @@
 		"PopupsBetaFeature": false,
 		"@PopupsSurveyLink": "@var bool|string: When defined a link will be rendered at the bottom of the popup for the user to provide feedback. The URL must start with https or http. If not, then an error is thrown client-side. The link is annotated with `rel=\"noreferrer\"` so no referrer information or `window.opener` is leaked to the survey hosting site (see https://html.spec.whatwg.org/multipage/semantics.html#link-type-noreferrer for more information).",
 		"PopupsSurveyLink": false,
-		"EnablePopupsMobile": false
+		"EnablePopupsMobile": false,
+		"@SchemaPopupsSamplingRate": "@var number: Sample rate for logging events to Schema:Popups.",
+		"SchemaPopupsSamplingRate": 0.1
 	},
 	"DefaultUserOptions": {
 		"popupsmobile": "1"

resources/ext.popups.core.js

@@ -140,4 +140,25 @@
 			} );
 	};
 
+	/**
+	 * Get action based on click event
+	 *
+	 * @method getAction
+	 * @param {Object} event
+	 * @return {string}
+	 */
+	mw.popups.getAction = function ( event ) {
+		if ( event.which === 2 ) { // middle click
+			return 'opened in new tab';
+		} else if ( event.which === 1 ) {
+			if ( event.ctrlKey || event.metaKey ) {
+				return 'opened in new tab';
+			} else if ( event.shiftKey ) {
+				return 'opened in new window';
+			} else {
+				return 'opened in same tab';
+			}
+		}
+	};
+
 } )( jQuery, mediaWiki );

resources/ext.popups.logger.js

@@ -1,64 +0,0 @@
-( function ( $, mw ) {
-
-	/**
-	 * @class mw.popups.logger
-	 * @singleton
-	 */
-	var logger = {};
-
-	/**
-	 * Sampling rate at which events are logged
-	 * @property samplingRate
-	 */
-	logger.samplingRate = 10;
-
-	/**
-	 * Get action based on click event
-	 *
-	 * @method getAction
-	 * @param {Object} event
-	 * @return {string}
-	 */
-	logger.getAction = function ( event ) {
-		if ( event.which === 2 ) { // middle click
-			return 'opened in new tab';
-		} else if ( event.which === 1 ) {
-			if ( event.ctrlKey || event.metaKey ) {
-				return 'opened in new tab';
-			} else if ( event.shiftKey ) {
-				return 'opened in new window';
-			} else {
-				return 'opened in same tab';
-			}
-		}
-	};
-
-	/**
-	 * Logs the popup event as defined in the following schema-
-	 * https://meta.wikimedia.org/wiki/Schema:Popups
-	 *
-	 * @method log
-	 * @param {Object} event
-	 * @return {jQuery.Promise}
-	 */
-	logger.log = function ( event ) {
-		if (
-			mw.eventLog === undefined ||
-			Math.floor( Math.random() * logger.samplingRate ) !== 0
-		) {
-			return $.Deferred().resolve();
-		}
-
-		// Get duration from  time
-		if ( $.isNumeric( event.time ) ) {
-			event.duration = Math.floor( mw.now() - event.time );
-			// FIXME: the time property should not be sent to the back-end regardless of its value.
-			delete event.time;
-		}
-
-		return mw.eventLog.logEvent( 'Popups', event );
-	};
-
-	mw.popups.logger = logger;
-
-} )( jQuery, mediaWiki );

resources/ext.popups.renderer/desktopRenderer.js

@@ -185,16 +185,13 @@
 		cache.process( link );
 
 		// Event logging
-		if ( mw.popups.logger ) {
-			mw.popups.render.logEvent = {
-				pageTitleHover: cache.settings.title,
-				pageTitleSource: mw.config.get( 'wgTitle' ),
-				popupEnabled: mw.popups.enabled,
-				time: mw.now(),
-				action: 'dismissed'
-			};
-			mw.popups.$popup.find( 'a.mwe-popups-extract, a.mwe-popups-discreet' ).click( mw.popups.render.clickHandler );
-		}
+		mw.popups.logData = {
+			pageTitleHover: cache.settings.title,
+			pageTitleSource: mw.config.get( 'wgTitle' ),
+			popupEnabled: mw.popups.enabled,
+			time: mw.now()
+		};
+		mw.popups.$popup.find( 'a.mwe-popups-extract, a.mwe-popups-discreet' ).click( mw.popups.render.clickHandler );
 
 		link
 			.off( 'mouseleave blur', mw.popups.render.leaveInactive )
@@ -210,12 +207,16 @@
 	 * @param {Object} event
 	 */
 	mw.popups.render.clickHandler = function ( event ) {
-		mw.popups.render.logEvent.action = mw.popups.logger.getAction( event );
-		if ( mw.popups.render.logEvent.action === 'opened in same tab' ) {
-			event.preventDefault();
-			mw.popups.logger.log( mw.popups.render.logEvent ).then( function () {
-				window.location.href = mw.popups.render.currentLink.attr( 'href' );
+		var action = mw.popups.getAction( event ),
+			logData;
+
+		if ( action === 'opened in same tab' ) {
+			logData = $.extend( {}, mw.popups.logData, {
+				action: action
 			} );
+			computeDurationFromTime( logData );
+			mw.track( 'ext.popups.schemaPopups', logData );
+			window.location.href = mw.popups.render.currentLink.attr( 'href' );
 		}
 	};
 
@@ -226,16 +227,18 @@
 	 * @method closePopup
 	 */
 	mw.popups.render.closePopup = function () {
-		var fadeInClass, fadeOutClass;
+		var fadeInClass, fadeOutClass,
+			logData = $.extend( {}, mw.popups.logData, {
+				action: 'dismissed'
+			} );
 
 		if ( mw.popups.render.currentLink === undefined ) {
 			return false;
 		}
 
 		// Event logging
-		if ( mw.popups.logger ) {
-			mw.popups.logger.log( mw.popups.render.logEvent );
-		}
+		computeDurationFromTime( logData );
+		mw.track( 'ext.popups.schemaPopups', logData );
 
 		$( mw.popups.render.currentLink ).off( 'mouseleave blur', mw.popups.render.leaveActive );
 
@@ -347,4 +350,16 @@
 		mw.popups.render.closeTimer = undefined;
 	};
 
+	/**
+	 * Utility function that computes duration from time.
+	 * Modifies the data so that it can be logged with EL.
+	 *
+	 * @ignore
+	 * @param {Object} data
+	 */
+	function computeDurationFromTime( data ) {
+		data.duration = Math.floor( mw.now() - data.time );
+		delete data.time;
+	}
+
 } )( jQuery, mediaWiki );

resources/ext.popups.schemaPopups.js

@@ -0,0 +1,15 @@
+( function ( $, mw ) {
+	/**
+	 * Log the popup event as defined in the schema
+	 *
+	 * https://meta.wikimedia.org/wiki/Schema:Popups
+	 */
+	var schemaPopups = new mw.eventLog.Schema(
+		'Popups',
+		mw.config.get( 'wgPopupsSchemaPopupsSamplingRate', 0 )
+	);
+
+	mw.trackSubscribe( 'ext.popups.schemaPopups', function ( topic, data ) {
+		schemaPopups.log( data );
+	} );
+} )( jQuery, mediaWiki );

resources/ext.popups.targets/desktopTarget.js

@@ -108,28 +108,20 @@
 			// Events are logged even when Hovercards are disabled
 			// See T88166 for details
 			$elements.on( 'click', function ( event ) {
-				var $this, href, action, logEvent, logPromise;
+				var $this = $( this ),
+					action = mw.popups.getAction( event ),
+					href = $this.attr( 'href' );
 
-				if ( mw.popups.logger === undefined ) {
-					return true;
-				}
-
-				$this = $( this );
-				href = $this.attr( 'href' );
-				action = mw.popups.logger.getAction( event );
-				logEvent = {
+				mw.track( 'ext.popups.schemaPopups', {
 					pageTitleHover: $this.attr( 'title' ),
 					pageTitleSource: mw.config.get( 'wgTitle' ),
 					popupEnabled: mw.popups.enabled,
 					action: action
-				};
-				logPromise = mw.popups.logger.log( logEvent );
+				} );
 
 				if ( action  === 'opened in same tab' ) {
 					event.preventDefault();
-					logPromise.then( function () {
-						window.location.href = href;
-					} );
+					window.location.href = href;
 				}
 			} );
 		}

tests/qunit/ext.popups.core.test.js

@@ -161,4 +161,28 @@
 		$link.remove();
 	} );
 
+	QUnit.test( 'getAction', function ( assert ) {
+		var i, expected, actual,
+			// 0 - main button, 1 - middle button
+			cases = [
+				[ { button: 0 }, 'opened in same tab' ],
+				[ { button: 0, ctrlKey: true }, 'opened in new tab' ],
+				[ { button: 0, metaKey: true }, 'opened in new tab' ],
+				[ { button: 0, ctrlKey: true, shiftKey: true }, 'opened in new tab' ],
+				[ { button: 0, metaKey: true, shiftKey: true }, 'opened in new tab' ],
+				[ { button: 0, ctrlKey: true, metaKey: true, shiftKey: true }, 'opened in new tab' ],
+				[ { button: 0, shiftKey: true }, 'opened in new window' ],
+				[ { button: 1 }, 'opened in new tab' ],
+				[ { button: 1, shiftKey: true }, 'opened in new tab' ]
+			];
+
+		QUnit.expect( cases.length );
+
+		for ( i = 0; i < cases.length; i++ ) {
+			expected = cases[ i ][ 1 ];
+			actual = mw.popups.getAction( new MouseEvent( 'CustomEvent', cases[ i ][ 0 ] ) );
+			assert.equal( actual, expected );
+		}
+	} );
+
 } )( jQuery, mediaWiki );

tests/qunit/ext.popups.logger.test.js

@@ -1,133 +0,0 @@
-( function ( $, mw ) {
-	QUnit.module( 'ext.popups.logger' );
-
-	QUnit.test( 'samplingRate', function ( assert ) {
-		QUnit.expect( 1 );
-
-		// make sure the sampling rate is not accidentally changed
-		assert.equal( mw.popups.logger.samplingRate, 10 );
-	} );
-
-	QUnit.test( 'getAction', function ( assert ) {
-		var i, expected, actual,
-			// 0 - main button, 1 - middle button
-			cases = [
-				[ { button: 0 }, 'opened in same tab' ],
-				[ { button: 0, ctrlKey: true }, 'opened in new tab' ],
-				[ { button: 0, metaKey: true }, 'opened in new tab' ],
-				[ { button: 0, ctrlKey: true, shiftKey: true }, 'opened in new tab' ],
-				[ { button: 0, metaKey: true, shiftKey: true }, 'opened in new tab' ],
-				[ { button: 0, ctrlKey: true, metaKey: true, shiftKey: true }, 'opened in new tab' ],
-				[ { button: 0, shiftKey: true }, 'opened in new window' ],
-				[ { button: 1 }, 'opened in new tab' ],
-				[ { button: 1, shiftKey: true }, 'opened in new tab' ]
-			];
-
-		QUnit.expect( cases.length );
-
-		for ( i = 0; i < cases.length; i++ ) {
-			expected = cases[ i ][ 1 ];
-			actual = mw.popups.logger.getAction( new MouseEvent( 'CustomEvent', cases[ i ][ 0 ] ) );
-			assert.equal( actual, expected );
-		}
-	} );
-
-	QUnit.module( 'ext.popups.logger (with EventLogging)', {
-		setup: function () {
-			this.eventLog = mw.eventLog;
-			if ( !mw.eventLog ) {
-				mw.eventLog = { logEvent: $.noop };
-			}
-			this.sandbox.stub( mw.eventLog, 'logEvent' );
-			this.logData = {
-				pageTitleHover: 'Main Page',
-				pageTitleSource: 'Popups test page',
-				popupEnabled: true,
-				action: 'opened in same tab',
-				time: new Date().getTime()
-			};
-		},
-		teardown: function () {
-			mw.eventLog.logEvent.restore();
-			mw.eventLog = this.eventLog;
-		}
-	} );
-
-	QUnit.test( 'log', function ( assert ) {
-		QUnit.expect( 6 );
-
-		// not sampled
-		this.sandbox.stub( Math, 'random' ).returns( 1 );
-		mw.popups.logger.log( $.extend( {}, this.logData ) ).done( function ( result ) {
-			assert.equal(
-				result,
-				undefined,
-				'Logger resolves with `undefined` when the page is not sampled.'
-			);
-		} );
-		Math.random.restore();
-
-		// Sampled
-		this.sandbox.stub( Math, 'random' ).returns( 0 );
-		mw.popups.logger.log( $.extend( {}, this.logData ) );
-		assert.ok(
-			mw.eventLog.logEvent.firstCall.args[ 1 ].hasOwnProperty( 'duration' ),
-			'The `duration` property has been added when `time` is a number.'
-		);
-		assert.notOk(
-			mw.eventLog.logEvent.firstCall.args[ 1 ].hasOwnProperty( 'time' ),
-			'The `time` property has been removed when it is a number.'
-		);
-
-		delete this.logData.time;
-		mw.popups.logger.log( this.logData );
-		assert.notOk(
-			mw.eventLog.logEvent.secondCall.args[ 1 ].hasOwnProperty( 'duration' ),
-			'The `duration` property has not been added when `time` is `undefined`.'
-		);
-
-		this.logData.time = 'September, 2046';
-		mw.popups.logger.log( this.logData );
-		assert.notOk(
-			mw.eventLog.logEvent.thirdCall.args[ 1 ].hasOwnProperty( 'duration' ),
-			'The `duration` property has not been added when `time` is non-numeric.'
-		);
-		assert.ok(
-			mw.eventLog.logEvent.thirdCall.args[ 1 ].hasOwnProperty( 'time' ),
-			'The `time` property has not been removed when it is non-numeric.'
-		);
-		Math.random.restore();
-
-	} );
-
-	QUnit.module( 'ext.popups.logger (without EventLogging)', {
-		setup: function () {
-			this.eventLog = mw.eventLog;
-			delete mw.eventLog;
-			// make sure we're sampled
-			this.sandbox.stub( Math, 'random' ).returns( 0 );
-			this.logData = {
-				pageTitleHover: 'Main Page',
-				pageTitleSource: 'Popups test page',
-				popupEnabled: true,
-				action: 'opened in same tab',
-				time: new Date().getTime()
-			};
-		},
-		teardown: function () {
-			mw.eventLog = this.eventLog;
-			Math.random.restore();
-		}
-	} );
-	QUnit.test( 'log', function ( assert ) {
-		QUnit.expect( 1 );
-
-		mw.popups.logger.log( this.logData ).done( function ( result ) {
-			assert.equal(
-				result,
-				undefined,
-				'Logger resolves with `undefined` when mw.eventLog is not available.'
-			);
-		} );
-	} );
-} )( jQuery, mediaWiki );