Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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 GET requests without a body', () => {
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
2 changes: 2 additions & 0 deletions src/constructorio.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,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 +142,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 = encodeURIComponent(this.options.apiKey);

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

validKeys.forEach((additionalKey) => {
const encodedKey = encodeURIComponent(additionalKey);
const swappedUrl = url.replace(`key=${encodedOriginalKey}`, `key=${encodedKey}`);
Comment thread
Copilot marked this conversation as resolved.
Outdated

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

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