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

Metax: Update bidder (#3631) #3660

Merged
merged 5 commits into from
Jan 23, 2025
Merged

Conversation

przemkaczmarek
Copy link
Collaborator

@przemkaczmarek przemkaczmarek commented Jan 7, 2025

🔧 Type of changes

  • bid adapter update

✨ What's the context?

Add BidVideo for video ads

#3631

@przemkaczmarek przemkaczmarek self-assigned this Jan 7, 2025
@AntoxaAntoxic AntoxaAntoxic linked an issue Jan 8, 2025 that may be closed by this pull request
Comment on lines 398 to 403
private static String givenBidResponse(Bid... bids) throws JsonProcessingException {
return mapper.writeValueAsString(BidResponse.builder()
.seatbid(singletonList(SeatBid.builder().bid(asList(bids)).build()))
.build());
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one looks redundant

Comment on lines 411 to 413
private static Bid givenBid(UnaryOperator<Bid.BidBuilder> bidCustomizer) {
return bidCustomizer.apply(Bid.builder()).build();
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one is also redundant

AntoxaAntoxic
AntoxaAntoxic previously approved these changes Jan 8, 2025
@AntoxaAntoxic AntoxaAntoxic requested a review from CTMBNara January 8, 2025 14:52
.bidCurrency(bidResponse.getCur())
.videoInfo(videoInfo(bid))
.build())

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, remove empty line

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also see a slight difference in logic with GO:

    private String resolveEndpoint(ExtImpMetax extImpMetax) {
        final String publisherIdAsString = Optional.ofNullable(extImpMetax.getPublisherId())
                .map(Object::toString)
                .orElse(StringUtils.EMPTY);
        final String adUnitAsString = Optional.ofNullable(extImpMetax.getAdUnit())
                .map(Object::toString)
                .orElse(StringUtils.EMPTY);

In both cases we need to set the default value to "0" instead of "".
Please, fix this if you don't mind.

@CTMBNara CTMBNara merged commit 9739aeb into master Jan 23, 2025
8 checks passed
@CTMBNara CTMBNara deleted the MetaX-Add-BidVideo-for-video-ads-#3631 branch January 23, 2025 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Port PR from PBS-Go: MetaX: Add BidVideo for video ads
3 participants