bidderSettings: Allow Currency Code control through adjustCurrency #12645
Conversation
…the bid currency to the converted currency
|
Tread carefully! This PR adds 1 linter error (possibly disabled through directives):
|
…ectly in Prebid Professor
|
Tread carefully! This PR adds 1 linter error (possibly disabled through directives):
|
|
Do not know why the test is failing, One can reproduce by adding currency module in their build file, then set adServerCurrency in the pbjs.setConfig, set the bid.currency in the auction.js according to my commit and this will update currency code along with the bid cpm |
|
we updated this ad server currency pattern -> #12102 |
src/auction.js
Outdated
| @@ -910,8 +910,9 @@ function setKeys(keyValues, bidderSettings, custBidObj, bidReq) { | |||
| export function adjustBids(bid) { | |||
| let bidPriceAdjusted = adjustCpm(bid.cpm, bid); | |||
|
|
|||
| if (bidPriceAdjusted >= 0) { | |||
| if (bidPriceAdjusted.cpm >= 0) { | |||
There was a problem hiding this comment.
The tests right now are failing because of linting errors, however, I hope they'd fail on this as well. bidPriceAdjusted is the adjusted CPM, on its own this cannot make sense.
There was a problem hiding this comment.
@dgirardi Kindly check it out again as I reverted bidPriceAdjusted as it was before. Now just fetching the adServerCurrency here for the currency code update
|
Tread carefully! This PR adds 1 linter error (possibly disabled through directives):
|
|
Tread carefully! This PR adds 1 linter error (possibly disabled through directives):
|
|
Tread carefully! This PR adds 1 linter error (possibly disabled through directives):
|
|
Tread carefully! This PR adds 1 linter error (possibly disabled through directives):
|
|
Tread carefully! This PR adds 1 linter error (possibly disabled through directives):
|
|
Tread carefully! This PR adds 1 linter error (possibly disabled through directives):
|
|
Tread carefully! This PR adds 1 linter error (possibly disabled through directives):
|
|
@dgirardi Now the lint error has also been fixed, kindly look into it now |
This one is for the teams using bidcpmadjustment function, in that case the currency code is not updated, however the cpm changes. |
|
@hi-ubaid why not use the currency module to adjust the bid currency instead of the bid adjustment function? Also it seems your change always adjusts the currency to the ad server currency when the bid adjustment function is used, and while it seems one may want that, certainly all cases of use of this function would not want it. Eg, suppose I have the currency module taking bids in Euros and converting them to USD and my bid adjustment function says to give the bid a 50% haircut because it is outstream competing with display. If the currency is adjusted here in the way it is in the pr, I would be required to have my bidadjustment cpm function be responsible for the currency value conversion as well, yet I am relying on another module to do that. tl;dr: it seems like you're mandating everyone handle currency conversions in bidadjustment function; while that may work for you, your pr may not work for everyone. |
|
Tread carefully! This PR adds 4 linter errors (possibly disabled through directives):
|
|
@patmmccann Can you take a look at it now, it's totally optional and addresses the concern of not making it mandatory for everyone, but it can be helpful for certain use cases as ours |
hi-ubaid
changed the title
Currently, bidCPMAdjustment receives bid.cpm and adjusts it according to the defined process. However, this change updates the behavior to allow passing the entire bid object. With this enhancement, not only will the bid.cpm be adjusted, but the bid's currency code will also be updated, as demonstrated in the Prebid Professor extension.
Previously, bidCPMAdjustment only adjusted the CPM value but did not account for changes to the currency code. This enhancement addresses that limitation.