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.

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

Quantcast adaptor #1063

Merged
merged 39 commits into from
May 2, 2017
Merged

Quantcast adaptor #1063

merged 39 commits into from
May 2, 2017

Conversation

ShreeniwasIyer
Copy link
Contributor

@ShreeniwasIyer ShreeniwasIyer commented Mar 22, 2017

We are implementing a new adaptor for Quantcast with a code name qcx for the bidderCode.

Type of change

  • Bugfix
  • Feature
  • New bidder adapter
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other

Description of change

  • test parameters for validating bids
{
  bidder: 'quantcast',
  params: {
    publisherId: 'LCOf845vzU',
    battr : [1,2]
  }
}

Other information

@aneuway2
Copy link
Contributor

Hey @ShreeniwasIyer are publisherId and battr both required parameters for your adaptor? Might be good to submit a PR to have this documented :-)

https://github.com/prebid/prebid.github.io/tree/master/dev-docs/bidders

@ShreeniwasIyer
Copy link
Contributor Author

Thanks @aneuway2 for pointing out. Done now through prebid/prebid.github.io#204

@ShreeniwasIyer
Copy link
Contributor Author

Hi @aneuway2 , @mkendall07 - does this need any more inputs before review? I see that PRs submitted after this one have already been reviewed - so was wondering if I missed something..

@mkendall07
Copy link
Member

@ShreeniwasIyer
We will review this for the next release. Thanks

@jaiminpanchal27 jaiminpanchal27 self-assigned this Apr 10, 2017
Copy link
Collaborator

@jaiminpanchal27 jaiminpanchal27 left a comment

Choose a reason for hiding this comment

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

@ShreeniwasIyer Please check comments

Test parameters has bidder: 'qcx', it should be bidder:'quantcast' as defined in adapters.json

Also i am not able get bids back while testing from localhost due to CORS.


const DEFAULT_BID_FLOOR = 0.0000000001;
// The following 2 constants are adopted from bidfactory.js codes
const BID_STATUS_CODE_AVAILABLE = 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use STATUS.GOOD and STATUS.NO_BID defined in constants.json

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Thanks. @jaiminpanchal27

if(utils.isEmpty(responseText)) {
return;
}
let response = JSON.parse(responseText);
Copy link
Collaborator

Choose a reason for hiding this comment

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

wrap this in try catch

