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

Update renderAd to replace ${AUCTION_PRICE} in adUrl #1795

Merged
merged 2 commits into from
Nov 14, 2017
Merged

Update renderAd to replace ${AUCTION_PRICE} in adUrl #1795

merged 2 commits into from
Nov 14, 2017

Conversation

klawil
Copy link
Contributor

@klawil klawil commented Nov 2, 2017

Type of change

  • Bugfix
  • Feature
  • New bidder adapter
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other

Description of change

This PR updates the renderAd function to replace ${AUCTION_PRICE} in adUrl.

In addition to this, this PR uses adUrl for the url parameter to match with the key used in the bid object (if this should be removed or needs to be a separate PR, let me know)

Other information

Associated Issue: #1794

The `bid.url` parameter is not used to render the iframe for an ad that
returns a url but is being treated as such when the auction price is
being inserted. This commit inserts the auction price into `bid.adUrl`
which is used to render the ad.
Instead of renaming the parameter `adUrl` to `url` for the rendering
function, use `adUrl` to be consistent with the key value returned in
the bid object.
@ckbo3hrk
Copy link
Contributor

ckbo3hrk commented Nov 7, 2017

Is it legit to substitute ${AUCTION_PRICE} with ad's own price? Up till now I thought that prebid doesn't support 2nd price auction so sending original price back to DSP has no sense as in general DSP expects to be charged with lesser price (and that lesser price is passed via ${AUCTION_PRICE}) than it bids in case of 2nd price auction.

@klawil
Copy link
Contributor Author

klawil commented Nov 7, 2017

@ckbo3hrk It looks like this functionality was added in #1165 (diff) with the integration of the Prebid S2S adapter, so I believe that is what its primary use is (but I'm sure someone else has a better idea of the specifics, we don't use S2S).

It could be beneficial to add support for a winning_cpm value to be passed into renderAd if Prebid is run as a second price auction (we handle it by modifying the bid.cpm before calling renderAd).

@mkendall07
Copy link
Member

Correct, there is no 2nd price but this gives us flexibility to support it if we want to. Nice catch @klawil

@matthewlane matthewlane merged commit 9bd0c46 into prebid:master Nov 14, 2017
Millerrok pushed a commit to Vertamedia/Prebid.js that referenced this pull request Nov 21, 2017
* unstream/master: (36 commits)
  + Add Optimatic Bid Adapter (prebid#1837)
  Add Bridgewell adapter (prebid#1825)
  Kumma adapter updated for Prebid 1.0 (prebid#1766)
  Touchup add bid response (prebid#1822)
  Fix skipped test (prebid#1836)
  Added new size in Rubicon pbjs Adapter (prebid#1842)
  HuddledMasses header bidding adapter (prebid#1806)
  Increment pre version
  Prebid 0.33.0 Release
  Update AOL adapter for v1.0  (prebid#1693)
  Sovrn 1.0 compliance (prebid#1796)
  Platform.io Bidder Adapter update (prebid#1817)
  Drop non-video bidders from video ad units (prebid#1815)
  Update renderAd to replace ${AUCTION_PRICE} in adUrl (prebid#1795)
  Pulsepoint adapter: fixing bid rejection due to missing mandatory bid params. (prebid#1823)
  Remove require.ensure entirely (prebid#1816)
  Add custom keyword support for pbs bid adapter (prebid#1763)
  OpenX Video Adapter update to Prebid v1.0 (prebid#1724)
  Fix test that hard-coded pbjs global. (prebid#1786)
  Update Pollux Adapter to v1.0 (prebid#1694)
  ...
vzhukovsky added a commit to aol/Prebid.js that referenced this pull request Dec 28, 2017
….33.0 to aolgithub-master

* commit '3e9756098bb20ecbe0314f16eed5298c5675b24c': (32 commits)
  Wrapped content type in options object.
  Added partners ids.
  Added changelog entry.
  Prebid 0.33.0 Release
  Update AOL adapter for v1.0  (prebid#1693)
  Sovrn 1.0 compliance (prebid#1796)
  Platform.io Bidder Adapter update (prebid#1817)
  Drop non-video bidders from video ad units (prebid#1815)
  Update renderAd to replace ${AUCTION_PRICE} in adUrl (prebid#1795)
  Pulsepoint adapter: fixing bid rejection due to missing mandatory bid params. (prebid#1823)
  Remove require.ensure entirely (prebid#1816)
  Add custom keyword support for pbs bid adapter (prebid#1763)
  OpenX Video Adapter update to Prebid v1.0 (prebid#1724)
  Fix test that hard-coded pbjs global. (prebid#1786)
  Update Pollux Adapter to v1.0 (prebid#1694)
  PubMatic adapter (prebid#1707)
  Added sizes to Rubicon Adapter (prebid#1818)
  jsonpFunction name should match the namespace (prebid#1785)
  Adding 33Across adapter (prebid#1805)
  Unit test fix (prebid#1812)
  ...
dluxemburg pushed a commit to Genius/Prebid.js that referenced this pull request Jul 17, 2018
* Modify `adUrl` instead of `url` with bid price

The `bid.url` parameter is not used to render the iframe for an ad that
returns a url but is being treated as such when the auction price is
being inserted. This commit inserts the auction price into `bid.adUrl`
which is used to render the ad.

* Use the key for adUrl instead of using url

Instead of renaming the parameter `adUrl` to `url` for the rendering
function, use `adUrl` to be consistent with the key value returned in
the bid object.
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