Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

refactor: Migrate CookieSyncManager to TypeScript #944

Merged
merged 23 commits into from
Nov 12, 2024
Merged

refactor: Migrate CookieSyncManager to TypeScript #944

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
d5fa429
refactor: Migrate CookieSyncManager to TypeScript
alexs-mparticle Oct 31, 2024
54e057f
rename import
alexs-mparticle Oct 31, 2024
b75ba9f
Remove migrated methods
alexs-mparticle Oct 31, 2024
e139d03
Remove empty lines
alexs-mparticle Oct 31, 2024
ac7b53b
Remove coverage build files from bundle
alexs-mparticle Nov 7, 2024
619debb
Apply suggestions from code review
alexs-mparticle Nov 7, 2024
b6a0be6
Address PR Comments
alexs-mparticle Nov 7, 2024
537e2ea
Address PR Comments
alexs-mparticle Nov 7, 2024
2b371ee
Address PR Comments
alexs-mparticle Nov 7, 2024
c93fa27
Address PR Comments
alexs-mparticle Nov 7, 2024
e2c6d5c
Address PR Comments
alexs-mparticle Nov 7, 2024
0e9cbfe
Address PR Comments
alexs-mparticle Nov 7, 2024
6ba361d
Address PR Comments
alexs-mparticle Nov 7, 2024
90d681f
Address PR Comments
alexs-mparticle Nov 7, 2024
8c51198
Address PR Comments
alexs-mparticle Nov 7, 2024
096f338
Address PR Comments
alexs-mparticle Nov 7, 2024
46eb534
Address PR Comments
alexs-mparticle Nov 8, 2024
6791291
Address PR Comments
alexs-mparticle Nov 8, 2024
010389b
Address PR Comments
alexs-mparticle Nov 8, 2024
6dd5dbb
Address PR Comments
alexs-mparticle Nov 8, 2024
d72b41c
Address PR Comments
alexs-mparticle Nov 8, 2024
4f2cc28
Merge branch 'development' into test/SQDSDKS-5084-cookie-sync-tests
alexs-mparticle Nov 8, 2024
15f082e
Apply suggestions from code review
alexs-mparticle Nov 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/cookieSyncManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
132 changes: 93 additions & 39 deletions test/jest/cookieSyncManager.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -18,7 +18,7 @@ const pixelSettings: PixelConfiguration = {
describe.only('CookieSyncManager', () => {
describe('#attemptCookieSync', () => {
alexs-mparticle marked this conversation as resolved.
Show resolved Hide resolved
it('should perform a cookie sync with defaults', () => {
const mockSDK = ({
const mockMPInstance = ({
_Store: {
webviewBridgeEnabled: false,
pixelConfigurations: [pixelSettings],
Expand All @@ -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);
Expand All @@ -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],
Expand All @@ -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);
Expand All @@ -68,7 +68,7 @@ describe.only('CookieSyncManager', () => {
});

it('should return early if webviewBridgeEnabled is true', () => {
const mockSDK = ({
const mockMPInstance = ({
_Store: {
webviewBridgeEnabled: true,
pixelConfigurations: [pixelSettings],
Expand All @@ -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);
Expand All @@ -95,7 +95,7 @@ describe.only('CookieSyncManager', () => {
},
};

const mockSDK = ({
const mockMPInstance = ({
_Store: {
webviewBridgeEnabled: false,
pixelConfigurations: [{...pixelSettings, ...myPixelSettings}],
Expand All @@ -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);
Expand All @@ -129,7 +129,7 @@ describe.only('CookieSyncManager', () => {
redirectUrl: '?redirect=https://redirect.com&mpid=%%mpid%%',
};

const mockSDK = ({
const mockMPInstance = ({
_Store: {
webviewBridgeEnabled: false,
pixelConfigurations: [{...pixelSettings, ...myPixelSettings}],
Expand All @@ -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);
Expand All @@ -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 = ({
alexs-mparticle marked this conversation as resolved.
Show resolved Hide resolved
const mockMPInstance = ({
_Store: {
webviewBridgeEnabled: false,
pixelConfigurations: [pixelSettings],
Expand All @@ -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);
Expand All @@ -187,7 +187,7 @@ describe.only('CookieSyncManager', () => {
});

it('should call performCookieSync with mpid and csd if previousMpid and mpid do not match', () => {
const mockSDK = ({
Copy link
Member

Choose a reason for hiding this comment

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

This test name and the previous test name make it sound like the same test. but this one has stuff in CSD. This one is probably more realistic because testMPID1 will have a CSD, before logging in to a new MPID. So I think you can remove the previous test.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think both tests are valid because they each test two different code paths in what is a somewhat squirrely and complex function. I'd rather change the assertion to make them more distinct.

const mockMPInstance = ({
_Store: {
webviewBridgeEnabled: false,
pixelConfigurations: [pixelSettings],
Expand All @@ -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);
Expand All @@ -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 = ({
alexs-mparticle marked this conversation as resolved.
Show resolved Hide resolved
const mockMPInstance = ({
_Store: {
webviewBridgeEnabled: false,
pixelConfigurations: [pixelSettings],
Expand All @@ -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);
Expand All @@ -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],
Expand All @@ -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);
Expand All @@ -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();
alexs-mparticle marked this conversation as resolved.
Show resolved Hide resolved

const mockSDK = ({
const mockMPInstance = ({
_Store: {
webviewBridgeEnabled: false,
pixelConfigurations: [pixelSettings],
Expand All @@ -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);
Expand All @@ -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],
Expand All @@ -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);
});
});

Expand All @@ -342,7 +342,7 @@ describe.only('CookieSyncManager', () => {
(mockImage as unknown) as HTMLImageElement
);

const mockSDK = ({
const mockMPInstance = ({
_Store: {
webviewBridgeEnabled: false,
pixelConfigurations: [pixelSettings],
Expand All @@ -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(
Expand Down Expand Up @@ -397,7 +397,7 @@ describe.only('CookieSyncManager', () => {

const loggerSpy = jest.fn();

const mockSDK = ({
const mockMPInstance = ({
_Store: {
webviewBridgeEnabled: false,
pixelConfigurations: [pixelSettings],
Expand All @@ -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(
Expand All @@ -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 = {
alexs-mparticle marked this conversation as resolved.
Show resolved Hide resolved
onload: jest.fn(),
src: '',
Expand All @@ -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 = {
alexs-mparticle marked this conversation as resolved.
Show resolved Hide resolved
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],
Expand All @@ -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(
Expand All @@ -483,7 +537,7 @@ describe.only('CookieSyncManager', () => {
cookieSyncDates,
null,
true,
true
true,
);

// Simulate image load
Expand All @@ -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',
Expand All @@ -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',
Expand Down
Loading