-
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
Prebid 7: remove support for legacy FPD #8372
Conversation
@@ -40,7 +41,7 @@ export const spec = { | |||
uspConsent: bidderRequest.uspConsent, | |||
currencyCode: config.getConfig('currency.adServerCurrency'), | |||
coppa: config.getConfig('coppa'), | |||
firstPartyData: config.getLegacyFpd(bidderRequest.ortb2), | |||
firstPartyData: getLegacyFpd(bidderRequest.ortb2), |
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.
@zandree-owneriq we welcome your feedback on if you still need the data formatted this way or if you would prefer the newer ortb2 formatting?
@@ -453,6 +457,8 @@ function newOrtbBidRequest(bidRequest, bidderRequest, currentImps) { | |||
deepSetValue(data, 'ext.prebid.bidderconfig.0', bidderData); | |||
} | |||
|
|||
// TODO: bidRequest.fpd is not the right place for pbadslot - who's filling that in, if anyone? | |||
// is this meant to be bidRequest.ortb2Imp.ext.data.pbadslot? |
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.
@adxpremium fyi
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.
@dgirardi : it feels safe to assume, largely based on the inability of @adxpremium to add sufficient coverage on their adapter in the outstanding pr, that this is indeed not working correctly and that inmar is the only user of getLegacyFPD
src/utils/legacyFPD.js
Outdated
import {deepAccess, mergeDeep} from '../utils.js'; | ||
|
||
/** | ||
* Returns backwards compatible FPD data for modules |
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.
my only thought here is if it turns out only one adapter is using, eg inmar, would it be better to put this in the inmar adapter itself?
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, that would avoid extra weight for anyone who's not using inmar. I'll update if it turns out that luponmedia does not need the legacy conversion logic. Ideally, inmar would also drop the old format eventually.
#7796 for linkage |
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.
Looks good!
+1 to moving the legacyFPD code into those two adapters. If they have not responded by now, then it is going to take them a long time.
Keeping this code in core for 2 / 250 bid adapters seems wrong. Even though I know having it in two places is wrong as well.
Type of change
Description of change
This removes support for legacy FPD:
setConfig('fpd')
is no longer converted to an equivalentsetConfig('ortb2')
adUnit.fpd
is no longer converted to an equivalentadUnit.ortb2Imp
getLegacyFpd
andgetLegacyImpFpd
have been moved away fromconfig
to standalone utils functions, and their use avoided when possibleI have kept the ortb2-to-legacy conversion logic available because of one, or possibly two adapters:
inmar
(@zandree-owneriq), sends legacy-formatted FPD verbatim to its backend;luponmedia
(@adxpremium) may have intended to do the same - but the implementation should not work even in version 6 (see code comments in this PR)Other information
Closes #8283
Documentation PR: TBD, together with other Prebid 7 FPD-related changes (#7651)