From 24eb26f8525955abc716b71e2f5e21e8eacfa6ee Mon Sep 17 00:00:00 2001
From: Greg Leonard <45019882+greg-el@users.noreply.github.com>
Date: Tue, 28 Nov 2023 13:41:30 +0000
Subject: [PATCH] Ensure latest config is used on each
 `enableButtonClickTracking` call

---
 .../src/api.ts                                | 28 +++++++++++--------
 .../src/types.ts                              | 11 --------
 .../test/integration/buttonClick.test.ts      | 13 +++++++--
 .../test/pages/button-click-tracking.html     | 11 ++++++++
 4 files changed, 38 insertions(+), 25 deletions(-)

diff --git a/plugins/browser-plugin-button-click-tracking/src/api.ts b/plugins/browser-plugin-button-click-tracking/src/api.ts
index aa027c600..9b8e94b1f 100644
--- a/plugins/browser-plugin-button-click-tracking/src/api.ts
+++ b/plugins/browser-plugin-button-click-tracking/src/api.ts
@@ -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
@@ -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) {
@@ -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]);
+    }
   }
 }
 
diff --git a/plugins/browser-plugin-button-click-tracking/src/types.ts b/plugins/browser-plugin-button-click-tracking/src/types.ts
index d87305bd3..936e7b7b8 100644
--- a/plugins/browser-plugin-button-click-tracking/src/types.ts
+++ b/plugins/browser-plugin-button-click-tracking/src/types.ts
@@ -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
  *
diff --git a/trackers/javascript-tracker/test/integration/buttonClick.test.ts b/trackers/javascript-tracker/test/integration/buttonClick.test.ts
index c9440a81f..a4a879510 100644
--- a/trackers/javascript-tracker/test/integration/buttonClick.test.ts
+++ b/trackers/javascript-tracker/test/integration/buttonClick.test.ts
@@ -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) {
@@ -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);
     });
 
@@ -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);
+    });
   });
 });
diff --git a/trackers/javascript-tracker/test/pages/button-click-tracking.html b/trackers/javascript-tracker/test/pages/button-click-tracking.html
index 7ca858d6c..a543dc315 100644
--- a/trackers/javascript-tracker/test/pages/button-click-tracking.html
+++ b/trackers/javascript-tracker/test/pages/button-click-tracking.html
@@ -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();