-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
fix and move logic that updates adUnitsS2SCopy based sizeMapping results #2795
Conversation
@jsnellbaker tested the pull found an issue. The defined sizes for each label dont pass correctly into the adUnitsS2SCopy array. So for example if we have label1 = desktop - sizes = 728,90 300,250 If appnexus labelany: ['desktop','moble'] If fired from desktop prebid will pass all sizes to adUnitsS2SCopy insted of just "728,90 300,250"
|
Hi @mercuryyy Can you provide a markup of the adUnits array that replicated the issue you described above? I have been attempting a few configurations but I haven't replicated the issue yet. |
@jsnellbaker Are you on the adops Slack channel or have an email i can DM a test page? |
Hi @mercuryyy, you can send it to jsnellbaker@appnexus.com |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Update on issue mentioned here; I did investigate the test pages provided, but I wasn't able to see the issue described by @mercuryyy. There were other issues related to Prebid Server, but they were separate from this PR. |
LGTM. Thanks |
Type of change
Description of change
This PR:
bidRequests.adUnitsS2SCopy
object. This update ensured only valid bids were being included in the s2s request.adUnitsS2SCopy
object is properly reflected in the console log messages for easier debugging/troubleshooting.Additional Context
Currently for s2s requests, we resync the
adUnitsS2SCopy
object based on the various bids that exist in thebidRequest
objects. This resync is to remove anybids
that were rejected by the sizeMapping/label filtering checks.Keeping the
adUnitsS2SCopy
in sync is important, as this object (not thebidRequest
) is used to build the POST request meant for PBS; thus this ensures only validated bids get sent to PBS.Notes on Fix
The logic used for the sync operated on a combination of the
bidder
andadUnitCode
s to matchadUnit.bid
withbidRequest.bids
to know if they passed the sizeMapping checks. This logic was insufficient when the sameadUnitCode
was used across different adUnits that used different labels. It allowed bids for the wrong label to pass the screening and get added to the PBS request.The new logic operates on the
bid_id
values, which are generated uniquely for eachadUnit.bid
object and then stored in the correspondingbidRequest.bid
object.By moving this logic to the
makeBidRequest
function, we can display the updatedadUnitsS2SCopy
object in the 'Bids Requested' log message (here https://github.com/prebid/Prebid.js/blob/master/src/auction.js#L188 ) instead of digging through debugger.