Skip to content

Commit

Permalink
Pubmatic: Making Adslot param optional (prebid#3788)
Browse files Browse the repository at this point in the history
* changes for multiformat support

* added new constant NATIVE_ASSETS in PubMatic adapter

* removed NATIVE_ASSET_KEY reference in PubMatic adapter

* removed reference of const NATIVE_ASSET_ID from PubMatic adapter

* removed reference of const NATIVE_ASSET_DATA_TYPE in PubMatic adapter

* using _commonNativeRequestObject in PubMatic adapter to avoid repeating code block

* removed new-lines in const declaration

* generating NATIVE_ASSET_REVERSE_ID from NATIVE_ASSETS

* renamed NATIVE_ASSET_REVERSE_ID to NATIVE_ASSET_ID_TO_KEY_MAP

* little modification in _checkParamDataType in PubMatic adapter

* a minor improvement

* using let instead of var

* added NATIVE_ASSET_KEY_TO_ASSET_MAP and combining switch cases

* lint update

* removed some stale comments

* using LOG_WARN_PREFIX

* using const UNDEFINED

* added a logWarn in catch

* using arrow functions

* code review changes

* making adSlot parameter optional

* added a test case for without adSlot config

* changes to checkMediaType function

* suppress warning of missing mediaTypes.banner for native and video impression

* ignore fluid size, if present, in banner impression

* fix for ignoring fluid size in banner impression

* added relevant comments and test cases for fluid case in banner request

* added sample config for multiformat adunit

* commented redundant check

* marking adSlot param as optional in examples

* fixed some lint errors

* removed commnented code

* removed a trailing white space

* removed a todo comment as suggested in review

* deleting commented test cases
  • Loading branch information
pm-harshad-mane authored and jacekburys-quantcast committed May 15, 2019
1 parent de25c54 commit 61895b0
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 32 deletions.
16 changes: 3 additions & 13 deletions modules/pubmaticBidAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ function _createImpressionObject(bid, conf) {

impObj = {
id: bid.bidId,
tagid: bid.params.adUnit,
tagid: bid.params.adUnit || undefined,
bidfloor: _parseSlotParam('kadfloor', bid.params.kadfloor),
secure: window.location.protocol === 'https:' ? 1 : 0,
ext: {
Expand Down Expand Up @@ -746,10 +746,6 @@ export const spec = {
utils.logWarn(LOG_WARN_PREFIX + 'Error: publisherId is mandatory and cannot be numeric. Call to OpenBid will not be sent for ad unit: ' + JSON.stringify(bid));
return false;
}
if (!utils.isStr(bid.params.adSlot)) {
utils.logWarn(LOG_WARN_PREFIX + 'Error: adSlotId is mandatory and cannot be numeric. Call to OpenBid will not be sent for ad unit: ' + JSON.stringify(bid));
return false;
}
// video ad validation
if (bid.params.hasOwnProperty('video')) {
if (!bid.params.video.hasOwnProperty('mimes') || !utils.isArray(bid.params.video.mimes) || bid.params.video.mimes.length === 0) {
Expand Down Expand Up @@ -783,17 +779,11 @@ export const spec = {
var blockedIabCategories = [];
validBidRequests.forEach(originalBid => {
bid = utils.deepClone(originalBid);
bid.params.adSlot = bid.params.adSlot || '';
_parseAdSlot(bid);
if (bid.params.hasOwnProperty('video')) {
if (!(bid.params.adSlot && bid.params.adUnit && bid.params.adUnitIndex)) {
utils.logWarn(LOG_WARN_PREFIX + 'Skipping the non-standard adslot: ', bid.params.adSlot, JSON.stringify(bid));
return;
}
// Nothing to do
} else {
if (!(bid.params.adSlot && bid.params.adUnit && bid.params.adUnitIndex)) {
utils.logWarn(LOG_WARN_PREFIX + 'Skipping the non-standard adslot: ', bid.params.adSlot, JSON.stringify(bid));
return;
}
// If we have a native mediaType configured alongside banner, its ok if the banner size is not set in width and height
// The corresponding banner imp object will not be generated, but we still want the native object to be sent, hence the following check
if (!(bid.hasOwnProperty('mediaTypes') && bid.mediaTypes.hasOwnProperty(NATIVE)) && bid.params.width === 0 && bid.params.height === 0) {
Expand Down
8 changes: 4 additions & 4 deletions modules/pubmaticBidAdapter.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ var adUnits = [
bidder: 'pubmatic',
params: {
publisherId: '156209', // required
adSlot: 'pubmatic_test2', // required
adSlot: 'pubmatic_test2', // optional
pmzoneid: 'zone1, zone11', // optional
lat: '40.712775', // optional
lon: '-74.005973', // optional
Expand Down Expand Up @@ -56,7 +56,7 @@ var adVideoAdUnits = [
bidder: 'pubmatic',
params: {
publisherId: '156209', // required
adSlot: 'pubmatic_video1', // required
adSlot: 'pubmatic_video1', // optional
video: {
mimes: ['video/mp4','video/x-flv'], // required
skippable: true, // optional
Expand Down Expand Up @@ -104,7 +104,7 @@ var adUnits = [
bidder: 'pubmatic',
params: {
publisherId: '156295', // required
adSlot: 'pubmatic_test2@1x1', // required
adSlot: 'pubmatic_test2@1x1', // optional
}
}]
}];
Expand Down Expand Up @@ -146,7 +146,7 @@ var adUnits = [
bidder: 'pubmatic',
params: {
publisherId: '156209', // required
adSlot: 'pubmatic_test2@300x250', // required
adSlot: 'pubmatic_test2@300x250', // optional
pmzoneid: 'zone1, zone11', // optional
lat: '40.712775', // optional
lon: '-74.005973', // optional
Expand Down
51 changes: 36 additions & 15 deletions test/spec/modules/pubmaticBidAdapter_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -668,27 +668,15 @@ describe('PubMatic adapter', function () {
expect(isValid).to.equal(false);
});

it('invalid bid case: adSlot not passed', function () {
let validBid = {
bidder: 'pubmatic',
params: {
publisherId: '301'
}
},
isValid = spec.isBidRequestValid(validBid);
expect(isValid).to.equal(false);
});

it('invalid bid case: adSlot is not string', function () {
it('valid bid case: adSlot is not passed', function () {
let validBid = {
bidder: 'pubmatic',
params: {
publisherId: '301',
adSlot: 15671365
publisherId: '301'
}
},
isValid = spec.isBidRequestValid(validBid);
expect(isValid).to.equal(false);
expect(isValid).to.equal(true);
});
});

Expand Down Expand Up @@ -742,6 +730,39 @@ describe('PubMatic adapter', function () {
expect(data.imp[0].bidfloorcur).to.equal(bidRequests[0].params.currency);
});

it('Request params check: without adSlot', function () {
delete bidRequests[0].params.adSlot;
let request = spec.buildRequests(bidRequests);
let data = JSON.parse(request.data);
expect(data.at).to.equal(1); // auction type
expect(data.cur[0]).to.equal('USD'); // currency
expect(data.site.domain).to.be.a('string'); // domain should be set
expect(data.site.page).to.equal(bidRequests[0].params.kadpageurl); // forced pageURL
expect(data.site.publisher.id).to.equal(bidRequests[0].params.publisherId); // publisher Id
expect(data.site.ext).to.exist.and.to.be.an('object'); // dctr parameter
expect(data.site.ext.key_val).to.exist.and.to.equal(bidRequests[0].params.dctr);
expect(data.user.yob).to.equal(parseInt(bidRequests[0].params.yob)); // YOB
expect(data.user.gender).to.equal(bidRequests[0].params.gender); // Gender
expect(data.device.geo.lat).to.equal(parseFloat(bidRequests[0].params.lat)); // Latitude
expect(data.device.geo.lon).to.equal(parseFloat(bidRequests[0].params.lon)); // Lognitude
expect(data.user.geo.lat).to.equal(parseFloat(bidRequests[0].params.lat)); // Latitude
expect(data.user.geo.lon).to.equal(parseFloat(bidRequests[0].params.lon)); // Lognitude
expect(data.ext.wrapper.wv).to.equal($$REPO_AND_VERSION$$); // Wrapper Version
expect(data.ext.wrapper.transactionId).to.equal(bidRequests[0].transactionId); // Prebid TransactionId
expect(data.ext.wrapper.wiid).to.equal(bidRequests[0].params.wiid); // OpenWrap: Wrapper Impression ID
expect(data.ext.wrapper.profile).to.equal(parseInt(bidRequests[0].params.profId)); // OpenWrap: Wrapper Profile ID
expect(data.ext.wrapper.version).to.equal(parseInt(bidRequests[0].params.verId)); // OpenWrap: Wrapper Profile Version ID

expect(data.imp[0].id).to.equal(bidRequests[0].bidId); // Prebid bid id is passed as id
expect(data.imp[0].bidfloor).to.equal(parseFloat(bidRequests[0].params.kadfloor)); // kadfloor
expect(data.imp[0].tagid).to.deep.equal(undefined); // tagid
expect(data.imp[0].banner.w).to.equal(728); // width
expect(data.imp[0].banner.h).to.equal(90); // height
expect(data.imp[0].banner.format).to.deep.equal([{w: 160, h: 600}]);
expect(data.imp[0].ext.pmZoneId).to.equal(bidRequests[0].params.pmzoneid.split(',').slice(0, 50).map(id => id.trim()).join()); // pmzoneid
expect(data.imp[0].bidfloorcur).to.equal(bidRequests[0].params.currency);
});

it('Request params multi size format object check', function () {
let bidRequests = [
{
Expand Down

0 comments on commit 61895b0

Please sign in to comment.