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

Bugfix/suppress prebid request if sizeMapping undefined for device width #758

Merged
merged 5 commits into from
Nov 1, 2016

Conversation

mkendall07
Copy link
Member

Type of change

  • Bugfix

Description of change

Currently when using adUnit.sizeMapping, if the device size does not match any mapping, requests are still made to bidders with undefined sizes. This causes issues with many bidders which expect adUnit.sizes to be required.

This PR changes the behavior to omit those requests.

Other information

@prebid/core to review

Copy link
Collaborator

@matthewlane matthewlane left a comment

Choose a reason for hiding this comment

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

Test request with undefined sizeMapping was suppressed as advertised. Checked this by looking at the network tab and seeing no request made for that bidder. Is some sort of messaging in the console needed when a bid is suppressed? Otherwise LGTM

…d, short circuit the callback if _bidsRequested.length == 0
@@ -571,6 +571,9 @@ $$PREBID_GLOBAL$$.requestBids = function ({ bidsBackHandler, timeout, adUnits, a
}

adaptermanager.callBids({ adUnits, adUnitCodes, cbTimeout });
if($$PREBID_GLOBAL$$._bidsRequested.length === 0) {
Copy link
Member Author

Choose a reason for hiding this comment

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

@protonate need your review on this change.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not clear why this is needed. Also if two or more bid requests have been made this _bidsRequested array could have bid data for placements from the previous request for bids.

Copy link
Collaborator

@protonate protonate left a comment

Choose a reason for hiding this comment

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

LGTM, tests are green, bids requests suppressed.

@protonate protonate merged commit a7e2bdf into master Nov 1, 2016
@mkendall07 mkendall07 deleted the bugfix/suppress_prebid_req_undefined_mapping branch January 10, 2017 15:10
mp-12301 pushed a commit to aol/Prebid.js that referenced this pull request Apr 10, 2017
…ebid-0.14.0 to release/1.7.0

* commit '3eefcf043466f5457c81bfec18b032b53490b78b': (52 commits)
  New adapters ids.
  Prebid 0.14.0 Release
  add workaround to prevent IX adapter from ending auction earlier due to one request becoming many responses (prebid#763)
  Add pbjs.getHighestCpmBids for getting winning bids (prebid#755)
  Fix build
  Drop Sekindo adapter.
  Bugfix/suppress prebid request if sizeMapping undefined for device width (prebid#758)
  amp integration (prebid#756)
  Add contribution guidelines (prebid#761)
  Add api method to shuffle the order bidders are called in (prebid#760)
  build adapter from custom source path (prebid#753)
  reduce duplication and ignore ga.js in coverage (prebid#754)
  Log /ut response errors in dev console (prebid#747)
  Fix indentation (code style error when building) (prebid#751)
  Add package keywords (prebid#746)
  added new rubiconLite adapter (prebid#740)
  Show warning if bidCpmAdjustment is set for AOL bidder (closes prebid#725) (prebid#728)
  IX adapter code refactoring (prebid#711)
  Update memeglobal.js (prebid#737)
  Pulsepoint Analytics adapter for Prebid (prebid#706)
  ...
mp-12301 pushed a commit to aol/Prebid.js that referenced this pull request Apr 10, 2017
…7.0 to master

* commit '262f371a5c855419c3ef357fb1f0cf87a107749f': (52 commits)
  New adapters ids.
  Prebid 0.14.0 Release
  add workaround to prevent IX adapter from ending auction earlier due to one request becoming many responses (prebid#763)
  Add pbjs.getHighestCpmBids for getting winning bids (prebid#755)
  Fix build
  Drop Sekindo adapter.
  Bugfix/suppress prebid request if sizeMapping undefined for device width (prebid#758)
  amp integration (prebid#756)
  Add contribution guidelines (prebid#761)
  Add api method to shuffle the order bidders are called in (prebid#760)
  build adapter from custom source path (prebid#753)
  reduce duplication and ignore ga.js in coverage (prebid#754)
  Log /ut response errors in dev console (prebid#747)
  Fix indentation (code style error when building) (prebid#751)
  Add package keywords (prebid#746)
  added new rubiconLite adapter (prebid#740)
  Show warning if bidCpmAdjustment is set for AOL bidder (closes prebid#725) (prebid#728)
  IX adapter code refactoring (prebid#711)
  Update memeglobal.js (prebid#737)
  Pulsepoint Analytics adapter for Prebid (prebid#706)
  ...
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.

3 participants