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