Correct the trackContentImpression command argument with the appropri…#4
Correct the trackContentImpression command argument with the appropri…#4OphelieMicoine wants to merge 3 commits into
Conversation
…ate field names for Content Impressions
- enableCookies if data.useCookies is true - change order of setUserId option to get it before user anonimization - deanonymizeUser if data.ipCollectionMode is true - deanonymizeUser if data.setSessionIdStrictPrivacyMode is true - if resetUserId and setUserIsAnonymous to true if data.setSessionIdStrictPrivacyMode is false
|
Hello @harmasz @abartczakpiwik @mbaersch and happy new year !!! When you get time, I will be very honored if you can review this pull request please ? |
|
Hi @OphelieMicoine, It looks like there are few additional new things related to data anonymization, cookies and user ID. Are those changes intentional? If so, please make sure to only address content impression related issues in this PR as 1. the PR name implies this, 2. it would be best to discuss potential breaking changes separately, so they don't end up in the master branch without proper approval. |
|
Yes It was intentionnal because this options were missing to apply hybrid data collection. Ok I will make a new commit for this feature. |
As a side note - I will be working on a separate anonymization/deanonymization template in the upcoming weeks. After initial inspection it looks like the base template (this repository) might not require changes as most of the fields already support variables (anonymization, cookies, device data and so on). |
|
OK so maybe decline this Commit and I stay tuned with your updates.
Le mer. 8 janv. 2025 à 11:48, abartczakpiwik ***@***.***> a
écrit :
… Yes It was intentionnal because this options were missing to apply hybrid
data collection. Ok I will make a new commit for this feature.
As a side note - I will be working on a separate
anonymization/deanonymization template in the upcoming weeks. After initial
inspection it looks like the base template (this repository) might not
require changes as most of the fields already support variables
(anonymization, cookies, device data and so on).
—
Reply to this email directly, view it on GitHub
<#4 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BKXJL52TUKZED2KJXTQVPID2JT665AVCNFSM6AAAAABTPTZICKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNZXGM3DMNBQGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
Ophélie Micoine
|
|
@OphelieMicoine could you push another commit with only the content impression changes? Don't want to close the whole PR :). |
…te field names for Content Impressions Like mentionned, the data.fieldName for the argument of the command trackContentImpression was the ones for Content Interaction. That pull request solved the issue.
|
Hello @abartczakpiwik sorry for the delay. It is done. |
2 participants
…ate field names for Content Impressions
Like mentionned, the data.fieldName for the argument of the command trackContentImpression was the ones for Content Interaction. That pull request solved the issue.