Implement bid adjustments by media type #1226
Conversation
…djustments/implekenting
| resolveImpMediaType(bid.getImpid(), requestImps, bidderBid.getType()); | ||
|
|
||
| final BigDecimal priceAdjustmentFactor = | ||
| bidAdjustmentForBidder(bidRequest, bidderResponse.getBidder(), mediaType); |
There was a problem hiding this comment.
Pls set bidder as first argument since you call it.
|
|
||
| import com.fasterxml.jackson.annotation.JsonProperty; | ||
|
|
||
| public enum AdjustmentsMediaType { |
There was a problem hiding this comment.
Would it better to name it as BidAdjustmentMediaType ?
| case video: | ||
| return resolveVideoMediaType(bidImpId, imps); | ||
| default: | ||
| throw new IllegalArgumentException("BidType not present for bidderBid"); |
There was a problem hiding this comment.
This error should be PreBidException type (since indirectly it is verfied by RequestValidator) and should not fail entire auction.
Ideally, it can be covered by unit-test.
There was a problem hiding this comment.
Actually it's just for check style(switch default), because this exception is not possible at any case.
But replaced IllegalArgumentException with PreBidException
| } | ||
| } | ||
|
|
||
| private static AdjustmentsMediaType resolveVideoMediaType(String bidImpId, List<Imp> imps) { |
There was a problem hiding this comment.
Pls rename to resolveBidAdjustmentVideoMediaType()
| : null; | ||
| } | ||
|
|
||
| private static BigDecimal resolveAdjustmentFactor(ExtRequestBidadjustmentfactors extBidadjustmentfactors, |
There was a problem hiding this comment.
Pls rename to resolveBidAdjustmentFactor()
| @@ -1084,6 +1088,61 @@ private BidderResponse applyBidPriceChanges(BidderResponse bidderResponse, BidRe | |||
| return bidderResponse.with(BidderSeatBid.of(updatedBidderBids, seatBid.getHttpCalls(), errors)); | |||
| } | |||
|
|
|||
| private static AdjustmentsMediaType resolveImpMediaType(String bidImpId, List<Imp> imps, BidType bidType) { | |||
There was a problem hiding this comment.
Pls rename to resolveBidAdjustmentMediaType().
| final AdjustmentsMediaType mediaType = | ||
| resolveImpMediaType(bid.getImpid(), requestImps, bidderBid.getType()); | ||
|
|
||
| final BigDecimal priceAdjustmentFactor = |
There was a problem hiding this comment.
Price adjustment can be refactored to separate method.
Too much details for applyBidPriceChanges().
| @@ -43,7 +43,14 @@ | |||
| "ext": { | |||
| "prebid": { | |||
| "bidadjustmentfactors": { | |||
| "33across": 0.7 | |||
| "mediatypes": { | |||
There was a problem hiding this comment.
Pls change rubicon bidder test instead. Other bidder tests should be simple as possible.
Probably we'll need to divide rubicon-tests into some functional way.
There was a problem hiding this comment.
Moved this logic to multifid rubicon test, because current rubicon-appnexus test, has already to many test conditions
* Implement bid adjustments by media type * Add additional validation * Code style fixes * Logic update * Fixes after review * Fixes after review 2.0 * Fixes after review 2.0.1 * Fixes after review 2.0-STABLE
* Implement bid adjustments by media type * Add additional validation * Code style fixes * Logic update * Fixes after review * Fixes after review 2.0 * Fixes after review 2.0.1 * Fixes after review 2.0-STABLE
* Implement bid adjustments by media type * Add additional validation * Code style fixes * Logic update * Fixes after review * Fixes after review 2.0 * Fixes after review 2.0.1 * Fixes after review 2.0-STABLE
No description provided.