Bug: bid.getCpmInNewCurrency does not use latest bid.cpm value and returns value as per original bid value.

[pm-harshad-mane]

Description

Bug: bid.getCpmInNewCurrency does not use the latest bid.cpm value and returns value as per original bid value.
We need an API to convert bid from one currency to another.

Scenario

I have set ad-server-currency in the config to INR, let's say USD to INR rate is 1:100.
I want to send bid value in INR to my ad-server but I want to send bid value in USD to my analytics.

The partner sends bid of $5.
I have put a hook on "bidResponse" so when my handler gets a bid object

• bid.cpm is now 500 (INR)
• bid.getCpmInNewCurrency('USD') returns 5 ($)
• now I change bid.cpm = 100
• but now bid.getCpmInNewCurrency('USD') still returns 5 ($)
  I do not have a way to convert the 100INR to equivalent $
  I think the bug is introduced due to keeping the actual value of bid.cpm in a variable at https://github.com/prebid/Prebid.js/blob/master/modules/currency.js#L184 and keeping the actual currency value in https://github.com/prebid/Prebid.js/blob/master/modules/currency.js#L183 which are refered in function getCpmInNewCurrency @ https://github.com/prebid/Prebid.js/blob/master/modules/currency.js#L187

Expected results

Approach 1: Quick fix: Can we extend getCpmInNewCurrency which has only one argument (toCurrency) to have multiple arguments (toCurrency, fromCurrency = fromCurrency (var on line 183), cpm = cpm (var on line 184) )
This will help publisher to change bids and also get corresponding bid value in othe currency (USD). Here still all attributes related to cpm (originalCpm etc.) are not updated and publisher will need to update those manually.

Approach 2:
Provide a separate API to do bid conversion in pbjs name-space.

Approach 3:
Provide an API to set the bid cpm value again in bid object which will recalculate values for all cpm related attributes.

[snapwich]

I don't like the idea of modifying bid.getCpmInNewCurrency to accept multiple arguments. It is a method of a bid instance and the instance should know what currency it is holding so the fromCurrency argument would be redundant unless you're giving it a currency that it's not holding; but then why are you calling bid.getCpmInNewCurrency()?

I might be okay with having bid.getCpmInNewCurrency() acting upon the the bid.cpm rather than the cached result, although that could give you erroneous results if you're updating bid.cpm because of currency conversion reasons and not also updating cpm.currency; and I'm not sure how I feel about updating cpm.currency in a bidResponse hook... So I guess the question is, why are you updating the bid.cpm?

I guess I'm not totally opposed to the idea of exposing our currency conversion as a public API on the pbjs namespace; however there's only few places where it's really applicable and I'd prefer we provide helpers to use the currency conversion in appropriate ways as opposed to opening the floodgates for users who want to use it for everything and anywhere. We aren't a currency conversion library. So I guess I'm interested in knowing where you'd be interested in using Prebid's currency conversion generically as a public API?

[pm-harshad-mane]

Hello @snapwich ,

Thank you for your feedback.
I am also not in favor of Approach 1.
I will prefer Approach 2, as we already have the necessary data available for converting cpm value from one currency to another, I think we can provide an API in pbjs namespace.

I also agree that if someone changes bid.currency upon getting the bid it may be a very messy scenario, let's exclude this scenario in the discussion.

Why I need this API?
As a publisher, I will like to boost the bids if I see a particular value for bid.dealId, and as I am using the currency module with adservercurrency set to INR, value of bid.cpm is in INR, now i need to send INR value to DFP and USD value in my analytics, but as i boosted the bid there is no relation in INR and USD value(returned by bid.getCpmInNewCurrency('USD') )

[pm-harshad-mane]

Hello @snapwich ,

This is a bug when one uses bidCpmAdjustment to update the bid.cpm.

[pm-harshad-mane]

Hello @snapwich ,

I have made changes to solve the issue, #3845 , can you please review the changes?
I have just made a change to use the latest value of bid.cpm and bid.currency than using a cached value of both params. The change is not changing the definition of the API.

[pm-harshad-mane]

Test Page: http://jsfiddle.net/tfsercgk/

[pm-harshad-mane]

Hello @snapwich,

We can close this issue as we have merged the #3845 .