fixed bug with sortByDealAndPriceBucketOrCpm #5504
Conversation
|
This pull request fixes 1 alert when merging 697613f into 1049d07 - view on LGTM.com fixed alerts:
|
FilipStamenkovic
added
needs update
needs 2nd review
ghost
force-pushed
the
bugfix/biddealprioritization
branch
from
697613f
to
d06c7fc
Compare
|
This pull request fixes 1 alert when merging d06c7fc into 3a8f5cd - view on LGTM.com fixed alerts:
|
|
I still don't see that fix is covered by a test case, can you add test case that will cover it? |
ghost
force-pushed
the
bugfix/biddealprioritization
branch
from
d06c7fc
to
b269d69
Compare
|
This pull request fixes 1 alert when merging b269d69 into f9631fb - view on LGTM.com fixed alerts:
|
| @@ -240,13 +240,13 @@ export function newTargeting(auctionManager) { | |||
| let targetingMap = Object.keys(targetingCopy).map(adUnitCode => { | |||
| return { | |||
| adUnitCode, | |||
| adUnitTargeting: targetingCopy[adUnitCode] | |||
| adserverTargeting: targetingCopy[adUnitCode] | |||
There was a problem hiding this comment.
curious why this name was changed?
There was a problem hiding this comment.
The sortByDealAndPriceBucketOrCpm is now also used in getHighestCpmBidsFromBidPool and the bids received object has the original name of adserverTargeting. If we want to leave the adUnitTargeting name for that cloned targeting map I could change it to work a different way. Just felt it made sense to go with the same name and went with the original adserverTargeting name.
Type of change
Description of change
Fixes a bug related to sortByDealAndPriceBucketOrCpm when sendBidsControl.dealPrioritization = true
Other information
#5424