UserID Module: allow userid to ppid sync #6448
Conversation
|
This pull request introduces 1 alert when merging 58e7a30 into 8c218d9 - view on LGTM.com new alerts:
|
harpere
added
feature
minor
needs 2nd review
|
@smenzer docs pr is linked above I am not sure how best to test this, as googletag doesn't have some sort of ppid successfully set functionality i can test for, what do you recommend? |
Could you maybe try to mock this part and then just check that the |
ChrisHuie
changed the title
|
I can't seem to restore the branch this was on, I plan to close this and reopen |
|
@patmmccann you still closing this pr? |
|
@robertrmartinez checking in to see if you were able to take a look at making the unit test |
| @@ -820,6 +821,16 @@ export function init(config) { | |||
| auctionDelay = utils.isNumber(userSync.auctionDelay) ? userSync.auctionDelay : NO_AUCTION_DELAY; | |||
| updateSubmodules(); | |||
| } | |||
| // userSync.ppid should be one of the 'source' values in getUserIdsAsEids() eg pubcid.org or id5-sync.com | |||
There was a problem hiding this comment.
Hello, how/where would the publisher define the value for userSync.ppid?
Also, my understanding from this comment is that it will leave the choice to the publisher onto which ID from EIDS they choose to use as PPID: do you confirm?
There was a problem hiding this comment.
The pub will need to set ppid inside of their userSync setConfig.
Something like so:
config.setConfig({
userSync: {
ppid: 'pubcid.org',
userIds: [
.. userIds
]
}
});
where the pubcid.org matches the userId submodules associated eids source value defined in eids.js
This PR has been closed as another one was open to fix the tests etc.
There was a problem hiding this comment.
Very clear, thanks @robertrmartinez!
|
Closing as new PR to address #7681 |
Type of change
Description of change
Solution to #6430