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

Widespace adapter #2283

Merged
merged 31 commits into from
Apr 13, 2018
Merged

Widespace adapter #2283

merged 31 commits into from
Apr 13, 2018

Conversation

mizmaar3
Copy link
Contributor

Type of change

  • New bidder adapter

Description of change

Widespace bidder adapter for prebid 1.x.x

  • test parameters for validating bids
 /** WS dev server url **/
 var wisp = wisp || {};
 wisp.ENGINE_URL = "https://nova-dev-engine.widespace.com/map/engine/dynadreq";

/** bid params **/
{
  bidder: 'widespace',
  params: {
   sid: '7b6589bf-95c8-4656-90b9-af9737bb9ad3',
   currency: 'EUR'
  }
}

/** Please append a hash parameter `#WS_DEBUG_FORCEADID=47696` to the url to always get a test ad  **/ 

* master:
  Update Vertoz adapter for Prebid 1.0 (prebid#2104)
  Add multiple bids request  (prebid#2136)
  [NEW Adapter] RTBHouseBidAdapter (prebid#2184)
  Update Innity Adapter to Prebid.js v1.0 (prebid#2180)
  Update Adyoulike Adapter to prebid 1.0 (prebid#2077)
  Change bidderCode for DAN Marketplace Bid Adapter (prebid#2183)
  only count bid timeouts if bidder didn't call done. fixes prebid#2146 (prebid#2154)
  [Edit BidAdapter] rxrtb adapter for Perbid.js 1.0 (prebid#2182)
  Update NasmediaAdmixer adapter (prebid#2164)
  only do video caching if we don't already have a videoCacheKey (prebid#2143)
* master: (27 commits)
  Increment pre version
  Prebid 1.5.0 Release
  Fix cross-platform test failures (prebid#2228)
  Fix uncahced video bids from multi-response array triggering callback early (prebid#2219)
  Add vuble adapter (prebid#2201)
  Update Vidazoo domain (prebid#2223)
  InSkin Bid Adapter: remove referrer field from request body (prebid#2217)
  Gamma Support UserSync Endpoint (prebid#2216)
  Feature/stale bot (prebid#2192)
  33Across Bid Adapter: updated user sync endpoint (prebid#2193)
  Adding PR_REVIEW guideline (prebid#2159)
  Add FairTrade Bid Adapter (prebid#2147)
  Add banner support to Beachfront adapter (prebid#2117)
  Smartyads Adapter 1.x (prebid#2080)
  Audience Network: allow native bids for non-IAB sizes (prebid#2203)
  Update position default value in rubicon (prebid#2214)
  Auctionmanager spec refactor pr (prebid#2155)
  fix mediaType being overwritten by undefined in rubicon bid adapter (prebid#2209)
  Fix lint error (prebid#2208)
  VAST support in adform adapter (prebid#2173)
  ...
* master:
  Audience Network: Add 'pbv' and 'cb' query params (prebid#2252)
  Add e-planning analytics adapter (prebid#2211)
  Add vastUrl for Gamma Adapter Video (prebid#2261)
  update params for test bid (prebid#2267)
  Updated adUnitCode (prebid#2262)
  vastUrl is set based on nurl for video. (prebid#2249)
  Added ad id to a4g bid (prebid#2250)
  Add billing url (burl) support (prebid#2246)
  Fix: add mediatype in bid response (prebid#2260)
  use b64EncodeUnicode to encode strings with unicode chars in them (prebid#2245)
  create RELEASE_SCHEDULE.md (prebid#2255)
  Update Platform.io Adapter (prebid#2230)
  Update Lifestreet adapter to 1.0 (prebid#2197)
  PBS adapter not sending app or device (prebid#2206)
  Fix prebid#2229 - Edge cookie string form (prebid#2236)
  Add Invibes Adapter (prebid#2202)
@mkendall07
Copy link
Member

This pull request introduces 1 alert when merging c246d9d into 1c862a8 - view on lgtm.com

new alerts:

  • 1 for Superfluous trailing arguments

Comment posted by lgtm.com

@mike-chowla
Copy link
Contributor

Use of $$PREBID_GLOBAL$$ is not permitted in 1.x adapters:
const PBJS = window['$$PREBID_GLOBAL$$'];

Additionally, it appears that you are storing winning bids of other partners and then sending off to your backend. Is that correct?

@mizmaar3
Copy link
Contributor Author

yes it is correct

@mizmaar3
Copy link
Contributor Author

And if you wonder why we are storing/sending winning bid info to backend when we lose! is because it would correspond to the "Loss Notice URL" we're getting from publishers over OpenRTB.

Please let me know what is alternative to $$PREBID_GLOBAL$$ if any

@mike-chowla
Copy link
Contributor

I checked with the head of the Prebid.js PMC and sending back winning bids is not permitted.

In general, you can not depend on receiving an event that signals a loss as if you win the Prebid auction but ad server selects a non-prebid line item, prebid will not get called and thus the only indication of a loss is your win pixel inside the creative not firing. With the current adapter code, you'd also not get notified when there is no subsequent bid request on the page.

My understanding is the current adapter interface doesn't allow for sending loss notifications back. If you think there should be a method to do this, I'd recommend opening a new issue on the project.

@mizmaar3
Copy link
Contributor Author

Ok I removed event listener and storing/sending winning bid data now

@mike-chowla
Copy link
Contributor

Test coverage is below 80%

screen shot 2018-03-22 at 5 50 52 pm

@mizmaar3
Copy link
Contributor Author

Its increased now

@mizmaar3
Copy link
Contributor Author

const REQUEST_SERVER_URL = getEngineUrl();
const DEMO_DATA_PARAMS = ['gender', 'country', 'region', 'postal', 'city', 'yob'];
const PERF_DATA = getData(LS_KEYS.PERF_DATA).map(perf_data => JSON.parse(perf_data));
const CUST_DATA = getData(LS_KEYS.CUST_DATA, false)[0];
Copy link
Contributor

@mike-chowla mike-chowla Mar 30, 2018

Choose a reason for hiding this comment

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

I don't see a storeData call for CUST_DATA so wouldn't it always be empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will be set by our server and I will just pass it on in next request when available

Copy link
Contributor

Choose a reason for hiding this comment

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

I couldn't see how it ever gets set to local storage. Am I just missing where that happens?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, you are not missing anything because CUST_DATA is not set by adapter but rather by adCreative code. This allows us get some sort of feedback when our won bid creative is rendered. Example: - If we detected some problem when trying to render ad i.e. unable to measure inscreen for some reason ? we will store the reason. - Get device information i.e. fine grained device resolution for iPhone models which not available in user agent HTTP header, it helps us to deliver better creative next time.

} else if (COOKIE_ENABLED) {
const theDate = new Date();
const expDate = new Date(theDate.setMonth(theDate.getMonth() + 12)).toGMTString();
window.document.cookie = `${name}=${value};path=/;expires=${expDate}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

This creates a cookie in the publisher's cookie space which seems like something the publisher should at least informed is happening. I don't see any mention in the docs that adapter creates cookies. However, I see other adapters doing this too and do not know of specific prohibition against it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I havnt found any adapter doc mentioning it, also cookie is just fallback and we have seen that 99.9% chances are for localStorage to be used.

@jonasmattsson1
Copy link
Contributor

@mike-chowla Hallo Sir, Is there anything more that we have to fix in the adapter to get it approved?

Br,
Jonas Mattsson

@mike-chowla mike-chowla merged commit 033d733 into prebid:master Apr 13, 2018
ArmandChoy pushed a commit to RockYou-Ads/Prebid.js that referenced this pull request Apr 18, 2018
* 'master' of https://github.com/prebid/Prebid.js: (211 commits)
  Increment Pre Version
  Prebid 1.8.0 Release
  Make eslint aware of the custom import paths (prebid#2292)
  send travis-ci notifications to slack (prebid#2404)
  send appnexus usePaymentRule info to prebid-server ortb request (prebid#2351)
  convert AN bid params to underscore formatting for pbs (prebid#2385)
  EbdrAdapter add usersync  (prebid#2407)
  Add outstream renderer to Beachfront adapter (prebid#2403)
  Add analytics adapter by Sigmoid (prebid#2316)
  deprecate loadScript and add loadExternalScript (prebid#2391)
  Removed the ability for to override any standard query parameters (prebid#2402)
  Unit test failures (prebid#2405)
  Add Unruly Bid Adapter (prebid#2326)
  Added VIS.X Bidder Adapter (prebid#2359)
  Smart: Add prebid version in the data payload (prebid#2394)
  add support for video bids to use an impression tracking URL  (prebid#2365)
  Create rtbdemandAdkBidAdapter_spec.js (prebid#2352)
  Widespace adapter (prebid#2283)
  Add: Vuble Analytics Adapter (prebid#2331)
  fixes prebid#2353 - not appending hb_uuid and hb_cache_id (prebid#2363)
  ...

# Conflicts:
#	test/spec/modules/rockyouBidAdapter_spec.js
dluxemburg pushed a commit to Genius/Prebid.js that referenced this pull request Jul 17, 2018
Update WideSpce Adapter for Prebid 1.x
@anniewalter anniewalter deleted the Widespace_adapter branch February 20, 2019 09:56
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.

5 participants