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

Add Sortable bid adapter #4907

Merged
merged 7 commits into from Mar 5, 2020
Merged

Add Sortable bid adapter #4907

merged 7 commits into from Mar 5, 2020

Conversation

shannonAB
Copy link
Contributor

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
  • Does this change affect user-facing APIs or examples documented on http://prebid.org?
  • Other

Description of change

Add the Sortable bidder adapter.

  • test parameters for validating bids
var adUnits = [
  {
    code: 'test-pb-leaderboard',
    sizes: [[728, 90]],
    bids: [{
      bidder: 'sortable',
      params: {
        tagId: 'test-pb-leaderboard',
        siteId: 'prebid.example.com',
        'keywords': {
          'key1': 'val1',
          'key2': 'val2'
        }
      }
    }]
  }, {
    code: 'test-pb-banner',
    sizes: [[300, 250]],
    bids: [{
      bidder: 'sortable',
      params: {
        tagId: 'test-pb-banner',
        siteId: 'prebid.example.com'
      }
    }]
  }, {
    code: 'test-pb-sidebar',
    size: [[160, 600]],
    bids: [{
      bidder: 'sortable',
      params: {
        tagId: 'test-pb-sidebar',
        siteId: 'prebid.example.com',
        'keywords': {
          'keyA': 'valA'
        }
      }
    }]
  }
]
  • contact email of the adapter’s maintainer: [email protected]
  • official adapter submission

For any changes that affect user-facing APIs or example code documented on http://prebid.org, please provide:

Copy link
Contributor

@Fawke Fawke left a comment

Choose a reason for hiding this comment

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

Hi @shannonAB,

Thanks for the PR!
Having gone through your code and having tested it, here are my review comments:

  1. Your SERVER_URL is not using https. It's a strict requirement that Prebid.js v3.0.0 onwards all adapters use a secure endpoint.

  2. The examples given in your markdown file is not accurate. Again, with Prebid.js v3.0.0 onwards, the property sizes in adUnit object is no longer supported. Instead, you should use the mediaTypes property. For example, in case of a banner ad, you should use,

mediaTypes: {
  banner: {
    sizes: [[728, 90]]
  }
}

Details about the Prebid.js v3.0.0 release.

  1. Am getting a 404 BAD REQUEST on sending a request to your endpoint. I tested with the hello world page

I've attached a screenshot for your reference.
Screenshot 2020-03-02 at 12 41 33 PM

- use https for URL
- fix examples in markdown
- request to endpoint should work now
@Fawke
Copy link
Contributor

Fawke commented Mar 3, 2020

@shannonAB,

Thanks for making the changes. I was expecting one more thing in your md file, since you support native and video formats as well, can you provide some examples of those. It'll be easier for us to test that way.

@Fawke
Copy link
Contributor

Fawke commented Mar 5, 2020

LGTM!

@Fawke Fawke merged commit 5794bdc into prebid:master Mar 5, 2020
rjvelicaria pushed a commit to openx/Prebid.js that referenced this pull request Apr 9, 2020
* Add Sortable adapter for Prebid 3.x

Update tests to reflect changes.

* Add .js in imports

* hostname not host: don't include port

* Trivial change to trigger build: failure wasn't our adapter

* More failures in other adapters

* PR Feedback

- use https for URL
- fix examples in markdown
- request to endpoint should work now

* Feedback: add native and video examples
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants