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

fledgeForGPT module may assume Prebid adunit.code matches GPT slot #11584

Closed
bretg opened this issue May 28, 2024 · 4 comments
Closed

fledgeForGPT module may assume Prebid adunit.code matches GPT slot #11584

bretg opened this issue May 28, 2024 · 4 comments
Assignees
Labels

Comments

@bretg
Copy link
Collaborator

bretg commented May 28, 2024

Type of issue

Potential bug

Description

In today's Tools meeting, @florianerl reported a potential issue with the fledgeForGPT module - that it's not working when the Prebid Adunit.code doesn't match a GPT slot. It's valid for Prebid adunit.codes to be divId other even a customSlotMatching function value.

Perhaps this line of code is near the source of the potential issue -- could this be generalized to first check if adunit.code is a GPT slot, and if not, check for div or customSlotMatching?

const gptSlot = getGptSlotForAdUnitCode(adUnitCode);

Unfortunately I don't have first hand data or a test page, but we've had issues before along these lines, so it seems plausible. So the first step would be to see if we have test cases that cover the divid and customSlotMatching scenarios.

@bretg bretg changed the title fledgeForGPT module assumes Prebid adunit.code matches GPT slot fledgeForGPT module may assume Prebid adunit.code matches GPT slot May 28, 2024
@dgirardi
Copy link
Collaborator

the current logic expects the ad unit code to be either a GAM ad unit path or the div ID:

export const compareCodeAndSlot = (slot, adUnitCode) => slot.getAdUnitPath() === adUnitCode || slot.getSlotElementId() === adUnitCode;

customSlotMatching is a parameter to setTargetingForGptAsync. We can replicate it for setPAAPIConfigForGPT - or we could extract it into a configuration parameter.

@ptomasroos
Copy link
Contributor

ptomasroos commented May 29, 2024

Just bumped into this when I was about to enable fledge on my ad slots which purely uses customSlotMatching.
I would say, adding customSlotMatching into the setPAAPIConfigForGPT function would be right way to approach this, having the same flexibility to filter gpt slots

Object.keys(targetingConfig).filter(customSlotMatching ? customSlotMatching(slot) : isAdUnitCodeMatchingSlot(slot))

@adtechnology
Copy link

When will this be fixed?

@mkomorski mkomorski self-assigned this Jun 18, 2024
@patmmccann
Copy link
Collaborator

fixed in #11714

@github-project-automation github-project-automation bot moved this from Ready for Dev to Done in Prebid.js Tactical Issues table Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

No branches or pull requests

6 participants