Skip to content

[CDX-379] Validate testCells #429

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
esezen merged 5 commits into from
Feb 25, 2026
+217 −12
Merged

[CDX-379] Validate testCells #429

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0e7c6e8
Validate testCells
esezen Feb 11, 2026
f31da08
Update src/utils/helpers.js
esezen Feb 20, 2026
82bc312
Update spec/src/constructorio.js
esezen Feb 20, 2026
7964938
Merge branch 'master' into cdx-379-client-js-add-validation-to-testcells
esezen Feb 20, 2026
b044467
Fix issue originating from conflict
esezen Feb 20, 2026
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
63 changes: 53 additions & 10 deletions spec/src/constructorio.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,49 @@ describe(`ConstructorIO${bundledDescriptionSuffix}`, () => {
expect(instance.options).to.have.property('testCells').to.deep.equal(newTestCells);
});

it('Should filter out non-string testCell values in constructor', () => {
const testCells = {
valid: 'bar',
nullVal: null,
numVal: 123,
objVal: { nested: 'value' },
emptyStr: '',
boolean: true,
};
const instance = new ConstructorIO({
apiKey: validApiKey,
testCells,
});

expect(instance.options.testCells).to.deep.equal({ valid: 'bar' });
});

it('Should filter out non-string testCell values in setClientOptions', () => {
const instance = new ConstructorIO({
apiKey: validApiKey,
testCells: { initial: 'value' },
});

instance.setClientOptions({
testCells: {
valid: 'baz',
nullVal: null,
numVal: 42,
},
});

expect(instance.options.testCells).to.deep.equal({ valid: 'baz' });
});

it('Should handle null testCells in constructor without error', () => {
const instance = new ConstructorIO({
apiKey: validApiKey,
testCells: null,
});

expect(instance.options.testCells).to.deep.equal({});
});

