Skip to content
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

Merged
merged 6 commits into from
May 20, 2019

Conversation

pm-harshad-mane
Copy link
Contributor

Type of change

  • Feature

Description of change

PubMatic adapter support to read TTD Id from UserId module

@pm-harshad-mane
Copy link
Contributor Author

Hello @robertrmartinez ,
Can you please review the code changes?

let adsrvrOrgId = config.getConfig('adsrvrOrgId');
if (adsrvrOrgId && utils.isStr(adsrvrOrgId.TDID)) {
if (validBidRequests[0] && validBidRequests[0].userId && utils.isStr(validBidRequests[0].userId.tdid)) {
Copy link
Collaborator

@robertrmartinez robertrmartinez May 18, 2019

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)) {

Copy link
Collaborator

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!

Copy link
Contributor Author

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 :)

@robertrmartinez
Copy link
Collaborator

@pm-harshad-mane Cool! Thanks :)

@robertrmartinez robertrmartinez merged commit 006eecc into prebid:master May 20, 2019
@pm-harshad-mane
Copy link
Contributor Author

Thank you @robertrmartinez !! :)

@bretg bretg removed the needs review label Jun 5, 2019
VideoReach pushed a commit to VideoReach/Prebid.js that referenced this pull request Aug 1, 2019
* in-dev

* added unit test cases

* adding isStr check on userId.tdid

* review suggestion
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants