Skip to content
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

PBS adapter: fix bug with priceFloors sometimes not being set in request #8309

Merged
merged 5 commits into from
Apr 22, 2022

Conversation

dgirardi
Copy link
Collaborator

Type of change

  • Bugfix

Description of change

This addresses #8307 by picking what's likely to be the minimum floor for an adUnit (instead of just looking at the floor defined by the first request it finds, which may not be there)

This addresses prebid#8307 by picking what's likely to be the minimum floor for an adUnit (instead of just looking at the floor defined by the first request it finds, which may not be there)
@dgirardi dgirardi linked an issue Apr 20, 2022 that may be closed by this pull request
@@ -755,21 +756,64 @@ Object.assign(ORTB2.prototype, {
deepSetValue(imp, 'ext.prebid.storedauctionresponse.id', storedAuctionResponseBid.storedAuctionResponse.toString());
}

const getFloorBid = find(firstBidRequest.bids, bid => bid.adUnitCode === adUnit.code && typeof bid.getFloor === 'function');
const floor = (() => {
// we have to pick a floor for the imp - here we attempt to find the minimimum floor
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spelling on minimum

@patmmccann
Copy link
Collaborator

@robertrmartinez could you take this one?

@robertrmartinez
Copy link
Collaborator

Ahhh yes, makes sense this bug existed!

Copy link
Collaborator

@robertrmartinez robertrmartinez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, Why the decision to take the min?

I don't really know to be honest what matters just curious.

Could have also just grabbed the first valid floor found I guess.

Or the max?

Does not really matter to me, most of the time it will prob always be the same!

@dgirardi
Copy link
Collaborator Author

@robertrmartinez my reasoning is that if you don't pick the minimum one you might lose bids that would have otherwise been available - say that you somehow manage to set a floor of 1 for bidderA and 2 for bidderB; if the request to PBS sets bidfloor: 2 and neither bidder can beat it, you won't get any bid. But bidderA might be able to beat 1, which the publisher has indicated as acceptable.

But I'm also not sure this makes sense - maybe the revenue from when they can beat the higher floor outweigh the lost bids.

If noone thinks this is necessary, I'd rather remove the added complexity and just pick the first one.

@patmmccann
Copy link
Collaborator

i find the reason to use the min here persuasive, seems safer

@robertrmartinez
Copy link
Collaborator

Sounds good to me!

@patmmccann patmmccann merged commit d8b0509 into prebid:master Apr 22, 2022
JoelPM pushed a commit to JoelPM/Prebid.js that referenced this pull request Apr 25, 2022
…est (prebid#8309)

* PBS adapter: fix bug with priceFloors sometimes not being set in request

This addresses prebid#8307 by picking what's likely to be the minimum floor for an adUnit (instead of just looking at the floor defined by the first request it finds, which may not be there)

* Do not send pricefloor if any bid cannot provide one

* Test errors from currency conversion

* Revert whitespace changes

* fix spepelling
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No floor price set in prebidServerBidAdapter
3 participants