Skip to content
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.

Sign up for GitHub

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

Concert Bid Adapter: adding referer (Resubmit of Pr #8580) #8676

Merged
merged 3 commits into from
Jul 25, 2022
Merged

Concert Bid Adapter: adding referer (Resubmit of Pr #8580) #8676

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fffb445
Concert Bid Adapter refererInfo update
ChrisHuie Jul 12, 2022
9ef93d8
fix linting
ChrisHuie Jul 12, 2022
4d45192
update with purpose1consent
ChrisHuie Jul 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions modules/concertBidAdapter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { logWarn, logMessage, debugTurnedOn, generateUUID } from '../src/utils.js';
import { registerBidder } from '../src/adapters/bidderFactory.js';
import { getStorageManager } from '../src/storageManager.js'
import { getStorageManager } from '../src/storageManager.js';

const BIDDER_CODE = 'concert';
const CONCERT_ENDPOINT = 'https://bids.concert.io';
Expand Down Expand Up @@ -45,7 +45,7 @@ export const spec = {
uspConsent: bidderRequest.uspConsent,
gdprConsent: bidderRequest.gdprConsent
}
}
};

payload.slots = validBidRequests.map(bidRequest => {
let slot = {
Expand All @@ -57,8 +57,9 @@ export const spec = {
slotType: bidRequest.params.slotType,
adSlot: bidRequest.params.slot || bidRequest.adUnitCode,
placementId: bidRequest.params.placementId || '',
site: bidRequest.params.site || bidderRequest.refererInfo.page
}
site: bidRequest.params.site || bidderRequest.refererInfo.page,
ref: bidderRequest.refererInfo.ref
};

return slot;
});
Expand All @@ -69,7 +70,7 @@ export const spec = {
method: 'POST',
url: `${CONCERT_ENDPOINT}/bids/prebid`,
data: JSON.stringify(payload)
}
};
},
/**
* Unpack the response from the server into a list of bids.
Expand Down Expand Up @@ -101,7 +102,7 @@ export const spec = {
creativeId: bid.creativeId,
netRevenue: bid.netRevenue,
currency: bid.currency
}
};
});

if (debugTurnedOn() && serverBody.debug) {
Expand All @@ -122,7 +123,7 @@ export const spec = {
* @return {UserSync[]} The user syncs which should be dropped.
*/
getUserSyncs: function(syncOptions, serverResponses, gdprConsent, uspConsent) {
const syncs = []
const syncs = [];
if (syncOptions.iframeEnabled && !hasOptedOutOfPersonalization()) {
let params = [];

Expand Down Expand Up @@ -203,9 +204,8 @@ function hasOptedOutOfPersonalization() {
* @param {BidderRequest} bidderRequest Object which contains any data consent signals
*/
function consentAllowsPpid(bidderRequest) {
/* NOTE: We cannot easily test GDPR consent, without the
/* NOTE: We can't easily test GDPR consent, without the
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does this comment mean? why can't you look at the tcf consent string without this npm module?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can view the consent string, yes, but if parsing it is non-trivial enough that there exists an official, specialised, and sanctioned npm module to do it, it does not seem appropriate that we would try to add that functionality ourselves to our bid adapter; especially since we do that exact step (using consent-string) in our bid server.

(For the record, this change only exists in this PR because it was a NOP commit, to try to activate the CircleCI on a fresh push.)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we provide a utility function, eg

Prebid.js/modules/appnexusBidAdapter.js

Line 559 in 0be4560

if (!hasPurpose1Consent(bidderRequest?.gdprConsent)) {

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw, this is not a change request, just trying to be helpful

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate this, @patmmccann , thank you! (I don't think those functions existed when we first wrote our adapter, so it's good to know that so we can update it.) I've updated our use of consent strings, as per these comments, and will ask Chris if they can merge these changes to this branch (which I can't do). 🙌

* `consent-string` npm module; so will have to rely on that
* happening on the bid-server. */
return !(bidderRequest.uspConsent === 'string' &&
bidderRequest.uspConsent.toUpperCase().substring(0, 2) === '1YY')
return !(bidderRequest.uspConsent === 'string' && bidderRequest.uspConsent.toUpperCase().substring(0, 2) === '1YY');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are you checking for the second character https://github.com/InteractiveAdvertisingBureau/USPrivacy/blob/f8d659f58665b4e797b99b59079fd787a386eaaa/CCPA/US%20Privacy%20String.md?plain=1#L123

Copy link
Contributor

@andrew-a-dev andrew-a-dev Jul 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is checking the first 3 characters of the string; we do not mark as consenting to PPID usage unless: (1) USPrivacy string version matches what we expect, and (2) both Notice to Opt-out and Opt-out Sale are marked as consenting. (And we don't care about LSPA.)

(Also, similarly to below: this line only shows in the diff because it looks like our linter took out the line-break; nothing consequential was changed.)

Copy link
Collaborator

@patmmccann patmmccann Jul 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the string is '1YY' or '1NY', the user is opted out

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand what you're saying, we should consider the user "opted-out" if the CCPA string starts 1NY? If so, I suspect we didn't realise that was a possible value (i.e. can they have opted-out if they weren't notified of the opt-out option?). I guess we assumed that a Y in the third character implied a Y in the second character. If that's not the case, I'll happily update the PR to ignore the second character then. (And also, your original comment makes more sense to me now; thank you for that.)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some people have started pushing '1NY' if the GPC headers in the browser indicate opt out.

}