Skip to content

Commit 6bdefc8

Browse files
esezenCopilotMudaafi
authored
[CDX-379] Validate testCells (#429)
* Validate testCells * Update src/utils/helpers.js Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update spec/src/constructorio.js Co-authored-by: Ahmad Mudaafi' <ahmad.mudaafi@constructor.io> * Fix issue originating from conflict --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Ahmad Mudaafi' <ahmad.mudaafi@constructor.io>
1 parent da6aed7 commit 6bdefc8

8 files changed

Lines changed: 217 additions & 12 deletions

File tree

‎spec/src/constructorio.js‎

Lines changed: 53 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,49 @@ describe(`ConstructorIO${bundledDescriptionSuffix}`, () => {
280280
expect(instance.options).to.have.property('testCells').to.deep.equal(newTestCells);
281281
});
282282

283+
it('Should filter out non-string testCell values in constructor', () => {
284+
const testCells = {
285+
valid: 'bar',
286+
nullVal: null,
287+
numVal: 123,
288+
objVal: { nested: 'value' },
289+
emptyStr: '',
290+
boolean: true,
291+
};
292+
const instance = new ConstructorIO({
293+
apiKey: validApiKey,
294+
testCells,
295+
});
296+
297+
expect(instance.options.testCells).to.deep.equal({ valid: 'bar' });
298+
});
299+
300+
it('Should filter out non-string testCell values in setClientOptions', () => {
301+
const instance = new ConstructorIO({
302+
apiKey: validApiKey,
303+
testCells: { initial: 'value' },
304+
});
305+
306+
instance.setClientOptions({
307+
testCells: {
308+
valid: 'baz',
309+
nullVal: null,
310+
numVal: 42,
311+
},
312+
});
313+
314+
expect(instance.options.testCells).to.deep.equal({ valid: 'baz' });
315+
});
316+
317+
it('Should handle null testCells in constructor without error', () => {
318+
const instance = new ConstructorIO({
319+
apiKey: validApiKey,
320+
testCells: null,
321+
});
322+
323+
expect(instance.options.testCells).to.deep.equal({});
324+
});
325+
283326
it('Should update the client options with new sendTrackingEvents value', () => {
284327
const instance = new ConstructorIO({
285328
apiKey: validApiKey,
@@ -507,22 +550,22 @@ describe(`ConstructorIO${bundledDescriptionSuffix}`, () => {
507550
});
508551

509552
expect(instance.options).to.have.property('testCells').to.deep.equal(oldTestCells);
510-
expect(instance.search.options).to.have.property('testCells').to.equal(oldTestCells);
511-
expect(instance.autocomplete.options).to.have.property('testCells').to.equal(oldTestCells);
512-
expect(instance.browse.options).to.have.property('testCells').to.equal(oldTestCells);
513-
expect(instance.recommendations.options).to.have.property('testCells').to.equal(oldTestCells);
514-
expect(instance.tracker.options).to.have.property('testCells').to.equal(oldTestCells);
553+
expect(instance.search.options).to.have.property('testCells').to.deep.equal(oldTestCells);
554+
expect(instance.autocomplete.options).to.have.property('testCells').to.deep.equal(oldTestCells);
555+
expect(instance.browse.options).to.have.property('testCells').to.deep.equal(oldTestCells);
556+
expect(instance.recommendations.options).to.have.property('testCells').to.deep.equal(oldTestCells);
557+
expect(instance.tracker.options).to.have.property('testCells').to.deep.equal(oldTestCells);
515558

516559
instance.setClientOptions({
517560
testCells: newTestCells,
518561
});
519562

520563
expect(instance.options).to.have.property('testCells').to.deep.equal(newTestCells);
521-
expect(instance.search.options).to.have.property('testCells').to.equal(newTestCells);
522-
expect(instance.autocomplete.options).to.have.property('testCells').to.equal(newTestCells);
523-
expect(instance.browse.options).to.have.property('testCells').to.equal(newTestCells);
524-
expect(instance.recommendations.options).to.have.property('testCells').to.equal(newTestCells);
525-
expect(instance.tracker.options).to.have.property('testCells').to.equal(newTestCells);
564+
expect(instance.search.options).to.have.property('testCells').to.deep.equal(newTestCells);
565+
expect(instance.autocomplete.options).to.have.property('testCells').to.deep.equal(newTestCells);
566+
expect(instance.browse.options).to.have.property('testCells').to.deep.equal(newTestCells);
567+
expect(instance.recommendations.options).to.have.property('testCells').to.deep.equal(newTestCells);
568+
expect(instance.tracker.options).to.have.property('testCells').to.deep.equal(newTestCells);
526569
});
527570

528571
it('Should update the client options with a new user id', () => {

‎spec/src/modules/autocomplete.js‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,24 @@ describe(`ConstructorIO - Autocomplete${bundledDescriptionSuffix}`, () => {
112112
});
113113
});
114114

115+
it('Should only include valid string testCells in request params', (done) => {
116+
const testCells = { valid: 'bar', invalid: null, numVal: 123 };
117+
const { autocomplete } = new ConstructorIO({
118+
apiKey: testApiKey,
119+
testCells,
120+
fetch: fetchSpy,
121+
});
122+
123+
autocomplete.getAutocompleteResults(query).then(() => {
124+
const requestedUrlParams = helpers.extractUrlParamsFromFetch(fetchSpy);
125+
126+
expect(requestedUrlParams).to.have.property('ef-valid').to.equal('bar');
127+
expect(requestedUrlParams).to.not.have.property('ef-invalid');
128+
expect(requestedUrlParams).to.not.have.property('ef-numVal');
129+
done();
130+
});
131+
});
132+
115133
it('Should return a response with a valid query, and segments', (done) => {
116134
const segments = ['foo', 'bar'];
117135
const { autocomplete } = new ConstructorIO({

‎spec/src/modules/browse.js‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,24 @@ describe(`ConstructorIO - Browse${bundledDescriptionSuffix}`, () => {
9797
});
9898
});
9999

100+
it('Should only include valid string testCells in request params', (done) => {
101+
const testCells = { valid: 'bar', invalid: null, numVal: 123 };
102+
const { browse } = new ConstructorIO({
103+
apiKey: testApiKey,
104+
testCells,
105+
fetch: fetchSpy,
106+
});
107+
108+
browse.getBrowseResults(filterName, filterValue).then(() => {
109+
const requestedUrlParams = helpers.extractUrlParamsFromFetch(fetchSpy);
110+
111+
expect(requestedUrlParams).to.have.property('ef-valid').to.equal('bar');
112+
expect(requestedUrlParams).to.not.have.property('ef-invalid');
113+
expect(requestedUrlParams).to.not.have.property('ef-numVal');
114+
done();
115+
});
116+
});
117+
100118
it('Should return a response with a valid filterName, filterValue and segments', (done) => {
101119
const segments = ['foo', 'bar'];
102120
const { browse } = new ConstructorIO({

‎spec/src/modules/search.js‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,24 @@ describe(`ConstructorIO - Search${bundledDescriptionSuffix}`, () => {
9393
});
9494
});
9595

96+
it('Should only include valid string testCells in request params', (done) => {
97+
const testCells = { valid: 'bar', invalid: null, numVal: 123 };
98+
const { search } = new ConstructorIO({
99+
apiKey: testApiKey,
100+
testCells,
101+
fetch: fetchSpy,
102+
});
103+
104+
search.getSearchResults(query, { section }).then(() => {
105+
const requestedUrlParams = helpers.extractUrlParamsFromFetch(fetchSpy);
106+
107+
expect(requestedUrlParams).to.have.property('ef-valid').to.equal('bar');
108+
expect(requestedUrlParams).to.not.have.property('ef-invalid');
109+
expect(requestedUrlParams).to.not.have.property('ef-numVal');
110+
done();
111+
});
112+
});
113+
96114
it('Should return a response with a valid query, section and segments', (done) => {
97115
const segments = ['foo', 'bar'];
98116
const { search } = new ConstructorIO({

‎spec/src/modules/tracker.js‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,29 @@ describe(`ConstructorIO - Tracker${bundledDescriptionSuffix}`, () => {
349349
expect(tracker.trackSessionStart()).to.equal(true);
350350
});
351351

352+
it('Should only include valid string testCells in request params', (done) => {
353+
const testCells = { valid: 'bar', invalid: null, numVal: 123 };
354+
const { tracker } = new ConstructorIO({
355+
apiKey: testApiKey,
356+
fetch: fetchSpy,
357+
testCells,
358+
...requestQueueOptions,
359+
});
360+
361+
tracker.on('success', () => {
362+
const requestParams = helpers.extractUrlParamsFromFetch(fetchSpy);
363+
364+
expect(fetchSpy).to.have.been.called;
365+
expect(requestParams).to.have.property('ef-valid').to.equal('bar');
366+
expect(requestParams).to.not.have.property('ef-invalid');
367+
expect(requestParams).to.not.have.property('ef-numVal');
368+
369+
done();
370+
});
371+
372+
expect(tracker.trackSessionStart()).to.equal(true);
373+
});
374+
352375
if (!skipNetworkTimeoutTests) {
353376
it('Should be rejected when network request timeout is provided and reached', (done) => {
354377
const { tracker } = new ConstructorIO({

‎spec/src/utils/helpers.js‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ const {
2626
addHTTPSToString,
2727
trimUrl,
2828
cleanAndValidateUrl,
29+
toValidTestCells,
2930
} = require('../../../test/utils/helpers'); // eslint-disable-line import/extensions
3031
const jsdom = require('./jsdom-global');
3132
const store = require('../../../test/utils/store'); // eslint-disable-line import/extensions
@@ -699,4 +700,71 @@ describe('ConstructorIO - Utils - Helpers', () => {
699700
});
700701
});
701702
}
703+
704+
describe('toValidTestCells', () => {
705+
it('Should return an empty object for null input', () => {
706+
expect(toValidTestCells(null)).to.deep.equal({});
707+
});
708+
709+
it('Should return an empty object for undefined input', () => {
710+
expect(toValidTestCells(undefined)).to.deep.equal({});
711+
});
712+
713+
it('Should return an empty object for an empty object input', () => {
714+
expect(toValidTestCells({})).to.deep.equal({});
715+
});
716+
717+
it('Should return an empty object for an array input', () => {
718+
expect(toValidTestCells(['foo'])).to.deep.equal({});
719+
});
720+
721+
it('Should return an empty object for a string input', () => {
722+
expect(toValidTestCells('foo')).to.deep.equal({});
723+
});
724+
725+
it('Should return an empty object for a number input', () => {
726+
expect(toValidTestCells(123)).to.deep.equal({});
727+
});
728+
729+
it('Should filter out entries with null values', () => {
730+
expect(toValidTestCells({ key: null })).to.deep.equal({});
731+
});
732+
733+
it('Should filter out entries with undefined values', () => {
734+
expect(toValidTestCells({ key: undefined })).to.deep.equal({});
735+
});
736+
737+
it('Should filter out entries with numeric values', () => {
738+
expect(toValidTestCells({ key: 123 })).to.deep.equal({});
739+
});
740+
741+
it('Should filter out entries with object values', () => {
742+
expect(toValidTestCells({ key: { nested: 'value' } })).to.deep.equal({});
743+
});
744+
745+
it('Should filter out entries with boolean values', () => {
746+
expect(toValidTestCells({ key: true })).to.deep.equal({});
747+
});
748+
749+
it('Should filter out entries with empty string values', () => {
750+
expect(toValidTestCells({ key: '' })).to.deep.equal({});
751+
});
752+
753+
it('Should keep entries with valid non-empty string values', () => {
754+
expect(toValidTestCells({ key: 'valid' })).to.deep.equal({ key: 'valid' });
755+
});
756+
757+
it('Should keep only valid string entries from mixed input', () => {
758+
const input = {
759+
valid1: 'bar',
760+
nullVal: null,
761+
numVal: 123,
762+
valid2: 'baz',
763+
emptyStr: '',
764+
objVal: {},
765+
undefVal: undefined,
766+
};
767+
expect(toValidTestCells(input)).to.deep.equal({ valid1: 'bar', valid2: 'baz' });
768+
});
769+
});
702770
});

‎src/constructorio.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ class ConstructorIO {
130130
clientId: clientId || client_id,
131131
userId,
132132
segments,
133-
testCells,
133+
testCells: helpers.toValidTestCells(testCells),
134134
fetch: fetchFromOptions || fetch,
135135
trackingSendDelay,
136136
sendTrackingEvents,
@@ -179,7 +179,7 @@ class ConstructorIO {
179179
}
180180

181181
if (testCells) {
182-
this.options.testCells = testCells;
182+
this.options.testCells = helpers.toValidTestCells(testCells);
183183
}
184184

185185
if (typeof sendTrackingEvents === 'boolean') {

‎src/utils/helpers.js‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,23 @@ const utils = {
373373

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

376+
// Filter testCells to only include entries with non-empty string values
377+
toValidTestCells: (testCells) => {
378+
if (!testCells || typeof testCells !== 'object' || Array.isArray(testCells)) {
379+
return {};
380+
}
381+
382+
const filtered = {};
383+
384+
Object.keys(testCells).forEach((key) => {
385+
if (typeof testCells[key] === 'string' && testCells[key].trim() !== '') {
386+
filtered[key] = testCells[key];
387+
}
388+
});
389+
390+
return filtered;
391+
},
392+
376393
getBehaviorUrl: (mediaServiceUrl) => {
377394
const baseUrl = new URL(mediaServiceUrl);
378395

0 commit comments

Comments
 (0)