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 Bid Adapter: custom price granularity fix #3670

Conversation

idettman
Copy link
Contributor

@idettman idettman commented Mar 23, 2019

Type of change

  • Bugfix

Description of change

In the Rubicon Bid Adapter, a config property used in getPriceGranularity(), was renamed from customPriceGranularity to customPriceBucket.
Added a test to verify all granularity values return the expected results.

@idettman idettman changed the title Fix for rubicon adapter custom price granularity Rubicon Bid Adapter: custom price granularity fix Mar 23, 2019
@idettman idettman added the bug label Mar 23, 2019
@idettman idettman requested a review from harpere March 23, 2019 08:09
Copy link
Collaborator

@robertrmartinez robertrmartinez left a comment

Choose a reason for hiding this comment

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

I have tested this and it LGTM

Sorry I did not catch this before :(

{min: 3.00, max: 8.00, increment: 0.05},
{min: 8.00, max: 20.00, increment: 0.50}
],
custom: config.getConfig('priceGranularity') === 'custom' ? config.getConfig('customPriceBucket').buckets : undefined
Copy link
Collaborator

Choose a reason for hiding this comment

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

there's no need for the ternary operator because the custom param will only be chosen when config.getConfig('priceGranularity') === 'custom'

@robertrmartinez
Copy link
Collaborator

robertrmartinez commented Mar 26, 2019

Example PBS Requests in browser:

With custom granularity:
image

With high granularity:
image

@robertrmartinez
Copy link
Collaborator

MERGING!

@robertrmartinez robertrmartinez merged commit 0f01ce3 into prebid:master Mar 26, 2019
pycnvr pushed a commit to conversant/Prebid.js that referenced this pull request Apr 4, 2019
* Add microadBidAdapter

* Remove unnecessary encodeURIComponent from microadBidAdapter

* Submit Advangelists Prebid Adapter

* Submit Advangelists Prebid Adapter 1.1

* Correct procudtion endpoint for prebid

* Fix rubicon adapter price granularity, rename config property name customPriceBucket from customPriceGranularity

* add unit test for getPriceGranularity

* update getPriceGranularity function for better readability

* Update rubiconBidAdapter.js

updating per review

* adding check before setting custom
jacekburys-quantcast pushed a commit to jacekburys-quantcast/Prebid.js that referenced this pull request May 15, 2019
* Add microadBidAdapter

* Remove unnecessary encodeURIComponent from microadBidAdapter

* Submit Advangelists Prebid Adapter

* Submit Advangelists Prebid Adapter 1.1

* Correct procudtion endpoint for prebid

* Fix rubicon adapter price granularity, rename config property name customPriceBucket from customPriceGranularity

* add unit test for getPriceGranularity

* update getPriceGranularity function for better readability

* Update rubiconBidAdapter.js

updating per review

* adding check before setting custom
@idettman idettman deleted the fix-rubicon-adapter-price-granularity branch June 5, 2019 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants