From 425c567386bfd754eca29e327e41e3bf5fedd042 Mon Sep 17 00:00:00 2001 From: bretg Date: Thu, 19 Sep 2019 16:30:16 -0400 Subject: [PATCH] rubicon: avoid passing unknown position (#4207) * rubicon: not passing pos if not specified * added comment * not sending pos for video when undefined * cleaning up test * fixed unit test --- modules/rubiconBidAdapter.js | 14 ++++++- test/spec/modules/rubiconBidAdapter_spec.js | 43 +++++++++++++++------ 2 files changed, 43 insertions(+), 14 deletions(-) diff --git a/modules/rubiconBidAdapter.js b/modules/rubiconBidAdapter.js index f309e4b7cac..a1cdfdf8fea 100644 --- a/modules/rubiconBidAdapter.js +++ b/modules/rubiconBidAdapter.js @@ -395,7 +395,6 @@ export const spec = { 'zone_id': params.zoneId, 'size_id': parsedSizes[0], 'alt_size_ids': parsedSizes.slice(1).join(',') || undefined, - 'p_pos': params.position === 'atf' || params.position === 'btf' ? params.position : 'unknown', 'rp_floor': (params.floor = parseFloat(params.floor)) > 0.01 ? params.floor : 0.01, 'rp_secure': isSecure() ? '1' : '0', 'tk_flint': `${configIntType || DEFAULT_INTEGRATION}_v$prebid.version$`, @@ -409,6 +408,11 @@ export const spec = { 'rf': _getPageUrl(bidRequest, bidderRequest) }; + // add p_pos only if specified and valid + if (params.position === 'atf' || params.position === 'btf') { + data['p_pos'] = params.position; + } + if ((bidRequest.userId || {}).tdid) { data['tpid_tdid'] = bidRequest.userId.tdid; } @@ -768,8 +772,14 @@ function addVideoParameters(data, bidRequest) { if (typeof data.imp[0].video === 'object' && data.imp[0].video.skipafter === undefined) { data.imp[0].video.skipafter = bidRequest.params.video.skipdelay; } + // video.pos can already be specified by adunit.mediatypes.video.pos. + // but if not, it might be specified in the params if (typeof data.imp[0].video === 'object' && data.imp[0].video.pos === undefined) { - data.imp[0].video.pos = bidRequest.params.position === 'atf' ? 1 : (bidRequest.params.position === 'btf' ? 3 : 0); + if (bidRequest.params.position === 'atf') { + data.imp[0].video.pos = 1; + } else if (bidRequest.params.position === 'btf') { + data.imp[0].video.pos = 3; + } } const size = parseSizes(bidRequest, 'video') diff --git a/test/spec/modules/rubiconBidAdapter_spec.js b/test/spec/modules/rubiconBidAdapter_spec.js index f7432435060..8d65e1e97b4 100644 --- a/test/spec/modules/rubiconBidAdapter_spec.js +++ b/test/spec/modules/rubiconBidAdapter_spec.js @@ -353,6 +353,28 @@ describe('the rubicon adapter', function () { }); }); + it('should not send p_pos to AE if not params.position specified', function() { + var noposRequest = utils.deepClone(bidderRequest); + delete noposRequest.bids[0].params.position; + + let [request] = spec.buildRequests(noposRequest.bids, noposRequest); + let data = parseQuery(request.data); + + expect(data['site_id']).to.equal('70608'); + expect(data['p_pos']).to.equal(undefined); + }); + + it('should not send p_pos to AE if not params.position is invalid', function() { + var badposRequest = utils.deepClone(bidderRequest); + badposRequest.bids[0].params.position = 'bad'; + + let [request] = spec.buildRequests(badposRequest.bids, badposRequest); + let data = parseQuery(request.data); + + expect(data['site_id']).to.equal('70608'); + expect(data['p_pos']).to.equal(undefined); + }); + it('ad engine query params should be ordered correctly', function () { sandbox.stub(Math, 'random').callsFake(() => 0.1); let [request] = spec.buildRequests(bidderRequest.bids, bidderRequest); @@ -1135,14 +1157,6 @@ describe('the rubicon adapter', function () { expect(post.ext.prebid.cache.vastxml.returnCreative).to.equal(false) }); - it('should send request with proper ad position', function () { - createVideoBidderRequest(); - let positionBidderRequest = utils.deepClone(bidderRequest); - positionBidderRequest.bids[0].mediaTypes.video.pos = 1; - let [request] = spec.buildRequests(positionBidderRequest.bids, positionBidderRequest); - expect(request.data.imp[0].video.pos).to.equal(1); - }); - it('should send correct bidfloor to PBS', function() { createVideoBidderRequest(); @@ -1171,13 +1185,18 @@ describe('the rubicon adapter', function () { expect(request.data.imp[0]).to.not.haveOwnProperty('bidfloor'); }); - it('should send request with proper ad position when mediaTypes.video.pos is not defined', function () { + it('should send request with proper ad position', function () { createVideoBidderRequest(); let positionBidderRequest = utils.deepClone(bidderRequest); + positionBidderRequest.bids[0].mediaTypes.video.pos = 1; + let [request] = spec.buildRequests(positionBidderRequest.bids, positionBidderRequest); + expect(request.data.imp[0].video.pos).to.equal(1); + + positionBidderRequest = utils.deepClone(bidderRequest); positionBidderRequest.bids[0].params.position = undefined; positionBidderRequest.bids[0].mediaTypes.video.pos = undefined; - let [request] = spec.buildRequests(positionBidderRequest.bids, positionBidderRequest); - expect(request.data.imp[0].video.pos).to.equal(0); + [request] = spec.buildRequests(positionBidderRequest.bids, positionBidderRequest); + expect(request.data.imp[0].video.pos).to.equal(undefined); positionBidderRequest = utils.deepClone(bidderRequest); positionBidderRequest.bids[0].params.position = 'atf' @@ -1195,7 +1214,7 @@ describe('the rubicon adapter', function () { positionBidderRequest.bids[0].params.position = 'foobar'; positionBidderRequest.bids[0].mediaTypes.video.pos = undefined; [request] = spec.buildRequests(positionBidderRequest.bids, positionBidderRequest); - expect(request.data.imp[0].video.pos).to.equal(0); + expect(request.data.imp[0].video.pos).to.equal(undefined); }); it('should properly enforce video.context to be either instream or outstream', function () {