From 8802bef012561b8c790348a745d674341b1b4bd0 Mon Sep 17 00:00:00 2001 From: Rich Snapp Date: Fri, 26 Oct 2018 10:51:15 -0600 Subject: [PATCH] fix deal targeting for cpm 0 --- src/targeting.js | 13 +---- test/spec/unit/core/targeting_spec.js | 78 ++++++++++++++++----------- 2 files changed, 49 insertions(+), 42 deletions(-) diff --git a/src/targeting.js b/src/targeting.js index dcd59f362c6..30407994b8e 100644 --- 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.} 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 index 9910645be09..427ceeca74c 100644 --- 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.