Skip to content

Commit

Permalink
Drop non-video bidders from video ad units (prebid#1815)
Browse files Browse the repository at this point in the history
* Check mediaTypes when validating video bidders

* Update error message

* Drop non-compatible bidders rather than entire ad unit
  • Loading branch information
matthewlane authored and dluxemburg committed Jul 17, 2018
1 parent 59b5247 commit 6e86850
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 14 deletions.
26 changes: 17 additions & 9 deletions src/prebid.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import { getGlobal } from './prebidGlobal';
import { flatten, uniques, isGptPubadsDefined, adUnitsFilter } from './utils';
import { videoAdUnit, hasNonVideoBidder } from './video';
import { videoAdUnit, videoBidder, hasNonVideoBidder } from './video';
import { nativeAdUnit, nativeBidder, hasNonNativeBidder } from './native';
import './polyfill';
import { parse as parseURL, format as formatURL } from './url';
Expand Down Expand Up @@ -380,13 +380,18 @@ $$PREBID_GLOBAL$$.requestBids = function ({ bidsBackHandler, timeout, adUnits, a
adUnitCodes = adUnits && adUnits.map(unit => unit.code);
}

// for video-enabled adUnits, only request bids if all bidders support video
const invalidVideoAdUnits = adUnits.filter(videoAdUnit).filter(hasNonVideoBidder);
invalidVideoAdUnits.forEach(adUnit => {
utils.logError(`adUnit ${adUnit.code} has 'mediaType' set to 'video' but contains a bidder that doesn't support video. No Prebid demand requests will be triggered for this adUnit.`);
for (let i = 0; i < adUnits.length; i++) {
if (adUnits[i].code === adUnit.code) { adUnits.splice(i, 1); }
}
// for video-enabled adUnits, only request bids for bidders that support video
adUnits.filter(videoAdUnit).filter(hasNonVideoBidder).forEach(adUnit => {
const nonVideoBidders = adUnit.bids
.filter(bid => !videoBidder(bid))
.map(bid => bid.bidder)
.join(', ');

utils.logError(`
${adUnit.code} is a 'video' ad unit but contains non-video bidder(s) ${nonVideoBidders}.
No Prebid demand requests will be triggered for those bidders.
`);
adUnit.bids = adUnit.bids.filter(videoBidder);
});

// for native-enabled adUnits, only request bids for bidders that support native
Expand All @@ -396,7 +401,10 @@ $$PREBID_GLOBAL$$.requestBids = function ({ bidsBackHandler, timeout, adUnits, a
.map(bid => bid.bidder)
.join(', ');

utils.logError(`adUnit ${adUnit.code} has 'mediaType' set to 'native' but contains non-native bidder(s) ${nonNativeBidders}. No Prebid demand requests will be triggered for those bidders.`);
utils.logError(`
${adUnit.code} is a 'native' ad unit but contains non-native bidder(s) ${nonNativeBidders}.
No Prebid demand requests will be triggered for those bidders.
`);
adUnit.bids = adUnit.bids.filter(nativeBidder);
});

Expand Down
10 changes: 7 additions & 3 deletions src/video.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,14 @@ const OUTSTREAM = 'outstream';
/**
* Helper functions for working with video-enabled adUnits
*/
export const videoAdUnit = adUnit => adUnit.mediaType === VIDEO_MEDIA_TYPE;
const nonVideoBidder = bid => !videoAdapters.includes(bid.bidder);
export const videoAdUnit = adUnit => {
const mediaType = adUnit.mediaType === VIDEO_MEDIA_TYPE;
const mediaTypes = deepAccess(adUnit, 'mediaTypes.video');
return mediaType || mediaTypes;
};
export const videoBidder = bid => videoAdapters.includes(bid.bidder);
export const hasNonVideoBidder = adUnit =>
adUnit.bids.filter(nonVideoBidder).length;
adUnit.bids.filter(bid => !videoBidder(bid)).length;

/**
* @typedef {object} VideoBid
Expand Down
32 changes: 30 additions & 2 deletions test/spec/unit/pbjs_api_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -779,7 +779,7 @@ describe('Unit: Prebid Module', function () {
adaptermanager.callBids.restore();
});

it('should not callBids if a video adUnit has non-video bidders', () => {
it('should only request video bidders on video adunits', () => {
sinon.spy(adaptermanager, 'callBids');
const videoAdaptersBackup = adaptermanager.videoAdapters;
adaptermanager.videoAdapters = ['appnexusAst'];
Expand All @@ -793,7 +793,35 @@ describe('Unit: Prebid Module', function () {
}];

$$PREBID_GLOBAL$$.requestBids({adUnits});
sinon.assert.notCalled(adaptermanager.callBids);
sinon.assert.calledOnce(adaptermanager.callBids);

const spyArgs = adaptermanager.callBids.getCall(0);
const biddersCalled = spyArgs.args[0].adUnits[0].bids;
expect(biddersCalled.length).to.equal(1);

adaptermanager.callBids.restore();
adaptermanager.videoAdapters = videoAdaptersBackup;
});

it('should only request video bidders on video adunits configured with mediaTypes', () => {
sinon.spy(adaptermanager, 'callBids');
const videoAdaptersBackup = adaptermanager.videoAdapters;
adaptermanager.videoAdapters = ['appnexusAst'];
const adUnits = [{
code: 'adUnit-code',
mediaTypes: {video: {context: 'instream'}},
bids: [
{bidder: 'appnexus', params: {placementId: 'id'}},
{bidder: 'appnexusAst', params: {placementId: 'id'}}
]
}];

$$PREBID_GLOBAL$$.requestBids({adUnits});
sinon.assert.calledOnce(adaptermanager.callBids);

const spyArgs = adaptermanager.callBids.getCall(0);
const biddersCalled = spyArgs.args[0].adUnits[0].bids;
expect(biddersCalled.length).to.equal(1);

adaptermanager.callBids.restore();
adaptermanager.videoAdapters = videoAdaptersBackup;
Expand Down

0 comments on commit 6e86850

Please sign in to comment.