MobfoxBidAdapter compliant to prebid1.0 #1757
Conversation
There was a problem hiding this comment.
Left some comments.
Please add markdown file containing key information about the adapter
Template can be found here: http://prebid.org/dev-docs/bidder-adapter-1.html
modules/mobfoxBidAdapter.js
Outdated
| let bidRequestData = bidRequest.data.split('&'); | ||
| const bidResponse = { | ||
| requestId: bidRequest.requestId, | ||
| bidderCode: BIDDER_CODE, |
There was a problem hiding this comment.
bidderCode can be removed, it will be added by bidderFactory.
| }; | ||
| }, | ||
| interpretResponse: function (serverResponse, bidRequest) { | ||
| const bidResponses = []; |
There was a problem hiding this comment.
#1742 got merged recently.
The first argument to interpretResponse now looks like this:
{
body: responseBody,
headers: {
get: function(header) { /* returns a header from the HTTP response */ }
}
}
You'll have to pull master and update the spec so that it looks digs into that object as well now.
| }]; | ||
|
|
||
| describe('test valid MF request success', function () { | ||
| let isValid = adapter.spec.isBidRequestValid(bidRequest[0]); |
There was a problem hiding this comment.
Test cases need to be in it to be executed. As of now none of the tests are executed.
This https://gist.github.com/samwize/8877226 will be helpful
modules/mobfoxBidAdapter.js
Outdated
| return bidResponse; | ||
| return bidResponses; | ||
| }, | ||
| getUserSyncs: function (syncOptions) { |
There was a problem hiding this comment.
getUserSyncs is optional. Can be removed
|
Hi, thanks for the comments. I made changes and added markdown file according the comments. |
modules/mobfoxBidAdapter.js
Outdated
| interpretResponse: function (serverResponse, bidRequest) { | ||
| const bidResponses = []; | ||
|
|
||
| if (!serverResponse || serverResponse.error) { |
There was a problem hiding this comment.
Its LGTM except this one change.
I think you missed to update this condition. Your can access error response by serverResponse.body.error
|
Changed error filed. Thanks |
| @@ -1,162 +1,142 @@ | |||
| describe('mobfox adapter tests', function () { | |||
| describe('mobfox adapter tests', () => { | |||
| const expect = require('chai').expect; | |||
| const utils = require('src/utils'); | |||
| const adapter = require('modules/mobfoxBidAdapter'); | |||
There was a problem hiding this comment.
Should be ES6 imports now.
| expect(bidResponse1.height).to.equal(bidderRequest.bids[0].sizes[0][1]); | ||
| }); | ||
| }); | ||
| let bidResponses = adapter.spec.interpretResponse(serverResponse, request); |
There was a problem hiding this comment.
should prefer const unless the variable is definitely being reassigned. Many of these are not.
* Upgrade MobfoxAdapter to be compliant with Prebid 1.0 - missing cpm * MfBidAdapter changes for preBid1.0 * Upgrade MobfoxAdapter to be compliant with Prebid 1.0 - final version * removed accidentally added files * Update MobfoxBidAdapter after review * Update MobfoxBidAdapter after review - Final * Add markdown file containing key information about Mobfox adapter * Fix error field - under request in serverResponse
Type of change
Description of change
Be sure to test the integration with your adserver using the Hello World sample page.
Other information