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

Rubicon adapter: add a floor variable #964

Merged
merged 3 commits into from
Feb 7, 2017

Conversation

benjaminclot
Copy link
Contributor

@benjaminclot benjaminclot commented Feb 1, 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

Added a new "floor" variable to the Rubicon adapter, in order to be able to use rp_floor dynamically and not have a static 0.01 value.

  • test parameters for validating bids
{
  bidder: 'rubicon',
  params: {
    // ...
    floor: 0.5,
    // ...
  }
}

@benjaminclot benjaminclot changed the title Add a floor variable Add a floor variable to Rubicon adapter Feb 1, 2017
@benjaminclot benjaminclot changed the title Add a floor variable to Rubicon adapter Rubicon adapter: add a floor variable Feb 1, 2017
@protonate protonate self-assigned this Feb 1, 2017
@protonate protonate self-requested a review February 1, 2017 19:20
@mkendall07 mkendall07 assigned snapwich and protonate and unassigned protonate Feb 2, 2017
@mkendall07 mkendall07 requested a review from snapwich February 2, 2017 13:56
@snapwich
Copy link
Collaborator

snapwich commented Feb 2, 2017

Needs to be updated in the unit test, but otherwise LGTM. Verifying with @bretg about documentation and endpoint.

Also this will need to be duplicated in #950 I imagine, so we might want to wait until that is merged.

@bretg
Copy link
Collaborator

bretg commented Feb 2, 2017

This is fine. I'll update the Prebid.org docs in the coming weeks.

@benjaminclot
Copy link
Contributor Author

Is this going to be merged soon? I kinda need it and I'd rather not use a "custom" adapter.
Also I can't make a unit test that responds differently based on the floor (and always the same for the tests), only the Rubicon guys can do that.

@mkendall07
Copy link
Member

@benjaminclot
Can you resolve conflicts? I think we can merge after that.

@snapwich
Copy link
Collaborator

snapwich commented Feb 7, 2017

@benjaminclot the unit test responses are mocked so anyone should be able to add the appropriate tests. I'm willing to add those though, if you'd like. I think that's possible if the "allow maintainers to edit" checkbox is clicked?

@benjaminclot
Copy link
Contributor Author

@mkendall07 conflicts are resolved.
@snapwich the checkbox is checked.

@snapwich
Copy link
Collaborator

snapwich commented Feb 7, 2017

For whatever reason it won't let me push my change due to lack of permissions. I thought being a Prebid.js member would be enough, or maybe i'm doing it wrong... But either way, here's the patch if you would like to apply it:

From ea495bb8f305269d7a3266adba81fb62c125bfc6 Mon Sep 17 00:00:00 2001
From: Rich Snapp <rsnapp@rubiconproject.com>
Date: Tue, 7 Feb 2017 10:46:00 -0700
Subject: [PATCH] added unit test to rubicon_spec for rp_floor override

---
 test/spec/adapters/rubicon_spec.js | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/test/spec/adapters/rubicon_spec.js b/test/spec/adapters/rubicon_spec.js
index a875def..853e99e 100644
--- a/test/spec/adapters/rubicon_spec.js
+++ b/test/spec/adapters/rubicon_spec.js
@@ -263,6 +263,19 @@ describe('the rubicon adapter', () => {

         });

+        it('should allow a floor override', () => {
+          bidderRequest.bids[0].params.floor = 2;
+
+          rubiconAdapter.callBids(bidderRequest);
+
+          let request = xhr.requests[0];
+
+          let [path, query] = request.url.split('?');
+          query = parseQuery(query);
+
+          expect(query['rp_floor']).to.equal('2');
+        });
+
         it('should not send a request and register an error bid if no valid sizes', () => {

           var sizesBidderRequest = clone(bidderRequest);
--
2.10.1 (Apple Git-78)

@benjaminclot
Copy link
Contributor Author

@snapwich The test has been added (and lightly edited)

@mkendall07
Copy link
Member

LGTM. Thanks!

@mkendall07 mkendall07 merged commit 316fdfa into prebid:master Feb 7, 2017
Walexander pushed a commit to MbidIO/Prebid.js that referenced this pull request Mar 6, 2017
* Add a floor variable

* Added floor parameter test
mp-12301 pushed a commit to aol/Prebid.js that referenced this pull request Apr 10, 2017
…ebid-official-0.19.0 to release/1.14.0

* commit 'b13f7ba7ee8b3c168dc0af7c8bfc94747d017e70': (34 commits)
  Add changelog entry.
  Prebid 0.19.0 Release
  check truthiness of adUnitCode (prebid#990)
  fixed exception when refreshing individual Ad Units and bidder responds slowly (prebid#989)
  Stub pixel drop to prevent network request in test (prebid#988)
  Updating Komoona adapter to support future Prebid requirements (prebid#974)
  Use stable version of Chrome (prebid#984)
  Revert to running browser tests in Travis (prebid#983)
  Fix issue with appnexusAst sending `user` object in the wrong place. (prebid#980)
  Integrate Browserstack tests into Travis CI build (prebid#839)
  Add StickyAdsTV Bidder adapter (prebid#916)
  Stronger xdomain checks (prebid#971)
  added matomy as an alias for appnexus (prebid#850)
  Added 152Media Appnexus Alias (prebid#952)
  OpenX Adapter: Handles fallback ads correctly as a no fill (prebid#39) (prebid#963)
  Use package dependencies for ES6 Array shims (prebid#962)
  Make x-domain safe frame example work out of the box (prebid#955)
  added usersync for adkernel adapter (prebid#951)
  Rubicon adapter: add a floor variable (prebid#964)
  Added Lifestreet adapter. (prebid#965)
  ...
mp-12301 pushed a commit to aol/Prebid.js that referenced this pull request Apr 10, 2017
…14.0 to master

* commit 'c008f3f531ae3409f4a16bf03470d84e82aead0e': (35 commits)
  Add adapters in aolPartnersIds.json.
  Add changelog entry.
  Prebid 0.19.0 Release
  check truthiness of adUnitCode (prebid#990)
  fixed exception when refreshing individual Ad Units and bidder responds slowly (prebid#989)
  Stub pixel drop to prevent network request in test (prebid#988)
  Updating Komoona adapter to support future Prebid requirements (prebid#974)
  Use stable version of Chrome (prebid#984)
  Revert to running browser tests in Travis (prebid#983)
  Fix issue with appnexusAst sending `user` object in the wrong place. (prebid#980)
  Integrate Browserstack tests into Travis CI build (prebid#839)
  Add StickyAdsTV Bidder adapter (prebid#916)
  Stronger xdomain checks (prebid#971)
  added matomy as an alias for appnexus (prebid#850)
  Added 152Media Appnexus Alias (prebid#952)
  OpenX Adapter: Handles fallback ads correctly as a no fill (prebid#39) (prebid#963)
  Use package dependencies for ES6 Array shims (prebid#962)
  Make x-domain safe frame example work out of the box (prebid#955)
  added usersync for adkernel adapter (prebid#951)
  Rubicon adapter: add a floor variable (prebid#964)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants