From 6e868508249f3d8a27bcd4517696fb9aec675c82 Mon Sep 17 00:00:00 2001
From: Matt Lane <mlane@appnexus.com>
Date: Tue, 14 Nov 2017 10:04:42 -0500
Subject: [PATCH] Drop non-video bidders from video ad units (#1815)

* Check mediaTypes when validating video bidders

* Update error message

* Drop non-compatible bidders rather than entire ad unit
---
 src/prebid.js                   | 26 +++++++++++++++++---------
 src/video.js                    | 10 +++++++---
 test/spec/unit/pbjs_api_spec.js | 32 ++++++++++++++++++++++++++++++--
 3 files changed, 54 insertions(+), 14 deletions(-)

diff --git a/src/prebid.js b/src/prebid.js
index f22e734515bd..947cebcac7e8 100644
--- a/src/prebid.js
+++ b/src/prebid.js
@@ -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';
@@ -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
@@ -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);
   });
 
diff --git a/src/video.js b/src/video.js
index f5203e4b198d..22255068cc05 100644
--- a/src/video.js
+++ b/src/video.js
@@ -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
diff --git a/test/spec/unit/pbjs_api_spec.js b/test/spec/unit/pbjs_api_spec.js
index d6f1451b65cc..5a91fe218ef8 100644
--- a/test/spec/unit/pbjs_api_spec.js
+++ b/test/spec/unit/pbjs_api_spec.js
@@ -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'];
@@ -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;