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 #2824

Merged
merged 5 commits into from Jul 11, 2018
Merged

Add Sortable bid adapter #2824

merged 5 commits into from Jul 11, 2018

Conversation

shannonAB
Copy link
Contributor

@shannonAB shannonAB commented Jul 6, 2018

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: 1,
        'keywords': {
          'key1': 'val1',
          'key2': 'val2'
        }
      }
    }]
  }, {
    code: 'test-pb-banner',
    sizes: [[300, 250]],
    bids: [{
      bidder: 'sortable',
      params: {
        tagId: 'test-pb-banner',
        siteId: 1
      }
    }]
  }, {
    code: 'test-pb-sidebar',
    size: [[160, 600]],
    bids: [{
      bidder: 'sortable',
      params: {
        tagId: 'test-pb-sidebar',
        siteId: 1,
        'keywords': {
          'keyA': 'valA'
        }
      }
    }]
  }
]

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

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

Other information

- [x] New bidder adapter

This change adds the Sortable adapter to Prebid.js.
Copy link
Collaborator

@jsnellbaker jsnellbaker 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 submitting this adapter. Overall, everything seems good. There is a unit test that should be reviewed as well as a potential update based on the feedback that was made in the related docs PR.

Once these items are addressed; we should be good to merge.


it('sends screen dimensions', () => {
expect(requestBody.site.device.w).to.equal(800);
expect(requestBody.site.device.h).to.equal(600);
Copy link
Collaborator

Choose a reason for hiding this comment

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

These checks are failing when I run gulp test locally. Could you look into a different approach than hard-coded values?

supportedMediaTypes: [BANNER],

isBidRequestValid: function(bid) {
const haveSiteId = !!config.getConfig('sortableId');
Copy link
Collaborator

Choose a reason for hiding this comment

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

In lieu of the feedback provided on the docs PR (prebid/prebid.github.io#872); this may need to be updated.

@bgahagan
Copy link
Contributor

Hi @jsnellbaker , I'll be helping out while Shannon is on vacation for the next few days. I've made the requested changes, let me know what you think.

@jsnellbaker
Copy link
Collaborator

Hi @bgahagan,

Thanks for making the updates. Those changes look good; however when I went to retest the adapter, I'm now getting a Bad Request response from the server.

The only error it's providing is unknown site. I believe this was working before, could you take a look?

Below is a copy of the request object:

{"id":"3d706ce860fa1e","imp":[{"id":"2185af434aa992","tagid":"test-pb-banner","banner":{"format":[{"w":300,"h":250},{"w":300,"h":600}]},"ext":{}}],"site":{"domain":"ap.localhost:9999","page":"http://ap.localhost:9999/integrationExamples/gpt/hello_world.html?pbjs_debug=true","ref":"http://ap.localhost:9999/integrationExamples/gpt/hello_world.html?pbjs_debug=true","publisher":{"id":"example.com"},"device":{"w":1680,"h":1050}}}

@bgahagan
Copy link
Contributor

Hi @jsnellbaker , apologies, I when I updated the siteIds to be strings, I used the wrong value. Instead of example.com it should be prebid.example.com to get a test bid. I updated the examples.

Copy link
Collaborator

@jsnellbaker jsnellbaker left a comment

Choose a reason for hiding this comment

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

Thanks for making the updates; the test deliveries are working fine now.

@jsnellbaker jsnellbaker merged commit 4ad1a00 into prebid:master Jul 11, 2018
@jsnellbaker
Copy link
Collaborator

@bgahagan & @shannonAB
FYI #2837

florevallatmrf pushed a commit to Marfeel/Prebid.js that referenced this pull request Sep 6, 2018
* Add Sortable bid adapter

- [x] New bidder adapter

This change adds the Sortable adapter to Prebid.js.

* move global sortable config into its own object

* update siteId

* config can be undefined when module loads

* support floor param
StefanWallin pushed a commit to mittmedia/Prebid.js that referenced this pull request Sep 28, 2018
* Add Sortable bid adapter

- [x] New bidder adapter

This change adds the Sortable adapter to Prebid.js.

* move global sortable config into its own object

* update siteId

* config can be undefined when module loads

* support floor param
ghost pushed a commit to devunrulymedia/Prebid.js that referenced this pull request Jan 30, 2019
* Add Sortable bid adapter

- [x] New bidder adapter

This change adds the Sortable adapter to Prebid.js.

* move global sortable config into its own object

* update siteId

* config can be undefined when module loads

* support floor param
AlessandroDG pushed a commit to simplaex/Prebid.js that referenced this pull request Mar 26, 2019
* Add Sortable bid adapter

- [x] New bidder adapter

This change adds the Sortable adapter to Prebid.js.

* move global sortable config into its own object

* update siteId

* config can be undefined when module loads

* support floor param
AlessandroDG pushed a commit to simplaex/Prebid.js that referenced this pull request Mar 26, 2019
* Add Sortable bid adapter

- [x] New bidder adapter

This change adds the Sortable adapter to Prebid.js.

* move global sortable config into its own object

* update siteId

* config can be undefined when module loads

* support floor param
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants