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

nanointeractive bid adapter #1627

Merged
merged 6 commits into from
Oct 26, 2017
Merged

nanointeractive bid adapter #1627

merged 6 commits into from
Oct 26, 2017

Conversation

rade-popovic
Copy link
Contributor

@rade-popovic rade-popovic commented Sep 27, 2017 •
edited
Loading

Type of change

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

https://github.com/nanointeractive/prebid.github.io/blob/2a57cea7d15b3e3a69b24db288d8e3364c3996b5/dev-docs/bidders/nanointeractive.md

Description of change

New bid adapter

  • test parameters for validating bids
    {
    bidder: 'nanointeractive',
    params: {
    sec: '04a0cb7fb9ac02840f7f33d68a883780',
    dpid: '58bfec94eb0a1916fa380162',
    pid: '58bfec94eb0a1916fa380163'
    }
    }

  • contact email of the adapter’s maintainer
    rade@nanointeractive.com

@mkendall07
Copy link
Member

hi @nanointeractive
We have recently updated the format for adapters. Please submit the adapter in the form specified here http://prebid.org/dev-docs/bidder-adapter-1.html

You can see an example of the new format implemented here: https://github.com/prebid/Prebid.js/blob/master/modules/appnexusAstBidAdapter.js

Thanks!

@mkendall07 mkendall07 self-assigned this Oct 17, 2017
@rade-popovic
Copy link
Contributor Author

Dear Matt, can you please give us an info when can we expect approval of the pull request. If there is anything from our side we can do to fasten the process please let us know.

@mkendall07
Copy link
Member

@nanointeractive
Thanks for the update. I didn't see the most recent commits. I'll review this week. Thanks!

@rade-popovic
Copy link
Contributor Author

Great, thanks !

mkendall07
mkendall07 requested changes Oct 24, 2017
View reviewed changes
Copy link
Member

@mkendall07 mkendall07 left a comment

Choose a reason for hiding this comment

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

@nanointeractive
Few comments to address. I'm not seeing bids returned for a 300x250 ad unit with the included bid params. Please advise.

Also please include a markdown file with you bid params example. Thanks

