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

- New Adaptor - Inneractive #1048

Merged
merged 4 commits into from
Apr 3, 2017
Merged

Conversation

uriw-ia
Copy link
Contributor

@uriw-ia uriw-ia commented Mar 14, 2017

Type of change

  • New bidder adapter

Description of change

New Inneractive PrebidJS Adaptor

  • test parameters for validating bids
{
  bidder: 'inneractive',
  params: {
    appId: 'Inneractive_Superman_iPhone',
    spotType: 'rectangle',
    customParams: {
      portal: 7002
    }
  }
}

Other information

@aneuway2
Copy link
Contributor

Hey @uriw-ia are appId and spotType both required parameters for your adaptor? Might be good to submit a PR to have this documented on prebid.org :-)

https://github.com/prebid/prebid.github.io/tree/master/dev-docs/bidders

@uriw-ia
Copy link
Contributor Author

uriw-ia commented Mar 22, 2017

Hi @aneuway2,

I've reopened a pull request on https://github.com/prebid/prebid.github.io/ with the documentation
prebid/prebid.github.io#200

(please also see: prebid/prebid.github.io#195).

Thanks,
Uri

@matthewlane
Copy link
Collaborator

Thanks for the adapter PR. I'm not able to get any bids back with the test parameters, are those still valid?

Also this adapter seems to be doing some analytic reporting, can you describe what's happening there?

@uriw-ia
Copy link
Contributor Author

uriw-ia commented Mar 27, 2017

@matthewlane ,
Could you please try again?
There seems to have been a temporary issue receiving the ad.

Regarding analytics - our adapter is sending out the following events:

  • "HBPreBidImpression" - for ad impression
  • "HBPreBidClick" - for ad click
  • "HBPreBidError" - for adapter JS error (unused)
  • "HBPreBidNoAd" - for cases in which no ad was received from our servers
  • "HBNoWin" - for cases in which an ad was received from our servers but didn't eventually win the bid

Copy link
Collaborator

@matthewlane matthewlane left a comment

Choose a reason for hiding this comment

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

Thanks for the clarification. We don't allow listening for the BID_WON event in adapters, but the other events you've listed are fine. Was also able to get bid responses with the test params now, thanks

const _reportNoBidWon = () => this._getStoredBids().forEach(bid => !wonAdIds.includes(bid.adId) && Reporter.reportEvent('HBNoWin', bid.params));
let timer = null;
$$PREBID_GLOBAL$$.onEvent(EVENTS.AUCTION_END, () => timer = setTimeout(_reportNoBidWon, $$PREBID_GLOBAL$$.bidderTimeout), null);
$$PREBID_GLOBAL$$.onEvent(EVENTS.BID_WON, (bid) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please drop this event listener

@uriw-ia
Copy link
Contributor Author

uriw-ia commented Mar 30, 2017

@matthewlane, sure thing.
The method listening for the event and the call to it, have been removed

Copy link
Collaborator

@matthewlane matthewlane left a comment

Choose a reason for hiding this comment

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

@uriw-ia I missed one other thing in the initial review, noted below, but after the requested change I think this will be ready for approval. Thanks for the update

* @type {{defaultsParams: *, serverParamNameBySettingParamName: {referrer: string, keywords: string, appId: string, portal: string, age: string, gender: string, isSecured: (boolean|null)}, toServerParams: (function(*)), unwantedValues: *[], getUrlParams: (function(*=))}}
*/
const Url = {
defaultsParams: Object.assign({}, Helpers.defaultsQsParams, {f: CONSTANTS.DISPLAY_AD,fs: false,ref: IaWindow.document.referrer}),
Copy link
Collaborator

@matthewlane matthewlane Mar 30, 2017

Choose a reason for hiding this comment

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

Accessing IaWindow.document.referrer here causes Uncaught DOMException: Blocked a frame with origin "<url>" from accessing a cross-origin frame in setups such as the prebid.org example pages. Wrapping this in a try/catch, either in this section, or (if you don't need IaWindow anywhere else), changing lines 34 - 39 to something like

let ref;
try{
  ref = window.top.document.referrer;
}catch(e){
  ref = window.document.referrer;
}

then setting ref: ref on this line should work, though feel free to catch the exception however you'd prefer

@uriw-ia
Copy link
Contributor Author

uriw-ia commented Apr 1, 2017

@matthewlane, the code is updated based on your comment. Thanks.

Copy link
Member

@mkendall07 mkendall07 left a comment

Choose a reason for hiding this comment

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

LGTM

* @param str: string
* @returns string
*/
stringToCamel(str){
Copy link
Member

Choose a reason for hiding this comment

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

This would be a good candidate to move to utils at some point.

* The returned string is either <code>http://</code> or <code>https://</code>
* @returns {string}
*/
getPageProtocol(){
Copy link
Member

Choose a reason for hiding this comment

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

Also good candidate for utils

* InnerActiveAdapter for requesting bids
* @class
*/
class InnerActiveAdapter{
Copy link
Member

Choose a reason for hiding this comment

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

We havne't been pushing to use class but it seems to work ok and transpile.

@matthewlane matthewlane merged commit c9b2bf4 into prebid:master Apr 3, 2017
outoftime added a commit to Genius/Prebid.js that referenced this pull request Apr 19, 2017
…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)
  ...
vzhukovsky added a commit to aol/Prebid.js that referenced this pull request May 19, 2017
…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)
  ...
vzhukovsky added a commit to aol/Prebid.js that referenced this pull request May 22, 2017
…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)
  ...
dluxemburg pushed a commit to Genius/Prebid.js that referenced this pull request Jul 17, 2018
* - New Adaptor - Inneractive

* refactor: removed checkIfBidWon call and method

* refactor: also removing EVENTS reference from src/constants

* Changed referrer to match access recommendations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants