fix deal targeting for cpm 0 (prebid#3233)

Marfeel · Mar 18, 2019 · 09c04fd · 09c04fd
1 parent 54ad583
commit 09c04fd
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 42 deletions.
diff --git a/src/targeting.js b/src/targeting.js
@@ -35,20 +35,11 @@ export function getHighestCpmBidsFromBidPool(bidsReceived, highestCpmCallback) {
   // filter top bid for each bucket by bidder
   Object.keys(buckets).forEach(bucketKey => {
     let bidsByBidder = groupBy(buckets[bucketKey], 'bidderCode');
-    Object.keys(bidsByBidder).forEach(key => bids.push(bidsByBidder[key].reduce(highestCpmCallback, getEmptyBid())));
+    Object.keys(bidsByBidder).forEach(key => bids.push(bidsByBidder[key].reduce(highestCpmCallback)));
   });
   return bids;
 }
 
-function getEmptyBid(adUnitCode) {
-  return {
-    adUnitCode: adUnitCode,
-    cpm: 0,
-    adserverTargeting: {},
-    timeToRespond: 0
-  };
-}
-
 /**
  * @typedef {Object.<string,string>} targeting
  * @property {string} targeting_key
@@ -222,7 +213,7 @@ export function newTargeting(auctionManager) {
       .filter(uniques)
       .map(adUnitCode => bidsReceived
         .filter(bid => bid.adUnitCode === adUnitCode ? bid : null)
-        .reduce(getHighestCpm, getEmptyBid(adUnitCode)));
+        .reduce(getHighestCpm));
   };
 
   /**

diff --git a/test/spec/unit/core/targeting_spec.js b/test/spec/unit/core/targeting_spec.js
@@ -99,42 +99,68 @@ const bid3 = {
 };
 
 describe('targeting tests', function () {
+  let sandbox;
+  let enableSendAllBids = false;
+
+  beforeEach(function() {
+    sandbox = sinon.sandbox.create();
+
+    let origGetConfig = config.getConfig;
+    sandbox.stub(config, 'getConfig').callsFake(function (key) {
+      if (key === 'enableSendAllBids') {
+        return enableSendAllBids;
+      }
+      return origGetConfig.apply(config, arguments);
+    });
+  });
+
+  afterEach(function () {
+    sandbox.restore();
+  });
+
   describe('getAllTargeting', function () {
     let amBidsReceivedStub;
     let amGetAdUnitsStub;
     let bidExpiryStub;
+    let bidsReceived;
 
     beforeEach(function () {
-      $$PREBID_GLOBAL$$._sendAllBids = false;
-      amBidsReceivedStub = sinon.stub(auctionManager, 'getBidsReceived').callsFake(function() {
-        return [bid1, bid2, bid3];
+      bidsReceived = [bid1, bid2, bid3];
+
+      amBidsReceivedStub = sandbox.stub(auctionManager, 'getBidsReceived').callsFake(function() {
+        return bidsReceived;
       });
-      amGetAdUnitsStub = sinon.stub(auctionManager, 'getAdUnitCodes').callsFake(function() {
+      amGetAdUnitsStub = sandbox.stub(auctionManager, 'getAdUnitCodes').callsFake(function() {
         return ['/123456/header-bid-tag-0'];
       });
-      bidExpiryStub = sinon.stub(targetingModule, 'isBidNotExpired').returns(true);
-    });
-
-    afterEach(function () {
-      auctionManager.getBidsReceived.restore();
-      auctionManager.getAdUnitCodes.restore();
-      targetingModule.isBidNotExpired.restore();
+      bidExpiryStub = sandbox.stub(targetingModule, 'isBidNotExpired').returns(true);
     });
 
     describe('when hb_deal is present in bid.adserverTargeting', function () {
+      let bid4;
+
+      beforeEach(function() {
+        bid4 = utils.deepClone(bid1);
+        bid4.adserverTargeting['hb_bidder'] = bid4.bidder = bid4.bidderCode = 'appnexus';
+        bid4.cpm = 0;
+        enableSendAllBids = true;
+
+        bidsReceived.push(bid4);
+      });
+
       it('returns targeting with both hb_deal and hb_deal_{bidder_code}', function () {
         const targeting = targetingInstance.getAllTargeting(['/123456/header-bid-tag-0']);
 
         // We should add both keys rather than one or the other
-        expect(targeting['/123456/header-bid-tag-0']).to.contain.keys('hb_deal', `hb_deal_${bid1.bidderCode}`);
+        expect(targeting['/123456/header-bid-tag-0']).to.contain.keys('hb_deal', `hb_deal_${bid1.bidderCode}`, `hb_deal_${bid4.bidderCode}`);
 
         // We should assign both keys the same value
         expect(targeting['/123456/header-bid-tag-0']['hb_deal']).to.deep.equal(targeting['/123456/header-bid-tag-0'][`hb_deal_${bid1.bidderCode}`]);
       });
     });
 
-    it('selects the top bid when _sendAllBids true', function () {
-      config.setConfig({ enableSendAllBids: true });
+    it('selects the top bid when enableSendAllBids true', function () {
+      enableSendAllBids = true;
       let targeting = targetingInstance.getAllTargeting(['/123456/header-bid-tag-0']);
 
       // we should only get the targeting data for the one requested adunit
@@ -155,14 +181,13 @@ describe('targeting tests', function () {
     let bidExpiryStub;
 
     beforeEach(function () {
-      $$PREBID_GLOBAL$$._sendAllBids = false;
-      amBidsReceivedStub = sinon.stub(auctionManager, 'getBidsReceived').callsFake(function() {
+      amBidsReceivedStub = sandbox.stub(auctionManager, 'getBidsReceived').callsFake(function() {
         return [];
       });
-      amGetAdUnitsStub = sinon.stub(auctionManager, 'getAdUnitCodes').callsFake(function() {
+      amGetAdUnitsStub = sandbox.stub(auctionManager, 'getAdUnitCodes').callsFake(function() {
         return ['/123456/header-bid-tag-0'];
       });
-      bidExpiryStub = sinon.stub(targetingModule, 'isBidNotExpired').returns(true);
+      bidExpiryStub = sandbox.stub(targetingModule, 'isBidNotExpired').returns(true);
     });
 
     afterEach(function () {
@@ -184,13 +209,8 @@ describe('targeting tests', function () {
       let bidExpiryStub;
       let auctionManagerStub;
       beforeEach(function () {
-        bidExpiryStub = sinon.stub(targetingModule, 'isBidNotExpired').returns(true);
-        auctionManagerStub = sinon.stub(auctionManager, 'getBidsReceived');
-      });
-
-      afterEach(function () {
-        bidExpiryStub.restore();
-        auctionManagerStub.restore();
+        bidExpiryStub = sandbox.stub(targetingModule, 'isBidNotExpired').returns(true);
+        auctionManagerStub = sandbox.stub(auctionManager, 'getBidsReceived');
       });
 
       it('should use bids from pool to get Winning Bid', function () {
@@ -248,14 +268,10 @@ describe('targeting tests', function () {
       let auctionManagerStub;
       let timestampStub;
       beforeEach(function () {
-        auctionManagerStub = sinon.stub(auctionManager, 'getBidsReceived');
-        timestampStub = sinon.stub(utils, 'timestamp');
+        auctionManagerStub = sandbox.stub(auctionManager, 'getBidsReceived');
+        timestampStub = sandbox.stub(utils, 'timestamp');
       });
 
-      afterEach(function () {
-        auctionManagerStub.restore();
-        timestampStub.restore();
-      });
       it('should not include expired bids in the auction', function () {
         timestampStub.returns(200000);
         // Pool is having 4 bids from 2 auctions. All the bids are expired and only bid #3 is passing the bidExpiry check.