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

Consistent targeting set #2592

Merged
merged 3 commits into from
Jun 5, 2018
Merged

Consistent targeting set #2592

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
86c5f5a
Populate adUnitCode's even though there's no bid available
ptomasroos May 22, 2018
3a1f928
Add new targeting test for no bids, and upgrade old implicit getAdser…
ptomasroos May 22, 2018
b2dd5b2
Updated to a more simplex generation method
ptomasroos May 30, 2018
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
19 changes: 19 additions & 0 deletions src/targeting.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,25 @@ export function newTargeting(auctionManager) {
.concat(getCustomBidTargeting(adUnitCodes, bidsReceived))
.concat(config.getConfig('enableSendAllBids') ? getBidLandscapeTargeting(adUnitCodes, bidsReceived) : []);

// make sure at least there is a entry per adUnitCode in the targeting so receivers of SET_TARGETING call's can know what ad units are being invoked

adUnitCodes.forEach(key => {
if (!targeting.some(target => {
let hasMatchingAdUnitCode = false;

Object.getOwnPropertyNames(target).forEach(targetKey => {
if (targetKey === key) {
hasMatchingAdUnitCode = true;
}
});
return hasMatchingAdUnitCode;
})) {
const emptyTargeting = {};
emptyTargeting[key] = [];
targeting.push(emptyTargeting);
}
});

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this could be greatly simplified by replacing it with the following code after the flattenTargeting is called below

    adUnitCodes.forEach(code => {
      if (!targeting[code]) {
        targeting[code] = {};
      }
    });

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! Will give it a try

// store a reference of the targeting keys
targeting.map(adUnitCode => {
Object.keys(adUnitCode).map(key => {
Expand Down
30 changes: 30 additions & 0 deletions test/spec/unit/core/targeting_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,36 @@ describe('targeting tests', () => {
});
}); // end getAllTargeting tests

describe('getAllTargeting without bids return empty object', () => {
let amBidsReceivedStub;
let amGetAdUnitsStub;
let bidExpiryStub;

beforeEach(() => {
$$PREBID_GLOBAL$$._sendAllBids = false;
amBidsReceivedStub = sinon.stub(auctionManager, 'getBidsReceived').callsFake(function() {
return [];
});
amGetAdUnitsStub = sinon.stub(auctionManager, 'getAdUnitCodes').callsFake(function() {
return ['/123456/header-bid-tag-0'];
});
bidExpiryStub = sinon.stub(targetingModule, 'isBidExpired').returns(true);
});

afterEach(() => {
auctionManager.getBidsReceived.restore();
auctionManager.getAdUnitCodes.restore();
targetingModule.isBidExpired.restore();
});

it('returns targetingSet correctly', () => {
let targeting = targetingInstance.getAllTargeting(['/123456/header-bid-tag-0']);

// we should only get the targeting data for the one requested adunit to at least exist even though it has no keys to set
expect(Object.keys(targeting).length).to.equal(1);
});
}); // end getAllTargeting without bids return empty object

describe('Targeting in concurrent auctions', () => {
describe('check getOldestBid', () => {
let bidExpiryStub;
Expand Down
18 changes: 9 additions & 9 deletions test/spec/unit/pbjs_api_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,13 +202,13 @@ describe('Unit: Prebid Module', function () {

it('should return current targeting data for slots', function () {
$$PREBID_GLOBAL$$.setConfig({ enableSendAllBids: true });
const targeting = $$PREBID_GLOBAL$$.getAdserverTargeting();
const expected = getAdServerTargeting();
const targeting = $$PREBID_GLOBAL$$.getAdserverTargeting(['/19968336/header-bid-tag-0', '/19968336/header-bid-tag1']);
const expected = getAdServerTargeting(['/19968336/header-bid-tag-0, /19968336/header-bid-tag1']);
assert.deepEqual(targeting, expected, 'targeting ok');
});

it('should return correct targeting with default settings', () => {
var targeting = $$PREBID_GLOBAL$$.getAdserverTargeting();
var targeting = $$PREBID_GLOBAL$$.getAdserverTargeting(['/19968336/header-bid-tag-0', '/19968336/header-bid-tag1']);
var expected = {
'/19968336/header-bid-tag-0': {
foobar: '0x0,300x250,300x600',
Expand All @@ -230,8 +230,8 @@ describe('Unit: Prebid Module', function () {

it('should return correct targeting with bid landscape targeting on', () => {
$$PREBID_GLOBAL$$.setConfig({ enableSendAllBids: true });
var targeting = $$PREBID_GLOBAL$$.getAdserverTargeting();
var expected = getAdServerTargeting();
var targeting = $$PREBID_GLOBAL$$.getAdserverTargeting(['/19968336/header-bid-tag-0', '/19968336/header-bid-tag1']);
var expected = getAdServerTargeting(['/19968336/header-bid-tag-0', '/19968336/header-bid-tag1']);
assert.deepEqual(targeting, expected);
});

Expand All @@ -248,7 +248,7 @@ describe('Unit: Prebid Module', function () {

auction.getBidsReceived = function() { return _bidsReceived };

var targeting = $$PREBID_GLOBAL$$.getAdserverTargeting();
var targeting = $$PREBID_GLOBAL$$.getAdserverTargeting(['/19968336/header-bid-tag-0', '/19968336/header-bid-tag1']);

// Ensure targeting for both ad placements includes the custom key.
assert.equal(
Expand Down Expand Up @@ -312,7 +312,7 @@ describe('Unit: Prebid Module', function () {
}
};

var targeting = $$PREBID_GLOBAL$$.getAdserverTargeting();
var targeting = $$PREBID_GLOBAL$$.getAdserverTargeting(['/19968336/header-bid-tag-0', '/19968336/header-bid-tag1']);

var expected = {
'/19968336/header-bid-tag-0': {
Expand Down Expand Up @@ -346,7 +346,7 @@ describe('Unit: Prebid Module', function () {

auction.getBidsReceived = function() { return _bidsReceived };

var targeting = $$PREBID_GLOBAL$$.getAdserverTargeting();
var targeting = $$PREBID_GLOBAL$$.getAdserverTargeting(['/19968336/header-bid-tag-0', '/19968336/header-bid-tag1']);

var expected = {
'/19968336/header-bid-tag-0': {
Expand Down Expand Up @@ -1487,7 +1487,7 @@ describe('Unit: Prebid Module', function () {
assert.ok(spyCallBids.calledTwice, 'When two requests for bids are made both should be' +
' callBids immediately');

let result = targeting.getAllTargeting(); // $$PREBID_GLOBAL$$.getAdserverTargeting();
let result = targeting.getAllTargeting(['/19968336/header-bid-tag-0', '/19968336/header-bid-tag1']); // $$PREBID_GLOBAL$$.getAdserverTargeting();
let expected = {
'/19968336/header-bid-tag-0': {
'foobar': '0x0,300x250,300x600',
Expand Down