-
Notifications
You must be signed in to change notification settings - Fork 44
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.
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
feat: Include consent query parameters for cookie sync #966
Open
rmi22186
wants to merge
12
commits into
refactor/ts-migration-blackout-2024
Choose a base branch
from
feat/SQDSDKS-6991-cookie-sync-gdpr-2
base: refactor/ts-migration-blackout-2024
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+3,201
−1,258
Open
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
d2d6e61
refactor: Migrate IdentityAPIClient to TypeScript (#948)
alexs-mparticle 2cff95d
refactor: Migrate Types to TypeScript as enums and utils (#950)
alexs-mparticle ae3c671
refactor: Migrate Instance Manager Tests to TypeScript (#953)
alexs-mparticle 8219caf
refactor: Update Error handling for Identity API Client (#959)
alexs-mparticle c8340c3
refactor: Create Interfaces for Events (#962)
alexs-mparticle 78a0cb2
refactor: Create Interfaces for eCommerce (#964)
alexs-mparticle 85f1887
feat: Include consent query parameters for cookie sync
rmi22186 96a787b
refactor cookie sync manager to async
rmi22186 3de3997
Simplify url creation
rmi22186 df4c310
refactor procesPixelConfig to its own function
rmi22186 6884bff
turn comments
rmi22186 e623756
add function definition, remove test
rmi22186 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,8 @@ import CookieSyncManager, { | |
IPixelConfiguration, | ||
CookieSyncDates, | ||
isLastSyncDateExpired, | ||
isTcfApiAvailable | ||
isTcfApiAvailable, | ||
appendGdprConsentUrl | ||
} from '../../src/cookieSyncManager'; | ||
import { MParticleWebSDK } from '../../src/sdkRuntimeModels'; | ||
import { testMPID } from '../src/config/constants'; | ||
|
@@ -563,9 +564,10 @@ describe('CookieSyncManager', () => { | |
}); | ||
}); | ||
|
||
describe('#performCookieSyncWithGDPR', () => { | ||
describe('#appendGdprConsentUrl', () => { | ||
const verboseLoggerSpy = jest.fn(); | ||
const errorLoggerSpy = jest.fn(); | ||
const mockUrl = 'https://example.com/cookie-sync'; | ||
|
||
let cookieSyncManager: any; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we still need this? |
||
const mockMpInstance = ({ | ||
|
@@ -590,12 +592,7 @@ describe('CookieSyncManager', () => { | |
jest.clearAllMocks(); | ||
}) | ||
|
||
it('should append GDPR parameters to the URL if __tcfapi callback succeeds', () => { | ||
const mockCookieSyncDates = {}; | ||
const mockUrl = 'https://example.com/cookie-sync'; | ||
const moduleId = 'module1'; | ||
const mpid = '12345'; | ||
|
||
it('should append GDPR parameters to the URL if __tcfapi callback succeeds', async () => { | ||
// Mock __tcfapi to call the callback with success | ||
(window.__tcfapi as jest.Mock).mockImplementation(( | ||
command, | ||
|
@@ -611,82 +608,77 @@ describe('CookieSyncManager', () => { | |
); | ||
}); | ||
|
||
const performCookieSyncSpy = jest.spyOn(cookieSyncManager, 'performCookieSync'); | ||
|
||
// Call the function under test | ||
cookieSyncManager.performCookieSyncWithGDPR( | ||
mockUrl, | ||
moduleId, | ||
mpid, | ||
mockCookieSyncDates | ||
); | ||
const fullUrl = await appendGdprConsentUrl(mockUrl, mockMpInstance.Logger); | ||
|
||
expect(performCookieSyncSpy).toHaveBeenCalledWith( | ||
expect(fullUrl).toBe( | ||
`${mockUrl}&gdpr=1&gdpr_consent=test-consent-string`, | ||
moduleId, | ||
mpid, | ||
mockCookieSyncDates | ||
); | ||
}); | ||
|
||
it('should fall back to performCookieSync if __tcfapi callback fails', () => { | ||
const mockCookieSyncDates = {}; | ||
const mockUrl = 'https://example.com/cookie-sync'; | ||
const moduleId = 'module1'; | ||
const mpid = '12345'; | ||
|
||
it('should return only the base url if the __tcfapi callback fails to get tcData', async () => { | ||
// Mock __tcfapi to call the callback with failure | ||
(window.__tcfapi as jest.Mock).mockImplementation((command, version, callback) => { | ||
callback(null, false); // Simulate a failure | ||
}); | ||
|
||
// Spy on the `performCookieSync` method | ||
const performCookieSyncSpy = jest.spyOn(cookieSyncManager, 'performCookieSync'); | ||
|
||
// Call the function under test | ||
cookieSyncManager.performCookieSyncWithGDPR( | ||
mockUrl, | ||
moduleId, | ||
mpid, | ||
mockCookieSyncDates | ||
); | ||
|
||
const fullUrl = await appendGdprConsentUrl(mockUrl, mockMpInstance.Logger); | ||
// Assert that the fallback method was called with the original URL | ||
expect(performCookieSyncSpy).toHaveBeenCalledWith( | ||
mockUrl, | ||
moduleId, | ||
mpid, | ||
mockCookieSyncDates | ||
); | ||
expect(fullUrl).toBe(mockUrl); | ||
}); | ||
|
||
it('should handle exceptions thrown by __tcfapi gracefully', () => { | ||
const mockCookieSyncDates = {}; | ||
const mockUrl = 'https://example.com/cookie-sync'; | ||
const moduleId = 'module1'; | ||
const mpid = '12345'; | ||
|
||
it('should handle exceptions thrown by __tcfapi gracefully', async () => { | ||
// Mock __tcfapi to throw an error | ||
(window.__tcfapi as jest.Mock).mockImplementation(() => { | ||
throw new Error('Test Error'); | ||
}); | ||
|
||
// Spy on the `performCookieSync` method | ||
const performCookieSyncSpy = jest.spyOn(cookieSyncManager, 'performCookieSync'); | ||
|
||
// Call the function under test | ||
cookieSyncManager.performCookieSyncWithGDPR( | ||
mockUrl, | ||
moduleId, | ||
mpid, | ||
mockCookieSyncDates | ||
); | ||
|
||
// Assert that the fallback method was called with the original URL | ||
expect(performCookieSyncSpy).not.toHaveBeenCalled(); | ||
try { | ||
await appendGdprConsentUrl(mockUrl, mockMpInstance.Logger); | ||
} catch(e) { | ||
expect(errorLoggerSpy).toHaveBeenCalledWith('Test Error'); | ||
} | ||
}); | ||
|
||
// Ensure the error was logged (if applicable) | ||
expect(errorLoggerSpy).toHaveBeenCalledWith('Test Error'); | ||
describe.only('#integration test', () => { | ||
it('should handle errors properly when calling attemptCookieSync and the callback fails', () => { | ||
rmi22186 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
(window.__tcfapi as jest.Mock).mockImplementation(() => { | ||
throw new Error('Test Error'); | ||
}); | ||
const mockMPInstance = ({ | ||
_Store: { | ||
webviewBridgeEnabled: false, | ||
pixelConfigurations: [pixelSettings], | ||
}, | ||
_Persistence: { | ||
getPersistence: () => ({testMPID: {}}), | ||
}, | ||
_Consent: { | ||
isEnabledForUserConsent: jest.fn().mockReturnValue(true), | ||
}, | ||
Identity: { | ||
getCurrentUser: jest.fn().mockReturnValue({ | ||
getMPID: () => testMPID, | ||
}), | ||
}, | ||
Logger: { | ||
verbose: jest.fn(), | ||
error: jest.fn() | ||
}, | ||
} as unknown) as MParticleWebSDK; | ||
|
||
const cookieSyncManager = new CookieSyncManager(mockMPInstance); | ||
cookieSyncManager.performCookieSync = jest.fn(); | ||
|
||
cookieSyncManager.attemptCookieSync(testMPID, true); | ||
|
||
expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( | ||
pixelUrlAndRedirectUrl, | ||
'5', | ||
testMPID, | ||
{}, | ||
); | ||
}); | ||
}); | ||
|
||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.