eventLogging: Add missing *Hover properties

Action changes:
* Include the namespace ID in LINK_DWELL.

Reducer changes:
* Add the createEvent helper, which adds the pageTitleHover and
  namespaceIdHover properties in the event.
* Create "dismissed", "dwelledButAbandoned", and "opened" events using
  the createEvent helper.

Additional changes:
* Store the target page's associated mw.Title using jQuery.
* Update the eventLogging reducer's test cases so that all LINK_DWELL
  representations include title and namespaceID attributes.
* Create the createStubTitle factory function, which returns a minimal
  usable mw.Title stub, and use it in the actions and integration test
  cases.

Bug: T164256
Change-Id: I8a63d82a65324680dff9176020a8ea97695428c4
This commit is contained in:
Sam Smith 2017-05-09 19:41:23 +01:00
parent 7774e6312a
commit 35bf613964
10 changed files with 159 additions and 46 deletions

Binary file not shown.

Binary file not shown.

View file

@ -93,7 +93,9 @@ actions.boot = function (
* @return {Redux.Thunk}
*/
actions.fetch = function ( gateway, el, token ) {
var title = $( el ).data( 'page-previews-title' );
var title = $( el ).data( 'page-previews-title' ),
titleText = title.getPrefixedText(),
namespaceID = title.namespace;
return function ( dispatch ) {
var request;
@ -101,10 +103,11 @@ actions.fetch = function ( gateway, el, token ) {
dispatch( timedAction( {
type: types.FETCH_START,
el: el,
title: title
title: titleText,
namespaceID: namespaceID
} ) );
request = gateway.getPageSummary( title )
request = gateway.getPageSummary( titleText )
.then( function ( result ) {
dispatch( timedAction( {
type: types.FETCH_END,

View file

@ -36,10 +36,13 @@ function processLinks( $container, blacklist, config ) {
if ( !titleText ) {
return false;
}
// Is titleText in a content namespace?
title = mw.Title.newFromText( titleText );
if ( title && ( $.inArray( title.namespace, contentNamespaces ) >= 0 ) ) {
$( this ).data( 'page-previews-title', titleText );
$( this ).data( {
'page-previews-title': title
} );
return true;
}

View file

@ -29,6 +29,25 @@ function getBaseData( bootAction ) {
return result;
}
/**
* Takes data specific to the action and adds the following properties:
*
* * `linkInteractionToken`;
* * `pageTitleHover` and `namespaceIdHover`.
*
* @param {Object} interaction
* @param {Object} actionData Data specific to the action, e.g. see
* `createClosingEvent`
* @return {Object}
*/
function createEvent( interaction, actionData ) {
actionData.linkInteractionToken = interaction.token;
actionData.pageTitleHover = interaction.title;
actionData.namespaceIdHover = interaction.namespaceID;
return actionData;
}
/**
* Creates an event that, when mixed into the base data (see `getBaseData`),
* represents the user abandoning a link or preview.
@ -44,8 +63,7 @@ function getBaseData( bootAction ) {
* @return {Object|undefined}
*/
function createClosingEvent( interaction ) {
var result = {
linkInteractionToken: interaction.token,
var actionData = {
totalInteractionTime: Math.round( interaction.finished - interaction.started )
};
@ -57,14 +75,14 @@ function createClosingEvent( interaction ) {
// instrumentation, then the preview has been dismissed by the user
// rather than the user has abandoned the link.
if ( interaction.timeToPreviewShow !== undefined ) {
result.action = 'dismissed';
result.previewType = interaction.previewType;
result.perceivedWait = interaction.timeToPreviewShow;
actionData.action = 'dismissed';
actionData.previewType = interaction.previewType;
actionData.perceivedWait = interaction.timeToPreviewShow;
} else {
result.action = 'dwelledButAbandoned';
actionData.action = 'dwelledButAbandoned';
}
return result;
return createEvent( interaction, actionData );
}
/**
@ -182,6 +200,8 @@ module.exports = function ( state, action ) {
// this and the preview reducer.
interaction: {
link: action.el,
title: action.title,
namespaceID: action.namespaceID,
token: action.token,
started: action.timestamp,
@ -205,11 +225,10 @@ module.exports = function ( state, action ) {
interaction: nextState( state.interaction, {
finalized: true
} ),
event: {
event: createEvent( state.interaction, {
action: 'opened',
linkInteractionToken: state.interaction.token,
totalInteractionTime: Math.round( action.timestamp - state.interaction.started )
}
} )
} );
case actionTypes.ABANDON_START:

View file

@ -1,5 +1,5 @@
var mock = require( 'mock-require' ),
createStubUser = require( './stubs' ).createStubUser,
stubs = require( './stubs' ),
actions = require( '../../src/actions' ),
mw = mediaWiki;
@ -11,7 +11,7 @@ QUnit.module( 'ext.popups/actions' );
QUnit.test( '#boot', function ( assert ) {
var config = new Map(), /* global Map */
stubUser = createStubUser( /* isAnon = */ true ),
stubUser = stubs.createStubUser( /* isAnon = */ true ),
stubUserSettings,
action;
@ -89,13 +89,16 @@ function teardownWait() {
}
/**
* Sets up an `a` element that can be passed to action creators.
* Sets up a link/mw.Title stub pair that can be passed to the linkDwell action
* creator.
*
* @param {Object} module
*/
function setupEl( module ) {
var title = stubs.createStubTitle( 1, 'Foo' );
module.el = $( '<a>' )
.data( 'page-previews-title', 'Foo' )
.data( 'page-previews-title', title )
.eq( 0 );
}
@ -302,13 +305,16 @@ QUnit.test( 'it should fetch data from the gateway immediately', function ( asse
assert.ok( this.gateway.getPageSummary.calledWith( 'Foo' ) );
assert.ok(
this.dispatch.calledWith( {
assert.ok( this.dispatch.calledOnce );
assert.deepEqual(
this.dispatch.getCall( 0 ).args[ 0 ],
{
type: 'FETCH_START',
el: this.el,
title: 'Foo',
namespaceID: 1,
timestamp: this.now
} ),
},
'It dispatches the FETCH_START action immediately.'
);
} );

View file

@ -2,7 +2,9 @@ var mock = require( 'mock-require' ),
Redux = require( 'redux' ),
ReduxThunk = require( 'redux-thunk' ),
wait = require( '../../src/wait' ),
mw = mediaWiki;
mw = mediaWiki,
stubs = require( './stubs' ),
$ = jQuery;
function identity( x ) { return x; }
function constant( x ) { return function () { return x; }; }
@ -44,7 +46,8 @@ function constant( x ) { return function () { return x; }; }
QUnit.module( 'ext.popups preview @integration', {
beforeEach: function () {
var that = this,
reducers, actions, registerChangeListener;
reducers, actions, registerChangeListener,
title;
// The worst-case implementation of mw.now.
mw.now = function () { return Date.now(); };
@ -80,6 +83,11 @@ QUnit.module( 'ext.popups preview @integration', {
return registerChangeListener( that.store, fn );
};
title = stubs.createStubTitle( 1, 'Foo' );
this.el = $( '<a href="/wiki/Foo">' )
.data( 'page-previews-title', title );
this.actions.boot(
/* isEnabled: */
constant( true ),
@ -172,21 +180,23 @@ QUnit.test( 'in INACTIVE state, a link dwell switches it to ACTIVE', function (
};
this.actions.linkDwell(
'element', 'event',
this.el, 'event',
gateway,
constant( 'pagetoken' )
);
state = this.store.getState();
assert.equal( state.preview.activeLink, 'element', 'It has an active link' );
assert.equal( state.preview.activeLink, this.el, 'It has an active link' );
assert.equal( state.preview.shouldShow, false, 'Initializes with NO_DATA' );
} );
QUnit.test( 'in ACTIVE state, fetch end switches it to DATA', function ( assert ) {
var store = this.store,
done = assert.async();
this.dwellAndShowPreview( 'element', 'event', 42 ).then( function () {
done = assert.async(),
el = this.el;
this.dwellAndShowPreview( el, 'event', 42 ).then( function () {
var state = store.getState();
assert.equal( state.preview.activeLink, 'element' );
assert.equal( state.preview.activeLink, el );
assert.equal( state.preview.shouldShow, true, 'Should show when data has been fetched' );
done();
} );
@ -194,9 +204,11 @@ QUnit.test( 'in ACTIVE state, fetch end switches it to DATA', function ( assert
QUnit.test( 'in ACTIVE state, abandon start, and then end, switch it to INACTIVE', function ( assert ) {
var that = this,
done = assert.async();
this.dwellAndShowPreview( 'element', 'event', 42 ).then( function () {
return that.abandonAndWait( 'element' );
done = assert.async(),
el = this.el;
this.dwellAndShowPreview( el, 'event', 42 ).then( function () {
return that.abandonAndWait( el );
} ).then( function () {
var state = that.store.getState();
assert.equal( state.preview.activeLink, undefined, 'After abandoning, preview is back to INACTIVE' );
@ -206,29 +218,32 @@ QUnit.test( 'in ACTIVE state, abandon start, and then end, switch it to INACTIVE
QUnit.test( 'in ACTIVE state, abandon link, and then dwell preview, should keep it active after all delays', function ( assert ) {
var that = this,
done = assert.async();
this.dwellAndPreviewDwell( 'element', 'event', 42 )
done = assert.async(),
el = this.el;
this.dwellAndPreviewDwell( el, 'event', 42 )
.then( function () {
var state = that.store.getState();
assert.equal( state.preview.activeLink, 'element' );
assert.equal( state.preview.activeLink, el );
done();
} );
} );
QUnit.test( 'in ACTIVE state, abandon link, hover preview, back to link, should keep it active after all delays', function ( assert ) {
var that = this,
done = assert.async();
done = assert.async(),
el = this.el;
this.dwellAndPreviewDwell( 'element', 'event', 42 )
this.dwellAndPreviewDwell( el, 'event', 42 )
.then( function () {
var abandonPreviewDeferred, dwellPromise, dwellDeferred;
// Start abandoning the preview
that.abandonPreview( 'element' );
that.abandonPreview( el );
abandonPreviewDeferred = that.waitDeferred;
// Dwell back into the link, new event is triggered
dwellPromise = that.dwell( 'element', 'event2', 42 );
dwellPromise = that.dwell( el, 'event2', 42 );
dwellDeferred = that.waitDeferred;
// Preview abandon happens next, before the fetch
@ -241,7 +256,7 @@ QUnit.test( 'in ACTIVE state, abandon link, hover preview, back to link, should
} )
.then( function () {
var state = that.store.getState();
assert.equal( state.preview.activeLink, 'element' );
assert.equal( state.preview.activeLink, el );
done();
} );
} );

View file

@ -92,11 +92,17 @@ QUnit.test( 'it should not return links without valid namespace', function ( ass
} );
QUnit.test( 'it should return only valid links', function ( assert ) {
var $links,
$link,
title;
// Valid link
this.getTitle.withArgs( 'link', this.config ).returns( 'title' );
window.mediaWiki.Title.newFromText.withArgs( 'title' ).returns( {
title = {
namespace: 5
} );
};
this.getTitle.withArgs( 'link', this.config ).returns( 'title' );
window.mediaWiki.Title.newFromText.withArgs( 'title' ).returns( title );
// Invalid link because of namespace
this.$container.add( '<a href="link2" title="title">Banana</a>' );
@ -108,8 +114,15 @@ QUnit.test( 'it should return only valid links', function ( assert ) {
// Valid namespaces for content
this.config.set( 'wgContentNamespaces', [ 5 ] );
assert.deepEqual(
processLinks( this.$container, [], this.config ).length,
1
$links = processLinks( this.$container, [], this.config );
assert.strictEqual( $links.length, 1 );
$link = $links.eq( 0 );
assert.strictEqual(
$link.data( 'page-previews-title' ),
title,
'It should store the title object using jQuery\'s data method.'
);
} );

View file

@ -164,6 +164,8 @@ QUnit.test( 'LINK_DWELL starts an interaction', function ( assert ) {
action = {
type: 'LINK_DWELL',
el: this.link,
title: 'Foo',
namespaceID: 1,
token: '0987654321',
timestamp: Date.now()
};
@ -173,6 +175,8 @@ QUnit.test( 'LINK_DWELL starts an interaction', function ( assert ) {
{
interaction: {
link: action.el,
title: 'Foo',
namespaceID: 1,
token: action.token,
started: action.timestamp,
@ -195,6 +199,8 @@ QUnit.test( 'LINK_DWELL doesn\'t start a new interaction under certain condition
action = {
type: 'LINK_DWELL',
el: this.link,
title: 'Foo',
namespaceID: 1,
token: '0987654321',
timestamp: now
};
@ -210,6 +216,8 @@ QUnit.test( 'LINK_DWELL doesn\'t start a new interaction under certain condition
state.interaction,
{
link: action.el,
title: 'Foo',
namespaceID: 1,
token: '0987654321',
started: now,
@ -230,6 +238,8 @@ QUnit.test(
state = eventLogging( undefined, {
type: 'LINK_DWELL',
el: this.link,
title: 'Foo',
namespaceID: 1,
token: token,
timestamp: now
} );
@ -242,6 +252,8 @@ QUnit.test(
state = eventLogging( state, {
type: 'LINK_DWELL',
el: $( '<a>' ),
title: 'Bar',
namespaceID: 1,
token: '1234567890',
timestamp: now + 500
} );
@ -249,6 +261,8 @@ QUnit.test(
assert.deepEqual(
state.event,
{
pageTitleHover: 'Foo',
namespaceIdHover: 1,
linkInteractionToken: '0987654321',
totalInteractionTime: 250, // 250 - 0
action: 'dwelledButAbandoned'
@ -260,6 +274,8 @@ QUnit.test(
state = eventLogging( undefined, {
type: 'LINK_DWELL',
el: this.link,
title: 'Foo',
namespaceID: 1,
token: token,
timestamp: now
} );
@ -272,6 +288,8 @@ QUnit.test(
state = eventLogging( state, {
type: 'LINK_DWELL',
el: $( '<a>' ),
title: 'Bar',
namespaceID: 1,
token: 'banana',
timestamp: now + 500
} );
@ -297,6 +315,8 @@ QUnit.test( 'LINK_CLICK should enqueue an "opened" event', function ( assert ) {
expectedState = state = eventLogging( state, {
type: 'LINK_DWELL',
el: this.link,
title: 'Foo',
namespaceID: 1,
token: token,
timestamp: now
} );
@ -311,6 +331,8 @@ QUnit.test( 'LINK_CLICK should enqueue an "opened" event', function ( assert ) {
state.event,
{
action: 'opened',
pageTitleHover: 'Foo',
namespaceIdHover: 1,
linkInteractionToken: token,
totalInteractionTime: 250
},
@ -338,6 +360,8 @@ QUnit.test( 'PREVIEW_SHOW should update the perceived wait time of the interacti
state = eventLogging( state, {
type: 'LINK_DWELL',
el: this.link,
title: 'Foo',
namespaceID: 1,
token: token,
timestamp: now
} );
@ -350,6 +374,8 @@ QUnit.test( 'PREVIEW_SHOW should update the perceived wait time of the interacti
assert.deepEqual( state.interaction, {
link: this.link,
title: 'Foo',
namespaceID: 1,
token: token,
started: now,
@ -442,7 +468,10 @@ QUnit.test( 'ABANDON_END', function ( assert ) {
action = {
type: 'LINK_DWELL',
el: this.link,
token: '1234567890'
title: 'Foo',
namespaceID: 1,
token: '1234567890',
timestamp: Date.now()
};
state = eventLogging( state, action );
@ -510,6 +539,8 @@ QUnit.test( 'ABANDON_END should enqueue an event', function ( assert ) {
dwelledState = eventLogging( undefined, {
type: 'LINK_DWELL',
el: this.link,
title: 'Foo',
namespaceID: 1,
token: token,
timestamp: now
} );
@ -528,6 +559,8 @@ QUnit.test( 'ABANDON_END should enqueue an event', function ( assert ) {
assert.deepEqual(
state.event,
{
pageTitleHover: 'Foo',
namespaceIdHover: 1,
linkInteractionToken: token,
totalInteractionTime: 500,
action: 'dwelledButAbandoned'
@ -563,6 +596,8 @@ QUnit.test( 'ABANDON_END should enqueue an event', function ( assert ) {
assert.deepEqual(
state.event,
{
pageTitleHover: 'Foo',
namespaceIdHover: 1,
linkInteractionToken: token,
totalInteractionTime: 850,
action: 'dismissed',
@ -585,6 +620,8 @@ QUnit.test( 'ABANDON_END doesn\'t enqueue an event under certain conditions', fu
dwelledState = eventLogging( undefined, {
type: 'LINK_DWELL',
el: this.link,
title: 'Foo',
namespaceID: 1,
token: token,
timestamp: now
} );

View file

@ -48,3 +48,20 @@ exports.createStubExperiments = function createStubExperiments( isSampled ) {
}
};
};
/**
* Creates a **minimal** stub that can be used in place of an instance of
* `mw.Title`.
*
* @param {Number} namespace
* @param {String} prefixedText e.g. Foo, or File:Bar.jpg
* @return {Object}
*/
exports.createStubTitle = function createStubTitle( namespace, prefixedText ) {
return {
namespace: namespace,
getPrefixedText: function () {
return prefixedText;
}
};
};