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
JW Player RTD Module : populate content url, title and description #11178
Conversation
@@ -329,15 +353,30 @@ export function addOrtbSiteContent(ortb2, contentId, contentData) { | |||
content.id = contentId; | |||
} | |||
|
|||
const currentData = content.data = content.data || []; | |||
if (contentUrl) { | |||
content.url = contentUrl; |
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.
Should this override what might be in content
already?
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! If a publisher has requested that we enrich the bid request for a given media, then they have determined that the bid request is specific to that media, therefore we should override existing data in that field.
Unfortunately the oRTB spec only has room for 1 content description, which is unrealistic given that multiple videos could be rendering simultaneously on the page.
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.
this feels concerning, let's discuss in committee
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.
@patmmccann and @dgirardi this has been address by adding configuration params
Co-authored-by: Demetrio Girardi <demetrio.girardi@proton.me>
Co-authored-by: Demetrio Girardi <demetrio.girardi@proton.me>
@patmmccann what does the "on hold" label mean ? Does it block this from getting merged; If so, how can we resolve? |
Yes this blocked from merging, it is a breaking change |
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.
This is a breaking change, please require new configuration to enable overwriting existing fields
@@ -329,15 +353,30 @@ export function addOrtbSiteContent(ortb2, contentId, contentData) { | |||
content.id = contentId; | |||
} | |||
|
|||
const currentData = content.data = content.data || []; | |||
if (contentUrl) { | |||
content.url = contentUrl; |
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.
this feels concerning, let's discuss in committee
@@ -71,15 +81,31 @@ export function fetchTargetingInformation(jwTargeting) { | |||
}); | |||
} | |||
|
|||
export function setOverrides(params) { | |||
// For backwards compatibility, default to always unless overridden by Publisher. | |||
overrideContentId = sanitizeOverrideParam(params.overrideContentId, ENRICH_ALWAYS); |
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.
Assuming this should be updated for 9, do you have a way to track it - would a TODO comment be helpful?
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.
added!
@patmmccann can we remove the |
Type of change
Feature
Does this change affect user-facing APIs or examples documented on http://prebid.org?
Description of change
Includes content url, title and description when enriching the
ortb.site.content
.Other information
Docs PR: prebid/prebid.github.io#5190