-
Notifications
You must be signed in to change notification settings - Fork 187
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
Yieldmo Adapter: add currency conversion for bid floors #3200
Yieldmo Adapter: add currency conversion for bid floors #3200
Conversation
.build(); | ||
} | ||
|
||
private Price convertBidFloor(Price bidFloorPrice, String impId, BidRequest bidRequest) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic is not covered with unit tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests have now been added, sorry about that.
Example can be found here |
…nging behavior of Yieldmo Adapter to continue with original bid floor and bid floor currency if currency conversion fails
.flatExtracting(BidRequest::getImp).doesNotContainNull() | ||
.extracting(Imp::getBidfloor, Imp::getBidfloorcur) | ||
.containsOnly(tuple(convertedBidFloor, "USD")); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant empty line
.flatExtracting(BidRequest::getImp).doesNotContainNull() | ||
.extracting(Imp::getBidfloor, Imp::getBidfloorcur) | ||
.containsOnly(tuple(BigDecimal.ONE, "EUR")); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant empty line
final BidRequest bidRequest = givenBidRequest(impBuilder -> | ||
impBuilder.bidfloor(BigDecimal.ONE).bidfloorcur("EUR")); | ||
|
||
final var convertedBidFloor = new BigDecimal("1.5"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we are not using vars, only strict types
|
||
return Price.of(USD_CURRENCY, convertedPrice); | ||
} catch (PreBidException e) { | ||
//If currency conversion fails, we still want to recieve the bid request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick. Typo and space
// If currency conversion fails, we still want to receive the bid request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have fixed the above issues. Thanks for reviewing.
This branch adds currency conversion for bid floors to the yieldmo adapter. I wasn't able to find any documentation around this feature, so this is largely based off of the implementations found in other adapters.