it('Should update the client options with new sendTrackingEvents value', () => {
const instance = new ConstructorIO({
apiKey: validApiKey,
Expand Down Expand Up @@ -507,22 +550,22 @@ describe(`ConstructorIO${bundledDescriptionSuffix}`, () => {
});

expect(instance.options).to.have.property('testCells').to.deep.equal(oldTestCells);
expect(instance.search.options).to.have.property('testCells').to.equal(oldTestCells);
expect(instance.autocomplete.options).to.have.property('testCells').to.equal(oldTestCells);
expect(instance.browse.options).to.have.property('testCells').to.equal(oldTestCells);
expect(instance.recommendations.options).to.have.property('testCells').to.equal(oldTestCells);
expect(instance.tracker.options).to.have.property('testCells').to.equal(oldTestCells);
expect(instance.search.options).to.have.property('testCells').to.deep.equal(oldTestCells);
expect(instance.autocomplete.options).to.have.property('testCells').to.deep.equal(oldTestCells);
expect(instance.browse.options).to.have.property('testCells').to.deep.equal(oldTestCells);
expect(instance.recommendations.options).to.have.property('testCells').to.deep.equal(oldTestCells);
expect(instance.tracker.options).to.have.property('testCells').to.deep.equal(oldTestCells);

instance.setClientOptions({
testCells: newTestCells,
});

expect(instance.options).to.have.property('testCells').to.deep.equal(newTestCells);
expect(instance.search.options).to.have.property('testCells').to.equal(newTestCells);
expect(instance.autocomplete.options).to.have.property('testCells').to.equal(newTestCells);
expect(instance.browse.options).to.have.property('testCells').to.equal(newTestCells);
expect(instance.recommendations.options).to.have.property('testCells').to.equal(newTestCells);
expect(instance.tracker.options).to.have.property('testCells').to.equal(newTestCells);
expect(instance.search.options).to.have.property('testCells').to.deep.equal(newTestCells);
expect(instance.autocomplete.options).to.have.property('testCells').to.deep.equal(newTestCells);
expect(instance.browse.options).to.have.property('testCells').to.deep.equal(newTestCells);
expect(instance.recommendations.options).to.have.property('testCells').to.deep.equal(newTestCells);
expect(instance.tracker.options).to.have.property('testCells').to.deep.equal(newTestCells);
});

it('Should update the client options with a new user id', () => {
Expand Down
18 changes: 18 additions & 0 deletions spec/src/modules/autocomplete.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,24 @@ describe(`ConstructorIO - Autocomplete${bundledDescriptionSuffix}`, () => {
});
});

it('Should only include valid string testCells in request params', (done) => {
const testCells = { valid: 'bar', invalid: null, numVal: 123 };
const { autocomplete } = new ConstructorIO({
apiKey: testApiKey,
testCells,
fetch: fetchSpy,
});

autocomplete.getAutocompleteResults(query).then(() => {
const requestedUrlParams = helpers.extractUrlParamsFromFetch(fetchSpy);

expect(requestedUrlParams).to.have.property('ef-valid').to.equal('bar');
expect(requestedUrlParams).to.not.have.property('ef-invalid');
expect(requestedUrlParams).to.not.have.property('ef-numVal');
done();
});
});

it('Should return a response with a valid query, and segments', (done) => {
const segments = ['foo', 'bar'];
const { autocomplete } = new ConstructorIO({
Expand Down
18 changes: 18 additions & 0 deletions spec/src/modules/browse.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,24 @@ describe(`ConstructorIO - Browse${bundledDescriptionSuffix}`, () => {
});
});

it('Should only include valid string testCells in request params', (done) => {
const testCells = { valid: 'bar', invalid: null, numVal: 123 };
const { browse } = new ConstructorIO({
apiKey: testApiKey,
testCells,
fetch: fetchSpy,
});

browse.getBrowseResults(filterName, filterValue).then(() => {
const requestedUrlParams = helpers.extractUrlParamsFromFetch(fetchSpy);

expect(requestedUrlParams).to.have.property('ef-valid').to.equal('bar');
expect(requestedUrlParams).to.not.have.property('ef-invalid');
expect(requestedUrlParams).to.not.have.property('ef-numVal');
done();
});
});

it('Should return a response with a valid filterName, filterValue and segments', (done) => {
const segments = ['foo', 'bar'];
const { browse } = new ConstructorIO({
Expand Down
18 changes: 18 additions & 0 deletions spec/src/modules/search.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,24 @@ describe(`ConstructorIO - Search${bundledDescriptionSuffix}`, () => {
});
});

it('Should only include valid string testCells in request params', (done) => {
const testCells = { valid: 'bar', invalid: null, numVal: 123 };
const { search } = new ConstructorIO({
apiKey: testApiKey,
testCells,
fetch: fetchSpy,
});

search.getSearchResults(query, { section }).then(() => {
const requestedUrlParams = helpers.extractUrlParamsFromFetch(fetchSpy);

expect(requestedUrlParams).to.have.property('ef-valid').to.equal('bar');
expect(requestedUrlParams).to.not.have.property('ef-invalid');
expect(requestedUrlParams).to.not.have.property('ef-numVal');
done();
});
});

it('Should return a response with a valid query, section and segments', (done) => {
const segments = ['foo', 'bar'];
const { search } = new ConstructorIO({
Expand Down
23 changes: 23 additions & 0 deletions spec/src/modules/tracker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,29 @@ describe(`ConstructorIO - Tracker${bundledDescriptionSuffix}`, () => {
expect(tracker.trackSessionStart()).to.equal(true);
});

it('Should only include valid string testCells in request params', (done) => {
const testCells = { valid: 'bar', invalid: null, numVal: 123 };
const { tracker } = new ConstructorIO({
apiKey: testApiKey,
fetch: fetchSpy,
testCells,
...requestQueueOptions,
});

tracker.on('success', () => {
const requestParams = helpers.extractUrlParamsFromFetch(fetchSpy);

expect(fetchSpy).to.have.been.called;
expect(requestParams).to.have.property('ef-valid').to.equal('bar');
expect(requestParams).to.not.have.property('ef-invalid');
expect(requestParams).to.not.have.property('ef-numVal');

done();
});

expect(tracker.trackSessionStart()).to.equal(true);
});

if (!skipNetworkTimeoutTests) {
it('Should be rejected when network request timeout is provided and reached', (done) => {
const { tracker } = new ConstructorIO({
Expand Down
68 changes: 68 additions & 0 deletions spec/src/utils/helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const {
addHTTPSToString,
trimUrl,
cleanAndValidateUrl,
toValidTestCells,
} = require('../../../test/utils/helpers'); // eslint-disable-line import/extensions
const jsdom = require('./jsdom-global');
const store = require('../../../test/utils/store'); // eslint-disable-line import/extensions
Expand Down Expand Up @@ -699,4 +700,71 @@ describe('ConstructorIO - Utils - Helpers', () => {
});
});
}

describe('toValidTestCells', () => {
it('Should return an empty object for null input', () => {
expect(toValidTestCells(null)).to.deep.equal({});
});

it('Should return an empty object for undefined input', () => {
expect(toValidTestCells(undefined)).to.deep.equal({});
});

it('Should return an empty object for an empty object input', () => {
expect(toValidTestCells({})).to.deep.equal({});
});

it('Should return an empty object for an array input', () => {
expect(toValidTestCells(['foo'])).to.deep.equal({});
});

it('Should return an empty object for a string input', () => {
expect(toValidTestCells('foo')).to.deep.equal({});
});

it('Should return an empty object for a number input', () => {
expect(toValidTestCells(123)).to.deep.equal({});
});

it('Should filter out entries with null values', () => {
expect(toValidTestCells({ key: null })).to.deep.equal({});
});

it('Should filter out entries with undefined values', () => {
expect(toValidTestCells({ key: undefined })).to.deep.equal({});
});

it('Should filter out entries with numeric values', () => {
expect(toValidTestCells({ key: 123 })).to.deep.equal({});
});

it('Should filter out entries with object values', () => {
expect(toValidTestCells({ key: { nested: 'value' } })).to.deep.equal({});
});

it('Should filter out entries with boolean values', () => {
expect(toValidTestCells({ key: true })).to.deep.equal({});
});

it('Should filter out entries with empty string values', () => {
expect(toValidTestCells({ key: '' })).to.deep.equal({});
});

it('Should keep entries with valid non-empty string values', () => {
expect(toValidTestCells({ key: 'valid' })).to.deep.equal({ key: 'valid' });
});

it('Should keep only valid string entries from mixed input', () => {
const input = {
valid1: 'bar',
nullVal: null,
numVal: 123,
valid2: 'baz',
emptyStr: '',
objVal: {},
undefVal: undefined,
};
expect(toValidTestCells(input)).to.deep.equal({ valid1: 'bar', valid2: 'baz' });
});
});
});
4 changes: 2 additions & 2 deletions src/constructorio.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ class ConstructorIO {
clientId: clientId || client_id,
userId,
segments,
testCells,
testCells: helpers.toValidTestCells(testCells),
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

This changes the default options.testCells value from undefined (when testCells isn't provided) to {}. That can be a breaking behavior change for consumers who check presence vs emptiness. Consider only adding testCells to the options object when the caller actually provided it (e.g., conditionally set it, or have toValidTestCells return undefined when input is undefined).

Copilot uses AI. Check for mistakes.
fetch: fetchFromOptions || fetch,
trackingSendDelay,
sendTrackingEvents,
Expand Down Expand Up @@ -179,7 +179,7 @@ class ConstructorIO {
}

if (testCells) {
this.options.testCells = testCells;
this.options.testCells = helpers.toValidTestCells(testCells);
}
Comment on lines 181 to 183
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

Using if (testCells) prevents callers from explicitly clearing testCells via null/undefined in setClientOptions (the assignment block won't run). If clearing is intended/expected, switch the conditional to check whether testCells was provided (e.g., property existence check) and then set this.options.testCells to the filtered result (which could be {}) accordingly.

Copilot uses AI. Check for mistakes.

if (typeof sendTrackingEvents === 'boolean') {
Expand Down
17 changes: 17 additions & 0 deletions src/utils/helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,23 @@ const utils = {

truncateString: (string, maxLength) => string.slice(0, maxLength),

// Filter testCells to only include entries with non-empty string values
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

The comment says 'non-empty string values' but the implementation allows whitespace-only strings (e.g. ' '). If whitespace-only values should be treated as empty, update the predicate accordingly (and add a test case); otherwise, consider clarifying the comment to indicate that whitespace-only strings are considered valid.

Copilot uses AI. Check for mistakes.
toValidTestCells: (testCells) => {
if (!testCells || typeof testCells !== 'object' || Array.isArray(testCells)) {
return {};
}

const filtered = {};

Object.keys(testCells).forEach((key) => {
if (typeof testCells[key] === 'string' && testCells[key].trim() !== '') {
filtered[key] = testCells[key];
}
});

return filtered;
},

getBehaviorUrl: (mediaServiceUrl) => {
const baseUrl = new URL(mediaServiceUrl);

Expand Down
Loading