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

Legacy Sortable bid adapter #2915

Merged
merged 2 commits into from Aug 8, 2018
Merged

Legacy Sortable bid adapter #2915

merged 2 commits into from Aug 8, 2018

Conversation

edmonl
Copy link
Contributor

@edmonl edmonl commented Jul 31, 2018

Type of change

  • [x ] New bidder adapter

Description of change

The PR backporst Sortable bid adapter to legacy. GDPR support is removed and utils.isPlainObject(...) is replaced with utils.isA(..., 'Object').

  • 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'
        }
      }
    }]
  }
];

* copy over sortable adapter as well as its spce test and md
* fix missing utils.isPlainObject
Copy link
Contributor

@idettman idettman left a comment

Choose a reason for hiding this comment

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

LGTM. Verified test parameters from sortableBidAdapter.md, received valid results from the /openrtb2/auction endpoint.

},

onTimeout(details) {
fetch(`//${SERVER_URL}/prebid/timeout`, {
Copy link
Contributor

Choose a reason for hiding this comment

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

fetch isn't supported by Internet Explorer 10 or 11, and prebid supports IE 10+: https://caniuse.com/#search=fetch

The prebid ajax function should work as a substitute. If you search the adapters codebase, you'll find usage examples.

Copy link
Contributor

@idettman idettman left a comment

Choose a reason for hiding this comment

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

Remove usages of the 'fetch' api (fetch is not supported by Internet Explorer 10 - 11)

@edmonl
Copy link
Contributor Author

edmonl commented Aug 7, 2018

@idettman I removed fetch and used ajax instead. Checks failed but seem not relevant to this change?

Copy link
Contributor

@idettman idettman left a comment

Choose a reason for hiding this comment

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

LGTM, Note: the initial circle test failed with an error not related to this adapter, but I ran circleci again and it resolved :)

@idettman idettman merged commit fed6a3f into prebid:legacy Aug 8, 2018
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