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

Jump to bottom

Add ReadPeak Bid Adapter #1838

Merged
merged 9 commits into from
Dec 4, 2017
Merged

Add ReadPeak Bid Adapter #1838

merged 9 commits into from
Dec 4, 2017

Conversation

kurrestahlberg
Copy link
Contributor

@kurrestahlberg kurrestahlberg commented Nov 15, 2017 •
edited
Loading

Type of change

  • New bidder adapter

Description of change

Bid adapter for readpeak.com, a purely native bidder from Finland

  • test parameters for validating bids
{
  bidder: 'readpeak',
  params: {
     bidfloor: 5.00,
     publisherId: 'test'
   },
}

Be sure to test the integration with your adserver using the Hello World sample page.

Other information

@kurrestahlberg kurrestahlberg mentioned this pull request Nov 16, 2017
Merged
mkendall07
mkendall07 reviewed Nov 27, 2017
View reviewed changes
modules/readpeakBidAdapter.js Outdated
const bid = {
requestId: id,
cpm: idToBidMap[id].price,
creativeId: id,
Copy link
Member

Choose a reason for hiding this comment

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

Is the creativeId really just the request id? Doesn't seem correct.

modules/readpeakBidAdapter.js Outdated
currency: bidResponse.cur,
bidderCode: BIDDER_CODE,
};
bid['native'] = nativeResponse(idToImpMap[id], idToBidMap[id]);
Copy link
Member

Choose a reason for hiding this comment

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

no need to use bracket notation here.

mkendall07
mkendall07 reviewed Nov 27, 2017
View reviewed changes
modules/readpeakBidAdapter.js Outdated
requestId: id,
cpm: idToBidMap[id].price,
creativeId: id,
adId: id,
Copy link
Member

Choose a reason for hiding this comment

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

adId can be dropped. I forgot this was the legacy bid identifier. It will be found on the requestId now

@mkendall07
Copy link
Member

@kurrestahlberg
Thanks for the submission! Needs a few minor updates.

Also it looks like I'm not receiving bid response. Here is the request generated to assist with testing:

