-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
ozone adapter 2.1 - bug fix for multi bids + GDPR parameter handling #3916
ozone adapter 2.1 - bug fix for multi bids + GDPR parameter handling #3916
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @afsheenb
The changes largely look good. There were a few questions I had regarding the md updates. Can you take a look when you have the chance?
Thanks.
modules/ozoneBidAdapter.md
Outdated
@@ -36,8 +36,7 @@ adUnits = [{ | |||
publisherId: 'OZONENUK0001', /* an ID to identify the publisher account - required */ | |||
siteId: '4204204201', /* An ID used to identify a site within a publisher account - required */ | |||
placementId: '0420420421', /* an ID used to identify the piece of inventory - required - for appnexus test use 13144370. */ | |||
customData": [{"settings": {}, "targeting": {"key": "value", "key2": ["value1", "value2"],}}] /* optional array with 'targeting' placeholder for passing publisher specific key-values for targeting. */ | |||
ozoneData: {"key1": "value1", "key2": "value2"}, /* optional JSON placeholder for for passing ozone project key-values for targeting. */ | |||
customData: "[{"settings": {}, "targeting": {"key": "value", "key2": ["value1", "value2"]}}]",/* optional array with 'targeting' placeholder for passing publisher specific key-values for targeting. */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the value meant to be surrounded by double quotes? It looks like it should be an array of objects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
• the quotes around the customData array in the md file have been removed - thanks for flagging. The quotes in the array for 'key2' in the example are intentional as they are meant to be a array of strings.
@@ -36,8 +36,7 @@ adUnits = [{ | |||
publisherId: 'OZONENUK0001', /* an ID to identify the publisher account - required */ | |||
siteId: '4204204201', /* An ID used to identify a site within a publisher account - required */ | |||
placementId: '0420420421', /* an ID used to identify the piece of inventory - required - for appnexus test use 13144370. */ | |||
customData": [{"settings": {}, "targeting": {"key": "value", "key2": ["value1", "value2"],}}] /* optional array with 'targeting' placeholder for passing publisher specific key-values for targeting. */ | |||
ozoneData: {"key1": "value1", "key2": "value2"}, /* optional JSON placeholder for for passing ozone project key-values for targeting. */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you clarify on the change to remove this property ozoneData
from the md file? Was it intentional?
It's still present in the adapter code from what I can tell. It's also present on the prebid.org site (under http://prebid.org/dev-docs/bidders.html#ozone).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this was intentional. We'll be submitting a pull request to remove it from the prebid.org documentation shortly.
…a and more stringent GDPR checks.
Answers: • references to ozoneData have now been removed from our adapter code, md file and spec tests. • the quotes around the customData array in the md file have been removed - thanks for flagging. The quotes in the array for 'key2' in the example are intentional as they are meant to be a array of strings. • we will submit updates for the documentation on prebid.org to remove references to ozoneData. • we've also added some more stringent checks for the GDPR bits we are passing up. |
let us know if you have any further questions @jsnellbaker following on from @afsheenb's feedback - thanks in advance! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@afsheenb Thanks for making the updates, the code changes look good. Once we have the docs PR put together, we'll be good to merge.
Hey @jsnellbaker - can you confirm what documentation you are expecting to see/is missing please ? The .md file was updated at the same time the last set of code changes were submitted & the reference to ozoneData was removed from this page - http://prebid.org/dev-docs/bidders.html#ozone |
@AskRupert-DM Sorry about that - I didn't notice the docs PR was already merged. We should be good at this point. |
No worries... thanks @jsnellbaker ! |
…rebid#3916) * bug fix for multi bids + GDPR parameter handling * Updated spec files and adapter files to remove references to ozoneData and more stringent GDPR checks.
…rebid#3916) * bug fix for multi bids + GDPR parameter handling * Updated spec files and adapter files to remove references to ozoneData and more stringent GDPR checks.
Type of change
Description of change
*bug fix for single request with multiple bids.
*parameters for handling GDPR requests inside regs.ext object