Skip to content

New imonomy Bid Adapter#1345

Merged
jaiminpanchal27 merged 9 commits intoprebid:masterfrom
ofir1122:patch-1
Aug 2, 2017
Merged

New imonomy Bid Adapter#1345
jaiminpanchal27 merged 9 commits intoprebid:masterfrom
ofir1122:patch-1

Conversation

@ofir1122
Copy link
Contributor

@ofir1122 ofir1122 commented Jul 5, 2017

Adding support of imonomy prebid

test parameters for validating bids

  {
    bidder: 'imonomy',
    placementCode: 'foo',
    bidId: 'foo',
    sizes: [[300, 250]],
    params: {
      publisher_id: '14567721164',
    }
  }

contact email of the adapter’s maintainer
tech@imonomy.com
amit@imonmy.com
[X ] official adapter submission

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: 'imonomy',
  params: {
           publisher_id: '14567721164'
  }
}

Other information

Adding support of imonomy prebid

test parameters for validating bids
{
  bidder: 'imonomy',
  params: {
           publisher_id: '14567721164'
  }
}
contact email of the adapter’s maintainer
tech@imonomy.com
amit@imonmy.com
[X ] official adapter submission
@dbemiller
Copy link
Contributor

Per the project standards, this needs unit tests.

@ofir1122
Copy link
Contributor Author

@dbemiller Thank you for your comment. I added the unit tests.

Copy link
Contributor

@dbemiller dbemiller left a comment

Choose a reason for hiding this comment

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

I get no bids back with your test params.

One bug is that you're not registering your bid adapter. Check out some of the other adapter files for example:

adaptermanager.registerBidAdapter(new HiroMediaAdapter, 'hiromedia');

There may be others after that... but that's at least one.

@@ -0,0 +1,111 @@
/* jshint -W030 */
Copy link
Contributor

Choose a reason for hiding this comment

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

What's up with this? We don't use jshint...


function _callBids(params) {
var request = [];
var siteRef = "";
Copy link
Contributor

Choose a reason for hiding this comment

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

A recently merged PR fixed a bug which was preventing adapters from being linted.

This line (along with many other things in this file) will fail the linting rules... so you'll have to pull the master branch again and fix the errors before we can merge it.

request.push(formRequestUrl(bid.params));
}

$$PREBID_GLOBAL$$[callbackName] = handleCallback(bids);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just so you know: JSONP isn't going to be allowed in Prebid 1.0 (most likely coming in Q4 of this year). It's fine if you want to build your adapter this way now, but just be aware that you'll probably have to refactor to stay in the project long-term.

return null;
}

function encode64(input) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you just re-implementing atob here? Or am I missing something?

Denis Rogov added 2 commits July 26, 2017 20:14
+ fixed all lint isses on adapter
+ replace encode64 with btoa
+ add some more unit tests
@ofir1122
Copy link
Contributor Author

ofir1122 commented Jul 26, 2017

@dbemiller Thank you for the feedback.
I fix all comments and updated the test parameters to make sure you will get a bid.
About JSONP we will rewrite the adapter before Q4 (thanks for the notice)

@dbemiller
Copy link
Contributor

I'm seeing a bid now, but no ad in the page.

The integrationExamples/gpt/hello_world.hmtl page exists to help test bid adapters end-to-end. Replace the AppNexus bid with your test params, run gulp serve, and then use that page to help debug.

@ofir1122
Copy link
Contributor Author

@dbemiller hi seems that the bid response was under 10 cent. I configured it and now it works.

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.

LGTM

@jaiminpanchal27 jaiminpanchal27 merged commit a261fb3 into prebid:master Aug 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

philipwatson pushed a commit to mbrtargeting/Prebid.js that referenced this pull request Sep 18, 2017
* New imonomy Bid Adapter

Adding support of imonomy prebid

test parameters for validating bids
{
  bidder: 'imonomy',
  params: {
           publisher_id: '14567721164'
  }
}
contact email of the adapter’s maintainer
tech@imonomy.com
amit@imonmy.com
[X ] official adapter submission

* identetion fixes

fix warning

* add imonomyBidAdapter unit testing

* Fix File Location
+Fix File references

* indentaion on unit test

* more identation fixes

* no-trailing-spaces

* remove jshint
+ fixed all lint isses on adapter
+ replace encode64 with btoa
+ add some more unit tests

* fix - A space is required after ','  comma-spacing
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.

4 participants

Comments