fix issue #2315 sizeMapping not working with s2s requests #2332
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
mkendall07
reviewed
Apr 2, 2018
src/adaptermanager.js
Outdated
| @@ -113,7 +113,7 @@ function getAdUnitCopyForPrebidServer(adUnits) { | |||
| let adUnitsCopy = utils.deepClone(adUnits); | |||
|
|
|||
| adUnitsCopy.forEach((adUnit) => { | |||
| adUnit.sizes = transformHeightWidth(adUnit); | |||
| adUnit.sizesS2S = transformHeightWidth(adUnit); | |||
There was a problem hiding this comment.
can we just move this function into prebidServerBidAdapter instead of adding a temporary sizesS2S?
There was a problem hiding this comment.
Yep - I'll move the function over and make other adjustments.
|
This should be good once the change is made |
|
@mkendall07 I just pushed in the changes. |
mkendall07
approved these changes
Apr 3, 2018
1 task
dluxemburg
pushed a commit
to Genius/Prebid.js
that referenced
this pull request
Jul 17, 2018
…id#2332) * initial commit to fix issue prebid#2315 * moved transformHeightWidth function to prebidServerBidAdapter
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
This PR addresses the issue reported in #2315 and a subsequent issue found during testing.
The issue with the sizeMapping functionality was due to the
adUnit.sizesbeing transformed from a list of integer sizes (ie[[300, 250], [300, 600]]to a list of objects (ie[{w: 300, h: 250}, {w: 300, h: 600}]). The sizeMapping code expected a list of integer sizes, not the object format sizes - so it never passed the filtering check properly.The fix places moves the
transformHeightWidthfunction from theadaptermanager.jsover to theprebidServerBidAdapter.jsfile, so that the size values are transformed when pbs needs them.During testing for this fix, a subsequent issue was observed and fixed. In this issue, a bidder that was rejected by the sizeMapping still had the opportunity to participate in the pbs auction and could win the bid. This is due to the fact that the sizeMapping functionality filters out the invalid
adUnitbidders and simply doesn't include them in the bids objects that are made in the request object. The sizeMapping functionality doesn't directly edit/remove the invalid bidders from theadUnits. This indirectly causes the issue, since the the actual request object made for the pbs request is built separately from the client-side request and it uses theadUnitsunmodified object.The fix for this issue is to introduce a filter that removes any bidders from the
adUnitsobject if there is no correspondingbidin the bidder's client-side request for thatadUnit. It performs a two stage check, first against theadUnit.codewith therequest.bid.adUnitCode(in case the same bidder is in multiple adUnits) and second against theadUnit.bid.bidderwith therequest.bidderCode.Other information
Fixes issue #2315