Skip to content

Commit

Permalink
Ensure latest config is used on each enableButtonClickTracking call
Browse files Browse the repository at this point in the history
  • Loading branch information
greg-el committed Nov 28, 2023
1 parent db2f319 commit 24eb26f
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 25 deletions.
28 changes: 16 additions & 12 deletions plugins/browser-plugin-button-click-tracking/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@ import { DynamicContext, CommonEventProperties, resolveDynamicContext } from '@s

import { createEventFromButton, filterFunctionFromFilter } from './util';
import { buildButtonClick } from './core';
import { ButtonClickEvent, ButtonClickTrackingConfiguration, Config, FilterFunction } from './types';
import { ButtonClickEvent, ButtonClickTrackingConfiguration, FilterFunction } from './types';

const _trackers: Record<string, BrowserTracker> = {};
const _config: Record<string, Config> = {};

// The event listeners added to the document, for each tracker
// This allows them to be removed with `removeEventListener` if button click tracking is disabled
const _listeners: Record<string, (event: MouseEvent) => void> = {};

/**
* Button click tracking
Expand Down Expand Up @@ -47,20 +50,19 @@ export function enableButtonClickTracking(
configuration: ButtonClickTrackingConfiguration = {},
trackers: Array<string> = Object.keys(_trackers)
) {
// Ensure that click tracking uses the latest configuration
// In the case of `enableButtonClickTracking` being called multiple times in a row
disableButtonClickTracking();

trackers.forEach((trackerId) => {
// Store the configuration for this tracker, if it doesn't already exist
// This allows us to enable click tracking for a tracker if it has been disabled
if (!_config[trackerId]) {
const filter = filterFunctionFromFilter(configuration.filter);
_config[trackerId] = {
filter,
context: configuration.context,
listener: (event: MouseEvent) => eventHandler(event, trackerId, filter, configuration.context),
};
}
_listeners[trackerId] = (event: MouseEvent) => {
eventHandler(event, trackerId, filterFunctionFromFilter(configuration.filter), configuration.context);
};

const addClickListener = () => {
document.addEventListener('click', _config[trackerId].listener);
document.addEventListener('click', _listeners[trackerId]);
};

if (_trackers[trackerId]?.sharedState.hasLoaded) {
Expand All @@ -80,7 +82,9 @@ export function enableButtonClickTracking(
*/
export function disableButtonClickTracking() {
for (const trackerId in _trackers) {
document.removeEventListener('click', _config[trackerId].listener);
if (_listeners[trackerId]) {
document.removeEventListener('click', _listeners[trackerId]);
}
}
}

Expand Down
11 changes: 0 additions & 11 deletions plugins/browser-plugin-button-click-tracking/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,6 @@ export interface ButtonClickTrackingConfiguration {
context?: DynamicContext;
}

// Internal config for a tracker
export type Config = {
// The filter function for this tracker after it has been created with `filterFunctionFromFilter`
filter: FilterFunction;
// The dynamic context for this tracker
context?: DynamicContext;
// The event listener added to the document, for this tracker
// This allows it to be removed with `removeEventListener` if button click tracking is disabled
listener: (event: MouseEvent) => void;
};

/**
* A Button Click event
*
Expand Down
13 changes: 11 additions & 2 deletions trackers/javascript-tracker/test/integration/buttonClick.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ describe('Snowplow Micro integration', () => {
await browser.pause(500);
await (await $('#enabled-click')).click();
await browser.pause(500);

await (await $('#set-multiple-configs')).click();
await browser.pause(500);

await (await $('#final-config')).click();
await browser.pause(500);
};

for (let method of eventMethods) {
Expand Down Expand Up @@ -122,8 +128,6 @@ describe('Snowplow Micro integration', () => {

it('should get button7 after it is added dynamically', async () => {
const ev = makeEvent({ id: 'button7', label: 'TestDynamicButton-' + method }, method);
// get event from log
console.log(log.filter((e) => e.event.unstruct_event.data.data.label === 'TestDynamicButton-' + method));
logContainsButtonClick(ev);
});

Expand All @@ -136,5 +140,10 @@ describe('Snowplow Micro integration', () => {
const ev = makeEvent({ id: 'enabled-click', label: 'EnabledClick' }, method);
logContainsButtonClick(ev);
});

it('should get `final-config` as it is the last config set', () => {
const ev = makeEvent({ id: 'final-config', classes: ['final-config'], label: 'Final Config' }, method);
logContainsButtonClick(ev);
});
});
});
11 changes: 11 additions & 0 deletions trackers/javascript-tracker/test/pages/button-click-tracking.html
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,17 @@
<button id="enable" onclick="snowplow('enableButtonClickTracking')">Enable</button>
<button id="enabled-click">EnabledClick</button>

<!-- Ensuring the final config is used after multiple calls to `enableButtonClickTracking` -->
<button id="set-multiple-configs" onClick="setMultipleConfigs">Set Multiple Configs</button>
<button id="final-config" class="final-config">Final Config</button>

<script>
function setMultipleConfigs() {
enableButtonClickTracking({ denylist: ['final-config'] });
enableButtonClickTracking();
}
</script>

<script>
var collector_endpoint = document.cookie.split('container=')[1].split(';')[0];
var testIdentifier = document.cookie.split('testIdentifier=')[1].split(';')[0].trim();
Expand Down

0 comments on commit 24eb26f

Please sign in to comment.