From d5fa42957a3ace3d682d96f7fa56e174f6251a34 Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Thu, 31 Oct 2024 12:37:57 -0500 Subject: [PATCH 01/22] refactor: Migrate CookieSyncManager to TypeScript --- src/consent.ts | 4 + ...kieSyncManager.js => cookieSyncManager.ts} | 74 ++- src/mp-instance.js | 2 +- src/sdkRuntimeModels.ts | 3 +- src/utils.ts | 9 + test/jest/cookieSyncManager.spec.ts | 454 ++++++++++++++++++ test/jest/utils.spec.ts | 18 +- ...kie-syncing.js => tests-cookie-syncing.ts} | 142 +++--- 8 files changed, 585 insertions(+), 121 deletions(-) rename src/{cookieSyncManager.js => cookieSyncManager.ts} (82%) create mode 100644 test/jest/cookieSyncManager.spec.ts rename test/src/{tests-cookie-syncing.js => tests-cookie-syncing.ts} (94%) diff --git a/src/consent.ts b/src/consent.ts index 6ff45c217..3dcac6afd 100644 --- a/src/consent.ts +++ b/src/consent.ts @@ -38,6 +38,10 @@ export interface SDKConsentApi { createConsentState: (consentState?: ConsentState) => ConsentState; ConsentSerialization: IConsentSerialization; createPrivacyConsent: ICreatePrivacyConsentFunction; + isEnabledForUserConsent: ( + consentRules: IConsentRules, + user: IMParticleUser + ) => boolean; } export interface IConsentSerialization { diff --git a/src/cookieSyncManager.js b/src/cookieSyncManager.ts similarity index 82% rename from src/cookieSyncManager.js rename to src/cookieSyncManager.ts index b5026149a..dd15c4439 100644 --- a/src/cookieSyncManager.js +++ b/src/cookieSyncManager.ts @@ -1,19 +1,26 @@ +import { replaceAmpWithAmpersand, replaceMPID } from './utils'; import Constants from './constants'; +import { ICookieSyncManager } from './cookieSyncManager.interfaces'; +import { MParticleWebSDK } from './sdkRuntimeModels'; +import { PixelConfiguration } from './store'; +import { Dictionary, MPID } from '@mparticle/web-sdk'; +import { IConsentRules } from './consent'; -var Messages = Constants.Messages; +const { Messages } = Constants; +const {InformationMessages} = Messages; -export default function cookieSyncManager(mpInstance) { - var self = this; +export default function CookieSyncManager(this: ICookieSyncManager, mpInstance: MParticleWebSDK) { + const self = this; // Public - this.attemptCookieSync = function(previousMPID, mpid, mpidIsNotInCookies) { + this.attemptCookieSync = function(previousMPID: MPID, mpid: MPID, mpidIsNotInCookies: boolean) { // TODO: These should move inside the for loop - var pixelConfig, - lastSyncDateForModule, - url, - redirect, - urlWithRedirect, - requiresConsent; + let pixelConfig: PixelConfiguration; + let lastSyncDateForModule: number; + let url: string; + let redirect: string; + let urlWithRedirect: string; + let requiresConsent: boolean; // TODO: Make this exit quicker instead of nested if (mpid && !mpInstance._Store.webviewBridgeEnabled) { @@ -41,11 +48,11 @@ export default function cookieSyncManager(mpInstance) { frequencyCap: pixelSettings.frequencyCap, // Url for cookie sync pixel - pixelUrl: self.replaceAmp(pixelSettings.pixelUrl), + pixelUrl: replaceAmpWithAmpersand(pixelSettings.pixelUrl), // TODO: Document requirements for redirectUrl redirectUrl: pixelSettings.redirectUrl - ? self.replaceAmp(pixelSettings.redirectUrl) + ? replaceAmpWithAmpersand(pixelSettings.redirectUrl) : null, // Filtering rules as defined in UI @@ -54,16 +61,18 @@ export default function cookieSyncManager(mpInstance) { }; // TODO: combine replaceMPID and replaceAmp into sanitizeUrl function - url = self.replaceMPID(pixelConfig.pixelUrl, mpid); + url = replaceMPID(pixelConfig.pixelUrl, mpid); redirect = pixelConfig.redirectUrl - ? self.replaceMPID(pixelConfig.redirectUrl, mpid) + ? replaceMPID(pixelConfig.redirectUrl, mpid) : ''; urlWithRedirect = url + encodeURIComponent(redirect); + // TODO: Refactor so that Persistence is only called once // outside of the loop var persistence = mpInstance._Persistence.getPersistence(); + // TODO: Is there a historic reason for checking for previousMPID? // it does not appear to be passed in anywhere if (previousMPID && previousMPID !== mpid) { @@ -140,33 +149,15 @@ export default function cookieSyncManager(mpInstance) { } }; - // Private - this.replaceMPID = function(string, mpid) { - return string.replace('%%mpid%%', mpid); - }; - - // Private - /** - * @deprecated replaceAmp has been deprecated, use replaceAmpersandWithAmp instead - */ - this.replaceAmp = function(string) { - return this.replaceAmpWithAmpersand(string); - }; - - // Private - this.replaceAmpWithAmpersand = function(string) { - return string.replace(/&/g, '&'); - }; - // Private this.performCookieSync = function( - url, - moduleId, - mpid, - cookieSyncDates, - filteringConsentRuleValues, - mpidIsNotInCookies, - requiresConsent + url: string, + moduleId: number, + mpid: MPID, + cookieSyncDates: Dictionary, + filteringConsentRuleValues: IConsentRules, + mpidIsNotInCookies: boolean, + requiresConsent: boolean ) { // if MPID is new to cookies, we should not try to perform the cookie sync // because a cookie sync can only occur once a user either consents or doesn't @@ -182,14 +173,15 @@ export default function cookieSyncManager(mpInstance) { // Currently, attemptCookieSync is called as a loop and therefore this // function polls the user object and consent multiple times. if ( + // https://go.mparticle.com/work/SQDSDKS-5009 mpInstance._Consent.isEnabledForUserConsent( filteringConsentRuleValues, mpInstance.Identity.getCurrentUser() ) ) { - var img = document.createElement('img'); + const img = document.createElement('img'); - mpInstance.Logger.verbose(Messages.InformationMessages.CookieSync); + mpInstance.Logger.verbose(InformationMessages.CookieSync); img.onload = function() { // TODO: Break this out into a convenience method so we can unit test cookieSyncDates[moduleId.toString()] = new Date().getTime(); diff --git a/src/mp-instance.js b/src/mp-instance.js index b18158339..00b5f1a92 100644 --- a/src/mp-instance.js +++ b/src/mp-instance.js @@ -21,7 +21,7 @@ import Constants from './constants'; import APIClient from './apiClient'; import Helpers from './helpers'; import NativeSdkHelpers from './nativeSdkHelpers'; -import CookieSyncManager from './cookieSyncManager'; +import CookieSyncManager from './CookieSyncManager'; import SessionManager from './sessionManager'; import Ecommerce from './ecommerce'; import Store from './store'; diff --git a/src/sdkRuntimeModels.ts b/src/sdkRuntimeModels.ts index 9f6c5a9c2..87956c6ba 100644 --- a/src/sdkRuntimeModels.ts +++ b/src/sdkRuntimeModels.ts @@ -6,7 +6,7 @@ import { Callback, IdentityApiData, } from '@mparticle/web-sdk'; -import { IStore } from './store'; +import { IStore, PixelConfiguration } from './store'; import Validators from './validators'; import { Dictionary } from './utils'; import { IServerModel } from './serverModel'; @@ -230,6 +230,7 @@ export interface SDKInitConfig sideloadedKits?: MPForwarder[]; dataPlanOptions?: KitBlockerOptions; flags?: Dictionary; + pixelConfigs?: PixelConfiguration[]; aliasMaxWindow?: number; deviceId?: string; diff --git a/src/utils.ts b/src/utils.ts index f25b7e35c..a7dad1554 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,3 +1,4 @@ +import { MPID } from '@mparticle/web-sdk'; import Constants from './constants'; const { Messages } = Constants; @@ -175,6 +176,10 @@ const replaceApostrophesWithQuotes = (value: string): string => const replaceQuotesWithApostrophes = (value: string): string => value.replace(/\"/g, "'"); +const replaceMPID = (value: string, mpid: MPID): string => value.replace('%%mpid%%', mpid); + +const replaceAmpWithAmpersand = (value: string): string => value.replace(/&/g, '&'); + // FIXME: REFACTOR for V3 // only used in store.js to sanitize server-side formatting of // booleans when checking for `isDevelopmentMode` @@ -333,6 +338,8 @@ const getHref = (): string => { : ''; }; + + export { createCookieString, revertCookieString, @@ -366,4 +373,6 @@ export { queryStringParser, getCookies, getHref, + replaceMPID, + replaceAmpWithAmpersand, }; diff --git a/test/jest/cookieSyncManager.spec.ts b/test/jest/cookieSyncManager.spec.ts new file mode 100644 index 000000000..440bec25c --- /dev/null +++ b/test/jest/cookieSyncManager.spec.ts @@ -0,0 +1,454 @@ +import CookieSyncManager from '../../src/cookieSyncManager'; +import { MParticleWebSDK } from '../../src/sdkRuntimeModels'; +import { PixelConfiguration } from '../../src/store'; +import { testMPID } from '../src/config/constants'; + +const pixelSettings: PixelConfiguration = { + name: 'TestPixel', + moduleId: 5, + esId: 24053, + isDebug: true, + isProduction: true, + settings: {}, + frequencyCap: 14, + pixelUrl: '', + redirectUrl: '', +}; + +describe('CookieSyncManager', () => { + describe('#attemptCookieSync', () => { + it('should perform a cookie sync with defaults', () => { + const mockSDK = ({ + _Store: { + webviewBridgeEnabled: false, + pixelConfigurations: [pixelSettings], + }, + _Persistence: { + getPersistence: () => ({testMPID: { + csd: {} + }}), + }, + } as unknown) as MParticleWebSDK; + + const cookieSyncManager = new CookieSyncManager(mockSDK); + cookieSyncManager.performCookieSync = jest.fn(); + + cookieSyncManager.attemptCookieSync(null, testMPID, true); + + expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( + '', + 5, + testMPID, + {}, + undefined, + true, + false, + ); + }); + + it('should toggle requiresConsent value if filtering filteringConsentRuleValues are defined', () => { + const myPixelSettings: PixelConfiguration = { + filteringConsentRuleValues: { + values: ['test'], + }, + }; + + const mockSDK = ({ + _Store: { + webviewBridgeEnabled: false, + pixelConfigurations: [{...pixelSettings, ...myPixelSettings}], + }, + _Persistence: { + getPersistence: () => ({testMPID: { + csd: {} + }}), + }, + } as unknown) as MParticleWebSDK; + + const cookieSyncManager = new CookieSyncManager(mockSDK); + cookieSyncManager.performCookieSync = jest.fn(); + + cookieSyncManager.attemptCookieSync(null, testMPID, true); + + expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( + '', + 5, + testMPID, + {}, + { values: ['test'] }, + true, + true, + ); + }); + + it('should update the urlWithRedirect if a redirectUrl is defined', () => { + const myPixelSettings: PixelConfiguration = { + pixelUrl: 'https://test.com', + redirectUrl: '?redirect=https://redirect.com&mpid=%%mpid%%', + }; + + const mockSDK = ({ + _Store: { + webviewBridgeEnabled: false, + pixelConfigurations: [{...pixelSettings, ...myPixelSettings}], + }, + _Persistence: { + getPersistence: () => ({testMPID: { + csd: {} + }}), + }, + } as unknown) as MParticleWebSDK; + + const cookieSyncManager = new CookieSyncManager(mockSDK); + cookieSyncManager.performCookieSync = jest.fn(); + + cookieSyncManager.attemptCookieSync(null, testMPID, true); + + expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( + 'https://test.com%3Fredirect%3Dhttps%3A%2F%2Fredirect.com%26mpid%3DtestMPID', + 5, + testMPID, + {}, + undefined, + true, + false, + ); + }); + + // QUESTION: What is the purpose of this code path? + it('should call performCookieSync with mpid if previousMpid and mpid do not match', () => { + const mockSDK = ({ + _Store: { + webviewBridgeEnabled: false, + pixelConfigurations: [pixelSettings], + }, + _Persistence: { + getPersistence: () => ({testMPID: { + }}), + }, + } as unknown) as MParticleWebSDK; + + const cookieSyncManager = new CookieSyncManager(mockSDK); + cookieSyncManager.performCookieSync = jest.fn(); + + cookieSyncManager.attemptCookieSync('other-mpid', testMPID, true); + + expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( + '', + 5, + testMPID, + {}, + undefined, + true, + false, + ); + }); + + it('should call performCookieSync with mpid and csd if previousMpid and mpid do not match', () => { + const mockSDK = ({ + _Store: { + webviewBridgeEnabled: false, + pixelConfigurations: [pixelSettings], + }, + _Persistence: { + getPersistence: () => ({testMPID: { + csd: { 5: 1234567890 } + }}), + }, + } as unknown) as MParticleWebSDK; + + const cookieSyncManager = new CookieSyncManager(mockSDK); + cookieSyncManager.performCookieSync = jest.fn(); + + cookieSyncManager.attemptCookieSync('other-mpid', testMPID, true); + + expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( + '', + 5, + testMPID, + { + 5: 1234567890 + }, + undefined, + true, + false, + ); + }); + + // QUESTION: What is the purpose of this code path? + it('should call performCookieSync with mpid if previousMpid and mpid match', () => { + const mockSDK = ({ + _Store: { + webviewBridgeEnabled: false, + pixelConfigurations: [pixelSettings], + }, + _Persistence: { + getPersistence: () => ({testMPID: { + }}), + }, + } as unknown) as MParticleWebSDK; + + const cookieSyncManager = new CookieSyncManager(mockSDK); + cookieSyncManager.performCookieSync = jest.fn(); + + cookieSyncManager.attemptCookieSync(testMPID, testMPID, true); + + expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( + '', + 5, + testMPID, + {}, + undefined, + true, + false, + ); + }); + + it('should perform a cookie sync if lastSyncDateForModule is past the frequency cap', () => { + const now = new Date().getTime(); + + // Rev the date by one + const cookieSyncDateInPast = now - ( pixelSettings.frequencyCap + 1 ) * 60 * 60 * 24 * 1000; + + const mockSDK = ({ + _Store: { + webviewBridgeEnabled: false, + pixelConfigurations: [pixelSettings], + }, + _Persistence: { + getPersistence: () => ({testMPID: { + csd: { 5: cookieSyncDateInPast } + }}), + }, + } as unknown) as MParticleWebSDK; + + const cookieSyncManager = new CookieSyncManager(mockSDK); + cookieSyncManager.performCookieSync = jest.fn(); + + cookieSyncManager.attemptCookieSync(null, testMPID, true); + + expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( + '', + 5, + testMPID, + { + 5: cookieSyncDateInPast + }, + undefined, + true, + false, + ); + }); + + it('should perform a cookie sync if lastSyncDateForModule is past the frequency cap even if csd is empty', () => { + const now = new Date().getTime(); + + const mockSDK = ({ + _Store: { + webviewBridgeEnabled: false, + pixelConfigurations: [pixelSettings], + }, + _Persistence: { + // This will make lastSyncDateForModule undefined, which bypasses the + // actual time check, but still performs a cookie sync + getPersistence: () => ({testMPID: {}}), + }, + } as unknown) as MParticleWebSDK; + + const cookieSyncManager = new CookieSyncManager(mockSDK); + cookieSyncManager.performCookieSync = jest.fn(); + + cookieSyncManager.attemptCookieSync(null, testMPID, true); + + expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( + '', + 5, + testMPID, + {}, + undefined, + true, + false, + ); + }); + + it('should sync cookies when there was not a previous cookie-sync', () => { + const mockSDK = ({ + _Store: { + webviewBridgeEnabled: false, + pixelConfigurations: [pixelSettings], + }, + _Persistence: { + getPersistence: () => ({}), + }, + } as unknown) as MParticleWebSDK; + + const cookieSyncManager = new CookieSyncManager(mockSDK); + + cookieSyncManager.attemptCookieSync(null, '456', true); + expect(mockSDK._Store.pixelConfigurations.length).toBe(1); + expect(mockSDK._Store.pixelConfigurations[0].moduleId).toBe(5); + }); + }); + + describe('#performCookieSync', () => { + it('should add cookie sync dates to a tracking pixel', () => { + const mockImage = { + onload: jest.fn(), + src: '', + }; + jest.spyOn(document, 'createElement').mockReturnValue( + (mockImage as unknown) as HTMLImageElement + ); + + const mockSDK = ({ + _Store: { + webviewBridgeEnabled: false, + pixelConfigurations: [pixelSettings], + }, + _Persistence: { + setCookieSyncDates: jest.fn(), + getPersistence: jest.fn(), + saveUserCookieSyncDatesToPersistence: jest.fn(), + }, + _Consent: { + isEnabledForUserConsent: jest.fn().mockReturnValue(true), + }, + Identity: { + getCurrentUser: jest.fn().mockReturnValue({ + getMPID: () => '123', + }), + }, + Logger: { + verbose: jest.fn(), + }, + } as unknown) as MParticleWebSDK; + + const cookieSyncManager = new CookieSyncManager(mockSDK); + + let cookieSyncDates = []; + cookieSyncManager.performCookieSync( + 'https://test.com', + 42, + '1234', + cookieSyncDates, + null, + false, + false + ); + + // Simulate image load + mockImage.onload(); + + expect(mockImage.src).toBe('https://test.com'); + expect(cookieSyncDates[42]).toBeDefined(); + expect(cookieSyncDates[42]).toBeGreaterThan(0); + }); + + it('should log a verbose message when a cookie sync is performed', () => { + const mockImage = { + onload: jest.fn(), + src: '', + }; + jest.spyOn(document, 'createElement').mockReturnValue( + (mockImage as unknown) as HTMLImageElement + ); + + const loggerSpy = jest.fn(); + + const mockSDK = ({ + _Store: { + webviewBridgeEnabled: false, + pixelConfigurations: [pixelSettings], + }, + _Persistence: { + setCookieSyncDates: jest.fn(), + getPersistence: jest.fn(), + saveUserCookieSyncDatesToPersistence: jest.fn(), + }, + _Consent: { + isEnabledForUserConsent: jest.fn().mockReturnValue(true), + }, + Identity: { + getCurrentUser: jest.fn().mockReturnValue({ + getMPID: () => '123', + }), + }, + Logger: { + verbose: loggerSpy, + }, + } as unknown) as MParticleWebSDK; + + const cookieSyncManager = new CookieSyncManager(mockSDK); + + let cookieSyncDates = []; + cookieSyncManager.performCookieSync( + 'https://test.com', + 42, + '1234', + cookieSyncDates, + null, + false, + false + ); + + // Simulate image load + mockImage.onload(); + + expect(loggerSpy).toHaveBeenCalledWith('Performing cookie sync'); + }); + + it('should return early if the user has not consented to the cookie sync', () => { + const mockImage = { + onload: jest.fn(), + src: '', + }; + jest.spyOn(document, 'createElement').mockReturnValue( + (mockImage as unknown) as HTMLImageElement + ); + + const loggerSpy = jest.fn(); + + const mockSDK = ({ + _Store: { + webviewBridgeEnabled: false, + pixelConfigurations: [pixelSettings], + }, + _Persistence: { + setCookieSyncDates: jest.fn(), + getPersistence: jest.fn(), + saveUserCookieSyncDatesToPersistence: jest.fn(), + }, + _Consent: { + isEnabledForUserConsent: jest.fn().mockReturnValue(true), + }, + Identity: { + getCurrentUser: jest.fn().mockReturnValue({ + getMPID: () => '123', + }), + }, + Logger: { + verbose: loggerSpy, + }, + } as unknown) as MParticleWebSDK; + + const cookieSyncManager = new CookieSyncManager(mockSDK); + + let cookieSyncDates = []; + cookieSyncManager.performCookieSync( + 'https://test.com', + 42, + '1234', + cookieSyncDates, + null, + true, + true + ); + + // Simulate image load + mockImage.onload(); + + expect(mockImage.src).toBe(''); + expect(cookieSyncDates[42]).toBeUndefined(); + }); + }); +}); diff --git a/test/jest/utils.spec.ts b/test/jest/utils.spec.ts index 8ae27d156..395adf17e 100644 --- a/test/jest/utils.spec.ts +++ b/test/jest/utils.spec.ts @@ -1,4 +1,4 @@ -import { queryStringParser, getCookies, getHref } from '../../src/utils'; +import { queryStringParser, getCookies, getHref, replaceMPID, replaceAmpWithAmpersand } from '../../src/utils'; import { deleteAllCookies } from './utils'; @@ -159,4 +159,20 @@ describe('Utils', () => { }); }); + describe('#replaceMPID', () => { + it('replaces the MPID in a string', () => { + const mpid = '1234'; + const string = 'https://www.google.com?mpid=%%mpid%%&foo=bar'; + + expect(replaceMPID(string, mpid)).toEqual('https://www.google.com?mpid=1234&foo=bar'); + }); + }); + + describe('#replaceAmpWithAmpersand', () => { + it('replaces all instances of amp with ampersand', () => { + const string = 'https://www.google.com?mpid=%%mpid%%&foo=bar'; + + expect(replaceAmpWithAmpersand(string)).toEqual('https://www.google.com?mpid=%%mpid%%&foo=bar'); + }); + }); }); diff --git a/test/src/tests-cookie-syncing.js b/test/src/tests-cookie-syncing.ts similarity index 94% rename from test/src/tests-cookie-syncing.js rename to test/src/tests-cookie-syncing.ts index 036b070d2..71d2236f9 100644 --- a/test/src/tests-cookie-syncing.js +++ b/test/src/tests-cookie-syncing.ts @@ -1,11 +1,15 @@ +import { expect } from 'chai'; import Utils from './config/utils'; import fetchMock from 'fetch-mock/esm/client'; import { urls, testMPID, MPConfig, v4LSKey, apiKey } from './config/constants'; +import { MParticleWebSDK } from '../../src/sdkRuntimeModels'; +import { PixelConfiguration } from '../../src/store'; +import { IMParticleUser } from '../../src/identity-user-interfaces'; const { fetchMockSuccess, waitForCondition, hasIdentifyReturned } = Utils; const { setLocalStorage, MockForwarder, getLocalStorage } = Utils; -let pixelSettings = { +const pixelSettings: PixelConfiguration = { name: 'TestPixel', moduleId: 5, esId: 24053, @@ -17,6 +21,15 @@ let pixelSettings = { redirectUrl: '', }; +declare global { + interface Window { + mParticle: MParticleWebSDK; + fetchMock: any; + } +} + +const mParticle = window.mParticle; + describe('cookie syncing', function() { const timeout = 100; // Have a reference to createElement function to reset after all cookie sync @@ -27,8 +40,8 @@ describe('cookie syncing', function() { // Mock the img create onload method // https://raminmousavi.medium.com/mock-img-element-in-jest-3341c495ca8b window.document.createElement = (function(create) { - return function() { - const element = create.apply(this, arguments); + return function(this: Document) { + const element = create.apply(this, arguments as unknown as [string, ElementCreationOptions?]); if (element.tagName === 'IMG') { setTimeout(() => { @@ -68,9 +81,9 @@ describe('cookie syncing', function() { waitForCondition(hasIdentifyReturned) .then(() => { setTimeout(function() { - Should( + expect( mParticle.getInstance()._Store.pixelConfigurations.length - ).equal(1); + ).to.equal(1); const data = mParticle.getInstance()._Persistence.getLocalStorage(); data[testMPID].csd.should.have.property('5'); @@ -94,14 +107,14 @@ describe('cookie syncing', function() { mParticle.init(apiKey, window.mParticle.config); setTimeout(function() { - Should( + expect( mParticle.getInstance()._Store.pixelConfigurations.length - ).equal(1); + ).to.equal(1); const data = mParticle.getInstance()._Persistence.getLocalStorage(); const updated = data[testMPID].csd['5'] > 500; - Should(updated).be.ok(); + expect(updated).to.be.ok; done(); }, timeout); @@ -126,9 +139,9 @@ describe('cookie syncing', function() { mParticle.getInstance()._Persistence.getLocalStorage().testMPID .csd['5'] ); - Should( + expect( mParticle.getInstance()._Store.pixelConfigurations.length - ).equal(1); + ).to.equal(1); done(); }, timeout); @@ -163,11 +176,11 @@ describe('cookie syncing', function() { const data2 = mParticle .getInstance() ._Persistence.getLocalStorage(); - data1[testMPID].csd[5].should.be.ok(); - data2['otherMPID'].csd[5].should.be.ok(); - Should( + data1[testMPID].csd[5].should.be.ok; + data2['otherMPID'].csd[5].should.be.ok; + expect( mParticle.getInstance()._Store.pixelConfigurations.length - ).equal(1); + ).to.equal(1); done(); }, timeout); @@ -201,9 +214,9 @@ describe('cookie syncing', function() { ._Persistence.getLocalStorage(); Object.keys(data1[testMPID]).should.not.have.property('csd'); - Should( + expect( mParticle.getInstance()._Store.pixelConfigurations.length - ).equal(0); + ).to.equal(0); done(); }, 500); @@ -233,39 +246,14 @@ describe('cookie syncing', function() { .getInstance() ._Persistence.getLocalStorage(); data1[testMPID].should.not.have.property('csd'); - Should( + expect( mParticle.getInstance()._Store.pixelConfigurations.length - ).equal(0); + ).to.equal(0); done(); }, timeout); }); - it('should replace mpID properly', function(done) { - const result = mParticle - .getInstance() - ._CookieSyncManager.replaceMPID( - 'www.google.com?mpid=%%mpid%%?foo=bar', - 123 - ); - - result.should.equal('www.google.com?mpid=123?foo=bar'); - - done(); - }); - - it("should remove 'amp;' from the URLs", function(done) { - const result = mParticle - .getInstance() - ._CookieSyncManager.replaceAmp( - 'www.google.com?mpid=%%mpid%%&foo=bar' - ); - - result.should.equal('www.google.com?mpid=%%mpid%%&foo=bar'); - - done(); - }); - it('parse and capture pixel settings properly from backend', function(done) { mParticle._resetForTests(MPConfig); window.mParticle.config.requestConfig = true; @@ -361,9 +349,9 @@ describe('cookie syncing', function() { mParticle.init(apiKey, window.mParticle.config); setTimeout(function() { - Should( + expect( mParticle.getInstance()._Store.pixelConfigurations.length - ).equal(1); + ).to.equal(1); const data = mParticle.getInstance()._Persistence.getLocalStorage(); data[testMPID].csd.should.have.property('5'); @@ -391,7 +379,7 @@ describe('cookie syncing', function() { .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, null); - enabled.should.not.be.ok(); + enabled.should.not.be.ok; done(); }); @@ -422,12 +410,12 @@ describe('cookie syncing', function() { 'foo purpose 1', mParticle.getInstance().Consent.createGDPRConsent(userConsent) ); - const user = MockUser(); + const user = MockUser() as IMParticleUser; user.setConsentState(consentState); const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.not.be.ok(); + enabled.should.not.be.ok; done(); }); @@ -456,13 +444,13 @@ describe('cookie syncing', function() { 'foo purpose 1', mParticle.getInstance().Consent.createGDPRConsent(userConsent) ); - const user = MockUser(); + const user = MockUser() as IMParticleUser; user.setConsentState(consentState); const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.not.be.ok(); + enabled.should.not.be.ok; done(); }); @@ -493,13 +481,13 @@ describe('cookie syncing', function() { 'foo purpose 1', mParticle.getInstance().Consent.createGDPRConsent(userConsent) ); - const user = MockUser(); + const user = MockUser() as IMParticleUser; user.setConsentState(consentState); const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.be.ok(); + enabled.should.be.ok; done(); }); @@ -530,12 +518,12 @@ describe('cookie syncing', function() { 'foo purpose 1', mParticle.getInstance().Consent.createGDPRConsent(userConsent) ); - const user = MockUser(); + const user = MockUser() as IMParticleUser; user.setConsentState(consentState); const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.be.ok(); + enabled.should.be.ok; done(); }); @@ -566,12 +554,12 @@ describe('cookie syncing', function() { 'foo purpose 1', mParticle.getInstance().Consent.createGDPRConsent(userConsented) ); - const user = MockUser(); + const user = MockUser() as IMParticleUser; user.setConsentState(consentState); const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.not.be.ok(); + enabled.should.not.be.ok; done(); }); @@ -602,12 +590,12 @@ describe('cookie syncing', function() { 'foo purpose 1', mParticle.getInstance().Consent.createGDPRConsent(userConsented) ); - const user = MockUser(); + const user = MockUser() as IMParticleUser; user.setConsentState(consentState); const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.be.ok(); + enabled.should.be.ok; done(); }); @@ -638,12 +626,12 @@ describe('cookie syncing', function() { 'foo purpose 1', mParticle.getInstance().Consent.createGDPRConsent(userConsented) ); - const user = MockUser(); + const user = MockUser() as IMParticleUser; user.setConsentState(consentState); const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.be.ok(); + enabled.should.be.ok; done(); }); @@ -674,12 +662,12 @@ describe('cookie syncing', function() { 'foo purpose 1', mParticle.getInstance().Consent.createGDPRConsent(userConsented) ); - const user = MockUser(); + const user = MockUser() as IMParticleUser; user.setConsentState(consentState); const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.be.ok(); + enabled.should.be.ok; done(); }); @@ -709,12 +697,12 @@ describe('cookie syncing', function() { .setCCPAConsentState( mParticle.getInstance().Consent.createCCPAConsent(ccpaPresent) ); - const user = MockUser(); + const user = MockUser() as IMParticleUser; user.setConsentState(consentState); const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.not.be.ok(); + enabled.should.not.be.ok; done(); }); @@ -744,12 +732,12 @@ describe('cookie syncing', function() { .setCCPAConsentState( mParticle.getInstance().Consent.createCCPAConsent(ccpaPresent) ); - const user = MockUser(); + const user = MockUser() as IMParticleUser; user.setConsentState(consentState); const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.not.be.ok(); + enabled.should.not.be.ok; done(); }); @@ -779,12 +767,12 @@ describe('cookie syncing', function() { .setCCPAConsentState( mParticle.getInstance().Consent.createCCPAConsent(ccpaPresent) ); - const user = MockUser(); + const user = MockUser() as IMParticleUser; user.setConsentState(consentState); const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.be.ok(); + enabled.should.be.ok; done(); }); @@ -814,12 +802,12 @@ describe('cookie syncing', function() { .setCCPAConsentState( mParticle.getInstance().Consent.createCCPAConsent(ccpaPresent) ); - const user = MockUser(); + const user = MockUser() as IMParticleUser; user.setConsentState(consentState); const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.be.ok(); + enabled.should.be.ok; done(); }); @@ -849,12 +837,12 @@ describe('cookie syncing', function() { .setCCPAConsentState( mParticle.getInstance().Consent.createCCPAConsent(ccpaPresent) ); - const user = MockUser(); + const user = MockUser() as IMParticleUser; user.setConsentState(consentState); const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.not.be.ok(); + enabled.should.not.be.ok; done(); }); @@ -884,12 +872,12 @@ describe('cookie syncing', function() { .setCCPAConsentState( mParticle.getInstance().Consent.createCCPAConsent(ccpaPresent) ); - const user = MockUser(); + const user = MockUser() as IMParticleUser; user.setConsentState(consentState); const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.be.ok(); + enabled.should.be.ok; done(); }); @@ -919,12 +907,12 @@ describe('cookie syncing', function() { .setCCPAConsentState( mParticle.getInstance().Consent.createCCPAConsent(ccpaPresent) ); - const user = MockUser(); + const user = MockUser() as IMParticleUser; user.setConsentState(consentState); const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.be.ok(); + enabled.should.be.ok; done(); }); @@ -1225,7 +1213,7 @@ describe('cookie syncing', function() { mParticle.config.isDevelopmentMode = false; // pixelSetting1 has consent required, and so should only perform a cookiesync after consent is saved to the user - const pixelSettings1 = { + const pixelSettings1: PixelConfiguration = { name: 'TestPixel', moduleId: 1, esId: 24053, From 54e057f5b74321be5e58011ea7fcbe344c6c60e0 Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Thu, 31 Oct 2024 12:40:13 -0500 Subject: [PATCH 02/22] rename import --- src/mp-instance.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mp-instance.js b/src/mp-instance.js index 00b5f1a92..b18158339 100644 --- a/src/mp-instance.js +++ b/src/mp-instance.js @@ -21,7 +21,7 @@ import Constants from './constants'; import APIClient from './apiClient'; import Helpers from './helpers'; import NativeSdkHelpers from './nativeSdkHelpers'; -import CookieSyncManager from './CookieSyncManager'; +import CookieSyncManager from './cookieSyncManager'; import SessionManager from './sessionManager'; import Ecommerce from './ecommerce'; import Store from './store'; From b75ba9f04693514cf6441bdf5d963d872a0b52b4 Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Thu, 31 Oct 2024 12:42:57 -0500 Subject: [PATCH 03/22] Remove migrated methods --- src/cookieSyncManager.interfaces.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/cookieSyncManager.interfaces.ts b/src/cookieSyncManager.interfaces.ts index 1f471a5b9..ff2ed651f 100644 --- a/src/cookieSyncManager.interfaces.ts +++ b/src/cookieSyncManager.interfaces.ts @@ -13,11 +13,4 @@ export interface ICookieSyncManager { mpidIsNotInCookies: boolean, requiresConsent: boolean ) => void; - replaceAmpWithAmpersand: (string: string) => string; - replaceMPID: (string: string, mpid: MPID) => string; - - /** - * @deprecated replaceAmp has been deprecated, use replaceAmpersandWithAmp instead - */ - replaceAmp: (string: string) => string; } \ No newline at end of file From e139d03957f41211c5f99707e94c674a1fd38dab Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Thu, 31 Oct 2024 12:48:31 -0500 Subject: [PATCH 04/22] Remove empty lines --- src/utils.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index a7dad1554..d53505870 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -338,8 +338,6 @@ const getHref = (): string => { : ''; }; - - export { createCookieString, revertCookieString, From ac7b53bb4f5a56fde1c09d5d97f998080375073b Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Thu, 7 Nov 2024 11:59:09 -0500 Subject: [PATCH 05/22] Remove coverage build files from bundle --- .gitignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index adfe36ad3..1c05a798e 100644 --- a/.gitignore +++ b/.gitignore @@ -12,4 +12,5 @@ test/integrations/requirejs/test-requirejs-bundle.js browserstack.err local.log test/cross-browser-testing/CBT-tests-es5.js -test/cross-browser-testing/CBT-tests.js \ No newline at end of file +test/cross-browser-testing/CBT-tests.js +coverage/ \ No newline at end of file From 619debb7046d1800d0d4294cece29320f20153fd Mon Sep 17 00:00:00 2001 From: Alex S <49695018+alexs-mparticle@users.noreply.github.com> Date: Thu, 7 Nov 2024 12:39:26 -0500 Subject: [PATCH 06/22] Apply suggestions from code review Co-authored-by: Robert Ing --- src/cookieSyncManager.ts | 3 +-- test/jest/cookieSyncManager.spec.ts | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/cookieSyncManager.ts b/src/cookieSyncManager.ts index dd15c4439..5e22182e3 100644 --- a/src/cookieSyncManager.ts +++ b/src/cookieSyncManager.ts @@ -7,7 +7,7 @@ import { Dictionary, MPID } from '@mparticle/web-sdk'; import { IConsentRules } from './consent'; const { Messages } = Constants; -const {InformationMessages} = Messages; +const { InformationMessages } = Messages; export default function CookieSyncManager(this: ICookieSyncManager, mpInstance: MParticleWebSDK) { const self = this; @@ -72,7 +72,6 @@ export default function CookieSyncManager(this: ICookieSyncManager, mpInstance: // outside of the loop var persistence = mpInstance._Persistence.getPersistence(); - // TODO: Is there a historic reason for checking for previousMPID? // it does not appear to be passed in anywhere if (previousMPID && previousMPID !== mpid) { diff --git a/test/jest/cookieSyncManager.spec.ts b/test/jest/cookieSyncManager.spec.ts index 440bec25c..030dc5f23 100644 --- a/test/jest/cookieSyncManager.spec.ts +++ b/test/jest/cookieSyncManager.spec.ts @@ -204,7 +204,7 @@ describe('CookieSyncManager', () => { ); }); - it('should perform a cookie sync if lastSyncDateForModule is past the frequency cap', () => { + it('should perform a cookie sync if lastSyncDateForModule is passed the frequency cap', () => { const now = new Date().getTime(); // Rev the date by one From b6a0be6bfebb5ffce2dad7cd853dbc2e89cf472d Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Thu, 7 Nov 2024 14:13:02 -0500 Subject: [PATCH 07/22] Address PR Comments --- src/cookieSyncManager.interfaces.ts | 21 ++- src/cookieSyncManager.ts | 263 +++++++++++++--------------- test/jest/cookieSyncManager.spec.ts | 99 ++++++++++- 3 files changed, 231 insertions(+), 152 deletions(-) diff --git a/src/cookieSyncManager.interfaces.ts b/src/cookieSyncManager.interfaces.ts index ff2ed651f..1b221f3b5 100644 --- a/src/cookieSyncManager.interfaces.ts +++ b/src/cookieSyncManager.interfaces.ts @@ -1,16 +1,25 @@ -import { MPID } from "@mparticle/web-sdk"; -import { Dictionary } from "./utils"; -import { IConsentRules } from "./consent"; +import { MPID } from '@mparticle/web-sdk'; +import { Dictionary } from './utils'; +import { IConsentRules } from './consent'; export interface ICookieSyncManager { - attemptCookieSync: (previousMPID: MPID, mpid: MPID, mpidIsNotInCookies: boolean) => void; + attemptCookieSync: ( + previousMPID: MPID, + mpid: MPID, + mpidIsNotInCookies: boolean + ) => void; performCookieSync: ( url: string, - moduleId: number, + moduleId: string, mpid: MPID, cookieSyncDates: Dictionary, filteringConsentRuleValues: IConsentRules, mpidIsNotInCookies: boolean, requiresConsent: boolean ) => void; -} \ No newline at end of file + combineUrlWithRedirect: ( + mpid: MPID, + pixelUrl: string, + redirectUrl: string + ) => string; +} diff --git a/src/cookieSyncManager.ts b/src/cookieSyncManager.ts index 5e22182e3..731f5c07b 100644 --- a/src/cookieSyncManager.ts +++ b/src/cookieSyncManager.ts @@ -1,163 +1,153 @@ -import { replaceAmpWithAmpersand, replaceMPID } from './utils'; +import { isEmpty, replaceAmpWithAmpersand, replaceMPID } from './utils'; import Constants from './constants'; import { ICookieSyncManager } from './cookieSyncManager.interfaces'; import { MParticleWebSDK } from './sdkRuntimeModels'; -import { PixelConfiguration } from './store'; import { Dictionary, MPID } from '@mparticle/web-sdk'; import { IConsentRules } from './consent'; const { Messages } = Constants; const { InformationMessages } = Messages; -export default function CookieSyncManager(this: ICookieSyncManager, mpInstance: MParticleWebSDK) { +interface PixelConfig { + moduleId: string; + frequencyCap: number; + pixelUrl: string; + redirectUrl: string; + filteringConsentRuleValues: IConsentRules; +} + +const DAYS_IN_MILLISECONDS = 1000 * 60 * 60 * 24; + +const hasFrequencyCapExpired = ( + lastSyncDate: number, + frequencyCap: number +): boolean => { + return ( + new Date().getTime() > + new Date(lastSyncDate).getTime() + frequencyCap * DAYS_IN_MILLISECONDS + ); +} + +export default function CookieSyncManager( + this: ICookieSyncManager, + mpInstance: MParticleWebSDK +) { const self = this; // Public - this.attemptCookieSync = function(previousMPID: MPID, mpid: MPID, mpidIsNotInCookies: boolean) { - // TODO: These should move inside the for loop - let pixelConfig: PixelConfiguration; - let lastSyncDateForModule: number; - let url: string; - let redirect: string; - let urlWithRedirect: string; - let requiresConsent: boolean; - - // TODO: Make this exit quicker instead of nested - if (mpid && !mpInstance._Store.webviewBridgeEnabled) { - mpInstance._Store.pixelConfigurations.forEach(function( - pixelSettings - ) { - // set requiresConsent to false to start each additional pixel configuration - // set to true only if filteringConsenRuleValues.values.length exists - requiresConsent = false; - - // TODO: Replace with isEmpty - if ( - pixelSettings.filteringConsentRuleValues && - pixelSettings.filteringConsentRuleValues.values && - pixelSettings.filteringConsentRuleValues.values.length - ) { - requiresConsent = true; - } + this.attemptCookieSync = ( + previousMPID: MPID, + mpid: MPID, + mpidIsNotInCookies: boolean + ): void => { + if (!mpid || mpInstance._Store.webviewBridgeEnabled) { + return; + } + + const { pixelConfigurations } = mpInstance._Store; + const persistence = mpInstance._Persistence.getPersistence(); + + pixelConfigurations.forEach(pixelSettings => { + // set requiresConsent to false to start each additional pixel configuration + // set to true only if filteringConsenRuleValues.values.length exists + let requiresConsent = false; + + // Filtering rules as defined in UI + const { filteringConsentRuleValues } = pixelSettings; + const { values } = filteringConsentRuleValues || {}; + + if (!isEmpty(values)) { + requiresConsent = true; + } + + // Kit Module ID + const moduleId = pixelSettings.moduleId.toString(); + + // Tells you how often we should do a cookie sync (in days) + const frequencyCap = pixelSettings.frequencyCap; - pixelConfig = { - // Kit Module ID - moduleId: pixelSettings.moduleId, - - // Tells you how often we should do a cookie sync (in days) - frequencyCap: pixelSettings.frequencyCap, - - // Url for cookie sync pixel - pixelUrl: replaceAmpWithAmpersand(pixelSettings.pixelUrl), - - // TODO: Document requirements for redirectUrl - redirectUrl: pixelSettings.redirectUrl - ? replaceAmpWithAmpersand(pixelSettings.redirectUrl) - : null, - - // Filtering rules as defined in UI - filteringConsentRuleValues: - pixelSettings.filteringConsentRuleValues, - }; - - // TODO: combine replaceMPID and replaceAmp into sanitizeUrl function - url = replaceMPID(pixelConfig.pixelUrl, mpid); - redirect = pixelConfig.redirectUrl - ? replaceMPID(pixelConfig.redirectUrl, mpid) - : ''; - urlWithRedirect = url + encodeURIComponent(redirect); - - - // TODO: Refactor so that Persistence is only called once - // outside of the loop - var persistence = mpInstance._Persistence.getPersistence(); - - // TODO: Is there a historic reason for checking for previousMPID? - // it does not appear to be passed in anywhere - if (previousMPID && previousMPID !== mpid) { - if (persistence && persistence[mpid]) { - if (!persistence[mpid].csd) { - persistence[mpid].csd = {}; - } - self.performCookieSync( - urlWithRedirect, - pixelConfig.moduleId, - mpid, - persistence[mpid].csd, - pixelConfig.filteringConsentRuleValues, - mpidIsNotInCookies, - requiresConsent - ); + // Url for cookie sync pixel + const pixelUrl = replaceAmpWithAmpersand(pixelSettings.pixelUrl); + + // TODO: Document requirements for redirectUrl + const redirectUrl = pixelSettings.redirectUrl + ? replaceAmpWithAmpersand(pixelSettings.redirectUrl) + : null; + + const urlWithRedirect = this.combineUrlWithRedirect( + mpid, + pixelUrl, + redirectUrl + ); + + // TODO: Is there a historic reason for checking for previousMPID? + // it does not appear to be passed in anywhere + if (previousMPID && previousMPID !== mpid) { + if (persistence && persistence[mpid]) { + if (!persistence[mpid].csd) { + persistence[mpid].csd = {}; } + self.performCookieSync( + urlWithRedirect, + moduleId, + mpid, + persistence[mpid].csd, + filteringConsentRuleValues, + mpidIsNotInCookies, + requiresConsent + ); + } + return; + } else { + if (!persistence || !persistence[mpid]) { return; - } else { - // TODO: Refactor to check for the inverse and exit early - // rather than nesting - if (persistence[mpid]) { - if (!persistence[mpid].csd) { - persistence[mpid].csd = {}; - } - lastSyncDateForModule = persistence[mpid].csd[ - pixelConfig.moduleId.toString() - ] - ? persistence[mpid].csd[ - pixelConfig.moduleId.toString() - ] - : null; - - if (lastSyncDateForModule) { - // Check to see if we need to refresh cookieSync - if ( - // TODO: Turn this into a convenience method for readability? - // We use similar comparisons elsewhere in the SDK, - // so perhaps we can make a time comparison convenience method - new Date().getTime() > - new Date(lastSyncDateForModule).getTime() + - pixelConfig.frequencyCap * - // TODO: Turn these numbers into a constant so - // we can remember what this number is for - 60 * - 1000 * - 60 * - 24 - ) { - self.performCookieSync( - urlWithRedirect, - pixelConfig.moduleId, - mpid, - persistence[mpid].csd, - pixelConfig.filteringConsentRuleValues, - mpidIsNotInCookies, - requiresConsent - ); - } - } else { - self.performCookieSync( - urlWithRedirect, - pixelConfig.moduleId, - mpid, - persistence[mpid].csd, - pixelConfig.filteringConsentRuleValues, - mpidIsNotInCookies, - requiresConsent - ); - } - } } - }); - } + + if (!persistence[mpid].csd) { + persistence[mpid].csd = {}; + } + + const lastSyncDateForModule = persistence[mpid].csd[moduleId] + ? persistence[mpid].csd[moduleId] + : null; + + // Check to see if we need to refresh cookieSync + if (hasFrequencyCapExpired(lastSyncDateForModule, frequencyCap)) { + self.performCookieSync( + urlWithRedirect, + moduleId, + mpid, + persistence[mpid].csd, + filteringConsentRuleValues, + mpidIsNotInCookies, + requiresConsent + ); + } + + } + }); + }; + + this.combineUrlWithRedirect = ( + mpid: MPID, + pixelUrl: string, + redirectUrl + ): string => { + const url = replaceMPID(pixelUrl, mpid); + const redirect = redirectUrl ? replaceMPID(redirectUrl, mpid) : ''; + return url + encodeURIComponent(redirect); }; // Private - this.performCookieSync = function( + this.performCookieSync = ( url: string, - moduleId: number, + moduleId: string, mpid: MPID, cookieSyncDates: Dictionary, filteringConsentRuleValues: IConsentRules, mpidIsNotInCookies: boolean, requiresConsent: boolean - ) { + ): void => { // if MPID is new to cookies, we should not try to perform the cookie sync // because a cookie sync can only occur once a user either consents or doesn't // we should not check if its enabled if the user has a blank consent @@ -182,8 +172,7 @@ export default function CookieSyncManager(this: ICookieSyncManager, mpInstance: mpInstance.Logger.verbose(InformationMessages.CookieSync); img.onload = function() { - // TODO: Break this out into a convenience method so we can unit test - cookieSyncDates[moduleId.toString()] = new Date().getTime(); + cookieSyncDates[moduleId] = new Date().getTime(); mpInstance._Persistence.saveUserCookieSyncDatesToPersistence( mpid, cookieSyncDates diff --git a/test/jest/cookieSyncManager.spec.ts b/test/jest/cookieSyncManager.spec.ts index 030dc5f23..d80885a9b 100644 --- a/test/jest/cookieSyncManager.spec.ts +++ b/test/jest/cookieSyncManager.spec.ts @@ -15,7 +15,7 @@ const pixelSettings: PixelConfiguration = { redirectUrl: '', }; -describe('CookieSyncManager', () => { +describe.only('CookieSyncManager', () => { describe('#attemptCookieSync', () => { it('should perform a cookie sync with defaults', () => { const mockSDK = ({ @@ -37,7 +37,7 @@ describe('CookieSyncManager', () => { expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( '', - 5, + '5', testMPID, {}, undefined, @@ -46,6 +46,48 @@ describe('CookieSyncManager', () => { ); }); + it('should return early if mpid is not defined', () => { + const mockSDK = ({ + _Store: { + webviewBridgeEnabled: false, + pixelConfigurations: [pixelSettings], + }, + _Persistence: { + getPersistence: () => ({testMPID: { + csd: {} + }}), + }, + } as unknown) as MParticleWebSDK; + + const cookieSyncManager = new CookieSyncManager(mockSDK); + cookieSyncManager.performCookieSync = jest.fn(); + + cookieSyncManager.attemptCookieSync(null, null, true); + + expect(cookieSyncManager.performCookieSync).not.toHaveBeenCalled(); + }); + + it('should return early if webviewBridgeEnabled is true', () => { + const mockSDK = ({ + _Store: { + webviewBridgeEnabled: true, + pixelConfigurations: [pixelSettings], + }, + _Persistence: { + getPersistence: () => ({testMPID: { + csd: {} + }}), + }, + } as unknown) as MParticleWebSDK; + + const cookieSyncManager = new CookieSyncManager(mockSDK); + cookieSyncManager.performCookieSync = jest.fn(); + + cookieSyncManager.attemptCookieSync(null, testMPID, true); + + expect(cookieSyncManager.performCookieSync).not.toHaveBeenCalled(); + }); + it('should toggle requiresConsent value if filtering filteringConsentRuleValues are defined', () => { const myPixelSettings: PixelConfiguration = { filteringConsentRuleValues: { @@ -72,7 +114,7 @@ describe('CookieSyncManager', () => { expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( '', - 5, + '5', testMPID, {}, { values: ['test'] }, @@ -106,7 +148,7 @@ describe('CookieSyncManager', () => { expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( 'https://test.com%3Fredirect%3Dhttps%3A%2F%2Fredirect.com%26mpid%3DtestMPID', - 5, + '5', testMPID, {}, undefined, @@ -135,7 +177,7 @@ describe('CookieSyncManager', () => { expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( '', - 5, + '5', testMPID, {}, undefined, @@ -164,7 +206,7 @@ describe('CookieSyncManager', () => { expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( '', - 5, + '5', testMPID, { 5: 1234567890 @@ -195,7 +237,7 @@ describe('CookieSyncManager', () => { expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( '', - 5, + '5', testMPID, {}, undefined, @@ -229,7 +271,7 @@ describe('CookieSyncManager', () => { expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( '', - 5, + '5', testMPID, { 5: cookieSyncDateInPast @@ -262,7 +304,7 @@ describe('CookieSyncManager', () => { expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( '', - 5, + '5', testMPID, {}, undefined, @@ -451,4 +493,43 @@ describe('CookieSyncManager', () => { expect(cookieSyncDates[42]).toBeUndefined(); }); }); + + describe('#combineUrlWithRedirect', () => { + it('should combine a pixelUrl with a redirectUrl', () => { + const mockSDK = ({ + _Store: { + webviewBridgeEnabled: false, + pixelConfigurations: [pixelSettings], + }, + } as unknown) as MParticleWebSDK; + + const cookieSyncManager = new CookieSyncManager(mockSDK); + + const result = cookieSyncManager.combineUrlWithRedirect( + '1234', + 'https://test.com', + '?redirect=https://redirect.com&mpid=%%mpid%%', + ); + + expect(result).toBe('https://test.com%3Fredirect%3Dhttps%3A%2F%2Fredirect.com%26mpid%3D1234'); + }); + + it('should return the pixelUrl if no redirectUrl is defined', () => { + const mockSDK = ({ + _Store: { + webviewBridgeEnabled: false, + pixelConfigurations: [pixelSettings], + }, + } as unknown) as MParticleWebSDK; + + const cookieSyncManager = new CookieSyncManager(mockSDK); + + const result = cookieSyncManager.combineUrlWithRedirect( + '1234', + 'https://test.com', + ); + + expect(result).toBe('https://test.com'); + }); + }); }); From 537e2ea84f350e6ff4a9a7a45f1c0b08226f3203 Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Thu, 7 Nov 2024 14:29:14 -0500 Subject: [PATCH 08/22] Address PR Comments --- test/src/tests-cookie-syncing.ts | 38 ++++++++++++++++---------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/test/src/tests-cookie-syncing.ts b/test/src/tests-cookie-syncing.ts index 71d2236f9..20bf07431 100644 --- a/test/src/tests-cookie-syncing.ts +++ b/test/src/tests-cookie-syncing.ts @@ -176,9 +176,9 @@ describe('cookie syncing', function() { const data2 = mParticle .getInstance() ._Persistence.getLocalStorage(); - data1[testMPID].csd[5].should.be.ok; - data2['otherMPID'].csd[5].should.be.ok; - expect( + expect(data1[testMPID].csd[5]).to.be.ok; + expect(data2['otherMPID'].csd[5]).to.be.ok; + expect( mParticle.getInstance()._Store.pixelConfigurations.length ).to.equal(1); @@ -379,7 +379,7 @@ describe('cookie syncing', function() { .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, null); - enabled.should.not.be.ok; + expect(enabled).to.not.be.ok; done(); }); @@ -415,7 +415,7 @@ describe('cookie syncing', function() { const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.not.be.ok; + expect(enabled).to.not.be.ok; done(); }); @@ -450,7 +450,7 @@ describe('cookie syncing', function() { .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.not.be.ok; + expect(enabled).to.not.be.ok; done(); }); @@ -487,7 +487,7 @@ describe('cookie syncing', function() { const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.be.ok; + expect(enabled).to.be.ok; done(); }); @@ -523,7 +523,7 @@ describe('cookie syncing', function() { const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.be.ok; + expect(enabled).to.be.ok; done(); }); @@ -559,7 +559,7 @@ describe('cookie syncing', function() { const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.not.be.ok; + expect(enabled).to.not.be.ok; done(); }); @@ -595,7 +595,7 @@ describe('cookie syncing', function() { const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.be.ok; + expect(enabled).to.be.ok; done(); }); @@ -631,7 +631,7 @@ describe('cookie syncing', function() { const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.be.ok; + expect(enabled).to.be.ok; done(); }); @@ -667,7 +667,7 @@ describe('cookie syncing', function() { const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.be.ok; + expect(enabled).to.be.ok; done(); }); @@ -702,7 +702,7 @@ describe('cookie syncing', function() { const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.not.be.ok; + expect(enabled).to.not.be.ok; done(); }); @@ -737,7 +737,7 @@ describe('cookie syncing', function() { const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.not.be.ok; + expect(enabled).to.not.be.ok; done(); }); @@ -772,7 +772,7 @@ describe('cookie syncing', function() { const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.be.ok; + expect(enabled).to.be.ok; done(); }); @@ -807,7 +807,7 @@ describe('cookie syncing', function() { const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.be.ok; + expect(enabled).to.be.ok; done(); }); @@ -842,7 +842,7 @@ describe('cookie syncing', function() { const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.not.be.ok; + expect(enabled).to.not.be.ok; done(); }); @@ -877,7 +877,7 @@ describe('cookie syncing', function() { const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.be.ok; + expect(enabled).to.be.ok; done(); }); @@ -912,7 +912,7 @@ describe('cookie syncing', function() { const enabled = mParticle .getInstance() ._Consent.isEnabledForUserConsent(filteringConsentRuleValues, user); - enabled.should.be.ok; + expect(enabled).to.be.ok; done(); }); From 2b371ee8641b4abfbcd12734273aaf17aadc666c Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Thu, 7 Nov 2024 14:53:11 -0500 Subject: [PATCH 09/22] Address PR Comments --- src/cookieSyncManager.ts | 2 +- test/jest/cookieSyncManager.spec.ts | 132 ++++++++++++++++++++-------- 2 files changed, 94 insertions(+), 40 deletions(-) diff --git a/src/cookieSyncManager.ts b/src/cookieSyncManager.ts index 731f5c07b..4a3f1e864 100644 --- a/src/cookieSyncManager.ts +++ b/src/cookieSyncManager.ts @@ -16,7 +16,7 @@ interface PixelConfig { filteringConsentRuleValues: IConsentRules; } -const DAYS_IN_MILLISECONDS = 1000 * 60 * 60 * 24; +export const DAYS_IN_MILLISECONDS = 1000 * 60 * 60 * 24; const hasFrequencyCapExpired = ( lastSyncDate: number, diff --git a/test/jest/cookieSyncManager.spec.ts b/test/jest/cookieSyncManager.spec.ts index d80885a9b..00459c3ad 100644 --- a/test/jest/cookieSyncManager.spec.ts +++ b/test/jest/cookieSyncManager.spec.ts @@ -1,4 +1,4 @@ -import CookieSyncManager from '../../src/cookieSyncManager'; +import CookieSyncManager, { DAYS_IN_MILLISECONDS } from '../../src/cookieSyncManager'; import { MParticleWebSDK } from '../../src/sdkRuntimeModels'; import { PixelConfiguration } from '../../src/store'; import { testMPID } from '../src/config/constants'; @@ -18,7 +18,7 @@ const pixelSettings: PixelConfiguration = { describe.only('CookieSyncManager', () => { describe('#attemptCookieSync', () => { it('should perform a cookie sync with defaults', () => { - const mockSDK = ({ + const mockMPInstance = ({ _Store: { webviewBridgeEnabled: false, pixelConfigurations: [pixelSettings], @@ -30,7 +30,7 @@ describe.only('CookieSyncManager', () => { }, } as unknown) as MParticleWebSDK; - const cookieSyncManager = new CookieSyncManager(mockSDK); + const cookieSyncManager = new CookieSyncManager(mockMPInstance); cookieSyncManager.performCookieSync = jest.fn(); cookieSyncManager.attemptCookieSync(null, testMPID, true); @@ -47,7 +47,7 @@ describe.only('CookieSyncManager', () => { }); it('should return early if mpid is not defined', () => { - const mockSDK = ({ + const mockMPInstance = ({ _Store: { webviewBridgeEnabled: false, pixelConfigurations: [pixelSettings], @@ -59,7 +59,7 @@ describe.only('CookieSyncManager', () => { }, } as unknown) as MParticleWebSDK; - const cookieSyncManager = new CookieSyncManager(mockSDK); + const cookieSyncManager = new CookieSyncManager(mockMPInstance); cookieSyncManager.performCookieSync = jest.fn(); cookieSyncManager.attemptCookieSync(null, null, true); @@ -68,7 +68,7 @@ describe.only('CookieSyncManager', () => { }); it('should return early if webviewBridgeEnabled is true', () => { - const mockSDK = ({ + const mockMPInstance = ({ _Store: { webviewBridgeEnabled: true, pixelConfigurations: [pixelSettings], @@ -80,7 +80,7 @@ describe.only('CookieSyncManager', () => { }, } as unknown) as MParticleWebSDK; - const cookieSyncManager = new CookieSyncManager(mockSDK); + const cookieSyncManager = new CookieSyncManager(mockMPInstance); cookieSyncManager.performCookieSync = jest.fn(); cookieSyncManager.attemptCookieSync(null, testMPID, true); @@ -95,7 +95,7 @@ describe.only('CookieSyncManager', () => { }, }; - const mockSDK = ({ + const mockMPInstance = ({ _Store: { webviewBridgeEnabled: false, pixelConfigurations: [{...pixelSettings, ...myPixelSettings}], @@ -107,7 +107,7 @@ describe.only('CookieSyncManager', () => { }, } as unknown) as MParticleWebSDK; - const cookieSyncManager = new CookieSyncManager(mockSDK); + const cookieSyncManager = new CookieSyncManager(mockMPInstance); cookieSyncManager.performCookieSync = jest.fn(); cookieSyncManager.attemptCookieSync(null, testMPID, true); @@ -129,7 +129,7 @@ describe.only('CookieSyncManager', () => { redirectUrl: '?redirect=https://redirect.com&mpid=%%mpid%%', }; - const mockSDK = ({ + const mockMPInstance = ({ _Store: { webviewBridgeEnabled: false, pixelConfigurations: [{...pixelSettings, ...myPixelSettings}], @@ -141,7 +141,7 @@ describe.only('CookieSyncManager', () => { }, } as unknown) as MParticleWebSDK; - const cookieSyncManager = new CookieSyncManager(mockSDK); + const cookieSyncManager = new CookieSyncManager(mockMPInstance); cookieSyncManager.performCookieSync = jest.fn(); cookieSyncManager.attemptCookieSync(null, testMPID, true); @@ -159,7 +159,7 @@ describe.only('CookieSyncManager', () => { // QUESTION: What is the purpose of this code path? it('should call performCookieSync with mpid if previousMpid and mpid do not match', () => { - const mockSDK = ({ + const mockMPInstance = ({ _Store: { webviewBridgeEnabled: false, pixelConfigurations: [pixelSettings], @@ -170,7 +170,7 @@ describe.only('CookieSyncManager', () => { }, } as unknown) as MParticleWebSDK; - const cookieSyncManager = new CookieSyncManager(mockSDK); + const cookieSyncManager = new CookieSyncManager(mockMPInstance); cookieSyncManager.performCookieSync = jest.fn(); cookieSyncManager.attemptCookieSync('other-mpid', testMPID, true); @@ -187,7 +187,7 @@ describe.only('CookieSyncManager', () => { }); it('should call performCookieSync with mpid and csd if previousMpid and mpid do not match', () => { - const mockSDK = ({ + const mockMPInstance = ({ _Store: { webviewBridgeEnabled: false, pixelConfigurations: [pixelSettings], @@ -199,7 +199,7 @@ describe.only('CookieSyncManager', () => { }, } as unknown) as MParticleWebSDK; - const cookieSyncManager = new CookieSyncManager(mockSDK); + const cookieSyncManager = new CookieSyncManager(mockMPInstance); cookieSyncManager.performCookieSync = jest.fn(); cookieSyncManager.attemptCookieSync('other-mpid', testMPID, true); @@ -219,7 +219,7 @@ describe.only('CookieSyncManager', () => { // QUESTION: What is the purpose of this code path? it('should call performCookieSync with mpid if previousMpid and mpid match', () => { - const mockSDK = ({ + const mockMPInstance = ({ _Store: { webviewBridgeEnabled: false, pixelConfigurations: [pixelSettings], @@ -230,7 +230,7 @@ describe.only('CookieSyncManager', () => { }, } as unknown) as MParticleWebSDK; - const cookieSyncManager = new CookieSyncManager(mockSDK); + const cookieSyncManager = new CookieSyncManager(mockMPInstance); cookieSyncManager.performCookieSync = jest.fn(); cookieSyncManager.attemptCookieSync(testMPID, testMPID, true); @@ -246,13 +246,13 @@ describe.only('CookieSyncManager', () => { ); }); - it('should perform a cookie sync if lastSyncDateForModule is passed the frequency cap', () => { + it('should perform a cookie sync if lastSyncDateForModule has passed the frequency cap', () => { const now = new Date().getTime(); // Rev the date by one - const cookieSyncDateInPast = now - ( pixelSettings.frequencyCap + 1 ) * 60 * 60 * 24 * 1000; + const cookieSyncDateInPast = now - ( pixelSettings.frequencyCap + 1 ) * DAYS_IN_MILLISECONDS; - const mockSDK = ({ + const mockMPInstance = ({ _Store: { webviewBridgeEnabled: false, pixelConfigurations: [pixelSettings], @@ -264,7 +264,7 @@ describe.only('CookieSyncManager', () => { }, } as unknown) as MParticleWebSDK; - const cookieSyncManager = new CookieSyncManager(mockSDK); + const cookieSyncManager = new CookieSyncManager(mockMPInstance); cookieSyncManager.performCookieSync = jest.fn(); cookieSyncManager.attemptCookieSync(null, testMPID, true); @@ -285,7 +285,7 @@ describe.only('CookieSyncManager', () => { it('should perform a cookie sync if lastSyncDateForModule is past the frequency cap even if csd is empty', () => { const now = new Date().getTime(); - const mockSDK = ({ + const mockMPInstance = ({ _Store: { webviewBridgeEnabled: false, pixelConfigurations: [pixelSettings], @@ -297,7 +297,7 @@ describe.only('CookieSyncManager', () => { }, } as unknown) as MParticleWebSDK; - const cookieSyncManager = new CookieSyncManager(mockSDK); + const cookieSyncManager = new CookieSyncManager(mockMPInstance); cookieSyncManager.performCookieSync = jest.fn(); cookieSyncManager.attemptCookieSync(null, testMPID, true); @@ -314,7 +314,7 @@ describe.only('CookieSyncManager', () => { }); it('should sync cookies when there was not a previous cookie-sync', () => { - const mockSDK = ({ + const mockMPInstance = ({ _Store: { webviewBridgeEnabled: false, pixelConfigurations: [pixelSettings], @@ -324,11 +324,11 @@ describe.only('CookieSyncManager', () => { }, } as unknown) as MParticleWebSDK; - const cookieSyncManager = new CookieSyncManager(mockSDK); + const cookieSyncManager = new CookieSyncManager(mockMPInstance); cookieSyncManager.attemptCookieSync(null, '456', true); - expect(mockSDK._Store.pixelConfigurations.length).toBe(1); - expect(mockSDK._Store.pixelConfigurations[0].moduleId).toBe(5); + expect(mockMPInstance._Store.pixelConfigurations.length).toBe(1); + expect(mockMPInstance._Store.pixelConfigurations[0].moduleId).toBe(5); }); }); @@ -342,7 +342,7 @@ describe.only('CookieSyncManager', () => { (mockImage as unknown) as HTMLImageElement ); - const mockSDK = ({ + const mockMPInstance = ({ _Store: { webviewBridgeEnabled: false, pixelConfigurations: [pixelSettings], @@ -365,7 +365,7 @@ describe.only('CookieSyncManager', () => { }, } as unknown) as MParticleWebSDK; - const cookieSyncManager = new CookieSyncManager(mockSDK); + const cookieSyncManager = new CookieSyncManager(mockMPInstance); let cookieSyncDates = []; cookieSyncManager.performCookieSync( @@ -397,7 +397,7 @@ describe.only('CookieSyncManager', () => { const loggerSpy = jest.fn(); - const mockSDK = ({ + const mockMPInstance = ({ _Store: { webviewBridgeEnabled: false, pixelConfigurations: [pixelSettings], @@ -420,7 +420,7 @@ describe.only('CookieSyncManager', () => { }, } as unknown) as MParticleWebSDK; - const cookieSyncManager = new CookieSyncManager(mockSDK); + const cookieSyncManager = new CookieSyncManager(mockMPInstance); let cookieSyncDates = []; cookieSyncManager.performCookieSync( @@ -439,7 +439,7 @@ describe.only('CookieSyncManager', () => { expect(loggerSpy).toHaveBeenCalledWith('Performing cookie sync'); }); - it('should return early if the user has not consented to the cookie sync', () => { + it.only('should return early if the user has not consented to the cookie sync', () => { const mockImage = { onload: jest.fn(), src: '', @@ -450,7 +450,61 @@ describe.only('CookieSyncManager', () => { const loggerSpy = jest.fn(); - const mockSDK = ({ + const mockMPInstance = ({ + _Store: { + webviewBridgeEnabled: false, + pixelConfigurations: [pixelSettings], + }, + _Persistence: { + setCookieSyncDates: jest.fn(), + getPersistence: jest.fn(), + saveUserCookieSyncDatesToPersistence: jest.fn(), + }, + _Consent: { + isEnabledForUserConsent: jest.fn().mockReturnValue(false), + }, + Identity: { + getCurrentUser: jest.fn().mockReturnValue({ + getMPID: () => '123', + }), + }, + Logger: { + verbose: loggerSpy, + }, + } as unknown) as MParticleWebSDK; + + const cookieSyncManager = new CookieSyncManager(mockMPInstance); + + let cookieSyncDates = []; + cookieSyncManager.performCookieSync( + 'https://test.com', + 42, + '1234', + cookieSyncDates, + null, + false, + false, + ); + + // Simulate image load + mockImage.onload(); + + expect(mockImage.src).toBe(''); + expect(cookieSyncDates[42]).toBeUndefined(); + }); + + it.only('should return early if requiresConsent and mpidIsNotInCookies are both true', () => { + const mockImage = { + onload: jest.fn(), + src: '', + }; + jest.spyOn(document, 'createElement').mockReturnValue( + (mockImage as unknown) as HTMLImageElement + ); + + const loggerSpy = jest.fn(); + + const mockMPInstance = ({ _Store: { webviewBridgeEnabled: false, pixelConfigurations: [pixelSettings], @@ -473,7 +527,7 @@ describe.only('CookieSyncManager', () => { }, } as unknown) as MParticleWebSDK; - const cookieSyncManager = new CookieSyncManager(mockSDK); + const cookieSyncManager = new CookieSyncManager(mockMPInstance); let cookieSyncDates = []; cookieSyncManager.performCookieSync( @@ -483,7 +537,7 @@ describe.only('CookieSyncManager', () => { cookieSyncDates, null, true, - true + true, ); // Simulate image load @@ -496,14 +550,14 @@ describe.only('CookieSyncManager', () => { describe('#combineUrlWithRedirect', () => { it('should combine a pixelUrl with a redirectUrl', () => { - const mockSDK = ({ + const mockMPInstance = ({ _Store: { webviewBridgeEnabled: false, pixelConfigurations: [pixelSettings], }, } as unknown) as MParticleWebSDK; - const cookieSyncManager = new CookieSyncManager(mockSDK); + const cookieSyncManager = new CookieSyncManager(mockMPInstance); const result = cookieSyncManager.combineUrlWithRedirect( '1234', @@ -515,14 +569,14 @@ describe.only('CookieSyncManager', () => { }); it('should return the pixelUrl if no redirectUrl is defined', () => { - const mockSDK = ({ + const mockMPInstance = ({ _Store: { webviewBridgeEnabled: false, pixelConfigurations: [pixelSettings], }, } as unknown) as MParticleWebSDK; - const cookieSyncManager = new CookieSyncManager(mockSDK); + const cookieSyncManager = new CookieSyncManager(mockMPInstance); const result = cookieSyncManager.combineUrlWithRedirect( '1234', From c93fa2765a2fc35ab8f881187faed70aa2288537 Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Thu, 7 Nov 2024 15:34:17 -0500 Subject: [PATCH 10/22] Address PR Comments --- test/jest/cookieSyncManager.spec.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/jest/cookieSyncManager.spec.ts b/test/jest/cookieSyncManager.spec.ts index 00459c3ad..b1fbd9353 100644 --- a/test/jest/cookieSyncManager.spec.ts +++ b/test/jest/cookieSyncManager.spec.ts @@ -17,6 +17,7 @@ const pixelSettings: PixelConfiguration = { describe.only('CookieSyncManager', () => { describe('#attemptCookieSync', () => { + // https://go.mparticle.com/work/SQDSDKS-6915 it('should perform a cookie sync with defaults', () => { const mockMPInstance = ({ _Store: { @@ -157,7 +158,7 @@ describe.only('CookieSyncManager', () => { ); }); - // QUESTION: What is the purpose of this code path? + // https://go.mparticle.com/work/SQDSDKS-6915 it('should call performCookieSync with mpid if previousMpid and mpid do not match', () => { const mockMPInstance = ({ _Store: { @@ -186,7 +187,7 @@ describe.only('CookieSyncManager', () => { ); }); - it('should call performCookieSync with mpid and csd if previousMpid and mpid do not match', () => { + it('should include mpid AND csd when calling performCookieSync if previousMpid and mpid do not match', () => { const mockMPInstance = ({ _Store: { webviewBridgeEnabled: false, From e2c6d5c00f3be07b640363aa9bce081cdb073f0a Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Thu, 7 Nov 2024 17:09:18 -0500 Subject: [PATCH 11/22] Address PR Comments --- src/cookieSyncManager.interfaces.ts | 2 +- src/cookieSyncManager.ts | 8 +------- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/cookieSyncManager.interfaces.ts b/src/cookieSyncManager.interfaces.ts index 1b221f3b5..83facd52f 100644 --- a/src/cookieSyncManager.interfaces.ts +++ b/src/cookieSyncManager.interfaces.ts @@ -6,7 +6,7 @@ export interface ICookieSyncManager { attemptCookieSync: ( previousMPID: MPID, mpid: MPID, - mpidIsNotInCookies: boolean + mpidIsNotInCookies?: boolean ) => void; performCookieSync: ( url: string, diff --git a/src/cookieSyncManager.ts b/src/cookieSyncManager.ts index 4a3f1e864..4e218de67 100644 --- a/src/cookieSyncManager.ts +++ b/src/cookieSyncManager.ts @@ -38,7 +38,7 @@ export default function CookieSyncManager( this.attemptCookieSync = ( previousMPID: MPID, mpid: MPID, - mpidIsNotInCookies: boolean + mpidIsNotInCookies?: boolean ): void => { if (!mpid || mpInstance._Store.webviewBridgeEnabled) { return; @@ -151,16 +151,10 @@ export default function CookieSyncManager( // if MPID is new to cookies, we should not try to perform the cookie sync // because a cookie sync can only occur once a user either consents or doesn't // we should not check if its enabled if the user has a blank consent - // TODO: We should do this check outside of this function if (requiresConsent && mpidIsNotInCookies) { return; } - // TODO: Refactor so that check is made outside of the function. - // Cache or store the boolean so that it only gets called once per - // cookie sync attempt per module. - // Currently, attemptCookieSync is called as a loop and therefore this - // function polls the user object and consent multiple times. if ( // https://go.mparticle.com/work/SQDSDKS-5009 mpInstance._Consent.isEnabledForUserConsent( From 0e9cbfe8080734d9cf528f9c0396bffd395e6cfd Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Thu, 7 Nov 2024 17:14:06 -0500 Subject: [PATCH 12/22] Address PR Comments --- src/cookieSyncManager.interfaces.ts | 3 ++- src/cookieSyncManager.ts | 6 +++--- src/persistence.interfaces.ts | 7 +++---- src/store.ts | 3 ++- test/jest/cookieSyncManager.spec.ts | 4 ++-- 5 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/cookieSyncManager.interfaces.ts b/src/cookieSyncManager.interfaces.ts index 83facd52f..9daed8d06 100644 --- a/src/cookieSyncManager.interfaces.ts +++ b/src/cookieSyncManager.interfaces.ts @@ -2,6 +2,7 @@ import { MPID } from '@mparticle/web-sdk'; import { Dictionary } from './utils'; import { IConsentRules } from './consent'; +export type CookieSyncDates = Dictionary; export interface ICookieSyncManager { attemptCookieSync: ( previousMPID: MPID, @@ -12,7 +13,7 @@ export interface ICookieSyncManager { url: string, moduleId: string, mpid: MPID, - cookieSyncDates: Dictionary, + cookieSyncDates: CookieSyncDates, filteringConsentRuleValues: IConsentRules, mpidIsNotInCookies: boolean, requiresConsent: boolean diff --git a/src/cookieSyncManager.ts b/src/cookieSyncManager.ts index 4e218de67..f9c902cb9 100644 --- a/src/cookieSyncManager.ts +++ b/src/cookieSyncManager.ts @@ -1,8 +1,8 @@ import { isEmpty, replaceAmpWithAmpersand, replaceMPID } from './utils'; import Constants from './constants'; -import { ICookieSyncManager } from './cookieSyncManager.interfaces'; +import { CookieSyncDates, ICookieSyncManager } from './cookieSyncManager.interfaces'; import { MParticleWebSDK } from './sdkRuntimeModels'; -import { Dictionary, MPID } from '@mparticle/web-sdk'; +import { MPID } from '@mparticle/web-sdk'; import { IConsentRules } from './consent'; const { Messages } = Constants; @@ -143,7 +143,7 @@ export default function CookieSyncManager( url: string, moduleId: string, mpid: MPID, - cookieSyncDates: Dictionary, + cookieSyncDates: CookieSyncDates, filteringConsentRuleValues: IConsentRules, mpidIsNotInCookies: boolean, requiresConsent: boolean diff --git a/src/persistence.interfaces.ts b/src/persistence.interfaces.ts index 59a1bd767..7bfc65f15 100644 --- a/src/persistence.interfaces.ts +++ b/src/persistence.interfaces.ts @@ -14,6 +14,7 @@ import { import { Dictionary } from './utils'; import { IMinifiedConsentJSONObject } from './consent'; import { UserAttributes } from './identity-user-interfaces'; +import { CookieSyncDates } from './cookieSyncManager.interfaces'; export type UploadsTable = Dictionary; export interface iForwardingStatsBatches { @@ -79,10 +80,8 @@ export interface IPersistenceMinified extends Dictionary { // }; } -export type CookieSyncDate = Dictionary; - export interface IUserPersistenceMinified extends Dictionary { - csd: CookieSyncDate; // Cookie Sync Dates // list of timestamps for last cookie sync + csd: CookieSyncDates; // Cookie Sync Dates // list of timestamps for last cookie sync con: IMinifiedConsentJSONObject; // Consent State ui: UserIdentities; // User Identities ua: UserAttributes; // User Attributes @@ -121,7 +120,7 @@ export interface IPersistence { getDomain(doc: string, locationHostname: string): string; getCartProducts(mpid: MPID): Product[]; setCartProducts(allProducts: Product[]): void; - saveUserCookieSyncDatesToPersistence(mpid: MPID, csd: CookieSyncDate): void; + saveUserCookieSyncDatesToPersistence(mpid: MPID, csd: CookieSyncDates): void; savePersistence(persistance: IPersistenceMinified): void; getPersistence(): IPersistenceMinified; getFirstSeenTime(mpid: MPID): string | null; diff --git a/src/store.ts b/src/store.ts index d48158e10..0947cb7ca 100644 --- a/src/store.ts +++ b/src/store.ts @@ -37,6 +37,7 @@ import { IGlobalStoreV2MinifiedKeys, IPersistenceMinified, } from './persistence.interfaces'; +import { CookieSyncDates } from './cookieSyncManager.interfaces'; // This represents the runtime configuration of the SDK AFTER // initialization has been complete and all settings and @@ -168,7 +169,7 @@ export interface IStore { identifyCalled: boolean; isLoggedIn: boolean; sideloadedKitsCount?: number; - cookieSyncDates: Dictionary; + cookieSyncDates: CookieSyncDates; integrationAttributes: IntegrationAttributes; requireDelay: boolean; isLocalStorageAvailable: boolean | null; diff --git a/test/jest/cookieSyncManager.spec.ts b/test/jest/cookieSyncManager.spec.ts index b1fbd9353..fa0ad74a1 100644 --- a/test/jest/cookieSyncManager.spec.ts +++ b/test/jest/cookieSyncManager.spec.ts @@ -440,7 +440,7 @@ describe.only('CookieSyncManager', () => { expect(loggerSpy).toHaveBeenCalledWith('Performing cookie sync'); }); - it.only('should return early if the user has not consented to the cookie sync', () => { + it('should return early if the user has not consented to the cookie sync', () => { const mockImage = { onload: jest.fn(), src: '', @@ -494,7 +494,7 @@ describe.only('CookieSyncManager', () => { expect(cookieSyncDates[42]).toBeUndefined(); }); - it.only('should return early if requiresConsent and mpidIsNotInCookies are both true', () => { + it('should return early if requiresConsent and mpidIsNotInCookies are both true', () => { const mockImage = { onload: jest.fn(), src: '', From 6ba361da9e1ee65edf190041a6ecd4bddd5e83fb Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Thu, 7 Nov 2024 17:28:19 -0500 Subject: [PATCH 13/22] Address PR Comments --- src/cookieSyncManager.interfaces.ts | 13 +++++++++++++ src/cookieSyncManager.ts | 13 +------------ src/sdkRuntimeModels.ts | 6 +++--- src/store.ts | 5 ++--- test/jest/cookieSyncManager.spec.ts | 12 ++++++------ test/src/tests-cookie-syncing.ts | 9 +++++---- 6 files changed, 30 insertions(+), 28 deletions(-) diff --git a/src/cookieSyncManager.interfaces.ts b/src/cookieSyncManager.interfaces.ts index 9daed8d06..54b363b52 100644 --- a/src/cookieSyncManager.interfaces.ts +++ b/src/cookieSyncManager.interfaces.ts @@ -3,6 +3,19 @@ import { Dictionary } from './utils'; import { IConsentRules } from './consent'; export type CookieSyncDates = Dictionary; + +export interface IPixelConfiguration { + name?: string; + moduleId: number; + esId?: number; + isDebug?: boolean; + isProduction?: boolean; + settings: Dictionary; + frequencyCap: number; + pixelUrl: string; + redirectUrl: string; + filteringConsentRuleValues?: IConsentRules; +} export interface ICookieSyncManager { attemptCookieSync: ( previousMPID: MPID, diff --git a/src/cookieSyncManager.ts b/src/cookieSyncManager.ts index f9c902cb9..031ce55ec 100644 --- a/src/cookieSyncManager.ts +++ b/src/cookieSyncManager.ts @@ -8,14 +8,6 @@ import { IConsentRules } from './consent'; const { Messages } = Constants; const { InformationMessages } = Messages; -interface PixelConfig { - moduleId: string; - frequencyCap: number; - pixelUrl: string; - redirectUrl: string; - filteringConsentRuleValues: IConsentRules; -} - export const DAYS_IN_MILLISECONDS = 1000 * 60 * 60 * 24; const hasFrequencyCapExpired = ( @@ -107,9 +99,7 @@ export default function CookieSyncManager( persistence[mpid].csd = {}; } - const lastSyncDateForModule = persistence[mpid].csd[moduleId] - ? persistence[mpid].csd[moduleId] - : null; + const lastSyncDateForModule = persistence[mpid].csd[moduleId] || null; // Check to see if we need to refresh cookieSync if (hasFrequencyCapExpired(lastSyncDateForModule, frequencyCap)) { @@ -123,7 +113,6 @@ export default function CookieSyncManager( requiresConsent ); } - } }); }; diff --git a/src/sdkRuntimeModels.ts b/src/sdkRuntimeModels.ts index 87956c6ba..b4b76c197 100644 --- a/src/sdkRuntimeModels.ts +++ b/src/sdkRuntimeModels.ts @@ -6,7 +6,7 @@ import { Callback, IdentityApiData, } from '@mparticle/web-sdk'; -import { IStore, PixelConfiguration } from './store'; +import { IStore } from './store'; import Validators from './validators'; import { Dictionary } from './utils'; import { IServerModel } from './serverModel'; @@ -31,7 +31,7 @@ import { } from './identity-user-interfaces'; import { IIdentityType } from './types.interfaces'; import IntegrationCapture from './integrationCapture'; -import { ICookieSyncManager } from './cookieSyncManager.interfaces'; +import { ICookieSyncManager, IPixelConfiguration } from './cookieSyncManager.interfaces'; // TODO: Resolve this with version in @mparticle/web-sdk export type SDKEventCustomFlags = Dictionary; @@ -230,7 +230,7 @@ export interface SDKInitConfig sideloadedKits?: MPForwarder[]; dataPlanOptions?: KitBlockerOptions; flags?: Dictionary; - pixelConfigs?: PixelConfiguration[]; + pixelConfigs?: IPixelConfiguration[]; aliasMaxWindow?: number; deviceId?: string; diff --git a/src/store.ts b/src/store.ts index 0947cb7ca..91f6fd51d 100644 --- a/src/store.ts +++ b/src/store.ts @@ -37,7 +37,7 @@ import { IGlobalStoreV2MinifiedKeys, IPersistenceMinified, } from './persistence.interfaces'; -import { CookieSyncDates } from './cookieSyncManager.interfaces'; +import { CookieSyncDates, IPixelConfiguration } from './cookieSyncManager.interfaces'; // This represents the runtime configuration of the SDK AFTER // initialization has been complete and all settings and @@ -119,7 +119,6 @@ function createSDKConfig(config: SDKInitConfig): SDKConfig { // TODO: Placeholder Types to be filled in as we migrate more modules // to TypeScript -export type PixelConfiguration = Dictionary; export type ServerSettings = Dictionary; export type SessionAttributes = Dictionary; export type IntegrationAttributes = Dictionary>; @@ -179,7 +178,7 @@ export interface IStore { kits: Dictionary; sideloadedKits: MPForwarder[]; configuredForwarders: MPForwarder[]; - pixelConfigurations: PixelConfiguration[]; + pixelConfigurations: IPixelConfiguration[]; integrationDelayTimeoutStart: number; // UNIX Timestamp webviewBridgeEnabled?: boolean; wrapperSDKInfo: WrapperSDKInfo; diff --git a/test/jest/cookieSyncManager.spec.ts b/test/jest/cookieSyncManager.spec.ts index fa0ad74a1..63ff5e107 100644 --- a/test/jest/cookieSyncManager.spec.ts +++ b/test/jest/cookieSyncManager.spec.ts @@ -1,9 +1,9 @@ import CookieSyncManager, { DAYS_IN_MILLISECONDS } from '../../src/cookieSyncManager'; +import { IPixelConfiguration } from '../../src/cookieSyncManager.interfaces'; import { MParticleWebSDK } from '../../src/sdkRuntimeModels'; -import { PixelConfiguration } from '../../src/store'; import { testMPID } from '../src/config/constants'; -const pixelSettings: PixelConfiguration = { +const pixelSettings: IPixelConfiguration = { name: 'TestPixel', moduleId: 5, esId: 24053, @@ -90,11 +90,11 @@ describe.only('CookieSyncManager', () => { }); it('should toggle requiresConsent value if filtering filteringConsentRuleValues are defined', () => { - const myPixelSettings: PixelConfiguration = { + const myPixelSettings: IPixelConfiguration = { filteringConsentRuleValues: { values: ['test'], }, - }; + } as unknown as IPixelConfiguration; const mockMPInstance = ({ _Store: { @@ -125,10 +125,10 @@ describe.only('CookieSyncManager', () => { }); it('should update the urlWithRedirect if a redirectUrl is defined', () => { - const myPixelSettings: PixelConfiguration = { + const myPixelSettings: IPixelConfiguration = { pixelUrl: 'https://test.com', redirectUrl: '?redirect=https://redirect.com&mpid=%%mpid%%', - }; + } as unknown as IPixelConfiguration; const mockMPInstance = ({ _Store: { diff --git a/test/src/tests-cookie-syncing.ts b/test/src/tests-cookie-syncing.ts index 20bf07431..e30e27aab 100644 --- a/test/src/tests-cookie-syncing.ts +++ b/test/src/tests-cookie-syncing.ts @@ -3,13 +3,14 @@ import Utils from './config/utils'; import fetchMock from 'fetch-mock/esm/client'; import { urls, testMPID, MPConfig, v4LSKey, apiKey } from './config/constants'; import { MParticleWebSDK } from '../../src/sdkRuntimeModels'; -import { PixelConfiguration } from '../../src/store'; import { IMParticleUser } from '../../src/identity-user-interfaces'; +import { IPixelConfiguration } from '../../src/cookieSyncManager.interfaces'; +import { IConsentRules } from '../../src/consent'; const { fetchMockSuccess, waitForCondition, hasIdentifyReturned } = Utils; const { setLocalStorage, MockForwarder, getLocalStorage } = Utils; -const pixelSettings: PixelConfiguration = { +const pixelSettings: IPixelConfiguration = { name: 'TestPixel', moduleId: 5, esId: 24053, @@ -343,7 +344,7 @@ describe('cookie syncing', function() { it('should perform a cookiesync when consent is not configured on the cookiesync setting', function(done) { mParticle._resetForTests(MPConfig); - pixelSettings.filteringConsentRuleValues = {}; + pixelSettings.filteringConsentRuleValues = {} as unknown as IConsentRules; window.mParticle.config.pixelConfigs = [pixelSettings]; mParticle.init(apiKey, window.mParticle.config); @@ -1213,7 +1214,7 @@ describe('cookie syncing', function() { mParticle.config.isDevelopmentMode = false; // pixelSetting1 has consent required, and so should only perform a cookiesync after consent is saved to the user - const pixelSettings1: PixelConfiguration = { + const pixelSettings1: IPixelConfiguration = { name: 'TestPixel', moduleId: 1, esId: 24053, From 90d681fe244fe86d52ae1ba9ad680995ec6374bf Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Thu, 7 Nov 2024 17:30:28 -0500 Subject: [PATCH 14/22] Address PR Comments --- src/cookieSyncManager.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cookieSyncManager.ts b/src/cookieSyncManager.ts index 031ce55ec..3ecd0449e 100644 --- a/src/cookieSyncManager.ts +++ b/src/cookieSyncManager.ts @@ -11,8 +11,8 @@ const { InformationMessages } = Messages; export const DAYS_IN_MILLISECONDS = 1000 * 60 * 60 * 24; const hasFrequencyCapExpired = ( - lastSyncDate: number, - frequencyCap: number + frequencyCap: number, + lastSyncDate?: number, ): boolean => { return ( new Date().getTime() > @@ -102,7 +102,7 @@ export default function CookieSyncManager( const lastSyncDateForModule = persistence[mpid].csd[moduleId] || null; // Check to see if we need to refresh cookieSync - if (hasFrequencyCapExpired(lastSyncDateForModule, frequencyCap)) { + if (hasFrequencyCapExpired(frequencyCap, lastSyncDateForModule)) { self.performCookieSync( urlWithRedirect, moduleId, From 8c5119896f4c6cd272cc679799b32a81a0e1652c Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Thu, 7 Nov 2024 17:31:59 -0500 Subject: [PATCH 15/22] Address PR Comments --- test/jest/cookieSyncManager.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/jest/cookieSyncManager.spec.ts b/test/jest/cookieSyncManager.spec.ts index 63ff5e107..3c393d14f 100644 --- a/test/jest/cookieSyncManager.spec.ts +++ b/test/jest/cookieSyncManager.spec.ts @@ -15,7 +15,7 @@ const pixelSettings: IPixelConfiguration = { redirectUrl: '', }; -describe.only('CookieSyncManager', () => { +describe('CookieSyncManager', () => { describe('#attemptCookieSync', () => { // https://go.mparticle.com/work/SQDSDKS-6915 it('should perform a cookie sync with defaults', () => { From 096f3389af466e6f676e8a9a9cf28c05b14ea86b Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Thu, 7 Nov 2024 17:46:53 -0500 Subject: [PATCH 16/22] Address PR Comments --- src/cookieSyncManager.ts | 2 +- test/jest/cookieSyncManager.spec.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cookieSyncManager.ts b/src/cookieSyncManager.ts index 3ecd0449e..072eb7f28 100644 --- a/src/cookieSyncManager.ts +++ b/src/cookieSyncManager.ts @@ -120,7 +120,7 @@ export default function CookieSyncManager( this.combineUrlWithRedirect = ( mpid: MPID, pixelUrl: string, - redirectUrl + redirectUrl: string, ): string => { const url = replaceMPID(pixelUrl, mpid); const redirect = redirectUrl ? replaceMPID(redirectUrl, mpid) : ''; diff --git a/test/jest/cookieSyncManager.spec.ts b/test/jest/cookieSyncManager.spec.ts index 3c393d14f..dfb8a0b83 100644 --- a/test/jest/cookieSyncManager.spec.ts +++ b/test/jest/cookieSyncManager.spec.ts @@ -562,11 +562,11 @@ describe('CookieSyncManager', () => { const result = cookieSyncManager.combineUrlWithRedirect( '1234', - 'https://test.com', - '?redirect=https://redirect.com&mpid=%%mpid%%', + 'https://test.com/some/path', + 'https://redirect.mparticle.com/v1/sync?esid=1234&MPID=%%mpid%%&ID=$UID&Key=testMPID&env=2' ); - expect(result).toBe('https://test.com%3Fredirect%3Dhttps%3A%2F%2Fredirect.com%26mpid%3D1234'); + expect(result).toBe('https://test.com/some/pathhttps%3A%2F%2Fredirect.mparticle.com%2Fv1%2Fsync%3Fesid%3D1234%26amp%3BMPID%3D1234%26amp%3BID%3D%24UID%26amp%3BKey%3DtestMPID%26amp%3Benv%3D2'); }); it('should return the pixelUrl if no redirectUrl is defined', () => { From 46eb534580f6e59be1c23016ccd781987d9d3692 Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Fri, 8 Nov 2024 09:13:48 -0500 Subject: [PATCH 17/22] Address PR Comments --- src/configAPIClient.ts | 14 ++------------ src/cookieSyncManager.ts | 4 ++-- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/src/configAPIClient.ts b/src/configAPIClient.ts index 91b7db53c..fda5b2f71 100644 --- a/src/configAPIClient.ts +++ b/src/configAPIClient.ts @@ -13,6 +13,7 @@ import { FetchUploader, XHRUploader, } from './uploaders'; +import { IPixelConfiguration } from './cookieSyncManager.interfaces'; export interface IKitConfigs extends IKitFilterSettings { name: string; @@ -65,17 +66,6 @@ export interface IConsentRuleValue { hasConsented: boolean; } -export interface IPixelConfig { - name: string; - moduleId: number; - esId: number; - isDebug: boolean; - isProduction: boolean; - settings: Dictionary; - frequencyCap: number; - pixelUrl: string; - redirectUrl: string; -} export interface IConfigResponse { appName: string; @@ -85,7 +75,7 @@ export interface IConfigResponse { secureServiceUrl: string; minWebviewBridgeVersion: number; workspaceToken: string; - pixelConfigs: IPixelConfig[]; + pixelConfigs: IPixelConfiguration[]; flags: SDKEventCustomFlags; } diff --git a/src/cookieSyncManager.ts b/src/cookieSyncManager.ts index 072eb7f28..a70fedb37 100644 --- a/src/cookieSyncManager.ts +++ b/src/cookieSyncManager.ts @@ -1,6 +1,6 @@ import { isEmpty, replaceAmpWithAmpersand, replaceMPID } from './utils'; import Constants from './constants'; -import { CookieSyncDates, ICookieSyncManager } from './cookieSyncManager.interfaces'; +import { CookieSyncDates, ICookieSyncManager, IPixelConfiguration } from './cookieSyncManager.interfaces'; import { MParticleWebSDK } from './sdkRuntimeModels'; import { MPID } from '@mparticle/web-sdk'; import { IConsentRules } from './consent'; @@ -39,7 +39,7 @@ export default function CookieSyncManager( const { pixelConfigurations } = mpInstance._Store; const persistence = mpInstance._Persistence.getPersistence(); - pixelConfigurations.forEach(pixelSettings => { + pixelConfigurations.forEach((pixelSettings: IPixelConfiguration) => { // set requiresConsent to false to start each additional pixel configuration // set to true only if filteringConsenRuleValues.values.length exists let requiresConsent = false; From 679129100c561d1d8cac6b95e9dc5f187afd6965 Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Fri, 8 Nov 2024 09:17:23 -0500 Subject: [PATCH 18/22] Address PR Comments --- test/jest/utils.spec.ts | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/test/jest/utils.spec.ts b/test/jest/utils.spec.ts index 395adf17e..5a24f0373 100644 --- a/test/jest/utils.spec.ts +++ b/test/jest/utils.spec.ts @@ -1,7 +1,12 @@ -import { queryStringParser, getCookies, getHref, replaceMPID, replaceAmpWithAmpersand } from '../../src/utils'; +import { + queryStringParser, + getCookies, + getHref, + replaceMPID, + replaceAmpWithAmpersand, +} from '../../src/utils'; import { deleteAllCookies } from './utils'; - describe('Utils', () => { describe('getCookies', () => { beforeEach(() => { @@ -128,7 +133,12 @@ describe('Utils', () => { }); it('returns an empty object if there are no query parameters', () => { - expect(queryStringParser('https://www.example.com', ['foo', 'narf'])).toEqual({}); + expect( + queryStringParser('https://www.example.com', [ + 'foo', + 'narf', + ]) + ).toEqual({}); }); it('returns an object with all the query string parameters if no keys are passed', () => { @@ -164,7 +174,9 @@ describe('Utils', () => { const mpid = '1234'; const string = 'https://www.google.com?mpid=%%mpid%%&foo=bar'; - expect(replaceMPID(string, mpid)).toEqual('https://www.google.com?mpid=1234&foo=bar'); + expect(replaceMPID(string, mpid)).toEqual( + 'https://www.google.com?mpid=1234&foo=bar' + ); }); }); @@ -172,7 +184,9 @@ describe('Utils', () => { it('replaces all instances of amp with ampersand', () => { const string = 'https://www.google.com?mpid=%%mpid%%&foo=bar'; - expect(replaceAmpWithAmpersand(string)).toEqual('https://www.google.com?mpid=%%mpid%%&foo=bar'); + expect(replaceAmpWithAmpersand(string)).toEqual( + 'https://www.google.com?mpid=%%mpid%%&foo=bar' + ); }); }); }); From 010389bfed81303f46fe6bb2c7e98330fd2fa82a Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Fri, 8 Nov 2024 09:18:10 -0500 Subject: [PATCH 19/22] Address PR Comments --- test/jest/utils.spec.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/test/jest/utils.spec.ts b/test/jest/utils.spec.ts index 5a24f0373..6bca98922 100644 --- a/test/jest/utils.spec.ts +++ b/test/jest/utils.spec.ts @@ -133,12 +133,7 @@ describe('Utils', () => { }); it('returns an empty object if there are no query parameters', () => { - expect( - queryStringParser('https://www.example.com', [ - 'foo', - 'narf', - ]) - ).toEqual({}); + expect(queryStringParser('https://www.example.com', ['foo', 'narf'])).toEqual({}); }); it('returns an object with all the query string parameters if no keys are passed', () => { From 6dd5dbbd54b0ba72ff906868ddbb16f5aa0ec854 Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Fri, 8 Nov 2024 09:18:44 -0500 Subject: [PATCH 20/22] Address PR Comments --- test/jest/utils.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/jest/utils.spec.ts b/test/jest/utils.spec.ts index 6bca98922..f9ac67dcd 100644 --- a/test/jest/utils.spec.ts +++ b/test/jest/utils.spec.ts @@ -103,7 +103,6 @@ describe('Utils', () => { }); }); - describe('without URLSearchParams', () => { beforeEach(() => { URL = undefined; From d72b41c8c81128b8a73feac773cd6536b1287487 Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Fri, 8 Nov 2024 11:13:39 -0500 Subject: [PATCH 21/22] Address PR Comments --- src/cookieSyncManager.ts | 7 ++-- test/jest/cookieSyncManager.spec.ts | 54 +++++++++++++++++++++++++++-- 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/src/cookieSyncManager.ts b/src/cookieSyncManager.ts index a70fedb37..9f056fed7 100644 --- a/src/cookieSyncManager.ts +++ b/src/cookieSyncManager.ts @@ -14,6 +14,10 @@ const hasFrequencyCapExpired = ( frequencyCap: number, lastSyncDate?: number, ): boolean => { + if (!lastSyncDate) { + return true; + } + return ( new Date().getTime() > new Date(lastSyncDate).getTime() + frequencyCap * DAYS_IN_MILLISECONDS @@ -61,7 +65,6 @@ export default function CookieSyncManager( // Url for cookie sync pixel const pixelUrl = replaceAmpWithAmpersand(pixelSettings.pixelUrl); - // TODO: Document requirements for redirectUrl const redirectUrl = pixelSettings.redirectUrl ? replaceAmpWithAmpersand(pixelSettings.redirectUrl) : null; @@ -72,8 +75,6 @@ export default function CookieSyncManager( redirectUrl ); - // TODO: Is there a historic reason for checking for previousMPID? - // it does not appear to be passed in anywhere if (previousMPID && previousMPID !== mpid) { if (persistence && persistence[mpid]) { if (!persistence[mpid].csd) { diff --git a/test/jest/cookieSyncManager.spec.ts b/test/jest/cookieSyncManager.spec.ts index dfb8a0b83..860251ab7 100644 --- a/test/jest/cookieSyncManager.spec.ts +++ b/test/jest/cookieSyncManager.spec.ts @@ -218,7 +218,6 @@ describe('CookieSyncManager', () => { ); }); - // QUESTION: What is the purpose of this code path? it('should call performCookieSync with mpid if previousMpid and mpid match', () => { const mockMPInstance = ({ _Store: { @@ -247,6 +246,33 @@ describe('CookieSyncManager', () => { ); }); + it('should perform a cookie sync if lastSyncDateForModule is null', () => { + const mockMPInstance = ({ + _Store: { + webviewBridgeEnabled: false, + pixelConfigurations: [pixelSettings], + }, + _Persistence: { + getPersistence: () => ({testMPID: {}}), + }, + } as unknown) as MParticleWebSDK; + + const cookieSyncManager = new CookieSyncManager(mockMPInstance); + cookieSyncManager.performCookieSync = jest.fn(); + + cookieSyncManager.attemptCookieSync(null, testMPID, true); + + expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( + '', + '5', + testMPID, + {}, + undefined, + true, + false, + ); + }); + it('should perform a cookie sync if lastSyncDateForModule has passed the frequency cap', () => { const now = new Date().getTime(); @@ -331,6 +357,25 @@ describe('CookieSyncManager', () => { expect(mockMPInstance._Store.pixelConfigurations.length).toBe(1); expect(mockMPInstance._Store.pixelConfigurations[0].moduleId).toBe(5); }); + + it('should not perform a cookie sync if persistence is empty', () => { + const mockMPInstance = ({ + _Store: { + webviewBridgeEnabled: false, + pixelConfigurations: [pixelSettings], + }, + _Persistence: { + getPersistence: () => ({}), + }, + } as unknown) as MParticleWebSDK; + + const cookieSyncManager = new CookieSyncManager(mockMPInstance); + cookieSyncManager.performCookieSync = jest.fn(); + + cookieSyncManager.attemptCookieSync(null, testMPID, true); + + expect(cookieSyncManager.performCookieSync).not.toHaveBeenCalled(); + }); }); describe('#performCookieSync', () => { @@ -560,13 +605,16 @@ describe('CookieSyncManager', () => { const cookieSyncManager = new CookieSyncManager(mockMPInstance); + // Note: We expect that the input of the pixelUrl will include a `?` + // (ideally at the end of the string) + // so that the redirectUrl can be processed by the backend correctly const result = cookieSyncManager.combineUrlWithRedirect( '1234', - 'https://test.com/some/path', + 'https://test.com/some/path?', 'https://redirect.mparticle.com/v1/sync?esid=1234&MPID=%%mpid%%&ID=$UID&Key=testMPID&env=2' ); - expect(result).toBe('https://test.com/some/pathhttps%3A%2F%2Fredirect.mparticle.com%2Fv1%2Fsync%3Fesid%3D1234%26amp%3BMPID%3D1234%26amp%3BID%3D%24UID%26amp%3BKey%3DtestMPID%26amp%3Benv%3D2'); + expect(result).toBe('https://test.com/some/path?https%3A%2F%2Fredirect.mparticle.com%2Fv1%2Fsync%3Fesid%3D1234%26amp%3BMPID%3D1234%26amp%3BID%3D%24UID%26amp%3BKey%3DtestMPID%26amp%3Benv%3D2'); }); it('should return the pixelUrl if no redirectUrl is defined', () => { From 15f082e3c5e9588d8cd28361e18a6fe099187d5c Mon Sep 17 00:00:00 2001 From: Alex S <49695018+alexs-mparticle@users.noreply.github.com> Date: Fri, 8 Nov 2024 15:16:27 -0500 Subject: [PATCH 22/22] Apply suggestions from code review Co-authored-by: Robert Ing --- src/persistence.interfaces.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/persistence.interfaces.ts b/src/persistence.interfaces.ts index 7bfc65f15..4c89ec69d 100644 --- a/src/persistence.interfaces.ts +++ b/src/persistence.interfaces.ts @@ -81,7 +81,7 @@ export interface IPersistenceMinified extends Dictionary { } export interface IUserPersistenceMinified extends Dictionary { - csd: CookieSyncDates; // Cookie Sync Dates // list of timestamps for last cookie sync + csd: CookieSyncDates; // list of timestamps for last cookie sync per module con: IMinifiedConsentJSONObject; // Consent State ui: UserIdentities; // User Identities ua: UserAttributes; // User Attributes