-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
PubMatic adapter support to read TTD Id from UserId module #3834
Conversation
Hello @robertrmartinez , |
modules/pubmaticBidAdapter.js
Outdated
let adsrvrOrgId = config.getConfig('adsrvrOrgId'); | ||
if (adsrvrOrgId && utils.isStr(adsrvrOrgId.TDID)) { | ||
if (validBidRequests[0] && validBidRequests[0].userId && utils.isStr(validBidRequests[0].userId.tdid)) { |
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.
Could possibly use utils.deepAccess here instead:
if (utils.isStr(utils.deepAccess(validBidRequests, '0.userId.tdid'))) {
instead of
if (validBidRequests[0] && validBidRequests[0].userId && utils.isStr(validBidRequests[0].userId.tdid)) {
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.
Won't request changes because of this,
But will wait to merge in case the change is wanted to be made!
Don't worry we will get it in before the release!
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.
Sure @robertrmartinez , thank you for this suggestion :)
@pm-harshad-mane Cool! Thanks :) |
Thank you @robertrmartinez !! :) |
* in-dev * added unit test cases * adding isStr check on userId.tdid * review suggestion
Type of change
Description of change
PubMatic adapter support to read TTD Id from UserId module