Prebid 11: gpt slots matching to ad units logic unification#14480
Prebid 11: gpt slots matching to ad units logic unification#14480mkomorski wants to merge 4 commits intoprebid-11.0from
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de7e592a3d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return; | ||
| } | ||
| targeting.setTargetingForGPT(adUnit, customSlotMatching); | ||
| targeting.setTargetingForGPT(adUnit); |
There was a problem hiding this comment.
Preserve custom slot matcher arg in setTargetingForGPTAsync
This removes the per-call matcher from pbjs.setTargetingForGPTAsync, so integrations that currently call setTargetingForGPTAsync(adUnits, matcher) will silently fall back to default slot/path matching. On pages with duplicate adUnit.code values, that can target the wrong GPT slot and send incorrect key-values to GAM. Please keep backward compatibility by honoring the second argument (or mapping it to the new global matcher path).
Useful? React with 👍 / 👎.
| export const getMatchingWinningBidForGPTSlot = (slot) => { | ||
| const match = isAdUnitCodeMatchingSlot(slot); | ||
| return getGlobal().getAllWinningBids().find( | ||
| // supports custom match function from config | ||
| bid => isFn(globalModuleConfig[CONFIG_CUSTOM_MATCH]) | ||
| ? globalModuleConfig[CONFIG_CUSTOM_MATCH](bid, slot) | ||
| : isBidAdUnitCodeMatchingSlot(bid, slot) | ||
| ({ adUnitCode }) => match(adUnitCode) |
There was a problem hiding this comment.
Keep bidViewability customMatchFunction compatibility
getMatchingWinningBidForGPTSlot no longer evaluates bidViewability.customMatchFunction, so existing configs that relied on that hook now cannot customize bid↔slot matching. In those deployments, slots that only matched via the custom function will now return null, which suppresses BID_VIEWABLE emission and viewability pixel firing for otherwise valid impressions. Consider preserving the old module-level matcher as a fallback.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de7e592a3d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return; | ||
| } | ||
| targeting.setTargetingForGPT(adUnit, customSlotMatching); | ||
| targeting.setTargetingForGPT(adUnit); |
There was a problem hiding this comment.
Keep custom slot matcher support in setTargetingForGPTAsync
This call now always invokes targeting.setTargetingForGPT(adUnit) and no longer forwards a caller-provided matcher, so existing integrations that pass a per-call custom matcher (used when multiple GPT slots share an adUnit code) will silently fall back to default path/id matching and can set targeting on the wrong slot. Please preserve the second-argument behavior (or a compatibility fallback) to avoid breaking current publisher implementations.
Useful? React with 👍 / 👎.
Pull Request Test Coverage Report for Build 22109419656Details
💛 - Coveralls |
e71919d to
7333bf8
Compare
7333bf8 to
6e3fe2d
Compare
This PR unifies matching gpt slots to ad units logic between adapters (gptUtils), targeting,
bidViewabillityandgptPreAuctionmodules. Introduces new config paramcustomGptSlotMatchingType of change
Bugfix
Feature
New bidder adapter
Updated bidder adapter
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Does this change affect user-facing APIs or examples documented on http://prebid.org?
Other
Description of change
Other information
#14408