-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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.
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
Add TapSense Header Bidding Adapter and tests #1004
Conversation
Hi @mkendall07, we currently have a publisher waiting to integrate with us, anyway we can help to speed up the merge process? tagging @amitmanjhi |
src/adapters/tapsense.js
Outdated
@@ -0,0 +1,89 @@ | |||
//v0.0.1 | |||
var bidfactory = require('../bidfactory.js'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use const
/let
instead of var
as appropriate.
src/adapters/tapsense.js
Outdated
if (!bid.params.scriptURL) { | ||
continue; | ||
} | ||
var queryString = "?price=true&callback=tapsense.callback_with_price_" + bid.bidId + "&version=" + version + "&"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer string templates here instead of concatenation.
src/adapters/tapsense.js
Outdated
continue; | ||
} | ||
var queryString = "?price=true&callback=tapsense.callback_with_price_" + bid.bidId + "&version=" + version + "&"; | ||
window.tapsense["callback_with_price_" + bid.bidId] = generateCallback(bid.bidId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use the the prebid global object $$PREBID_GLOBAL$$
instead of window.
src/adapters/tapsense.js
Outdated
if (validParams.indexOf(keys[j]) < 0) continue; | ||
queryString += encodeURIComponent(keys[j]) + "=" + encodeURIComponent(bid.params[keys[j]]) + "&"; | ||
} | ||
var scriptURL = bid.params.scriptURL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you make scriptUrl
static or is it required to be passed in?
src/adapters/tapsense.js
Outdated
} | ||
|
||
function generateCallback(bidId){ | ||
return function(response, price) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason this is an anonymous function?
src/adapters/tapsense.js
Outdated
return; | ||
} | ||
for (var k = 0; k < bid.sizes.length; k++) { | ||
if (creativeSizes.indexOf(bid.sizes[k].join("x")) > -1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fails if the bid.sizes
is a single array and not a nested array (ie [300,250];
instead of [[300,250]];
)
@erikchau |
* changed var to es6 let/const * when checking for bid sizes, use utils.parseSizesInput to handle single/nested arrays * use template strings where applicable * use $$PREBID_GLOBAL$$ instead of window * scriptUrl is now static * named anonymous function in generateCallBack * add more tests in tapsense_spec.js
|
@mkendall07 ready for re-review |
const TapSenseAdapter = function TapSenseAdapter() { | ||
const version = "0.0.1"; | ||
const creativeSizes = [ | ||
"320x50" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only valid bid size?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For header bidding we only currently only support banner requests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, probably want to make a note about this in your bidder documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to be sure, to create our bidder documentation I will need to make a PR for this page with our information correct?? http://prebid.org/dev-docs/bidders.html
Thanks for the updates. With the changes, I'm getting |
Created bidder documentation PR here: prebid/prebid.github.io#186 |
LGTM. Thanks |
* Add TapSense Header Bidding Adapter and tests * Update for Tapsense Prebid Header * changed var to es6 let/const * when checking for bid sizes, use utils.parseSizesInput to handle single/nested arrays * use template strings where applicable * use $$PREBID_GLOBAL$$ instead of window * scriptUrl is now static * named anonymous function in generateCallBack * add more tests in tapsense_spec.js * Url Callback parameter needed prebid global object
…built * 'master' of https://github.com/prebid/Prebid.js: Add GourmetAds AppNexus Alias (prebid#1057) fix issue calling `requestBids();` (prebid#1058) explicit win url response format as pixel (prebid#1001) OpenX Adapter: Correctly gets the page domain for cross-domain iframes (prebid#1027) better http/s support (prebid#1010) Add a new generated field transactionId to each adunits. (prebid#1040) Update readme (prebid#1053) PulsePoint Lite adapter (prebid#1016) Add new adapter ServerBid (by Adzerk) (prebid#1024) Fix Mantis tests in negative timezone (prebid#1049) Add deal id handling (prebid#1044) sanitize bidderRequest to rubicon adapter to ensure accountId is sent (prebid#1030) Bidfluence Adapter (prebid#1023) Update uglify-js version (prebid#1041) Add dev dependencies. hb_adid should be uppercase in all cases (prebid#1037) Add TapSense Header Bidding Adapter and tests (prebid#1004) iOS Referrer fix (prebid#996) Change identification of JavaScript user matching (prebid#1022) Fixed mixed tabs/spaces in wideorbit adapter (prebid#1031)
@erikchau |
hi @mkendall07 , this has already been done and merged to master |
oh, great. You were fast :) thanks |
* Add TapSense Header Bidding Adapter and tests * Update for Tapsense Prebid Header * changed var to es6 let/const * when checking for bid sizes, use utils.parseSizesInput to handle single/nested arrays * use template strings where applicable * use $$PREBID_GLOBAL$$ instead of window * scriptUrl is now static * named anonymous function in generateCallBack * add more tests in tapsense_spec.js * Url Callback parameter needed prebid global object
Type of change
Description of change
Addition of the TapSense Header Bidding Adapter
[email protected]
Other information