-
Notifications
You must be signed in to change notification settings - Fork 181
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
InteractiveOffers - Parameters changed & dynamic endpoint #1417
Conversation
I'm not able to fix the Java CI Check, will need some help please |
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.
You need to modify url creation and use placeholder instead (BetweenBidder could be used as example) it will fix functional test
...a/org/prebid/server/proto/openrtb/ext/request/interactiveoffers/ExtImpInteractiveoffers.java
Show resolved
Hide resolved
final BidRequest bidRequest = BidRequest.builder() | ||
.imp(singletonList(Imp.builder() | ||
.ext(mapper.valueToTree(ExtPrebid.of(null, ExtImpInteractiveoffers.of(35)))) | ||
.ext(mapper.valueToTree(ExtPrebid.of(null, ExtImpInteractiveoffers.of("partnerId")))) |
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.
you must change value here to "abc123"
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.
changed
url += partnerId; | ||
} catch (PreBidException e) { | ||
return Result.withError(BidderError.badInput(e.getMessage())); | ||
} | ||
return Result.withValue(HttpRequest.<BidRequest>builder() |
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.
move this inside your try block
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.
Not sure what you'r asking to move inside the try block
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.
move this
return Result.withValue(HttpRequest.<BidRequest>builder() .method(HttpMethod.POST) .uri(url) .headers(HttpUtil.headers()) .payload(request) .body(mapper.encode(request)) .build());
to try {
block
and you dont need to create String url = endpointUrl;
@@ -38,9 +39,22 @@ public InteractiveOffersBidder(String endpointUrl, JacksonMapper mapper) { | |||
|
|||
@Override | |||
public Result<List<HttpRequest<BidRequest>>> makeHttpRequests(BidRequest request) { | |||
|
|||
ExtImpInteractiveoffers extImpInteractiveoffers = null; | |||
String url = endpointUrl; |
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.
you dont need this
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.
changed
|
||
ExtImpInteractiveoffers extImpInteractiveoffers = null; | ||
String url = endpointUrl; | ||
String partnerId; |
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.
move inside try block
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.
moved the variable that's only used inside the block
@@ -1,14 +1,14 @@ | |||
adapters: | |||
interactiveoffers: | |||
enabled: false | |||
endpoint: https://prebid-server.ioadx.com/bidRequest/?partnerId=d9e56d418c4825d466ee96c7a31bf1da6b62fa04 | |||
endpoint: https://prebid-server.ioadx.com/bidRequest/?partnerId= |
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.
use placeholder here like {{PARTNERID}} and use replace while constructing url
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.
changed
@@ -38,9 +39,22 @@ public InteractiveOffersBidder(String endpointUrl, JacksonMapper mapper) { | |||
|
|||
@Override | |||
public Result<List<HttpRequest<BidRequest>>> makeHttpRequests(BidRequest request) { | |||
|
|||
ExtImpInteractiveoffers extImpInteractiveoffers = null; |
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.
looks like this redundant, you dont use it
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.
changed
url += partnerId; | ||
} catch (PreBidException e) { | ||
return Result.withError(BidderError.badInput(e.getMessage())); | ||
} | ||
return Result.withValue(HttpRequest.<BidRequest>builder() |
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.
move this
return Result.withValue(HttpRequest.<BidRequest>builder() .method(HttpMethod.POST) .uri(url) .headers(HttpUtil.headers()) .payload(request) .body(mapper.encode(request)) .build());
to try {
block
and you dont need to create String url = endpointUrl;
No description provided.