{"id":"35bb8c9a63e32c","imp":[{"id":"486443e023cc9e","native":{"request":"{\"assets\":[{\"id\":1,\"required\":1,\"title\":{\"len\":70}},{\"id\":4,\"required\":1,\"data\":{\"type\":2,\"len\":120}}]}","ver":"1.1"},"bidfloor":5,"bidfloorcur":"USD"}],"site":{"publisher":{"id":"test"},"id":"test","ref":"","page":"http://ap.localhost:9999/integrationExamples/native_test.html","domain":"ap.localhost"},"app":null,"device":{"ua":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3202.94 Safari/537.36","language":"en-US"},"isPrebid":true}

@kurrestahlberg
Copy link
Contributor Author

Thanks for the review! I have fixed the issues you pointed out. We seem to have a minor configuration issue in our backend right now and thus it returns 204 for the test publisher id. I will get it fixed asap.

@kurrestahlberg
Copy link
Contributor Author

@mkendall07
Everything should be in order again. Please retest.

mkendall07
mkendall07 reviewed Nov 29, 2017
View reviewed changes
modules/readpeakBidAdapter.js Outdated
netRevenue: true,
mediaType: 'native',
currency: bidResponse.cur,
bidderCode: BIDDER_CODE,
Copy link
Member

Choose a reason for hiding this comment

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

please drop bidder code. This will be added automatically.

@mkendall07
Copy link
Member

@kurrestahlberg
if you make those changes quick I'll retest

@mkendall07
Copy link
Member

seeing bids now. Just need bidderCode dropped and we are good to merge

@kurrestahlberg
Copy link
Contributor Author

kurrestahlberg commented Nov 29, 2017 •
edited
Loading

@mkendall07
Should be okay now even though it fails with OOM. Sorry for the extra commits, the tests run so slow on my laptop that I always push while they are running...

@kurrestahlberg
Copy link
Contributor Author

I don't know how to trigger rerun for travis.

@mkendall07
Copy link
Member

restarted the build

@bretg bretg added the ready label Nov 29, 2017
@mkendall07
Copy link
Member

@kurrestahlberg
I'm seeing bid returned but our test creative isn't displaying. I haven't had a chance to dig further into it. Do you have a test page that is working?

@kurrestahlberg
Copy link
Contributor Author

@mkendall07
I'm having trouble getting anything to show here, but I can see that the native parts are sent and they look correct to me.

The following is sent as prev_scp:

hb_native_linkurl=https%253A%252F%252Fapp.readpeak.com%252Fads%252Ftracker%252F528%252F4_askelta_vaikuttavaan_natiivimainontaan%253Fl%253D95%2526a%253D3149%2526ka%253D1534%2526avn%253D1%2526b%253D487b0c08-74e5-49d4-8e48-9ad46630f622&hb_native_cta=&hb_native_image=https%3A%2F%2Fs3.eu-central-1.amazonaws.com%2Freadpeak%2Ffiles%2Farticle_thumbs%2Fthumb-408-c38808e3de9412fde8ce40b7a652f8cb.jpg&hb_native_brand=&hb_native_body=ReadPeak-k%C3%A4ytt%C3%B6liittym%C3%A4%20on%20kevyt%20ja%20ketter%C3%A4%20natiivimainonnan%20ty%C3%B6kalu%20jonka%20kautta%20tavoitat%20aiheistasi%20kiinnostun...&hb_native_title=4%20askelta%20vaikuttavaan%20natiivimainontaan&hb_size=0x0&hb_pb=5.80&hb_adid=24c0f8e5758435&hb_bidder=readpeak

The only odd bit to me is hb_size, and I am not sure if the regular test page will show both banners and native.

@mkendall07
Copy link
Member

Ok figured it out - I wasn't overidding the bid CPM and our native placement only responds to nice round numbers...

anyway LGTM:
image

@jaiminpanchal27 jaiminpanchal27 merged commit 05e4853 into prebid:master Dec 4, 2017
Millerrok pushed a commit to Vertamedia/Prebid.js that referenced this pull request Dec 8, 2017
Merge branch 'master' of https://github.com/prebid/Prebid.js
4273734 
* 'master' of https://github.com/prebid/Prebid.js: (43 commits)
  Merge Prebid 1.0 to Master (prebid#1936)
  Prebid.js 0.34.1 release
  Vertamedia adapter outstream support (prebid#1860)
  Expose native image-type asset dimensions on bid response object (prebid#1919)
  Remove for of (prebid#1932)
  Unit-test fix (prebid#1927)
  Remove description_url (prebid#1922)
  Trion Interactive Adapter Bugfix (prebid#1925)
  Remove config setting from Optimatic adapter (prebid#1909)
  IE bug fix (prebid#1930)
  Clarify ad unit media filtering warning (prebid#1903)
  Add ReadPeak Bid Adapter (prebid#1838)
  Change clone function to make deep copies (prebid#1910)
  Serverbid 1.0 (prebid#1802)
  sekindoUM for prebid1.0 (prebid#1777)
  update auctionId to be requestId (prebid#1896)
  bug fixed to populate userSync default values (prebid#1897)
  Increment pre version
  AdkernelAdn analytics adapter (prebid#1868)
  Justpremium Adapter: use `filter` instead of `...new Set` (prebid#1895)
  ...
vzhukovsky added a commit to aol/Prebid.js that referenced this pull request Dec 28, 2017
@vzhukovsky
Merge pull request prebid#134 in AOLP_ADS_JS/prebid.js from release/1…
b736fae 
….34.0 to aolgithub-master

* commit 'f0ba90afa8b52de7a646d43928b2d51ee42e74a1': (53 commits)
  Added changelog entry.
  Added partners ids.
  Added dynamic ttl property for One Display and One Mobile.
  Prebid.js 0.34.1 release
  Vertamedia adapter outstream support (prebid#1860)
  Expose native image-type asset dimensions on bid response object (prebid#1919)
  Remove for of (prebid#1932)
  Unit-test fix (prebid#1927)
  Remove description_url (prebid#1922)
  Trion Interactive Adapter Bugfix (prebid#1925)
  Remove config setting from Optimatic adapter (prebid#1909)
  IE bug fix (prebid#1930)
  Clarify ad unit media filtering warning (prebid#1903)
  Add ReadPeak Bid Adapter (prebid#1838)
  Change clone function to make deep copies (prebid#1910)
  Serverbid 1.0 (prebid#1802)
  sekindoUM for prebid1.0 (prebid#1777)
  update auctionId to be requestId (prebid#1896)
  bug fixed to populate userSync default values (prebid#1897)
  Increment pre version
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mkendall07 mkendall07 mkendall07 left review comments

@jaiminpanchal27 jaiminpanchal27 jaiminpanchal27 approved these changes

Assignees

@matthewlane matthewlane

@mkendall07 mkendall07

Labels
LGTM Prebid 1.0 ready
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kurrestahlberg @mkendall07 @jaiminpanchal27 @matthewlane @bretg