From d51e5e2fd5d46dbc4b5bb0b3b0fdd33ce32e633a Mon Sep 17 00:00:00 2001 From: Jagadeesh Branch <102190347+JagadeeshKaricherla-branch@users.noreply.github.com> Date: Tue, 27 Feb 2024 16:09:00 -0800 Subject: [PATCH] fix(core): fix issue with sending defaults for DMA params (#1000) -fix(): fix issue with sending defaults for DMA params --- src/1_utils.js | 59 +++++++++++++---------------- src/6_branch.js | 33 +++++++++++++---- test/1_utils.js | 15 ++++++++ test/6_branch_new.js | 88 +++++++++++++++++++++++++++++++++++++++----- test/branch-deps.js | 4 +- 5 files changed, 147 insertions(+), 52 deletions(-) diff --git a/src/1_utils.js b/src/1_utils.js index 01addb5c..afb80df5 100644 --- a/src/1_utils.js +++ b/src/1_utils.js @@ -1335,49 +1335,42 @@ utils.shouldAddDMAParams = function(endPointURL) { }; utils.setDMAParams = function(data, dmaObj = {}, endPoint) { - const DMA_EEA = "dma_eea"; - const DMA_Ad_Personalization = "dma_ad_personalization"; - const DMA_Ad_User_Data = "dma_ad_user_data"; - + const v1_DMAEndPoints = [ "/v1/open", "/v1/pageview" ]; + const v2_DMAEndPoints = [ "/v2/event/standard", "/v2/event/custom" ]; const dmaParams = { - [DMA_EEA]: dmaObj.eeaRegion || false, - [DMA_Ad_Personalization]: dmaObj.adPersonalizationConsent || false, - [DMA_Ad_User_Data]: dmaObj.adUserDataUsageConsent || false + dma_eea: dmaObj['eeaRegion'], + dma_ad_personalization: dmaObj['adPersonalizationConsent'], + dma_ad_user_data: dmaObj['adUserDataUsageConsent'] }; - - const allowDMAParamURLMap = utils.allowDMAParamURLMap; - - for (const [key, value] of Object.entries(allowDMAParamURLMap)) { - if (endPoint.includes(key)) { - - if (value === '') { - Object.assign(data, dmaParams); + if (v1_DMAEndPoints.includes(endPoint)) { + Object.assign(data, dmaParams); + } + else if (v2_DMAEndPoints.includes(endPoint)) { + try { + let user_data; + if (!data['user_data']) { + user_data = {}; } else { - let updatedValue; - const valueExists = value in data; - if (!valueExists || data[value] === '') { - updatedValue = JSON.stringify(dmaParams); - } - else { - try { - const innerJSONObject = JSON.parse(data[value]); - const mergedObject = Object.assign({}, innerJSONObject, dmaParams); - updatedValue = JSON.stringify(mergedObject); - } catch (error) { - console.error(`setDMAParams:: ${value} is not a valid JSON string`); - } - } - if (updatedValue) { - data[value] = updatedValue; - } + user_data = JSON.parse(data['user_data']); } - break; + Object.assign(user_data, dmaParams); + data['user_data'] = JSON.stringify(user_data); + } catch (error) { + console.error(`setDMAParams:: ${data['user_data']} is not a valid JSON string`); } } }; +/** + * @param {?} value + * Check if given value is boolean or not + */ + utils.isBoolean = function(value) { + return (value === true || value === false); +}; + /** * @param {String} url * A utility function to validate url diff --git a/src/6_branch.js b/src/6_branch.js index 86fded70..e66bc2fc 100644 --- a/src/6_branch.js +++ b/src/6_branch.js @@ -217,7 +217,7 @@ Branch.prototype._api = function(resource, obj, callback) { } if (utils.shouldAddDMAParams(resource.endpoint)) { var dmaData = this._storage.get('branch_dma_data', true); - obj["branch_dma_data"] = dmaData ? safejson.parse(dmaData) : {}; + obj["branch_dma_data"] = dmaData ? safejson.parse(dmaData) : null; } if (resource.endpoint !== '/_r') { resource.destination = config.api_endpoint; @@ -1957,14 +1957,31 @@ Branch.prototype['setAPIResponseCallback'] = wrap(callback_params.NO_CALLBACK, f */ Branch.prototype['setDMAParamsForEEA'] = wrap(callback_params.CALLBACK_ERR, function(done, eeaRegion, adPersonalizationConsent, adUserDataUsageConsent) { try { - var dmaObj = {}; - dmaObj.eeaRegion = eeaRegion || false; - dmaObj.adPersonalizationConsent = adPersonalizationConsent || false; - dmaObj.adUserDataUsageConsent = adUserDataUsageConsent || false; + const validateParam = (param, paramName) => { + if (!utils.isBoolean(param)) { + console.warn(`setDMAParamsForEEA: ${paramName} must be boolean, but got ${param}`); + return false; + } + return true; + }; + const isValid = ( + validateParam(eeaRegion, "eeaRegion") && + validateParam(adPersonalizationConsent, "adPersonalizationConsent") && + validateParam(adUserDataUsageConsent, "adUserDataUsageConsent") + ); + if (!isValid) { + return; + } + + const dmaObj = { + eeaRegion, + adPersonalizationConsent, + adUserDataUsageConsent + }; + this._storage.set('branch_dma_data', safejson.stringify(dmaObj), true); - } - catch (e) { - console.error("setDMAParamsForEEA::An error occured while setting DMA parameters for EEA", e); + } catch (e) { + console.error("setDMAParamsForEEA::An error occurred while setting DMA parameters for EEA", e); } done(); }, true); diff --git a/test/1_utils.js b/test/1_utils.js index d25577e8..2aeaaabd 100644 --- a/test/1_utils.js +++ b/test/1_utils.js @@ -1372,6 +1372,21 @@ describe('utils', function() { utils.setDMAParams(data2, dmaObj, '/v2/event/custom'); assert.deepEqual(data2, { "user_data":"{\"dma_eea\":true,\"dma_ad_personalization\":true,\"dma_ad_user_data\":false}" }); }); + it('should add DMA parameters for valid endpoints: v2/event/custom', () => { + const dmaObj = { + eeaRegion: true, + adPersonalizationConsent: true, + adUserDataUsageConsent: false + }; + + const data2 = { + }; + data2.user_data = JSON.stringify({ + "test": true + }); + utils.setDMAParams(data2, dmaObj, '/v2/event/custom'); + assert.deepEqual(data2, { "user_data": "{\"test\":true,\"dma_eea\":true,\"dma_ad_personalization\":true,\"dma_ad_user_data\":false}" }); + }); it('should add DMA parameters for valid endpoints: v1/pageview', () => { const dmaObj = { eeaRegion: true, diff --git a/test/6_branch_new.js b/test/6_branch_new.js index aa9449c7..669f3e9d 100644 --- a/test/6_branch_new.js +++ b/test/6_branch_new.js @@ -78,7 +78,7 @@ describe('Branch - new', function() { branch_instance.setDMAParamsForEEA.call(thisObj, dmaObj.eeaRegion, dmaObj.adPersonalizationConsent, dmaObj.adUserDataUsageConsent); sinon.assert.calledWith(storageSetStub, 'branch_dma_data', stringifieddmaObj, true); }); - it('should store default dma params inside branch_dma_data of storage', function() { + it('should not store dma params inside branch_dma_data of storage if eeaRegion is not set', function() { const thisObj = { _storage: { set: () => {} @@ -86,13 +86,83 @@ describe('Branch - new', function() { _queue: task_queue() }; const storageSetStub = sandbox.stub(thisObj._storage, 'set'); - const dmaObj = {}; - dmaObj.eeaRegion = false; - dmaObj.adPersonalizationConsent = false; - dmaObj.adUserDataUsageConsent = false; - const stringifieddmaObj = JSON.stringify(dmaObj); branch_instance.setDMAParamsForEEA.call(thisObj); - sinon.assert.calledWith(storageSetStub, 'branch_dma_data', stringifieddmaObj, true); + sinon.assert.notCalled(storageSetStub); + }); + it('should not store dma params inside branch_dma_data of storage if eeaRegion is null', function() { + const thisObj = { + _storage: { + set: () => {} + }, + _queue: task_queue() + }; + const storageSetStub = sandbox.stub(thisObj._storage, 'set'); + const dmaObj = {}; + dmaObj.eeaRegion = null; + dmaObj.adPersonalizationConsent = true; + dmaObj.adUserDataUsageConsent = true; + branch_instance.setDMAParamsForEEA.call(thisObj, dmaObj.eeaRegion, dmaObj.adPersonalizationConsent, dmaObj.adUserDataUsageConsent); + sinon.assert.notCalled(storageSetStub); + }); + it('should log warning if eeaRegion is not boolean', function() { + const thisObj = { + _storage: { + set: () => {} + }, + _queue: task_queue() + }; + const consoleErrorStub = sandbox.stub(console, 'warn'); + try { + const dmaObj = {}; + dmaObj.eeaRegion = null; + dmaObj.adPersonalizationConsent = true; + dmaObj.adUserDataUsageConsent = true; + branch_instance.setDMAParamsForEEA.call(thisObj, dmaObj.eeaRegion, dmaObj.adPersonalizationConsent, dmaObj.adUserDataUsageConsent); + + } catch (e) { + + } + sinon.assert.calledWith(consoleErrorStub, 'setDMAParamsForEEA: eeaRegion must be boolean, but got null'); + }); + it('should log warning if adPersonalizationConsent is not boolean', function() { + const thisObj = { + _storage: { + set: () => {} + }, + _queue: task_queue() + }; + const consoleErrorStub = sandbox.stub(console, 'warn'); + try { + const dmaObj = {}; + dmaObj.eeaRegion = true; + dmaObj.adPersonalizationConsent = null; + dmaObj.adUserDataUsageConsent = true; + branch_instance.setDMAParamsForEEA.call(thisObj, dmaObj.eeaRegion, dmaObj.adPersonalizationConsent, dmaObj.adUserDataUsageConsent); + + } catch (e) { + + } + sinon.assert.calledWith(consoleErrorStub, 'setDMAParamsForEEA: adPersonalizationConsent must be boolean, but got null'); + }); + it('should log warning if eeaRegion is not boolean', function() { + const thisObj = { + _storage: { + set: () => {} + }, + _queue: task_queue() + }; + const consoleErrorStub = sandbox.stub(console, 'warn'); + try { + const dmaObj = {}; + dmaObj.eeaRegion = true; + dmaObj.adPersonalizationConsent = true; + dmaObj.adUserDataUsageConsent = null; + branch_instance.setDMAParamsForEEA.call(thisObj, dmaObj.eeaRegion, dmaObj.adPersonalizationConsent, dmaObj.adUserDataUsageConsent); + + } catch (e) { + + } + sinon.assert.calledWith(consoleErrorStub, 'setDMAParamsForEEA: adUserDataUsageConsent must be boolean, but got null'); }); it('should catch and log exception', function() { const thisObj = { @@ -108,12 +178,12 @@ describe('Branch - new', function() { dmaObj.eeaRegion = false; dmaObj.adPersonalizationConsent = false; dmaObj.adUserDataUsageConsent = false; - branch_instance.setDMAParamsForEEA.call(thisObj); + branch_instance.setDMAParamsForEEA.call(thisObj, dmaObj.eeaRegion, dmaObj.adPersonalizationConsent, dmaObj.adUserDataUsageConsent); } catch (e) { } - sinon.assert.calledWith(consoleErrorStub, 'setDMAParamsForEEA::An error occured while setting DMA parameters for EEA', sinon.match.instanceOf(Error)); + sinon.assert.calledWith(consoleErrorStub, 'setDMAParamsForEEA::An error occurred while setting DMA parameters for EEA', sinon.match.instanceOf(Error)); }); }); describe('setAPIUrl', function() { diff --git a/test/branch-deps.js b/test/branch-deps.js index 149ac947..1a7ae194 100644 --- a/test/branch-deps.js +++ b/test/branch-deps.js @@ -5,12 +5,12 @@ goog.addDependency('../../../../src/1_utils.js', ['utils'], ['config', 'goog.jso goog.addDependency('../../../../src/2_resources.js', ['resources'], ['config', 'utils']); goog.addDependency('../../../../src/2_session.js', ['session'], ['goog.json', 'safejson', 'storage', 'utils']); goog.addDependency('../../../../src/2_storage.js', ['storage'], ['goog.json', 'utils']); -goog.addDependency('../../../../src/3_api.js', ['Server'], ['goog.json', 'safejson', 'storage', 'utils']); +goog.addDependency('../../../../src/3_api.js', ['Server'], ['goog.json', 'safejson', 'storage', 'utils'], {'lang': 'es6'}); goog.addDependency('../../../../src/3_banner_utils.js', ['banner_utils'], ['safejson', 'storage', 'utils']); goog.addDependency('../../../../src/4_banner_css.js', ['banner_css'], ['banner_utils', 'utils']); goog.addDependency('../../../../src/4_banner_html.js', ['banner_html'], ['banner_utils', 'session', 'storage', 'utils']); goog.addDependency('../../../../src/5_banner.js', ['banner'], ['banner_css', 'banner_html', 'banner_utils', 'utils']); -goog.addDependency('../../../../src/6_branch.js', ['Branch'], ['Server', 'banner', 'branch_view', 'config', 'goog.json', 'journeys_utils', 'resources', 'safejson', 'session', 'storage', 'task_queue', 'utils']); +goog.addDependency('../../../../src/6_branch.js', ['Branch'], ['Server', 'banner', 'branch_view', 'config', 'goog.json', 'journeys_utils', 'resources', 'safejson', 'session', 'storage', 'task_queue', 'utils'], {'lang': 'es6'}); goog.addDependency('../../../../src/7_initialization.js', ['branch_instance'], ['Branch', 'config']); goog.addDependency('../../../../src/branch_view.js', ['branch_view'], ['banner_css', 'journeys_utils', 'safejson', 'utils']); goog.addDependency('../../../../src/extern.js', [], []);