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.

Sign up for GitHub

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

Jump to bottom

fixed bug with sortByDealAndPriceBucketOrCpm #5504

Merged
merged 2 commits into from
Jul 31, 2020
Merged

fixed bug with sortByDealAndPriceBucketOrCpm #5504

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
195f63b
fixed bug with sortByDealAndPriceBucketOrCpm
RRSchweitzer Jul 16, 2020
b269d69
added unit test
RRSchweitzer Jul 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/targeting.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export function getHighestCpmBidsFromBidPool(bidsReceived, highestCpmCallback, a
Object.keys(bidsByBidder).forEach(key => bucketBids.push(bidsByBidder[key].reduce(highestCpmCallback)));
// if adUnitBidLimit is set, pass top N number bids
if (adUnitBidLimit > 0) {
bucketBids = dealPrioritization ? bucketBids(sortByDealAndPriceBucketOrCpm(true)) : bucketBids.sort((a, b) => b.cpm - a.cpm);
bucketBids = dealPrioritization ? bucketBids.sort(sortByDealAndPriceBucketOrCpm(true)) : bucketBids.sort((a, b) => b.cpm - a.cpm);
FilipStamenkovic marked this conversation as resolved.
Show resolved Hide resolved
bids.push(...bucketBids.slice(0, adUnitBidLimit));
} else {
bids.push(...bucketBids);
Expand Down Expand Up @@ -76,11 +76,11 @@ export function getHighestCpmBidsFromBidPool(bidsReceived, highestCpmCallback, a
*/
export function sortByDealAndPriceBucketOrCpm(useCpm = false) {
return function(a, b) {
if (a.adUnitTargeting.hb_deal !== undefined && b.adUnitTargeting.hb_deal === undefined) {
if (a.adserverTargeting.hb_deal !== undefined && b.adserverTargeting.hb_deal === undefined) {
return -1;
}

if ((a.adUnitTargeting.hb_deal === undefined && b.adUnitTargeting.hb_deal !== undefined)) {
if ((a.adserverTargeting.hb_deal === undefined && b.adserverTargeting.hb_deal !== undefined)) {
return 1;
}

Expand All @@ -89,7 +89,7 @@ export function sortByDealAndPriceBucketOrCpm(useCpm = false) {
return b.cpm - a.cpm;
}

return b.adUnitTargeting.hb_pb - a.adUnitTargeting.hb_pb;
return b.adserverTargeting.hb_pb - a.adserverTargeting.hb_pb;
}
}

Expand Down Expand Up @@ -240,13 +240,13 @@ export function newTargeting(auctionManager) {
let targetingMap = Object.keys(targetingCopy).map(adUnitCode => {
return {
adUnitCode,
adUnitTargeting: targetingCopy[adUnitCode]
adserverTargeting: targetingCopy[adUnitCode]
Copy link
Member

Choose a reason for hiding this comment

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

curious why this name was changed?

Copy link
Author

Choose a reason for hiding this comment

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

The sortByDealAndPriceBucketOrCpm is now also used in getHighestCpmBidsFromBidPool and the bids received object has the original name of adserverTargeting. If we want to leave the adUnitTargeting name for that cloned targeting map I could change it to work a different way. Just felt it made sense to go with the same name and went with the original adserverTargeting name.

};
}).sort(sortByDealAndPriceBucketOrCpm());

// iterate through the targeting based on above list and transform the keys into the query-equivalent and count characters
return targetingMap.reduce(function (accMap, currMap, index, arr) {
let adUnitQueryString = convertKeysToQueryForm(currMap.adUnitTargeting);
let adUnitQueryString = convertKeysToQueryForm(currMap.adserverTargeting);

// for the last adUnit - trim last encoded ampersand from the converted query string
if ((index + 1) === arr.length) {
Expand Down
125 changes: 79 additions & 46 deletions test/spec/unit/core/targeting_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { expect } from 'chai';
import { targeting as targetingInstance, filters, sortByDealAndPriceBucketOrCpm } from 'src/targeting.js';
import { targeting as targetingInstance, filters, getHighestCpmBidsFromBidPool, sortByDealAndPriceBucketOrCpm } from 'src/targeting.js';
import { config } from 'src/config.js';
import { getAdUnits, createBidReceived } from 'test/fixtures/fixtures.js';
import CONSTANTS from 'src/constants.json';
Expand Down Expand Up @@ -338,11 +338,44 @@ describe('targeting tests', function () {
bid4 = utils.deepClone(bid1);
bid4.adserverTargeting['hb_bidder'] = bid4.bidder = bid4.bidderCode = 'appnexus';
bid4.cpm = 2.25;
bid4.adId = '8383838';
enableSendAllBids = true;

bidsReceived.push(bid4);
});

it('when sendBidsControl.bidLimit is set greater than 0 in getHighestCpmBidsFromBidPool', function () {
config.setConfig({
sendBidsControl: {
bidLimit: 2,
dealPrioritization: true
}
});

const bids = getHighestCpmBidsFromBidPool(bidsReceived, utils.getHighestCpm, 2);

expect(bids.length).to.equal(3);
expect(bids[0].adId).to.equal('8383838');
expect(bids[1].adId).to.equal('148018fe5e');
expect(bids[2].adId).to.equal('48747745');
});

it('when sendBidsControl.bidLimit is set greater than 0 and deal priortization is false in getHighestCpmBidsFromBidPool', function () {
config.setConfig({
sendBidsControl: {
bidLimit: 2,
dealPrioritization: false
}
});

const bids = getHighestCpmBidsFromBidPool(bidsReceived, utils.getHighestCpm, 2);

expect(bids.length).to.equal(3);
expect(bids[0].adId).to.equal('8383838');
expect(bids[1].adId).to.equal('148018fe5e');
expect(bids[2].adId).to.equal('48747745');
});

it('selects the top n number of bids when enableSendAllBids is true and and bitLimit is set', function () {
config.setConfig({
sendBidsControl: {
Expand All @@ -369,7 +402,7 @@ describe('targeting tests', function () {
expect(limitedBids.length).to.equal(2);
});

it('Sends all bids when enableSendAllBids is true and and bitLimit is set to 0', function () {
it('Sends all bids when enableSendAllBids is true and and bidLimit is set to 0', function () {
config.setConfig({
sendBidsControl: {
bidLimit: 0
Expand Down Expand Up @@ -690,171 +723,171 @@ describe('targeting tests', function () {
describe('sortByDealAndPriceBucketOrCpm', function() {
it('will properly sort bids when some bids have deals and some do not', function () {
let bids = [{
adUnitTargeting: {
adserverTargeting: {
hb_adid: 'abc',
hb_pb: '1.00',
hb_deal: '1234'
}
}, {
adUnitTargeting: {
adserverTargeting: {
hb_adid: 'def',
hb_pb: '0.50',
}
}, {
adUnitTargeting: {
adserverTargeting: {
hb_adid: 'ghi',
hb_pb: '20.00',
hb_deal: '4532'
}
}, {
adUnitTargeting: {
adserverTargeting: {
hb_adid: 'jkl',
hb_pb: '9.00',
hb_deal: '9864'
}
}, {
adUnitTargeting: {
adserverTargeting: {
hb_adid: 'mno',
hb_pb: '50.00',
}
}, {
adUnitTargeting: {
adserverTargeting: {
hb_adid: 'pqr',
hb_pb: '100.00',
}
}];
bids.sort(sortByDealAndPriceBucketOrCpm());
expect(bids[0].adUnitTargeting.hb_adid).to.equal('ghi');
expect(bids[1].adUnitTargeting.hb_adid).to.equal('jkl');
expect(bids[2].adUnitTargeting.hb_adid).to.equal('abc');
expect(bids[3].adUnitTargeting.hb_adid).to.equal('pqr');
expect(bids[4].adUnitTargeting.hb_adid).to.equal('mno');
expect(bids[5].adUnitTargeting.hb_adid).to.equal('def');
expect(bids[0].adserverTargeting.hb_adid).to.equal('ghi');
expect(bids[1].adserverTargeting.hb_adid).to.equal('jkl');
expect(bids[2].adserverTargeting.hb_adid).to.equal('abc');
expect(bids[3].adserverTargeting.hb_adid).to.equal('pqr');
expect(bids[4].adserverTargeting.hb_adid).to.equal('mno');
expect(bids[5].adserverTargeting.hb_adid).to.equal('def');
});

it('will properly sort bids when all bids have deals', function () {
let bids = [{
adUnitTargeting: {
adserverTargeting: {
hb_adid: 'abc',
hb_pb: '1.00',
hb_deal: '1234'
}
}, {
adUnitTargeting: {
adserverTargeting: {
hb_adid: 'def',
hb_pb: '0.50',
hb_deal: '4321'
}
}, {
adUnitTargeting: {
adserverTargeting: {
hb_adid: 'ghi',
hb_pb: '2.50',
hb_deal: '4532'
}
}, {
adUnitTargeting: {
adserverTargeting: {
hb_adid: 'jkl',
hb_pb: '2.00',
hb_deal: '9864'
}
}];
bids.sort(sortByDealAndPriceBucketOrCpm());
expect(bids[0].adUnitTargeting.hb_adid).to.equal('ghi');
expect(bids[1].adUnitTargeting.hb_adid).to.equal('jkl');
expect(bids[2].adUnitTargeting.hb_adid).to.equal('abc');
expect(bids[3].adUnitTargeting.hb_adid).to.equal('def');
expect(bids[0].adserverTargeting.hb_adid).to.equal('ghi');
expect(bids[1].adserverTargeting.hb_adid).to.equal('jkl');
expect(bids[2].adserverTargeting.hb_adid).to.equal('abc');
expect(bids[3].adserverTargeting.hb_adid).to.equal('def');
});

it('will properly sort bids when no bids have deals', function () {
let bids = [{
adUnitTargeting: {
adserverTargeting: {
hb_adid: 'abc',
hb_pb: '1.00'
}
}, {
adUnitTargeting: {
adserverTargeting: {
hb_adid: 'def',
hb_pb: '0.10'
}
}, {
adUnitTargeting: {
adserverTargeting: {
hb_adid: 'ghi',
hb_pb: '10.00'
}
}, {
adUnitTargeting: {
adserverTargeting: {
hb_adid: 'jkl',
hb_pb: '10.01'
}
}, {
adUnitTargeting: {
adserverTargeting: {
hb_adid: 'mno',
hb_pb: '1.00'
}
}, {
adUnitTargeting: {
adserverTargeting: {
hb_adid: 'pqr',
hb_pb: '100.00'
}
}];
bids.sort(sortByDealAndPriceBucketOrCpm());
expect(bids[0].adUnitTargeting.hb_adid).to.equal('pqr');
expect(bids[1].adUnitTargeting.hb_adid).to.equal('jkl');
expect(bids[2].adUnitTargeting.hb_adid).to.equal('ghi');
expect(bids[3].adUnitTargeting.hb_adid).to.equal('abc');
expect(bids[4].adUnitTargeting.hb_adid).to.equal('mno');
expect(bids[5].adUnitTargeting.hb_adid).to.equal('def');
expect(bids[0].adserverTargeting.hb_adid).to.equal('pqr');
expect(bids[1].adserverTargeting.hb_adid).to.equal('jkl');
expect(bids[2].adserverTargeting.hb_adid).to.equal('ghi');
expect(bids[3].adserverTargeting.hb_adid).to.equal('abc');
expect(bids[4].adserverTargeting.hb_adid).to.equal('mno');
expect(bids[5].adserverTargeting.hb_adid).to.equal('def');
});

it('will properly sort bids when some bids have deals and some do not and by cpm when flag is set to true', function () {
let bids = [{
cpm: 1.04,
adUnitTargeting: {
adserverTargeting: {
hb_adid: 'abc',
hb_pb: '1.00',
hb_deal: '1234'
}
}, {
cpm: 0.50,
adUnitTargeting: {
adserverTargeting: {
hb_adid: 'def',
hb_pb: '0.50',
hb_deal: '4532'
}
}, {
cpm: 0.53,
adUnitTargeting: {
adserverTargeting: {
hb_adid: 'ghi',
hb_pb: '0.50',
hb_deal: '4532'
}
}, {
cpm: 9.04,
adUnitTargeting: {
adserverTargeting: {
hb_adid: 'jkl',
hb_pb: '9.00',
hb_deal: '9864'
}
}, {
cpm: 50.00,
adUnitTargeting: {
adserverTargeting: {
hb_adid: 'mno',
hb_pb: '50.00',
}
}, {
cpm: 100.00,
adUnitTargeting: {
adserverTargeting: {
hb_adid: 'pqr',
hb_pb: '100.00',
}
}];
bids.sort(sortByDealAndPriceBucketOrCpm(true));
expect(bids[0].adUnitTargeting.hb_adid).to.equal('jkl');
expect(bids[1].adUnitTargeting.hb_adid).to.equal('abc');
expect(bids[2].adUnitTargeting.hb_adid).to.equal('ghi');
expect(bids[3].adUnitTargeting.hb_adid).to.equal('def');
expect(bids[4].adUnitTargeting.hb_adid).to.equal('pqr');
expect(bids[5].adUnitTargeting.hb_adid).to.equal('mno');
expect(bids[0].adserverTargeting.hb_adid).to.equal('jkl');
expect(bids[1].adserverTargeting.hb_adid).to.equal('abc');
expect(bids[2].adserverTargeting.hb_adid).to.equal('ghi');
expect(bids[3].adserverTargeting.hb_adid).to.equal('def');
expect(bids[4].adserverTargeting.hb_adid).to.equal('pqr');
expect(bids[5].adserverTargeting.hb_adid).to.equal('mno');
});
});

Expand Down