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 floors module for #5511 #5538

Merged
merged 4 commits into from
Jul 29, 2020
Merged

Update floors module for #5511 #5538

merged 4 commits into from
Jul 29, 2020

Conversation

diDNA-matt
Copy link
Contributor

@diDNA-matt diDNA-matt commented Jul 24, 2020

Type of change

  • Other

Description

  • When no floor data is available after the floors module is invoked, records 'noData' in the bidRequest[ ].floorData.location field
  • Adds the field floorProvider, to the top level of the floor object. The value is a string and defaults to an empty string if none is provided.

Updates made for #5511

…ate from bidRequest[ ].floorData if undefined. If no floor data available set bidRequest[ ].floorData.location to 'noBid'.
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.

Hey @diDNA-matt Thanks a bunch for getting to this! Nice to have another developer working on the module.

Couple things that may have been a misunderstanding.

I'll let @bszekely1 confirm since he wrote the issue.

modules/priceFloors.js Outdated Show resolved Hide resolved
modules/priceFloors.js Outdated Show resolved Hide resolved
modules/priceFloors.js Show resolved Hide resolved
modules/priceFloors.js Show resolved Hide resolved
…e enforcement from floorProvider. Added floorProvider into return data.
@bszekely1
Copy link

Hey @diDNA-matt Thanks a bunch for getting to this! Nice to have another developer working on the module.

Couple things that may have been a misunderstanding.

I'll let @bszekely1 confirm since he wrote the issue.

I addressed the point in the main issue ticket, but to briefly reiterate that the Floors Module will always default to 0 when not supplied by the publisher or a floors provider. This makes it impossible for the skipRate to be undefined. The Floors Module will define when available.

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.

Awesome stuff thank you!

If you could just fix the .md file we are good.

Will get another prebid org member to review!

@robertrmartinez
Copy link
Collaborator

@Fawke

I have approved this pending the .md update.

Please give it a look when you can!

Copy link
Contributor

@Fawke Fawke left a comment

Choose a reason for hiding this comment

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

The PR looks good to me. Do we need to update the docs on Prebid.org since we're adding a new key, floorProvider?

@robertrmartinez
Copy link
Collaborator

@bszekely1

There should be some docs updated regarding floorProvider and how it is meant to be used.

Just a pass through from the floors module to the analytics adapters so they can filter out floors which do not come from their own data (wether that be in the analytics module itself, or simply passing along the provider name and their server side pipeline can do the filtering)

@robertrmartinez robertrmartinez merged commit 4a4e922 into prebid:master Jul 29, 2020
@bszekely1
Copy link

PR for documentation: prebid/prebid.github.io#2169

Brief note, I added additional functionality and flexibility of the floorProvider parameter to be at the top level of the floors object and within the data object to allow floor provider to self-declare.

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.

5 participants