Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions spec/src/modules/tracker.js
Original file line number Diff line number Diff line change
Expand Up @@ -16345,4 +16345,39 @@ describe(`ConstructorIO - Tracker${bundledDescriptionSuffix}`, () => {
).to.equal(true);
});
});

describe('additionalTrackingKeys', () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also add a test for verifying the body of the requests are the same in the duplicate request?

it('Should send tracking events to both primary and additional keys', (done) => {
const additionalKey = 'extra-test-key';
const { tracker } = new ConstructorIO({
apiKey: testApiKey,
fetch: fetchSpy,
...requestQueueOptions,
additionalTrackingKeys: [additionalKey],
});

let callCount = 0;

const checkComplete = () => {
callCount += 1;

if (callCount === 2) {
expect(fetchSpy).to.have.been.calledTwice;
Comment thread
constructor-claude-bedrock[bot] marked this conversation as resolved.
Comment thread
constructor-claude-bedrock[bot] marked this conversation as resolved.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important Issue: The integration test relies on fetchSpy.getCall(0) and getCall(1) being the two calls from this test. But fetchSpy is a module-level variable re-assigned in each beforeEach — it is not reset between describe blocks. If a prior test in the suite has already recorded calls on the spy, getCall(0) and getCall(1) may refer to the wrong invocations.

Use fetchSpy.args relative to the current test context, or (preferred) create a dedicated spy scoped to this test:

const localSpy = sinon.spy(fetch);
const { tracker } = new ConstructorIO({
  apiKey: testApiKey,
  fetch: localSpy,
  ...
});
// then assert localSpy.getCall(0) / localSpy.getCall(1)

Also, calledTwice is asserted inside a callback that fires only once (when callCount === 2). If the first call triggers an error and second triggers success (or vice versa), getCall(0).args[0] may be the secondary-key request, not the primary. Asserting on url.includes for both keys (without assuming order) would make this more robust.


const firstCallUrl = fetchSpy.getCall(0).args[0];
const secondCallUrl = fetchSpy.getCall(1).args[0];

expect(firstCallUrl).to.contain(`key=${testApiKey}`);
expect(secondCallUrl).to.contain(`key=${additionalKey}`);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also check for not.contain(testApiKey) here?


done();
}
};

tracker.on('success', checkComplete);
tracker.on('error', checkComplete);

expect(tracker.trackSessionStartV2()).to.equal(true);
});
});
});
115 changes: 115 additions & 0 deletions spec/src/utils/request-queue.js
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,121 @@ describe('ConstructorIO - Utils - Request Queue', function utilsRequestQueue() {
});
});

describe('additionalTrackingKeys', () => {
let defaultAgent;
let cleanup;

before(() => {
helpers.clearStorage();
});

beforeEach(() => {
global.CLIENT_VERSION = 'cio-mocha';
cleanup = jsdom();
defaultAgent = window.navigator.userAgent;
});

afterEach(() => {
window.navigator.__defineGetter__('userAgent', () => defaultAgent);
delete global.CLIENT_VERSION;
cleanup();
helpers.clearStorage();
});

it('Should add duplicate requests for each additional tracking key', () => {
store.session.set(humanityStorageKey, true);
const requests = new RequestQueue({
sendTrackingEvents: true,
trackingSendDelay: 1,
apiKey: 'primary-key',
additionalTrackingKeys: ['extra-key-1', 'extra-key-2'],
});

requests.queue('https://ac.cnstrc.com/behavior?action=session_start&key=primary-key&_dt=123', 'POST', { action: 'session_start', key: 'primary-key' });

const queue = RequestQueue.get();
expect(queue).to.be.an('array').length(3);

// Primary request
expect(queue[0].url).to.contain('key=primary-key');
expect(queue[0].body.key).to.equal('primary-key');

// First additional key
expect(queue[1].url).to.contain('key=extra-key-1');
expect(queue[1].url).to.not.contain('key=primary-key');
expect(queue[1].body.key).to.equal('extra-key-1');
Comment thread
constructor-claude-bedrock[bot] marked this conversation as resolved.
Comment thread
constructor-claude-bedrock[bot] marked this conversation as resolved.

// Second additional key
expect(queue[2].url).to.contain('key=extra-key-2');
expect(queue[2].url).to.not.contain('key=primary-key');
expect(queue[2].body.key).to.equal('extra-key-2');
});

it('Should not add duplicates when additionalTrackingKeys is an empty array', () => {
store.session.set(humanityStorageKey, true);
const requests = new RequestQueue({
sendTrackingEvents: true,
trackingSendDelay: 1,
apiKey: 'primary-key',
additionalTrackingKeys: [],
});

requests.queue('https://ac.cnstrc.com/behavior?action=session_start&key=primary-key&_dt=123');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important Issue: The GET test ('Should add duplicate requests for GET method') asserts queue[1].body.key equals 'extra-key-1', but the primary request was queued with no explicit body argument (defaults to {}). The assertion passes only because the duplicated entry spreads {} and then sets key. However, this conflates the GET-path body handling with POST semantics. The test should also assert queue[0].body to be {} (or undefined) to document the intended contract for GET requests, making the asymmetry visible and intentional.


expect(RequestQueue.get()).to.be.an('array').length(1);
});

it('Should not add duplicates when additionalTrackingKeys is not provided', () => {
store.session.set(humanityStorageKey, true);
const requests = new RequestQueue({
sendTrackingEvents: true,
trackingSendDelay: 1,
apiKey: 'primary-key',
});

requests.queue('https://ac.cnstrc.com/behavior?action=session_start&key=primary-key&_dt=123');

expect(RequestQueue.get()).to.be.an('array').length(1);
});

it('Should add duplicate requests for GET method', () => {
store.session.set(humanityStorageKey, true);
const requests = new RequestQueue({
sendTrackingEvents: true,
trackingSendDelay: 1,
apiKey: 'primary-key',
additionalTrackingKeys: ['extra-key-1'],
});

requests.queue('https://ac.cnstrc.com/behavior?action=session_start&key=primary-key&_dt=123');

const queue = RequestQueue.get();
expect(queue).to.be.an('array').length(2);

expect(queue[0].url).to.contain('key=primary-key');
expect(queue[1].url).to.contain('key=extra-key-1');
expect(queue[1].url).to.contain('action=session_start');
expect(queue[1].body.key).to.equal('extra-key-1');
});

it('Should skip invalid entries in additionalTrackingKeys', () => {
store.session.set(humanityStorageKey, true);
const requests = new RequestQueue({
sendTrackingEvents: true,
trackingSendDelay: 1,
apiKey: 'primary-key',
additionalTrackingKeys: ['valid-key', '', null, 123, 'another-valid-key'],
});

requests.queue('https://ac.cnstrc.com/behavior?action=session_start&key=primary-key&_dt=123', 'POST', { action: 'session_start', key: 'primary-key' });

const queue = RequestQueue.get();
expect(queue).to.be.an('array').length(3);
expect(queue[1].url).to.contain('key=valid-key');
expect(queue[2].url).to.contain('key=another-valid-key');
});
});

