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

Serverbid 1.0 #1802

Merged
merged 6 commits into from
Dec 4, 2017
Merged

Serverbid 1.0 #1802

merged 6 commits into from
Dec 4, 2017

Conversation

jcswart
Copy link
Contributor

@jcswart jcswart commented Nov 3, 2017

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

Refactor ServerBid adapter to conform to new 1.0 spec for bidders.

Other information

*/

buildRequests: function(validBidRequests) {
// Do we need to group by bidder? i.e. to make multiple requests for
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what is meant by grouping by bidder, but multiple requests would be required for different endpoints.

Copy link
Contributor

Choose a reason for hiding this comment

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

@idettman That answers this question, thanks! We aren't going to make a change for this now since it isn't a scenario that has come up yet, but will add in support for that at some point.

@ndhimehta ndhimehta assigned idettman and unassigned ndhimehta Nov 15, 2017
@jgrimes
Copy link
Contributor

jgrimes commented Nov 21, 2017

Hi @idettman
I just want to make sure that this PR is going to go in the next Prebid release. Is it go to go?

bidder: 'serverbid',
params: {
networkId: '9969',
siteId: '45678'
Copy link
Collaborator

@matthewlane matthewlane Nov 22, 2017

Choose a reason for hiding this comment

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

Are these params still valid for testing bid responses? Currently getting a 400 Bad Request from your endpoint with the message Out of 0 placements on the request, none were valid. I use these on the test page to make sure your interpretResponse function is working correctly. That function may require updating due to #1748

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, those aren't valid. But I just updated this PR with ones that are.

@@ -1,5 +1,5 @@
import { expect } from 'chai';
import Adapter from 'modules/serverbidBidAdapter';
import { spec } from 'modules/serverbidBidAdapter';
import bidmanager from 'src/bidmanager';
import * as utils from 'src/utils';
Copy link
Collaborator

Choose a reason for hiding this comment

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

bidmanager and utils aren't used in tests, both imports can be dropped

* @param {*} serverResponse A successful response from the server.
* @return {Bid[]} An array of bids which were nested inside the server.
*/
interpretResponse: function(serverResponse, bidRequest) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

#1748 changed the first argument of interpretResponse to:

{
  body: responseBody,
  headers: {
    get: function(header) { /* returns a header from the HTTP response */ }
  }
}

so adding something like

serverResponse = serverResponse.body;

just below this line, or however you'd prefer to grab the body, and updating corresponding tests if needed should get this back to working properly

bid = {};
bidObj = bids[i];
bidId = bidObj.bidId;
bid.bidderCode = bidObj.bidder;
Copy link
Collaborator

Choose a reason for hiding this comment

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

bidderCode will be set automatically by bidderFactory now, this line can be dropped

@mkendall07
Copy link
Member

@jgrimes @jcswart
Need a few updates before merge. Thanks

@jcswart
Copy link
Contributor Author

jcswart commented Dec 2, 2017

@mkendall07 I've made the updates. I would love to get this merged along with prebid/prebid.github.io#470 as soon as possible. Thank you!

@matthewlane matthewlane merged commit 1614a66 into prebid:master Dec 4, 2017
Millerrok pushed a commit to Vertamedia/Prebid.js that referenced this pull request Dec 8, 2017
* 'master' of https://github.com/prebid/Prebid.js: (43 commits)
  Merge Prebid 1.0 to Master (prebid#1936)
  Prebid.js 0.34.1 release
  Vertamedia adapter outstream support (prebid#1860)
  Expose native image-type asset dimensions on bid response object (prebid#1919)
  Remove for of (prebid#1932)
  Unit-test fix (prebid#1927)
  Remove description_url (prebid#1922)
  Trion Interactive Adapter Bugfix (prebid#1925)
  Remove config setting from Optimatic adapter (prebid#1909)
  IE bug fix (prebid#1930)
  Clarify ad unit media filtering warning (prebid#1903)
  Add ReadPeak Bid Adapter (prebid#1838)
  Change clone function to make deep copies (prebid#1910)
  Serverbid 1.0 (prebid#1802)
  sekindoUM for prebid1.0 (prebid#1777)
  update auctionId to be requestId (prebid#1896)
  bug fixed to populate userSync default values (prebid#1897)
  Increment pre version
  AdkernelAdn analytics adapter (prebid#1868)
  Justpremium Adapter: use `filter` instead of `...new Set` (prebid#1895)
  ...
vzhukovsky added a commit to aol/Prebid.js that referenced this pull request Dec 28, 2017
….34.0 to aolgithub-master

* commit 'f0ba90afa8b52de7a646d43928b2d51ee42e74a1': (53 commits)
  Added changelog entry.
  Added partners ids.
  Added dynamic ttl property for One Display and One Mobile.
  Prebid.js 0.34.1 release
  Vertamedia adapter outstream support (prebid#1860)
  Expose native image-type asset dimensions on bid response object (prebid#1919)
  Remove for of (prebid#1932)
  Unit-test fix (prebid#1927)
  Remove description_url (prebid#1922)
  Trion Interactive Adapter Bugfix (prebid#1925)
  Remove config setting from Optimatic adapter (prebid#1909)
  IE bug fix (prebid#1930)
  Clarify ad unit media filtering warning (prebid#1903)
  Add ReadPeak Bid Adapter (prebid#1838)
  Change clone function to make deep copies (prebid#1910)
  Serverbid 1.0 (prebid#1802)
  sekindoUM for prebid1.0 (prebid#1777)
  update auctionId to be requestId (prebid#1896)
  bug fixed to populate userSync default values (prebid#1897)
  Increment pre version
  ...
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.

8 participants