publisherId = '' + bids[0].params.publisherId;
utils._each(bids, function(bid) {
// make sure the "sizes" are an array of arrays
if (!(bid.sizes[0] instanceof Array)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

DFP allows sizes to be array and array of arrays. Support both.
parseSizesInput function https://github.com/prebid/Prebid.js/blob/master/src/utils.js#L117 will help

let returnEmptyBid = function() {
var bidsRequested = $$PREBID_GLOBAL$$._bidsRequested.find(bidSet => bidSet.bidderCode === BIDDER_CODE).bids;
if (bidsRequested.length > 0) {
let bid = bidfactory.createBid(BID_STATUS_CODE_EMPTY);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pass bidRequest as second param

let bidRequests = {};

let returnEmptyBid = function() {
var bidsRequested = $$PREBID_GLOBAL$$._bidsRequested.find(bidSet => bidSet.bidderCode === BIDDER_CODE).bids;
Copy link
Collaborator

Choose a reason for hiding this comment

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


beforeEach(() => {
bidsRequestedOriginal = pbjs._bidsRequested;
pbjs._bidsRequested = [];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use $$PREBID_GLOBAL$$ at all places in this file

@ShreeniwasIyer
Copy link
Contributor Author

@jaiminpanchal27 I fixed the code as per the comments above. We are also currently pushing an change to our domain handling to allow for testing from localhost on certain test cases, this one inclusive. I believe it should be done in the next 3 hours or so.

@ShreeniwasIyer
Copy link
Contributor Author

@jaiminpanchal27 All changes are done both in JS and server side and it seems we are good to go. Please review. (The last build is working from the PR perspective, but failing on some other test, which seems to pass sometimes and fail at other times. Please re-invoke the tests and I am confident they will pass.)

@jaiminpanchal27
Copy link
Collaborator

@ShreeniwasIyer I now see response but its not a valid bid. Here is the response i get.
{"bidderCode":"qcx","bids":[{"height":250,"placementCode":"div-gpt-ad-1438287399331-0","statusCode":0,"width":300}],"requestId":"280f6c64a34097"}

@ShreeniwasIyer
Copy link
Contributor Author

ShreeniwasIyer commented Apr 28, 2017

Hi @jaiminpanchal27 , thanks for your review comments. We have fixed a few issues we noticed along the way. Now our bidder is definitely responding with the bids and would do so, so long as you are testing this on localhost and you are doing this from an US IP.

Please note that we have updated the publisherId in the PR - so please use the revised one while testing.

Can you please review and let us know?

@jaiminpanchal27
Copy link
Collaborator

@ShreeniwasIyer I still see the same CORS error. I am testing from US ip.
Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at http://global.qc.rtb.quantserve.com:8080/qchb. (Reason: expected ‘true’ in CORS header ‘Access-Control-Allow-Credentials’). (unknown)

I am testing using this example page, https://jsfiddle.net/L618t79a/ Update prebid path before testing at your end.

Ajax request to your endpoint
curl 'http://global.qc.rtb.quantserve.com:8080/qchb' -H 'Cookie: mc=58f61b01-efd3a-4c41c-d26e7; d=EAUB4AEB4heBkwo9-Tk8LivvgwqLLx7NTB2MQsMAAAEPhSrqMLkgIgkgAAkQAAC-IADcMJiiKPURC6QAAAMNmGAQ6Vgri6KSuKwgDB7eEZ9Cn5LxnJVK1YLugfHI0YHeknji4f_bEO2i8f_ygpjH8ZzcIYsg' -H 'Origin: http://localhost:9999' -H 'Accept-Encoding: gzip, deflate' -H 'Accept-Language: en-US,en;q=0.8,gu;q=0.6' -H 'User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.81 Safari/537.36' -H 'Content-Type: text/plain' -H 'Accept: */*' -H 'Cache-Control: max-age=0' -H 'Referer: http://localhost:9999/integrationExamples/gpt/adapter/qcast.html' -H 'Connection: keep-alive' --data-binary '{"publisherId":"LCOf845vzU","requestId":"2c83a976870a59","bidId":"2c83a976870a59","site":{"page":"http://localhost:9999/integrationExamples/gpt/adapter/qcast.html","referrer":"http://localhost:9999/integrationExamples/gpt/adapter/qcast.html","domain":"localhost"},"imp":[{"banner":{"battr":[1,2],"sizes":[{"width":728,"height":90},{"width":300,"height":250},{"width":300,"height":600}]},"placementCode":"div-gpt-ad-1438287399331-0","bidFloor":1e-10}]}' --compressed

Let me know if i am doing something wrong here.
You can also reach out to me on jpanchal@appnexus.com

@ShreeniwasIyer
Copy link
Contributor Author

Thanks @jaiminpanchal27 for the review. Would you be merging it into the master when ready for next release? Do I need to do anything else?

@jaiminpanchal27
Copy link
Collaborator

jaiminpanchal27 commented May 2, 2017

@ShreeniwasIyer It looks good to me. No you don't need to do anything else from your side.
As per our process, any of the contributor from core team will check and give second LGTM and merge.

@protonate protonate requested a review from matthewlane May 2, 2017 18:19
@protonate protonate added this to the Prebid 0.23.0 milestone May 2, 2017
} catch(e) {
// Malformed JSON
utils.logError("Malformed JSON received from server - can't do anything here");
return;
Copy link
Member

Choose a reason for hiding this comment

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

Should really try to add a error bid here if possible

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mkendall07 I tried to, but at that point in code we don't have the placement code for which the empty text response came and hence returning an error bid isn't possible..

@protonate protonate merged commit 90e67f0 into prebid:master May 2, 2017
@jaiminpanchal27
Copy link
Collaborator

This is merged into master. Please submit a PR to the docs repo to add a file for your adapter to the bidders directory so your adapter's params will appear on the bidders page. Thank you for contributing

outoftime pushed a commit to Genius/Prebid.js that referenced this pull request May 4, 2017
…built

* 'master' of https://github.com/prebid/Prebid.js: (21 commits)
  add lodash as dependency (prebid#1174)
  fix size mapping for s2s (prebid#1175)
  Improve footer styling (prebid#1171)
  Bugfix: internal bids requested overwritten (prebid#1173)
  pre-release version bump
  Prebid 0.23.0 Release
  Yieldbot adapter - multiple requestBids per pageview (prebid#1146)
  Widespace adapter validate size fix (prebid#1140)
  Audience Network: bid when at least one valid slot size (prebid#1148)
  Quantcast adaptor (prebid#1063)
  AOL Adapter - ONE Mobile endpoint implemented. (prebid#1115)
  Prebid Server to Server (prebid#1165)
  Pubgears Header Bidding Adapter (prebid#953)
  remove old adloader#trackPixel (prebid#1159)
  added audit beacon to detect misuse of this bidder.  Detects auctions… (prebid#1134)
  Bidfluence CDN endpoint URL update (prebid#1163)
  AdSupply adapter (prebid#1162)
  Sonobi Adapter - Enable size overrides (prebid#1141)
  Added an editorconfig file to match jshint and jssrc files. (prebid#1147)
  force cpm to be a number (prebid#1161)
  ...
vzhukovsky added a commit to aol/Prebid.js that referenced this pull request May 25, 2017
…21.0 to aolgithub-master

* commit '4d573b42c5fbbabf23fed48fa042b75a72dd16b2': (31 commits)
  Added prebidServer in aolPartnersIds.json.
  Added adapters in aolPartnersIds.json.
  Added changelog entry.
  Prebid 0.23.1 Release
  Add trafficSourceCode + test (prebid#1184)
  Clear cookie sync to prevent multiple calls (prebid#1181)
  change s2s adapter to filter out units with empty sizes array (prebid#1179)
  Sharethrough: Change to using a closure for the callback from ajax to preserve bidObj (prebid#1108)
  check array length when mapping sizes (prebid#1180)
  Bugfix/encoding url (prebid#1178)
  add lodash as dependency (prebid#1174)
  fix size mapping for s2s (prebid#1175)
  Improve footer styling (prebid#1171)
  Bugfix: internal bids requested overwritten (prebid#1173)
  pre-release version bump
  Prebid 0.23.0 Release
  Yieldbot adapter - multiple requestBids per pageview (prebid#1146)
  Widespace adapter validate size fix (prebid#1140)
  Audience Network: bid when at least one valid slot size (prebid#1148)
  Quantcast adaptor (prebid#1063)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants