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

Prebid Core: Fix wrong targeting being applied when multibid module is included #21

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

i-atanasov-az
Copy link

@i-atanasov-az i-atanasov-az commented Jan 31, 2025

Type of change

  • Bugfix

Description of change

The refactor of targeting.js introduced in 9.16.0 relies on getHighestCpmBidsFromBidPool returning sorted bids and then when a winning bid should be chosen it simply reduces the sorted bids. The logic in getHighestCpmBidsFromBidPool, though, returns unchanged bids when hasModified is true. But getHighestCpmBidsFromBidPool is hookable and multibid in it's targetBidPoolHook sets hasModified to true when multibid is enabled causing bids not to be sorted and the winning bid chosen for targeting is the first rather than the best bid.

Changes:

  • Puts back the logic from before the change to choose the winning bid inside the getWinningBids method instead of choosing the winning bid as the first bid for the ad unit, relying on getHighestCpmBidsFromBidPool returning already sorted bids.
  • Changes the logic of getWinningBidTargeting to use getWinningBids instead of reducing the bidsReceived
  • Adds adUnitCodes to function interfaces

Other information

prebid#12273

…etWinningBids instead of the using the hookable getHighestCpmBidsFromBidPool which has a logic to return bids unchanged, to sort them causing wrong targeting being applied when multibid module is included.
@i-atanasov-az i-atanasov-az self-assigned this Jan 31, 2025
@i-atanasov-az i-atanasov-az force-pushed the bugfix/targeting-multibid branch from 3c675ba to 07c74f0 Compare January 31, 2025 12:40
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