[Review] [CDX-379] Validate testCells

spec/src/constructorio.js

@@ -280,6 +280,48 @@ 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: '',
+      };
+      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,
@@ -507,22 +549,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', () => {

spec/src/modules/autocomplete.js

@@ -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({

spec/src/modules/browse.js

@@ -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({

spec/src/modules/search.js

@@ -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({

spec/src/modules/tracker.js

@@ -347,6 +347,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({

spec/src/utils/helpers.js

@@ -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
@@ -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' });
+    });
+  });
 });

src/constructorio.js

@@ -128,7 +128,7 @@ class ConstructorIO {
       clientId: clientId || client_id,
       userId,
       segments,
-      testCells,
+      testCells: helpers.toValidTestCells(testCells),
       fetch: fetchFromOptions || fetch,
       trackingSendDelay,
       sendTrackingEvents,
@@ -176,7 +176,7 @@ class ConstructorIO {
       }
 
       if (testCells) {
-        this.options.testCells = testCells;
+        this.options.testCells = helpers.toValidTestCells(testCells);
       }
 
       if (typeof sendTrackingEvents === 'boolean') {

src/utils/helpers.js

@@ -372,6 +372,23 @@ const utils = {
   },
 
   truncateString: (string, maxLength) => string.slice(0, maxLength),
+
+  // Filter testCells to only include entries with non-empty string values
+  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] !== '') {
+        filtered[key] = testCells[key];
+      }
+    });
+
+    return filtered;
+  },
 };
 
 module.exports = utils;