-
Notifications
You must be signed in to change notification settings - Fork 2k
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
IX Bid Adapter: Add banner pos support #8892
IX Bid Adapter: Add banner pos support #8892
Conversation
lgtm |
@@ -171,7 +171,8 @@ describe('IndexexchangeAdapter', function () { | |||
sizes: [[300, 250], [300, 600]], | |||
mediaTypes: { | |||
banner: { | |||
sizes: [[300, 250], [300, 600]] | |||
sizes: [[300, 250], [300, 600]], | |||
pos: 0, |
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.
erroneous comma here, I think
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.
Nice catch! It's addressed.
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.
LGTM
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.
LGTM
modules/ixBidAdapter.js
Outdated
@@ -612,13 +612,15 @@ function buildRequest(validBidRequests, bidderRequest, impressions, version) { | |||
r.ext.ixdiag.err = cachedErrors; | |||
} | |||
|
|||
// populate request source.tid | |||
r.source = { | |||
tid: validBidRequests[0].transactionId, |
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.
@robhazan what's your guys goal here? You already have this field on imp.ext.tid
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.
@patmmccann thanks for pointing that out, spoke to @robhazan about it and removed source tid logic from this MR.
* IX Bid Adapter: Add source tid and banner pos support * add missing semi * remove extra comma * update to check banner pos being int * remove source tid logic Co-authored-by: shahin.rahbariasl <shahin.rahbariasl@indexexchange.com>
* IX Bid Adapter: Add source tid and banner pos support * add missing semi * remove extra comma * update to check banner pos being int * remove source tid logic Co-authored-by: shahin.rahbariasl <shahin.rahbariasl@indexexchange.com>
Type of change
Description of change
Add banner.pos support (video.pos is already supported).
contact shahin.rahbariasl@indexexchange.com for more information.