Skip to content

Commit

Permalink
Fix regressions caught by tests
Browse files Browse the repository at this point in the history
  • Loading branch information
philipwalton committed Aug 20, 2018
1 parent 6d55c73 commit 1f795ca
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 13 deletions.
16 changes: 13 additions & 3 deletions lib/plugins/page-visibility-tracker.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ class PageVisibilityTracker {

this.visibleThresholdTimeout_ = setTimeout(() => {
this.store.update(change);
this.sendPageview({pageviewTime: time});
this.sendPageview({pageviewTime: time, sessionDidExpire: true});
}, this.opts.visibleThreshold);
}
} else {
Expand Down Expand Up @@ -318,9 +318,13 @@ class PageVisibilityTracker {

/**
* Sends a pageview, optionally calculating an offset if time is passed.
* @param {{pageviewTime: (number), isPageLoad: (boolean|undefined)}} param1
* @param {{
* pageviewTime: (number),
* isPageLoad: (boolean|undefined),
* sessionDidExpire: (boolean|undefined),
* }} param1
*/
sendPageview({pageviewTime, isPageLoad}) {
sendPageview({pageviewTime, isPageLoad, sessionDidExpire}) {
this.queue.add(() => {
/** @type {FieldsObj} */
const defaultFields = {
Expand All @@ -335,6 +339,12 @@ class PageVisibilityTracker {
this.tracker.send('pageview',
createFieldsObj(defaultFields, this.opts.fieldsObj,
this.tracker, this.opts.hitFilter));

// If the session expired, sending a new pageview will generate a new
// session ID. We need to make sure the store has that updated ID.
if (sessionDidExpire) {
this.store.update({sessionId: this.session.id});
}
});
}

Expand Down
5 changes: 3 additions & 2 deletions lib/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,9 @@ function removeStorageListener() {
* @param {!Event} event The DOM event.
*/
function storageListener(event) {
const store = instances[event.key].value;
if (store) {
// Only care about storage events for keys matching stores in instances.
if (event.key in instances) {
const store = instances[event.key].value;
const oldData = assign({}, store.defaults_, parse(event.oldValue));
const newData = assign({}, store.defaults_, parse(event.newValue));

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"bin": "./bin/autotrack",
"scripts": {
"build": "node -r esm ./node_modules/.bin/gulp js:lib",
"build": "node -r esm ./node_modules/.bin/gulp lint",
"lint": "node -r esm ./node_modules/.bin/gulp lint",
"start": "node -r esm ./node_modules/.bin/gulp watch",
"test": "node -r esm ./node_modules/.bin/gulp test",
"selenium": "node -r esm ./node_modules/.bin/gulp selenium",
Expand Down
35 changes: 28 additions & 7 deletions test/e2e/page-visibility-tracker-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1080,10 +1080,7 @@ function clearStorage() {
* Manually expires the session.
*/
function expireSession() {
setStoreData('autotrack:UA-12345-1:session', {
isExpired: true,
hitTime: +new Date,
});
updateStoreData('autotrack:UA-12345-1:session', {isExpired: true});
}


Expand All @@ -1092,16 +1089,15 @@ function expireSession() {
* store data gets out of sync with the session store.
*/
function corruptSession() {
setStoreData('autotrack:UA-12345-1:session', {
updateStoreData('autotrack:UA-12345-1:session', {
id: 'new-id',
hitTime: +new Date,
isExpired: false,
});
}


/**
* Manually set a value for store in all open windows/tabs.
* Manually set a value for a store in all open windows/tabs.
* @param {string} key
* @param {!Object} value
*/
Expand All @@ -1125,6 +1121,31 @@ function setStoreData(key, value) {
}


/**
* Merges an object with the data in an existing store.
* @param {string} key
* @param {!Object} value
*/
function updateStoreData(key, value) {
browser.execute((key, value) => {
const oldValue = window.localStorage.getItem(key);
const newValue = JSON.stringify(Object.assign(JSON.parse(oldValue), value));

// IE11 doesn't support event constructors.
try {
// Set the value on localStorage so it triggers the storage event in
// other tabs. Also, manually dispatch the event in this tab since just
// writing to localStorage won't update the locally cached values.
window.localStorage.setItem(key, newValue);
window.dispatchEvent(
new StorageEvent('storage', {key, oldValue, newValue}));
} catch (err) {
// Do nothing
}
}, key, value);
}


/**
* Since function objects can't be passed via parameters from server to
* client, this one-off function must be used to set the value for
Expand Down
18 changes: 18 additions & 0 deletions test/unit/store-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,24 @@ describe('Store', () => {
store1.destroy();
store2.destroy();
});

it('is not invoked when the storage event fires for other keys', () => {
const store = Store.getOrCreate('UA-12345-1', 'ns1');

sandbox.spy(store, 'update');

// Simulate a storage event, meaning a `set()` call was made in
// another tab.
dispatchStorageEvent({
key: 'other-key',
oldValue: '',
newValue: JSON.stringify({foo: 12, bar: 34}),
});

assert(store.update.notCalled);

store.destroy();
});
});

describe('clear', () => {
Expand Down

0 comments on commit 1f795ca

Please sign in to comment.