Skip to content

PBS adapter not sending app or device#2206

Merged
mike-chowla merged 3 commits intomasterfrom
feature/include_app_device
Mar 8, 2018
Merged

PBS adapter not sending app or device#2206
mike-chowla merged 3 commits intomasterfrom
feature/include_app_device

Conversation

@mkendall07
Copy link
Contributor

Type of change

  • Bugfix

Description of change

s2sConfig.app and s2sConfig.device were not getting picked up on either the legacy or new ORTB PBS endpoints. This fixes and adds tests.

@muncha
Copy link

muncha commented Mar 2, 2018

While testing my s2s fork, the error I was seeing was saying it needed either "app" or "page". I just pulled commit 428c125 , and, locally, added the one line change to also send page, which is what I need in my test scenario. This worked fine for me :

request.site = {publisher: {id: _s2sConfig.accountId}, page: window.location.href};

@mkendall07
Copy link
Contributor Author

mkendall07 commented Mar 2, 2018

ready for review

@mkendall07
Copy link
Contributor Author

thanks @muncha - updated to include page

@mike-chowla
Copy link
Contributor

Code looks good. I'm a bit puzzled by the build failure for standard/object-curly-even-spacing since when I run lint locally on the PR, it passses

@mkendall07
Copy link
Contributor Author

yeah that is weird. I'll check into it

@mkendall07
Copy link
Contributor Author

mkendall07 commented Mar 8, 2018

not sure why my local lint wasn't picking up the bad style either. I fixed it so travis is happy now though.

@muncha
Copy link

muncha commented Mar 8, 2018

Maybe it makes sense for the request that I made in #2243 to include vastUrl to be included with this?

@mike-chowla
Copy link
Contributor

@muncha A separate PR seems to me to be the way to handle #2243 since it's not directly related to the issue this PR is fixing

@muncha
Copy link

muncha commented Mar 8, 2018

Thank you @mike-chowla

@matthewlane matthewlane deleted the feature/include_app_device branch March 9, 2018 18:27
Millerrok pushed a commit to Vertamedia/Prebid.js that referenced this pull request Mar 15, 2018
* 'master' of https://github.com/prebid/Prebid.js: (76 commits)
  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)
  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)
  ...
mizmaar3 added a commit to widespace-os/Prebid.js that referenced this pull request Mar 19, 2018
* 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)
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.

3 participants

Comments