Skip to content

Conversation

@EmilNadimanov
Copy link
Contributor

@EmilNadimanov EmilNadimanov commented Jan 15, 2025

🔧 Type of changes

  • bid adapter update
  • configuration

✨ What's the context?

Implementing #3608, i.e. porting this change from PBS-Go

🧠 Rationale behind the change

Keeping PBS-Java up-to-date

🧪 Test plan

Made changes to the fixtures in the integration test (added markupType to the Bid- and Auction Response)
Expanded/reworked unit tests for the "makeBids" method, which was affected by the changes.

🏎 Quality check

  • Are your changes following our code style guidelines?
  • Are there any breaking changes in your code?
  • Does your test coverage exceed 90%?
  • Are there any erroneous console logs, debuggers or leftover code in your changes?

  1. I tried to consult the code-style file while making the changes and made sure that mvn validate is happy, but I might have missed something, pls let me know if I have to make any changes.
  2. The changes don't seem to be breaking to me. Please correct me if I'm wrong.

@EmilNadimanov EmilNadimanov changed the title Port 3608 smartadserver parse openrtb2 markup type #FIX 3608 smartadserver parse openrtb2 markup type Jan 15, 2025
@EmilNadimanov EmilNadimanov changed the title #FIX 3608 smartadserver parse openrtb2 markup type FIX #3608 smartadserver parse openrtb2 markup type Jan 15, 2025
@EmilNadimanov EmilNadimanov changed the title FIX #3608 smartadserver parse openrtb2 markup type FIX #3608 Port from PBS-Go: Equativ: SmartAdserver alias with update to use mtype Jan 15, 2025
@EmilNadimanov EmilNadimanov marked this pull request as ready for review January 15, 2025 16:13
@EmilNadimanov
Copy link
Contributor Author

Hello everyone. I am going on vacation until February so I won't be able to react to any responses here. If this PR can wait until then, I will happily finish it in February. If not, I kindly ask @bretg to assign the Issue/PR to someone else, if any changes are required.

@osulzhenko
Copy link
Collaborator

@EmilNadimanov is it ready for review?

@EmilNadimanov
Copy link
Contributor Author

@osulzhenko it is. Am I supposed to mark it with a tag or something like that?

@osulzhenko
Copy link
Collaborator

@EmilNadimanov No need—if it doesn't contain "WIP" or "draft," it will be considered ready. I was just confused by the message, which is why I decided to ask. The PR will be reviewed soon.

Copy link
Collaborator

@CTMBNara CTMBNara left a comment

Choose a reason for hiding this comment

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

Also, please, add integration tests for new alias

@osulzhenko osulzhenko changed the title FIX #3608 Port from PBS-Go: Equativ: SmartAdserver alias with update to use mtype Equativ: SmartAdserver alias with update to use mtype Feb 28, 2025
@osulzhenko
Copy link
Collaborator

Required update based on the comments

@osulzhenko osulzhenko requested a review from CTMBNara March 21, 2025 08:10
@EmilNadimanov
Copy link
Contributor Author

Thanks for reviewing. I will get to it sooner or later, I am currently quite busy with our internal work. Sorry for that.

@EmilNadimanov
Copy link
Contributor Author

@CTMBNara I've made the requested changes. Sorry that it took so long.

Copy link
Collaborator

@CTMBNara CTMBNara left a comment

Choose a reason for hiding this comment

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

Rename the JSON files associated with aliases so that they contain the alias name instead of the original bidder name:

  • /alias/test-smartadserver-bid-response.json
  • /alias/test-smartadserver-bid-request.json
  • /alias/test-auction-smartadserver-response.json
  • /alias/test-auction-smartadserver-request.json

@EmilNadimanov EmilNadimanov requested a review from CTMBNara June 2, 2025 07:57
@EmilNadimanov EmilNadimanov requested a review from CTMBNara June 30, 2025 07:25
@CTMBNara CTMBNara merged commit e3216fb into prebid:master Aug 1, 2025
8 of 10 checks passed
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: Equativ: SmartAdserver alias with update to use mtype

4 participants