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.

Sign up for GitHub

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

Jump to bottom

HuddledMasses header bidding adapter #1806

Merged
merged 9 commits into from
Nov 14, 2017
Merged

HuddledMasses header bidding adapter #1806

merged 9 commits into from
Nov 14, 2017

Conversation

HuddledMasses
Copy link
Contributor

Type of change

  • New bidder adapter

Description of change

Add adapter and tests for the HuddledMasses header bidding adapter

  • test parameters for validating bids
{
   bidder: 'huddledmasses',
   params: {
      placement_id: 0
    }
}

@HuddledMasses HuddledMasses mentioned this pull request Nov 8, 2017
Closed
@mkendall07
Copy link
Member

@HuddledMasses
Why did you rename the previous adapter and now submitting a new one? This is confusing.

@HuddledMasses
Copy link
Contributor Author

@mkendall07
sorry fo confusion. at last minute we made a decision to keep two adapters; so we will have one for Colossus, another for Huddled Masses

@mkendall07
Copy link
Member

@HuddledMasses
And the code for the adapters is discrete (and the endpoints?)

@HuddledMasses
Copy link
Contributor Author

@mkendall07
yes, both the code and the endpoints are discrete.

@HuddledMasses
Copy link
Contributor Author

Hi @mkendall07 Any update on this? will appreciate. Thanks!

mkendall07
mkendall07 requested changes Nov 13, 2017
View reviewed changes
modules/huddledmassesBidAdapter.js Outdated
*/
isBidRequestValid: (bid) => {
return (!isNaN(bid.params.placement_id) &&
((bid.params.sizes != undefined && bid.params.sizes.length > 0 && bid.params.sizes.some((sizeIndex) => INDEX_SIZE[sizeIndex] != undefined)) ||
Copy link
Member

Choose a reason for hiding this comment

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

we prefer triple equality (!==).

modules/huddledmassesBidAdapter.js Outdated
'800x250': 118,
'200x600': 119
};
const INDEX_SIZE = {
Copy link
Member

Choose a reason for hiding this comment

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

You don't need to duplicate this data. You can do a reverse lookup by iterating the object properties.

modules/huddledmassesBidAdapter.js Outdated
} catch (e) {
console.log(e);
};
let location = winTop.location;
Copy link
Member

Choose a reason for hiding this comment

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

You can use utils.getTopWindowLocation() instead of the code above: https://github.com/prebid/Prebid.js/blob/master/src/utils.js#L157

modules/huddledmassesBidAdapter.js Outdated
}
}
} catch (e) {
console.log(e);
Copy link
Member

Choose a reason for hiding this comment

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

please use utils.logMessage() instead

modules/huddledmassesBidAdapter.md Outdated
```
Module Name: HuddledMasses Bidder Adapter
Module Type: Bidder Adapter
Maintainer: prebid@example.com
Copy link
Member

Choose a reason for hiding this comment

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

Please update email

modules/huddledmassesBidAdapter.js
interpretResponse: (serverResponse) => {
let response = [];
try {
serverResponse = serverResponse.body;
Copy link
Member

Choose a reason for hiding this comment

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

this line needs to be serverResponse.body.body;. Otherwise no bids are returned.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here I do not agree.
Example response:

{
    "body": [{
        "width": 300,
        "height": 250,
        "ad": "<img src='http://supply.huddledmasses.com/img/logo.png' style='position: absolute; left: 50%; top:50%; margin-left: -113px; margin-top:-79px;'>",
        "requestId": "26042378f97b21",
        "cpm": 5,
        "ttl": 120,
        "creativeId": "123",
        "netRevenue": true,
        "currency": "USD",
        "dealId": "HASH"
    }]
}

Copy link
Member

Choose a reason for hiding this comment

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

it's broke. Run the code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I cant understand whats wrong.
Why doesn't it work for you.
Here is the test page with the given adapter:
http://hb-test.huddledmassessupply.com/
I do not understand why it does not work for you.

Copy link
Member

Choose a reason for hiding this comment

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

@HuddledMasses
The API changed. Please rebase to master.

On your test page replace http://hb-test.huddledmassessupply.com/prebid-core.js with the latest code and you will see the issue.

Copy link
Member

Choose a reason for hiding this comment

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

Here is the code changed: https://github.com/prebid/Prebid.js/blob/master/src/adapters/bidderFactory.js#L285

Copy link
Contributor Author

@HuddledMasses HuddledMasses Nov 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I replaced the response at the server level.
Now it should work

[{
        "width": 300,
        "height": 250,
        "ad": "<img src='http://supply.huddledmasses.com/img/logo.png' style='position: absolute; left: 50%; top:50%; margin-left: -113px; margin-top:-79px;'>",
        "requestId": "26042378f97b21",
        "cpm": 5,
        "ttl": 120,
        "creativeId": "123",
        "netRevenue": true,
        "currency": "USD",
        "dealId": "HASH"
}]

@HuddledMasses
Copy link
Contributor Author

All rights are included.
Only the function interpretResponse remained the same.
The comment with an example of a response from the server is left above.

@mkendall07 mkendall07 merged commit d3905fe into prebid:master Nov 14, 2017
Millerrok pushed a commit to Vertamedia/Prebid.js that referenced this pull request Nov 21, 2017
Merge remote-tracking branch 'unstream/master'
77e66d3 
* unstream/master: (36 commits)
  + Add Optimatic Bid Adapter (prebid#1837)
  Add Bridgewell adapter (prebid#1825)
  Kumma adapter updated for Prebid 1.0 (prebid#1766)
  Touchup add bid response (prebid#1822)
  Fix skipped test (prebid#1836)
  Added new size in Rubicon pbjs Adapter (prebid#1842)
  HuddledMasses header bidding adapter (prebid#1806)
  Increment pre version
  Prebid 0.33.0 Release
  Update AOL adapter for v1.0  (prebid#1693)
  Sovrn 1.0 compliance (prebid#1796)
  Platform.io Bidder Adapter update (prebid#1817)
  Drop non-video bidders from video ad units (prebid#1815)
  Update renderAd to replace ${AUCTION_PRICE} in adUrl (prebid#1795)
  Pulsepoint adapter: fixing bid rejection due to missing mandatory bid params. (prebid#1823)
  Remove require.ensure entirely (prebid#1816)
  Add custom keyword support for pbs bid adapter (prebid#1763)
  OpenX Video Adapter update to Prebid v1.0 (prebid#1724)
  Fix test that hard-coded pbjs global. (prebid#1786)
  Update Pollux Adapter to v1.0 (prebid#1694)
  ...
@HuddledMasses HuddledMasses mentioned this pull request Dec 1, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mkendall07 mkendall07 mkendall07 requested changes

Assignees

@mkendall07 mkendall07

Labels
needs update not receiving bids
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@HuddledMasses @mkendall07 @matthewlane