describe('send', () => {
let fetchSpy = null;
let cleanup;
Expand Down
3 changes: 3 additions & 0 deletions src/constructorio.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ class ConstructorIO {
* @param {object} [parameters.networkParameters] - Parameters relevant to network requests
* @param {number} [parameters.networkParameters.timeout] - Request timeout (in milliseconds) - may be overridden within individual method calls
* @param {string} [parameters.humanityCheckLocation='session'] - Storage location for the humanity check flag ('session' for sessionStorage, 'local' for localStorage)
* @param {string[]} [parameters.additionalTrackingKeys] - Additional API keys to duplicate tracking events to

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am fine with the current version as well but this might explain this concept a bit further

Suggested change
* @param {string[]} [parameters.additionalTrackingKeys] - Additional API keys to duplicate tracking events to
* @param {string[]} [parameters.additionalTrackingKeys] - Additional API keys that each receive a duplicate of every tracking event. Events are always sent to the primary `apiKey` and, in addition, to every key in this
array.```

* @property {object} search - Interface to {@link module:search}
* @property {object} browse - Interface to {@link module:browse}
* @property {object} autocomplete - Interface to {@link module:autocomplete}
Expand Down Expand Up @@ -93,6 +94,7 @@ class ConstructorIO {
beaconMode,
networkParameters,
humanityCheckLocation,
additionalTrackingKeys,
Comment thread
constructor-claude-bedrock[bot] marked this conversation as resolved.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: additionalTrackingKeys is passed through to this.options as-is without any normalisation (no filtering of non-strings, no deduplication). The filtering is deferred to RequestQueue.queue(). This is a valid design choice, but consider documenting it or — better — normalising in ConstructorIO constructor so that this.options.additionalTrackingKeys always contains only valid, deduplicated strings. This prevents surprises if other code paths ever read the option directly without going through RequestQueue.

} = options;
Comment on lines +97 to 98

if (!apiKey || typeof apiKey !== 'string') {
Expand Down Expand Up @@ -141,6 +143,7 @@ class ConstructorIO {
beaconMode: (beaconMode === false) ? false : true, // Defaults to 'true',
networkParameters: networkParameters || {},
humanityCheckLocation: humanityCheckLocation || 'session',
additionalTrackingKeys,
};

// Expose global modules
Expand Down
1 change: 1 addition & 0 deletions src/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export interface ConstructorClientOptions {
beaconMode?: boolean;
networkParameters?: NetworkParameters;
humanityCheckLocation?: 'session' | 'local';
additionalTrackingKeys?: string[];
}

export interface RequestFeature extends Record<string, any> {
Expand Down
22 changes: 22 additions & 0 deletions src/utils/request-queue.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,28 @@ class RequestQueue {
body,
networkParameters,
});

// Duplicate request for each additional tracking key
const additionalKeys = this.options?.additionalTrackingKeys;

if (Array.isArray(additionalKeys) && additionalKeys.length) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big deal but can we:

  1. Make this array into a set to remove duplicates
  2. Check the keys in here against the original key to make sure we don't duplicate events

const encodedOriginalKey = helpers.encodeURIComponentRFC3986(this.options.apiKey);

const validKeys = additionalKeys.filter((key) => key && typeof key === 'string');

validKeys.forEach((additionalKey) => {
const encodedKey = helpers.encodeURIComponentRFC3986(additionalKey);
const swappedUrl = url.replace(`key=${encodedOriginalKey}`, `key=${encodedKey}`);

queue.push({
url: obfuscatePiiRequest(swappedUrl),
method,
body: { ...body, key: additionalKey },
networkParameters,
});
});
}

RequestQueue.set(queue);
}
}
Expand Down
Loading