Skip to content

Conversation

SamuelAlejandroNT
Copy link

🔧 Type of changes
[ x] new bid adapter
bid adapter update
new feature
new analytics adapter
new module
module update
bugfix
documentation
configuration
dependency update
tech debt (test coverage, refactorings, etc.)
✨ What's the context?
A new bid adapter AlvadsBidder has been implemented, and the bid type determination (BidType) logic has been corrected to properly distinguish between banner and video Imps. Test payloads were also updated to reflect separate banner and video Imps.

🧠 Rationale behind the change
Previously, the getBidType method always returned banner, even when the Imp was a video. This caused test failures and incorrect bid type mapping. The changes include:

Mapping each Bid to its corresponding AlvaAdsImp using impId.

Returning BidType.video when the Imp has a video object.

Updating test payloads to include separate banner and video Imps.

Ensuring lambda variables are final to comply with Java requirements.

No significant trade-offs were involved; the change is internal logic and test improvements.

🔎 New Bid Adapter Checklist
[x ] verify email contact works
[ x] NO fully dynamic hostnames
[ x] geographic host parameters are NOT required
[ x] direct use of HTTP is prohibited - implement an existing Bidder interface that will do all the job
[x ] if the ORTB is just forwarded to the endpoint, use the generic adapter - define the new adapter as the alias of the generic adapter
[ x] cover an adapter configuration with an integration test
🧪 Test plan
Bids were tested with both banner and video Imps. The results were verified to ensure:

Banner bids return BidType.banner.

Video bids return BidType.video.

Unit test makeBidsShouldReturnBidderBidsWithFullFields passes successfully.

🏎 Quality check
[x ] Are your changes following our code style guidelines?
[x ] Are there any breaking changes in your code?
[x ] Does your test coverage exceed 90%?
[x ] Are there any erroneous console logs, debuggers or leftover code in your changes?

@AntoxaAntoxic
Copy link
Collaborator

Hi @SamuelAlejandroNT why did you re-created the PR you've closed? Since you've done that the history of comments is left there, so please fix all the comments for the previous PR.

Thank you

@AntoxaAntoxic AntoxaAntoxic changed the title initial release with code fixes New Alvads Adapter Sep 19, 2025
@SamuelAlejandroNT
Copy link
Author

Hi @AntoxaAntoxic ,

The reason was an issue with branches caused by our team—apologies for that. We've also fixed the errors now.

Thank you.

Copy link
Collaborator

@AntoxaAntoxic AntoxaAntoxic left a comment

Choose a reason for hiding this comment

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

Some the comments from the previous PR have been ignored (especially test-related), please double check and fix them

Samuel Alejandro Maldonado Garcia added 3 commits September 25, 2025 09:06
@SamuelAlejandroNT
Copy link
Author

Hi @AntoxaAntoxic, the previous PR comments have been reviewed, addressed, and the changes have been pushed. Thanks for your feedback.

@AntoxaAntoxic
Copy link
Collaborator

@SamuelAlejandroNT please add an integration test for the bidder

@SamuelAlejandroNT
Copy link
Author

Added an integration test

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

Successfully merging this pull request may close these issues.

2 participants