-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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 Facebook Audience Network adapter #1068
Conversation
@lovell |
Hi Matt, the work for this PR was commissioned and paid for by Facebook, sorry I didn't make that clear in the first instance. I'll ask one of their full-time employees to verify this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @lovell, the bidder params in the provided integration example aren't returning valid responses, do you have test params we could use for validating bids?
Also a few initial notes below for your review. Thanks a lot for this adapter PR!
src/adapters/audienceNetwork.js
Outdated
// Handle response | ||
const data = parseJson(res); | ||
if (data.errors && data.errors.length) { | ||
addBidResponse(null, createFailureBidResponse()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addBidResponse
should take an ad unit code as the first param even for nobid
responses
src/adapters/audienceNetwork.js
Outdated
}, null, { withCredentials: true }); | ||
} else { | ||
// No valid bids | ||
addBidResponse(null, createFailureBidResponse()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please also pass in the ad unit code as first param here
Hello @matthewlane, you should be able to create a new "App" for testing purposes at https://developers.facebook.com/apps then add the "Audience Network" product to it. This will then allow you to create a "Placement" and therefore provide you with the We should probably add these steps to the example :) |
Thanks for the instructions @lovell. I've gone through and created a placementId, but am getting a response of |
What's your app ID, @matthewlane? It needs to be approved for Audience Network header bidding - I can do that. |
Thanks @wheresbarney, the app ID is 1780499291962017 |
@lovell |
@mkendall07 I expect to update this later today, thanks for the nudge. |
@mkendall07 - I've approved your app ID. You should be able to get bids as long as you're signed in to FB on the same session, and you've got a mobile user agent. |
5610a19
to
ddeec42
Compare
@mkendall07 The code is now updated to call |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was able to verify bid responses. Thanks for the update, looks good to me
* and therefore indicate testmode should be used? | ||
* @returns {String} "true" or "false" | ||
*/ | ||
const isTestmode = () => Boolean( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the test mode for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This detects the addition of ?anhb_testmode
in a given page's URL so only test bids are returned. Those test bids won't be honoured beyond the initial response. I guess/hope any official FB API docs will include mention of this but happy to add this to the "quick start" in the example too if required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly just curious. Thanks.
LGTM |
Thanks @matthewlane, @lovell. Do we need to do anything for the AN adaptor to end up in the configure-and-compile downloader at http://prebid.org/download.html? |
This is merged into master. Please submit a PR to the docs repo to add a file for your adapter to the bidders directory so your adapter's params will appear on the bidders page. Thank you for contributing |
@wheresbarney see above message. That's all that is required and we will update the rest. |
…built * 'master' of https://github.com/prebid/Prebid.js: (38 commits) Add optional domain parameter to AdButler adapter (prebid#1078) Send transactionID to Criteo Services (prebid#1113) Fix `buildMasterVideoTagFromAdserverTag()` not selecting winning bid (prebid#1106) Remove placement size selection and filtering (prebid#1107) revert `srcdoc` change (prebid#1130) Add new Adapter- Beachfront Media (prebid#1062) Fixes SpringServe adapter (prebid#1101) Update Widespace request param (prebid#1098) - New Adapter: Innity (prebid#1074) Update Roxot prebid analytic adapter (prebid#1034) Yarn Package Manager (prebid#1109) allow writing into current document if prebid is loaded inside an iframe (prebid#1066) Adapter bug fix (prebid#1096) fix typo added pr review process and governance model (prebid#1103) added support for sampling in ga and base adapter, fixed up some tests (prebid#1011) Add Inneractive adapter (prebid#1048) Add alias freewheel-ssp to stickyadstv bidder adapter (prebid#1043) Add Facebook Audience Network adapter (prebid#1068) Add Atomx support (prebid#1056) ...
…19.0 to aolgithub-master * commit '5109273bd4317535b23e26ff609345101a3d038d': (49 commits) Disable unit tests that fails on PhantomJs. Fixed unit tests for adapter loader. Changed way of including analytic adapters. Added adapters in aolPartnersIds.json. Added changelog entry. Replace removed utils.extend functionality by object.assign. Add Inneractive adapter (prebid#1048) Add alias freewheel-ssp to stickyadstv bidder adapter (prebid#1043) Add Facebook Audience Network adapter (prebid#1068) Add Atomx support (prebid#1056) Updated rubicon video bid endpoint in source and test files (prebid#1097) Pass through params to server (prebid#1084) Reset the list of slots to be requested between each action for pubmatic (prebid#1079) Support for downloading Analytics Adapters via http://prebid.org/download.html (prebid#1021) update PR template to include link to dev docs page (prebid#1075) Prebid 0.21.0 Lifestreet adapter: ignore unnecessary events from creative. (prebid#1054) Video header bidding support to RhythmOne bidder adapter (prebid#1052) Add rubicon targeting to rubicon bid responses for bidderSettings use (prebid#1045) Fix adapter getSize (prebid#1064) ...
…19.0 to master * commit '5109273bd4317535b23e26ff609345101a3d038d': (49 commits) Disable unit tests that fails on PhantomJs. Fixed unit tests for adapter loader. Changed way of including analytic adapters. Added adapters in aolPartnersIds.json. Added changelog entry. Replace removed utils.extend functionality by object.assign. Add Inneractive adapter (prebid#1048) Add alias freewheel-ssp to stickyadstv bidder adapter (prebid#1043) Add Facebook Audience Network adapter (prebid#1068) Add Atomx support (prebid#1056) Updated rubicon video bid endpoint in source and test files (prebid#1097) Pass through params to server (prebid#1084) Reset the list of slots to be requested between each action for pubmatic (prebid#1079) Support for downloading Analytics Adapters via http://prebid.org/download.html (prebid#1021) update PR template to include link to dev docs page (prebid#1075) Prebid 0.21.0 Lifestreet adapter: ignore unnecessary events from creative. (prebid#1054) Video header bidding support to RhythmOne bidder adapter (prebid#1052) Add rubicon targeting to rubicon bid responses for bidderSettings use (prebid#1045) Fix adapter getSize (prebid#1064) ...
Type of change
Description of change
Hello, I'm sure some contributors to this very useful header bidding product will be aware of the recent news that Facebook Audience Network now supports header bidding.
https://adexchanger.com/publishers/facebook-launches-header-bidding-turns-partners-tech/
As well as the adapter itself, this PR includes an integration example of its possible use.
(The branch coverage of the provided unit tests is 100% but due to the coverage reporter's use of ES5 this is currently displayed as only 59%. I hope this is OK.)
Feedback is very much welcome.