Jdlrobson has uploaded a new change for review.
https://gerrit.wikimedia.org/r/315583
Change subject: Log events in one central place
......................................................................
Log events in one central place
We calculate totalInteractionTime in every single log event. It's no
wonder we're seeing errors in our data.
This change moves the logging into one central place.
Callers are now dumb and pass just an action and additional data to
log.
This is still not perfect but it should help us manage totalInteractionTime
time better.
Change-Id: Ia9f4ae6e343c9114c9fde7d9e98899d4eb43998a
---
M resources/ext.popups.core.js
M resources/ext.popups.renderer.article.js
M resources/ext.popups.renderer/desktopRenderer.js
M resources/ext.popups.settings.js
M resources/ext.popups.targets/desktopTarget.js
5 files changed, 33 insertions(+), 33 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Popups
refs/changes/83/315583/1
diff --git a/resources/ext.popups.core.js b/resources/ext.popups.core.js
index 20a2054..b86aa22 100644
--- a/resources/ext.popups.core.js
+++ b/resources/ext.popups.core.js
@@ -35,6 +35,25 @@
'.cancelLink a'
];
+
+ /**
+ * Log events
+ *
+ * @method log
+ * @param {String} action that is being logged
+ * @param {Object} [logData]
+ */
+ mw.popups.log = function ( action, logData ) {
+ logData = logData || {};
+ mw.track( 'ext.popups.schemaPopups',
+ $.extend( {}, logData, {
+ action: action,
+ // Some events e.g. pageLoaded do not have an
interaction time.
+ totalInteractionTime: logData.dwellStartTime ?
Math.round( mw.now() - logData.dwellStartTime ) : undefined
+ } )
+ );
+ };
+
/**
* Temporarily remove the title attribute of the links so that
* the yellow tooltips don't show up alongside the Hovercard.
diff --git a/resources/ext.popups.renderer.article.js
b/resources/ext.popups.renderer.article.js
index 12b03c8..b38527d 100644
--- a/resources/ext.popups.renderer.article.js
+++ b/resources/ext.popups.renderer.article.js
@@ -68,11 +68,11 @@
currentRequest.fail( function ( textStatus, data ) {
// only log genuine errors, not client aborts
if ( data.textStatus !== 'abort' ) {
- mw.track( 'ext.popups.schemaPopups', $.extend(
logData, {
- action: 'error',
- errorState: textStatus,
- totalInteractionTime: Math.round(
mw.now() - logData.dwellStartTime )
- } ) );
+ mw.popups.log( 'error',
+ $.extend( logData, {
+ errorState: textStatus
+ } )
+ );
}
deferred.reject();
} )
@@ -552,10 +552,7 @@
mw.popups.settings.open( $.extend( {}, logData ) );
- mw.track( 'ext.popups.schemaPopups', $.extend( logData,
{
- action: 'tapped settings cog',
- totalInteractionTime: Math.round( mw.now() -
logData.dwellStartTime )
- } ) );
+ mw.popups.log( 'tapped settings cog', logData );
} );
if ( !flippedY && !tall && cache.settings.thumbnail.height <
SIZES.landscapeImage.h ) {
diff --git a/resources/ext.popups.renderer/desktopRenderer.js
b/resources/ext.popups.renderer/desktopRenderer.js
index 0907d3e..7b574d5 100644
--- a/resources/ext.popups.renderer/desktopRenderer.js
+++ b/resources/ext.popups.renderer/desktopRenderer.js
@@ -31,20 +31,14 @@
* @param {Object} event
*/
function logClickAction( event ) {
- mw.track( 'ext.popups.schemaPopups', $.extend( {}, logData, {
- action: mw.popups.getAction( event ),
- totalInteractionTime: Math.round( mw.now() -
logData.dwellStartTime )
- } ) );
+ mw.popups.log( mw.popups.getAction( event ), logData );
}
/**
* Logs when a popup is dismissed
*/
function logDismissAction() {
- mw.track( 'ext.popups.schemaPopups', $.extend( {}, logData, {
- action: 'dismissed',
- totalInteractionTime: Math.round( mw.now() -
logData.dwellStartTime )
- } ) );
+ mw.popups.log( 'dismissed', logData );
}
/**
* @class mw.popups.render
@@ -350,7 +344,7 @@
if ( event.keyCode === 27 && $activeLink ) {
mw.popups.render.closePopup( logDismissAction );
}
- };
+ }
/**
* Closes the box after a delay
@@ -382,10 +376,7 @@
logData.linkInteractionToken &&
mw.now() - logData.dwellStartTime >=
mw.popups.render.DWELL_EVENTS_MIN_INTERACTION_TIME
) {
- mw.track( 'ext.popups.schemaPopups', $.extend( {},
logData, {
- action: 'dwelledButAbandoned',
- totalInteractionTime: Math.round( mw.now() -
logData.dwellStartTime )
- } ) );
+ mw.popups.log( 'dwelledButAbandoned', logData );
}
// TODO: should `blur` also be here?
$activeLink.off( 'mouseleave', leaveInactive );
diff --git a/resources/ext.popups.settings.js b/resources/ext.popups.settings.js
index 1d23274..df93370 100644
--- a/resources/ext.popups.settings.js
+++ b/resources/ext.popups.settings.js
@@ -85,9 +85,7 @@
mw.popups.saveEnabledState( false );
$( '#mwe-popups-settings-form, #mwe-popups-settings
.save' ).hide();
$( '#mwe-popups-settings-help, #mwe-popups-settings
.okay' ).show();
- mw.track( 'ext.popups.schemaPopups', $.extend( {},
currentLinkLogData, {
- action: 'disabled'
- } ) );
+ mw.popups.log( 'disabled', currentLinkLogData );
}
};
diff --git a/resources/ext.popups.targets/desktopTarget.js
b/resources/ext.popups.targets/desktopTarget.js
index ceb4592..3982147 100644
--- a/resources/ext.popups.targets/desktopTarget.js
+++ b/resources/ext.popups.targets/desktopTarget.js
@@ -23,10 +23,8 @@
if ( data.dwellStartTime && data.linkInteractionToken &&
mw.now() - data.dwellStartTime >=
mw.popups.render.DWELL_EVENTS_MIN_INTERACTION_TIME
) {
- mw.track( 'ext.popups.schemaPopups', {
+ mw.popups.log( 'dwelledButAbandoned', {
pageTitleHover: $this.attr( 'title' ),
- action: 'dwelledButAbandoned',
- totalInteractionTime: Math.round( mw.now() -
data.dwellStartTime ),
linkInteractionToken: data.linkInteractionToken,
hovercardsSuppressedByGadget:
data.hovercardsSuppressedByGadget
} );
@@ -47,10 +45,8 @@
$this.off( 'click', onLinkClick );
- mw.track( 'ext.popups.schemaPopups', {
+ mw.popups.log( action, {
pageTitleHover: $this.attr( 'title' ),
- action: action,
- totalInteractionTime: Math.round( mw.now() -
data.dwellStartTime ),
linkInteractionToken: data.linkInteractionToken,
hovercardsSuppressedByGadget:
data.hovercardsSuppressedByGadget
} );
@@ -186,8 +182,7 @@
initPopups();
}
- mw.track( 'ext.popups.schemaPopups', {
- action: 'pageLoaded',
+ mw.popups.log( 'pageLoaded', {
hovercardsSuppressedByGadget:
isNavigationPopupsGadgetEnabled()
} );
} );
--
To view, visit https://gerrit.wikimedia.org/r/315583
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia9f4ae6e343c9114c9fde7d9e98899d4eb43998a
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Popups
Gerrit-Branch: master
Gerrit-Owner: Jdlrobson <jrob...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
