Fix multi-bid adId in Criteo bid adapter #3340
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
A single bid request can generate multiple bids if it has multiple sizes. However, by default 'adId' will be filled with the bid request's 'bidId' field, so if we create two bids from the same bid request, they will share the same 'adId'. Because of this, Prebid will not know which bid to use once DFP makes either of those bids win, always taking the first one (see 'auctionManager.findBidByAdId' used in 'pbjs.renderAd').
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
jsnellbaker
requested changes
Dec 17, 2018
There was a problem hiding this comment.
@Spark-NF Could you setup a unit-test for this scenario (or alter an existing one to cover this use-case)? We should be good at that point.
|
@jsnellbaker done, I added a test case covering the specific bug this PR fixes: one request generating two bids that must have different adIds. |
|
Thanks for making the update; LGTM. |
jsnellbaker
approved these changes
Dec 19, 2018
pedrolopezmrf
pushed a commit
to Marfeel/Prebid.js
that referenced
this pull request
Mar 18, 2019
* Fix multi-bid adId in Criteo bid adapter A single bid request can generate multiple bids if it has multiple sizes. However, by default 'adId' will be filled with the bid request's 'bidId' field, so if we create two bids from the same bid request, they will share the same 'adId'. Because of this, Prebid will not know which bid to use once DFP makes either of those bids win, always taking the first one (see 'auctionManager.findBidByAdId' used in 'pbjs.renderAd'). * Increment Criteo adapter version * Add test for Criteo multi-bid fix
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Type of change
Description of change
A single bid request to the Criteo endpoint can generate as many bids as there is sizes. However, by default
adIdwill be filled with the bid request'sbidIdfield, so if we create multiple bids from the same bid request, they will share the sameadId.Because of this, Prebid will not know which bid to use once DFP asks for the creative for either of those bids, always taking the first one, which might not be the one that actually won the auction (see auctionManager.findBidByAdId used in pbjs.renderAd).