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

Fixes issue #2327 - getBidLandScapeTargeting not using adUnitCode argument #2336

Merged
merged 2 commits into from
Apr 3, 2018
Merged

Fixes issue #2327 - getBidLandScapeTargeting not using adUnitCode argument #2336

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d26f0b4
initial commit to fix issue 2327
jsnellbaker Apr 3, 2018
a69b291
switched to different include function
jsnellbaker Apr 3, 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
6 changes: 5 additions & 1 deletion src/targeting.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,11 @@ export function newTargeting(auctionManager) {
});
// populate targeting keys for the remaining bids
return bids.map(bid => {
if (bid.adserverTargeting) {
if (
bid.adserverTargeting && adUnitCodes &&
((utils.isArray(adUnitCodes) && adUnitCodes.includes(bid.adUnitCode)) ||
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use includes by importing it from core-js
import includes from 'core-js/library/fn/array/includes';
Usage includes(<array>, <item>

includes doesn’t work in older versions of IE. The imported version is polyfilled to work with ie, rather than Babel doing any kind of transforms that might end up not working in target browsers

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the feedback; I just pushed a commit that switched it to the other includes function.

(typeof adUnitCodes === 'string' && bid.adUnitCode === adUnitCodes))
) {
return {
[bid.adUnitCode]: getTargetingMap(bid, standardKeys.filter(
key => typeof bid.adserverTargeting[key] !== 'undefined')
Expand Down
37 changes: 36 additions & 1 deletion test/spec/unit/core/targeting_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,36 @@ const bid2 = {
'ttl': 300
};

const bid3 = {
'bidderCode': 'rubicon',
'width': '300',
'height': '600',
'statusMessage': 'Bid available',
'adId': '48747745',
'cpm': 0.75,
'ad': 'markup',
'ad_id': '3163950',
'sizeId': '15',
'requestTimestamp': 1454535718610,
'responseTimestamp': 1454535724863,
'timeToRespond': 123,
'pbLg': '0.75',
'pbMg': '0.75',
'pbHg': '0.75',
'adUnitCode': '/123456/header-bid-tag-1',
'bidder': 'rubicon',
'size': '300x600',
'adserverTargeting': {
'hb_bidder': 'rubicon',
'hb_adid': '48747745',
'hb_pb': '0.75',
'foobar': '300x600'
},
'netRevenue': true,
'currency': 'USD',
'ttl': 300
};

describe('targeting tests', () => {
describe('getAllTargeting', () => {
let amBidsReceivedStub;
Expand All @@ -75,7 +105,7 @@ describe('targeting tests', () => {
beforeEach(() => {
$$PREBID_GLOBAL$$._sendAllBids = false;
amBidsReceivedStub = sinon.stub(auctionManager, 'getBidsReceived').callsFake(function() {
return [bid1, bid2];
return [bid1, bid2, bid3];
});
amGetAdUnitsStub = sinon.stub(auctionManager, 'getAdUnitCodes').callsFake(function() {
return ['/123456/header-bid-tag-0'];
Expand All @@ -92,9 +122,14 @@ describe('targeting tests', () => {
it('selects the top bid when _sendAllBids true', () => {
config.setConfig({ enableSendAllBids: true });
let targeting = targetingInstance.getAllTargeting(['/123456/header-bid-tag-0']);

// we should only get the targeting data for the one requested adunit
expect(Object.keys(targeting).length).to.equal(1);

let sendAllBidCpm = Object.keys(targeting['/123456/header-bid-tag-0']).filter(key => key.indexOf('hb_pb_') != -1)
// we shouldn't get more than 1 key for hb_pb_${bidder}
expect(sendAllBidCpm.length).to.equal(1);

// expect the winning CPM to be equal to the sendAllBidCPM
expect(targeting['/123456/header-bid-tag-0']['hb_pb_rubicon']).to.deep.equal(targeting['/123456/header-bid-tag-0']['hb_pb']);
});
Expand Down