-
Notifications
You must be signed in to change notification settings - Fork 748
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
Allow bidders to skip sync for GDPR/GPP #3265
Conversation
usersync/chooser.go
Outdated
@@ -177,9 +184,17 @@ func (c standardChooser) evaluate(bidder string, syncersSeen map[string]struct{} | |||
return nil, BidderEvaluation{Status: StatusBlockedByGDPR, Bidder: bidder, SyncerKey: syncer.Key()} | |||
} | |||
|
|||
if c.bidderInfo[bidder].Syncer != nil && c.bidderInfo[bidder].Syncer.SkipWhen != nil && c.bidderInfo[bidder].Syncer.SkipWhen.GDPR { |
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.
Shouldn't there be a check here to see if the GDPR applies? Otherwise we will always skip every request when they ask to skip when the GDPR applies. May want to put this in front of the GDPR check above so we don't bother checking if they pass the GDPR if they want to skip any GDPR requests.
usersync/chooser.go
Outdated
if !privacy.CCPAAllowsBidderSync(bidderNormalized.String()) { | ||
return nil, BidderEvaluation{Status: StatusBlockedByCCPA, Bidder: bidder, SyncerKey: syncer.Key()} | ||
} | ||
|
||
if c.bidderInfo[bidder].Syncer != nil && c.bidderInfo[bidder].Syncer.SkipWhen != nil && c.bidderInfo[bidder].Syncer.SkipWhen.GPPSID == GPPSID { |
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.
According to the linked issue, the configured GPPSID is a list of GPPSIDs that they want to skip on. So you need to transform the string from the config into a list of IDs, and then check if the incoming GPPSID matches any one of them.
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.
LGTM
endpoints/cookie_sync.go
Outdated
func (p usersyncPrivacy) GDPRInScope() bool { | ||
if p.gdprSignal == gdpr.SignalYes { | ||
return true | ||
} | ||
return false | ||
} |
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.
Super nitpick.
This func can be rewritten to
func (p usersyncPrivacy) GDPRInScope() bool {
return p.gdprSignal == gdpr.SignalYes
}
e43e3ea
Addresses this issue: #3102
This PR was re-uploaded because of issues I was facing with merge conflicts