tcfControl: add deferS2Sbidders flag #14252
Conversation
Pull Request Test Coverage Report for Build 20264476124Details
💛 - Coveralls |
modules/tcfControl.ts
Outdated
| return true; | ||
| } | ||
| const vendorConsentRequred = rule.enforceVendor && !((gvlId === VENDORLESS_GVLID || (rule.softVendorExceptions || []).includes(currentModule))); | ||
| const deferS2Sbidders = rule.purpose === 'basicAds' && !gvlId && rule.deferS2Sbidders; |
There was a problem hiding this comment.
I think the intent of #12084 is to do this only for bidders going to prebid server. As it is this means "do not require a gvl id for purpose 2", but that should not apply to client side bidders.
| enforceVendor: true, | ||
| vendorExceptions: ['bidderA'] | ||
| vendorExceptions: ['bidderA'], | ||
| deferS2Sbidders: false |
There was a problem hiding this comment.
Given that the value of deferS2Sbidders can change the output of validateRules(), do we think having a unit test that demonstrates at least one case where it is true would be useful?
|
Tread carefully! This PR adds 2 linter errors (possibly disabled through directives):
|
src/adapterManager.ts
Outdated
| // filter out bidders that cannot participate in the auction | ||
| au.bids = au.bids.filter((bid) => !bid.bidder || dep.isAllowed(ACTIVITY_FETCH_BIDS, activityParams(MODULE_TYPE_BIDDER, bid.bidder))) | ||
| au.bids = au.bids.filter((bid) => !bid.bidder || dep.isAllowed(ACTIVITY_FETCH_BIDS, activityParams(MODULE_TYPE_BIDDER, bid.bidder, { | ||
| isS2S: serverBiddersCodes.has(bid.bidder) |
There was a problem hiding this comment.
unfortunately a bidder can be both client and server and this would be too permissive in that case.
You get to know who's going to client / server only after partitionBidders below here, so I think you need to either:
a) move the partitionBidders above this, use it to determine isS2SOnly, then filter again bidders that were denied out of clientBidders / serverBidders (since we switched the order of operations), or
b) do two separate activity checks for client and server.
a) would be too restrictive in this edge case (which is better than too permissive). b) is of course more correct but I think more complex and I'm not sure it's worth it, since it is an edge case.
|
@mkomorski can you link the docs pull? |
dgirardi
dismissed
their stale review
Should not defer when gvlid is present?
| return true; | ||
| } | ||
| const vendorConsentRequred = rule.enforceVendor && !((gvlId === VENDORLESS_GVLID || (rule.softVendorExceptions || []).includes(currentModule))); | ||
| const deferS2Sbidders = params['isS2S'] && rule.purpose === 'basicAds' && rule.deferS2Sbidders; |
There was a problem hiding this comment.
I notice now this no longer checks that the gvl id is missing; shouldn't it?
There was a problem hiding this comment.
I assumed if it's called deferS2Sbidders then it applies to s2s bidders in general with assumption they are all going to be verified by server. But I have no strong opinion on that
This PR adds
deferS2Sbiddersflag to tcfControl module configuration which prevents from excluding server bidders from the auction because GVLID is unknownType of change
Bugfix
Feature
New bidder adapter
Updated bidder adapter
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Does this change affect user-facing APIs or examples documented on http://prebid.org?
Other
Description of change
Other information
#12084