modules/nanointeractiveBidAdapter.js Outdated
function createSingleBidResponse(serverBid) {
return {
requestId: serverBid.id,
bidderCode: serverBid.bidderCode,
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 bidderCode here - we add it automatically - this was an error in our docs sorry.

modules/nanointeractiveBidAdapter.js Outdated
return bid.params[CATEGORY] || null;
}

function getQueryParam(nq) {
Copy link
Member

Choose a reason for hiding this comment

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

I think you can use utils. getParameterByName() here:
https://github.com/prebid/Prebid.js/blob/master/src/utils.js#L257

@rade-popovic
- using utils.getParameterByName instead of utils.getTopWindowLocatio…
acf178f 
…n().href

- bidderCode is removed
- Default ALG changed to 'ihr'
- added protocol to 'cors' param
@rade-popovic
Copy link
Contributor Author

@mkendall07
Thanks for the overview and the help.
We deployed the requested changes.

I've also changed params for the bid validation, please use this one:
{
bidder: 'nanointeractive',
params: {
sec: '04a0cb7fb9ac02840f7f33d68a883780',
dpid: '58bfec94eb0a1916fa380162',
pid: '58bfec94eb0a1916fa380163'
}
}

Cheers

@mkendall07
Copy link
Member

@nanointeractive
thanks. Can you add the markdown file as well? see here for example: https://github.com/prebid/Prebid.js/blob/master/modules/appnexusAstBidAdapter.md

@rade-popovic
Copy link
Contributor Author

thanks, done: https://github.com/nanointeractive/Prebid.js/blob/master/modules/nanointeractiveBidAdapter.md

@mkendall07
Copy link
Member

@nanointeractive
getting a CORS error now on your endpoint: Failed to load http://tmp.audiencemanager.de/hb: The value of the 'Access-Control-Allow-Origin' header in the response must not be the wildcard '*' when the request's credentials mode is 'include'. Origin 'http://localhost:9999' is therefore not allowed access. The credentials mode of requests initiated by the XMLHttpRequest is controlled by the withCredentials attribute.

@rade-popovic
Copy link
Contributor Author

strange, I know the issue but we solved it by changing dynamically 'Access-Control-Allow-Origin' for each bid request from location.origin value. Can you please send me the Response headers and Request Payload.

@mkendall07
Copy link
Member

It's because of Access-Control-Allow-Origin: * instead of Access-Control-Allow-Origin: <domain>

Date: Wed, 25 Oct 2017 16:50:10 GMT
Server: Apache/2.4.27 (Amazon) PHP/5.6.25
X-Powered-By: PHP/5.6.25
Status: 200 OK
RequestId: e082476220652d6695d50c78b790b634
Hostname: 10-75-1-58#slave3
P3P: CP="NID DSP ALL COR"
Access-Control-Allow-Origin: *
responseTime: 90
Content-Length: 835
Keep-Alive: timeout=5, max=100
Connection: Keep-Alive
Content-Type: application/x-javascript```

@rade-popovic
Copy link
Contributor Author

mystery solved, we disabled localhost for bid requests. I have enabled localhost, now everything should be fine

mkendall07
mkendall07 approved these changes Oct 26, 2017
View reviewed changes
Copy link
Member

@mkendall07 mkendall07 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@mkendall07 mkendall07 merged commit 698c330 into prebid:master Oct 26, 2017
Millerrok pushed a commit to Vertamedia/Prebid.js that referenced this pull request Oct 31, 2017
Merge branch 'master' of https://github.com/prebid/Prebid.js
e45235f 
* 'master' of https://github.com/prebid/Prebid.js: (22 commits)
  Update GetIntent adapter to 1.0 version (prebid#1721)
  Add `usePaymentRule` param to AN bidders (prebid#1778)
  New hooks API (replaces monkey-patching for currency) (prebid#1683)
  Change prebidServer to call client user syncs if they exist (prebid#1734)
  Fix Centro adapter to allow requests of the same units (prebid#1746)
  add vastUrl + media type for video bids Prebid Server (prebid#1739)
  Update adxcg adapter for prebid 1.0 (prebid#1741)
  Update yieldmoBid adapter request url (prebid#1771)
  Upgrade Quantcast adapter for Prebid 1.0 (prebid#1753)
  Fidelity Media Adapter update. Prebid v1.0 (prebid#1719)
  Kargo Adapter for Prebid 1.0 (prebid#1729)
  updated for prebid 1.0 api (prebid#1722)
  Add AdOcean adapter (prebid#1735)
  Update Conversant adapter to Prebid 1.0 (prebid#1711)
  Fix test-coverage bug (prebid#1765)
  Migrating TrustX adapter to 1.0 (prebid#1709)
  Update Improve Digital adapter for Prebid 1.0 (prebid#1728)
  Fixed the argument type on getUserSyncs. (prebid#1767)
  nanointeractive bid adapter (prebid#1627)
  Validating bid response params (prebid#1738)
  ...
mattpr pushed a commit to mattpr/Prebid.js that referenced this pull request Oct 31, 2017
@rade-popovic @mattpr
nanointeractive bid adapter (prebid#1627)
09ce1b3 
* nanointeractive bid adapter

* nanointeractive bid adapter

* - using utils.getParameterByName instead of utils.getTopWindowLocation().href
- bidderCode is removed
- Default ALG changed to 'ihr'
- added protocol to 'cors' param

* markdown file

* enabling localhost for bid requests
@rade-popovic rade-popovic mentioned this pull request Apr 23, 2018
Merged
dluxemburg pushed a commit to Genius/Prebid.js that referenced this pull request Jul 17, 2018
@rade-popovic @dluxemburg
nanointeractive bid adapter (prebid#1627)
9e257b4 
* nanointeractive bid adapter

* nanointeractive bid adapter

* - using utils.getParameterByName instead of utils.getTopWindowLocation().href
- bidderCode is removed
- Default ALG changed to 'ihr'
- added protocol to 'cors' param

* markdown file

* enabling localhost for bid requests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mkendall07 mkendall07 mkendall07 approved these changes

Assignees

@mkendall07 mkendall07

Labels
needs update not receiving bids
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rade-popovic @mkendall07 @matthewlane