From 76b8c18ca52d01c9815a6d6f66fca8f60801cfb9 Mon Sep 17 00:00:00 2001 From: Sam Smith Date: Tue, 11 Apr 2017 13:13:54 +0100 Subject: [PATCH] reducers: Make PREVIEW_SHOW require a token Like the FETCH_COMPLETE and ABANDON_END actions, the PREVIEW_SHOW action was delayed but not conditionally reduced. As ABANDON_END is delayed, there's a potential for a race and if ABANDON_END is reduced before PREVIEW_SHOW, then there's no interaction to reduce the action into, which causes an error, e.g. T159490#3165276 and T162373. Making PREVIEW_SHOW require a token stops the error occurring in this scenario. An alternative would be to clear the timeout created in ext.popups.Preview#show in #hide. However, this would be inconsistent with actions#fetch and actions#abandon. Bug: T159490 Change-Id: Ibd2c0c6f45e4392582cc6ed08517f6ca1146d57a --- resources/dist/index.js | Bin 120501 -> 120530 bytes resources/dist/index.js.map | Bin 154639 -> 154677 bytes src/reducers/eventLogging.js | 3 ++- .../node-qunit/reducers/eventLogging.test.js | 19 ++++++++++++------ 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/resources/dist/index.js b/resources/dist/index.js index b6ef992857b90f2a8489fd99e90a814bd733f75f..6ed449d5a55c935740fa507e84808f2f7bc44912 100644 GIT binary patch delta 38 wcmV+>0NMYwtq0Pr2Y|Ez^gxsUq6L?cL;)BE3JD2aI|`SPRRJ!yC_w>@l`u^W$^ZZW delta 17 Zcmcb#mVN75_J%EtUmUgzIx-%d3IIv%2e|+M diff --git a/resources/dist/index.js.map b/resources/dist/index.js.map index a58e4916f1928b1b2e05d1585d12a90a0e12bf59..4729e82aa67059b002aa3cd2a8df0ba4560c20a7 100644 GIT binary patch delta 133 zcmeC*!MSw@=L8243rhnd delta 114 zcmdnGgR_4J=L83lRD(oIixf)>lN1YMBU3}mWP?PD$&QRt8}rPaCMN`oY~JV8z{FJM zH@VSNW^#ZA$L18*$0}HLHAiP{kIrH&^Wlogiz$gIiM5W&o349<(R{n{c1EV9084cx AXaE2J diff --git a/src/reducers/eventLogging.js b/src/reducers/eventLogging.js index 431e91f7d..501393d6f 100644 --- a/src/reducers/eventLogging.js +++ b/src/reducers/eventLogging.js @@ -84,7 +84,8 @@ module.exports = function ( state, action ) { var nextCount, actionTypesWithTokens = [ actionTypes.FETCH_COMPLETE, - actionTypes.ABANDON_END + actionTypes.ABANDON_END, + actionTypes.PREVIEW_SHOW ]; if ( state === undefined ) { diff --git a/tests/node-qunit/reducers/eventLogging.test.js b/tests/node-qunit/reducers/eventLogging.test.js index d807204cd..9d93911e8 100644 --- a/tests/node-qunit/reducers/eventLogging.test.js +++ b/tests/node-qunit/reducers/eventLogging.test.js @@ -111,7 +111,8 @@ QUnit.test( 'EVENT_LOGGED', function ( assert ) { QUnit.test( 'PREVIEW_SHOW', function ( assert ) { var state, count = 22, - expectedCount = count + 1; + expectedCount = count + 1, + token = '1234567890'; state = { previewCount: count, @@ -121,11 +122,14 @@ QUnit.test( 'PREVIEW_SHOW', function ( assert ) { event: undefined, // state.interaction.started is used in this part of the reducer. - interaction: {} + interaction: { + token: token + } }; state = eventLogging( state, { - type: 'PREVIEW_SHOW' + type: 'PREVIEW_SHOW', + token: token } ); assert.equal( @@ -284,7 +288,8 @@ QUnit.test( 'LINK_CLICK should enqueue an "opened" event', function ( assert ) { QUnit.test( 'PREVIEW_SHOW should update the perceived wait time of the interaction', function ( assert ) { var state, - now = Date.now(); + now = Date.now(), + token = '1234567890'; state = { interaction: undefined @@ -293,18 +298,19 @@ QUnit.test( 'PREVIEW_SHOW should update the perceived wait time of the interacti state = eventLogging( state, { type: 'LINK_DWELL', el: this.link, - token: '0987654321', + token: token, timestamp: now } ); state = eventLogging( state, { type: 'PREVIEW_SHOW', + token: token, timestamp: now + 500 } ); assert.deepEqual( state.interaction, { link: this.link, - token: '0987654321', + token: token, started: now, isUserDwelling: true, @@ -493,6 +499,7 @@ QUnit.test( 'ABANDON_END should enqueue an event', function ( assert ) { state = eventLogging( dwelledState, { type: 'PREVIEW_SHOW', + token: token, timestamp: now + 700 } );