OpenX Bid Adapter : support native #12625
Conversation
…e-imps' into EXCH-10877-add-support-for-native-imps
…e-imps' into EXCH-10877-add-support-for-native-imps
| @@ -1399,6 +1555,78 @@ describe('OpenxRtbAdapter', function () { | |||
| }); | |||
| } | |||
|
|
|||
| if (FEATURES.NATIVE) { | |||
There was a problem hiding this comment.
Tests don't get built, however, can you include the feature flag in your adapter? Why do you include it here?
There was a problem hiding this comment.
I'm not sure if I understand why/how to include the feature flag in the adapter. Our adapter utilizes ortbConverter library and there are some checks there which verifies if native feature is enabled.
As I understand gulp test firstly execute test-all-features-disabled task and then test-only.
I enclosed native-related tests in this if, so that they won't be run when native feature is disabled (test-all-features-disabled). However those tests should be run in test-only task.
I've also seen similar pattern in other adapters tests: example.
| } | ||
| }) | ||
|
|
||
| it('should be able to handle multiformat request - banner + native', () => { |
There was a problem hiding this comment.
Can you add discussion or example of this in your docs? So many large adapters need twins, happy to see you do not
There was a problem hiding this comment.
Actually I would prefer not to add it to our docs for now, since we're not supporting multi-format bid requests yet. Our system is able to accept the multi-format request, however we will default to one format. So when we get banner + native request, we will treat it as banner only.
There was a problem hiding this comment.
That's generally all you'll get from Raptive, we'll be sure to let you know if revenue is impacted
| let bannerBids = bids.filter(bid => isBannerBid(bid)); | ||
| let requests = bannerBids.length ? [createRequest(bannerBids, bidderRequest, BANNER)] : []; | ||
| let bannerAndNativeBids = bids.filter(bid => isBannerBid(bid) || isNativeBid(bid)) | ||
| // In case of multi-format bids remove `video` from mediaTypes as for video a separate bid request is built |
There was a problem hiding this comment.
When it comes to video we wanted to keep the implementation unchanged, so create separate bid request with video mediaType only. We didn't want to introduce two separate changes at once to our adapter. Also we're be changing it probably in near future as we start supporting multi-format requests.
|
Please name your pr with adapter type like all the others |
|
@patmmccann Could you please take a look at my comments above? I would really appreciate your review. |
* [EXCH-10877] Add support for native to OpenX bid adapter * [EXCH-10877] Add unit tests for interpretResponse and native * [EXCH-10877] Fix formatting * [EXCH-10877] Add support for native to OpenX bid adapter * [EXCH-10877] Add unit tests for interpretResponse and native * [EXCH-10877] Fix formatting * Revert "Merge remote-tracking branch 'origin/EXCH-10877-add-support-for-native-imps' into EXCH-10877-add-support-for-native-imps" This reverts commit c881386, reversing changes made to 55a23a5. * [EXCH-10877] Add info about native support to the docs * [EXCH-10877] create const, use null instead of undefined * [EXCH-10877] Add support for native to OpenX bid adapter * [EXCH-10877] Add unit tests for interpretResponse and native * [EXCH-10877] Fix formatting * [EXCH-10877] Add support for native to OpenX bid adapter * [EXCH-10877] Add unit tests for interpretResponse and native * [EXCH-10877] Fix formatting * Revert "Merge remote-tracking branch 'origin/EXCH-10877-add-support-for-native-imps' into EXCH-10877-add-support-for-native-imps" This reverts commit c881386, reversing changes made to 55a23a5. * [EXCH-10877] Add info about native support to the docs * [EXCH-10877] create const, use null instead of undefined * [EXCH-10877] Fix merge issues * [EXCH-10877] Use various params.platform in test
Type of change
Bugfix
Feature
New bidder adapter
Updated bidder adapter
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Does this change affect user-facing APIs or examples documented on http://prebid.org?
Other
Description of change
OpenX starts to support native inventory and we want to allow publishers to send requests for native ads to OpenX through Prebid.js. This change is aimed to add native bids support to the Prebid.js OpenX bidder adapter.