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

When currency module is enabled, all bid object should have originalCurrency and originalCpm properties #3855

Closed
pm-harshad-mane opened this issue May 22, 2019 · 2 comments
Labels

Comments

@pm-harshad-mane
Copy link
Contributor

Type of issue

BUG

Description

When we enable the currency module, the currency module adds originalCurrency and originalCpm properties to the bid object. But these properties are not added when bid.currency is same as config.currency.adServerCurrency. This makes some bid objects inconsistent with other bid objects. This makes it difficult for the bid object consuming codes like bidResponse event handler or analytics handlers to consume this information. Due to this in-consistency, the consuming codes need to handle these bid objects separately and may need to set value for these properties on own with some logic. Better if we make the bid objects consistently in the presence of the currency module by adding these missing properties even if bid.currency matches config.currency.adserverCurrency

Steps to reproduce

Example:
PreBid Config.currency.adServerCurrency = 'SEK'
PubMatic bidder bids with currency USD
AdForm bidder bids with currency SEK

then pubmaticBid.originalCurrency = 'USD' and pubmaticBid.originalCpm = 5
but adformBid.originalCurrency and adform.originalCpm are UNDEFINED

Refer: https://github.com/prebid/Prebid.js/blob/master/modules/currency.js#L192

@stale
Copy link

stale bot commented Jun 6, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 6, 2019
@pm-harshad-mane
Copy link
Contributor Author

we can close this issue as we have merged the changes to master.

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

No branches or pull requests

1 participant