Index explicit pass on bid #1406
Conversation
| @@ -419,6 +426,9 @@ var IndexExchangeAdapter = function IndexExchangeAdapter() { | |||
| _IndexRequestData.targetAggregate = {'open': {}, 'private': {}}; | |||
|
|
|||
| if (!utils.hasValidBidRequest(bidArr[0].params, requiredParams, ADAPTER_NAME)) { | |||
| for (var i = 0; i < bidArr.length; i++) { | |||
| passOnBid(bidArr[i].placementCode); | |||
| } | |||
There was a problem hiding this comment.
Can you explain this logic to me of passing on all bids if the first bid is invalid?
There was a problem hiding this comment.
I second you on this: bidRequested is called for non valid sizes.
Say I call this adapter with [[728, 90], [1000, 90]], the size 1000x90 is invalid for IX, but I'll get two calls to event bidRequested, when I would expect only one for the size of the adUnit that is accepted by IX.
There was a problem hiding this comment.
@snapwich This is an existing behaviour from previous endpoints. It is true that we no longer need this, but this shouldn't block this pull request. We'll fix this issue in the later releases.
There was a problem hiding this comment.
@dugwood Referencing: https://github.com/prebid/Prebid.js/blob/master/src/bidmanager.js#L54
bidmanager is expecting the number of bids returned from Index Exchange to match with the number of sizes in the slots.
There was a problem hiding this comment.
@arvending I've tested the fix, and it works (else this was raising the timeout). But @snapwich is right as the bidRequested event confirms it.
Nevertheless, I'll patch my 0.26.1 release with your fix, so that I can make it work (I've warned your team a while ago about it, so I'm okay with this temporary solution).
Thanks for your patch anyway.
There was a problem hiding this comment.
@arvending the bids array contains multiple bids across different adUnits though. Also, that bid order is pretty much arbitrary (I think it ends up being definition order or something), so why you would want to return NO_BIDs on all bids across all adUnits involved in an auction because one of those arbitrary bids is invalid makes no sense. Maybe I'm missing something with the indexExchange exception that changes things?
@mkendall07 what do you think about this?
There was a problem hiding this comment.
@snapwich
Our endpoint had changed in this PR - #826. The site id and slot id information from the first slot were treated uniquely in our original endpoint. Now that endpoint had changed, there are no needs to reject all bids if the first slot is invalid. Since this validation is an existing behaviour and introduced no behaviour changes to IX adapter, this issue should not block this particular pull request. In the later release, we will remove this first slot validation logic.
Type of change
Description of change
Other information