-
Notifications
You must be signed in to change notification settings - Fork 58
feat: disable product cookie when no targeting is true #1071
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
Changes from 2 commits
b7230d6
b14099e
bbe4f08
507b3e5
9f062ca
d09d4c9
273e170
0847f1e
bfcdb2b
6c0c0c8
95d8fd8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -88,7 +88,10 @@ export default function _Persistence(mpInstance) { | |||||||||||||||
|
|
||||||||||||||||
| // https://go.mparticle.com/work/SQDSDKS-6048 | ||||||||||||||||
| try { | ||||||||||||||||
| if (mpInstance._Store.isLocalStorageAvailable) { | ||||||||||||||||
| if ( | ||||||||||||||||
| mpInstance._Store.isLocalStorageAvailable && | ||||||||||||||||
| !mpInstance._Store.getNoTargeting() | ||||||||||||||||
| ) { | ||||||||||||||||
| var encodedProducts = localStorage.getItem( | ||||||||||||||||
| mpInstance._Store.prodStorageName | ||||||||||||||||
| ); | ||||||||||||||||
|
|
@@ -269,7 +272,10 @@ export default function _Persistence(mpInstance) { | |||||||||||||||
| }; | ||||||||||||||||
|
|
||||||||||||||||
| this.getUserProductsFromLS = function(mpid) { | ||||||||||||||||
| if (!mpInstance._Store.isLocalStorageAvailable) { | ||||||||||||||||
| if ( | ||||||||||||||||
| mpInstance._Store.getNoTargeting() || | ||||||||||||||||
| !mpInstance._Store.isLocalStorageAvailable | ||||||||||||||||
| ) { | ||||||||||||||||
| return []; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
|
|
@@ -308,6 +314,15 @@ export default function _Persistence(mpInstance) { | |||||||||||||||
| }; | ||||||||||||||||
|
|
||||||||||||||||
| this.getAllUserProductsFromLS = function() { | ||||||||||||||||
| if (mpInstance._Store.getNoTargeting()) { | ||||||||||||||||
| var currentUser = mpInstance.Identity.getCurrentUser(); | ||||||||||||||||
| var mpid = currentUser ? currentUser.getMPID() : null; | ||||||||||||||||
| var result = {}; | ||||||||||||||||
| if (mpid) { | ||||||||||||||||
| result[mpid] = { cp: [] }; | ||||||||||||||||
| } | ||||||||||||||||
| return result; | ||||||||||||||||
| } | ||||||||||||||||
| var decodedProducts, | ||||||||||||||||
| encodedProducts = localStorage.getItem( | ||||||||||||||||
| mpInstance._Store.prodStorageName | ||||||||||||||||
|
|
@@ -342,14 +357,20 @@ export default function _Persistence(mpInstance) { | |||||||||||||||
| ? allLocalStorageProducts[mpid].cp | ||||||||||||||||
| : [], | ||||||||||||||||
| }; | ||||||||||||||||
|
|
||||||||||||||||
| if (mpInstance._Store.getNoTargeting()) { | ||||||||||||||||
| return; | ||||||||||||||||
| } | ||||||||||||||||
| if (mpid) { | ||||||||||||||||
| allLocalStorageProducts = allLocalStorageProducts || {}; | ||||||||||||||||
| allLocalStorageProducts[mpid] = currentUserProducts; | ||||||||||||||||
| try { | ||||||||||||||||
| window.localStorage.setItem( | ||||||||||||||||
| encodeURIComponent(mpInstance._Store.prodStorageName), | ||||||||||||||||
| Base64.encode(JSON.stringify(allLocalStorageProducts)) | ||||||||||||||||
| ); | ||||||||||||||||
| if (!mpInstance._Store.getNoTargeting()) { | ||||||||||||||||
| window.localStorage.setItem( | ||||||||||||||||
| encodeURIComponent(mpInstance._Store.prodStorageName), | ||||||||||||||||
| Base64.encode(JSON.stringify(allLocalStorageProducts)) | ||||||||||||||||
| ); | ||||||||||||||||
|
Collaborator
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. I would also clean up the original code a bit so that i's more readable:
Suggested change
|
||||||||||||||||
| } | ||||||||||||||||
| } catch (e) { | ||||||||||||||||
| mpInstance.Logger.error( | ||||||||||||||||
| 'Error with setting products on localStorage.' | ||||||||||||||||
|
|
@@ -894,6 +915,9 @@ export default function _Persistence(mpInstance) { | |||||||||||||||
| }; | ||||||||||||||||
|
|
||||||||||||||||
| this.getCartProducts = function(mpid) { | ||||||||||||||||
| if (mpInstance._Store.getNoTargeting()) { | ||||||||||||||||
| return []; | ||||||||||||||||
| } | ||||||||||||||||
| var allCartProducts, | ||||||||||||||||
| cartProductsString = localStorage.getItem( | ||||||||||||||||
| mpInstance._Store.prodStorageName | ||||||||||||||||
|
|
@@ -913,7 +937,10 @@ export default function _Persistence(mpInstance) { | |||||||||||||||
| }; | ||||||||||||||||
|
|
||||||||||||||||
| this.setCartProducts = function(allProducts) { | ||||||||||||||||
| if (!mpInstance._Store.isLocalStorageAvailable) { | ||||||||||||||||
| if ( | ||||||||||||||||
| !mpInstance._Store.isLocalStorageAvailable && | ||||||||||||||||
|
Member
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. This should be OR, not AND. If local storage isn't available, or if getNoTargeting returns true, then don't set the cart products
Suggested change
|
||||||||||||||||
| mpInstance._Store.getNoTargeting() | ||||||||||||||||
| ) { | ||||||||||||||||
| return; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,33 @@ | ||||||
| import { | ||||||
| mParticle, | ||||||
| MPConfig, | ||||||
| testMPID, | ||||||
| } from '../src/config/constants'; | ||||||
|
|
||||||
| describe('Products Persistence', () => { | ||||||
| let mpInstance = mParticle.getInstance(); | ||||||
| beforeEach(() => { | ||||||
| mParticle._resetForTests(MPConfig); | ||||||
| }); | ||||||
|
|
||||||
| it('should update Products localStorage when noTargeting is false by default', () => { | ||||||
|
Member
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.
Suggested change
|
||||||
| const product = { Name: 'iphone', Sku: 'iphonesku', Price: 599, Quantity: 1 }; | ||||||
| mpInstance._Persistence.setCartProducts({ [testMPID]: { cp: [product] } } as any); | ||||||
|
Collaborator
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.
Suggested change
|
||||||
| expect(mpInstance._Persistence.getCartProducts(testMPID).length).toBe(1); | ||||||
| }); | ||||||
|
|
||||||
| it('should NOT update Products localStorage when noTargeting is true', () => { | ||||||
|
Member
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.
Suggested change
|
||||||
| mpInstance._Store.setNoTargeting(true); | ||||||
| const product = { Name: 'iphone', Sku: 'iphonesku', Price: 599, Quantity: 1 }; | ||||||
| mpInstance._Persistence.setCartProducts({ [testMPID]: { cp: [product] } } as any); | ||||||
|
Collaborator
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.
Suggested change
|
||||||
| expect(mpInstance._Persistence.getCartProducts(testMPID).length).toBe(0); | ||||||
|
jaissica12 marked this conversation as resolved.
Outdated
|
||||||
| expect(mpInstance._Persistence.getCartProducts(testMPID)).toEqual([]); | ||||||
| }); | ||||||
|
|
||||||
| it('should update Products localStorage when noTargeting is false', () => { | ||||||
|
Member
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.
Suggested change
|
||||||
| mpInstance._Store.setNoTargeting(false); | ||||||
| const product = { Name: 'iphone', Sku: 'iphonesku', Price: 599, Quantity: 1 }; | ||||||
| mpInstance._Persistence.setCartProducts({ [testMPID]: { cp: [product] } } as any); | ||||||
|
Collaborator
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.
Suggested change
|
||||||
| expect(mpInstance._Persistence.getCartProducts(testMPID).length).toBe(1); | ||||||
| }); | ||||||
| }); | ||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -1661,6 +1661,53 @@ describe('persistence', () => { | |||||
| parsedProductsAfter['testMPID'].cp.length.should.equal(0); | ||||||
| }); | ||||||
|
|
||||||
| describe('products persistence with noTargeting privacy flag', () => { | ||||||
| beforeEach(() => { | ||||||
| mParticle._resetForTests(MPConfig); | ||||||
| }); | ||||||
|
|
||||||
| it('should not save products LS when noTargeting is true', async () => { | ||||||
|
Member
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.
Suggested change
|
||||||
| mParticle.config.noTargeting = true; | ||||||
|
Member
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. I'm surprised this is working. Per the docs, this should be:
Suggested change
Contributor
Author
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. @rmi22186 Currently
Member
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. Now that you have updated the noTargeting and noFunctional to pull from |
||||||
|
|
||||||
| mParticle.init(apiKey, mParticle.config); | ||||||
| await waitForCondition(hasIdentifyReturned); | ||||||
| const localStorageProducts = localStorage.getItem(LocalStorageProductsV4WithWorkSpaceName); | ||||||
| expect(localStorageProducts).to.not.be.ok; | ||||||
| }); | ||||||
|
|
||||||
| it('should save products LS when noTargeting is false', async () => { | ||||||
|
Member
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.
Suggested change
|
||||||
| mParticle.config.noTargeting = false; | ||||||
|
Member
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.
Suggested change
|
||||||
|
|
||||||
| mParticle.init(apiKey, mParticle.config); | ||||||
| await waitForCondition(hasIdentifyReturned); | ||||||
| const iphone = mParticle.eCommerce.createProduct( | ||||||
| 'iphone', | ||||||
| 'iphonesku', | ||||||
| 599, | ||||||
| 1 | ||||||
| ); | ||||||
| mParticle.eCommerce.Cart.add(iphone, true); | ||||||
|
|
||||||
| const localStorageProducts = localStorage.getItem(LocalStorageProductsV4WithWorkSpaceName); | ||||||
| expect(localStorageProducts).to.be.ok; | ||||||
| }); | ||||||
|
|
||||||
| it('should save products LS when noTargeting is false by default', async () => { | ||||||
|
Member
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.
Suggested change
|
||||||
| mParticle.init(apiKey, mParticle.config); | ||||||
| await waitForCondition(hasIdentifyReturned); | ||||||
| const iphone = mParticle.eCommerce.createProduct( | ||||||
| 'iphone', | ||||||
| 'iphonesku', | ||||||
| 599, | ||||||
| 1 | ||||||
| ); | ||||||
| mParticle.eCommerce.Cart.add(iphone, true); | ||||||
|
|
||||||
| const localStorageProducts = localStorage.getItem(LocalStorageProductsV4WithWorkSpaceName); | ||||||
| expect(localStorageProducts).to.be.ok; | ||||||
| }); | ||||||
| }); | ||||||
|
|
||||||
| it('should only set setFirstSeenTime() once', async () => { | ||||||
| const cookies = JSON.stringify({ | ||||||
| gs: { | ||||||
|
|
||||||
Uh oh!
There was an error while loading. Please reload this page.