Skip to content

XCH-2897 coppa support #4

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

Open
anthonyjl92 wants to merge 3 commits into
base: master
Choose a base branch
from
Open

XCH-2897 coppa support #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
920e7ab
coppa support
Jun 22, 2023
c675633
set for all requests
Jun 22, 2023
2663b60
feedback
Jun 23, 2023
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
5 changes: 5 additions & 0 deletions modules/33acrossBidAdapter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@ function _createServerRequest({ bidRequests, gdprConsent = {}, uspConsent, pageU
const ttxRequest = {};
const firstBidRequest = bidRequests[0];
const { siteId, test } = firstBidRequest.params;
const coppaValue = config.getConfig('coppa');

/*
* Infer data for the request payload
Expand Down Expand Up @@ -296,6 +297,10 @@ function _createServerRequest({ bidRequests, gdprConsent = {}, uspConsent, pageU
});
}

if (coppaValue !== undefined) {
ttxRequest.regs.coppa = Number(!!coppaValue);
}

ttxRequest.ext = {
ttx: {
prebidStartedAt: Date.now(),
Expand Down
6 changes: 4 additions & 2 deletions modules/33acrossIdSystem.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import { logMessage, logError } from '../src/utils.js';
import { ajaxBuilder } from '../src/ajax.js';
import { submodule } from '../src/hook.js';
import { uspDataHandler } from '../src/adapterManager.js';
import { uspDataHandler, coppaDataHandler } from '../src/adapterManager.js';

const MODULE_NAME = '33acrossId';
const API_URL = 'https://lexicon.33across.com/v1/envelope';
Expand Down Expand Up @@ -37,11 +37,13 @@ function getEnvelope(response) {
function calculateQueryStringParams(pid, gdprConsentData) {
const uspString = uspDataHandler.getConsentData();
const gdprApplies = Boolean(gdprConsentData?.gdprApplies);
const coppaValue = coppaDataHandler.getCoppa();
const params = {
pid,
gdpr: Number(gdprApplies),
src: CALLER_NAME,
ver: '$prebid.version$'
ver: '$prebid.version$',
coppa: Number(coppaValue)
};

if (uspString) {
Expand Down
51 changes: 51 additions & 0 deletions test/spec/modules/33acrossBidAdapter_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,14 @@ describe('33acrossBidAdapter:', function () {
return this;
};

this.withCoppa = coppaValue => {
Copy link

@carlosfelix carlosfelix Jun 23, 2023

Choose a reason for hiding this comment

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

what if you just add the coppa signal when it's present.

e.g.

const coppaValue = config.getConfig('coppa');

if (coppaValue !== undefined) {
  ttxRequest.regs.coppa = Number(!!coppaValue);
}

wouldn't that help to reduce the .withCoppa(0) calls in the specs and also be more in sync with other signals that we add to the request only when they are present such as the us_privacy and gpp. LMK what you think about this suggestion.

Object.assign(ttxRequest.regs, {
coppa: coppaValue
});

return this;
};

this.withSite = site => {
Object.assign(ttxRequest, { site });
return this;
Expand Down Expand Up @@ -1059,6 +1067,7 @@ describe('33acrossBidAdapter:', function () {
.withBanner()
.withProduct()
.withGdprConsent('foobarMyPreference', 1)
.withCoppa(1)
.build();
const serverRequest = new ServerRequestBuilder()
.withData(ttxRequest)
Expand Down Expand Up @@ -1094,6 +1103,7 @@ describe('33acrossBidAdapter:', function () {
const ttxRequest = new TtxRequestBuilder()
.withBanner()
.withProduct()
.withCoppa(1)
.build();
const serverRequest = new ServerRequestBuilder()
.withData(ttxRequest)
Expand Down Expand Up @@ -1138,6 +1148,7 @@ describe('33acrossBidAdapter:', function () {
.withBanner()
.withProduct()
.withUspConsent('foo')
.withCoppa(1)
.build();
const serverRequest = new ServerRequestBuilder()
.withData(ttxRequest)
Expand Down Expand Up @@ -1173,6 +1184,7 @@ describe('33acrossBidAdapter:', function () {
const ttxRequest = new TtxRequestBuilder()
.withBanner()
.withProduct()
.withCoppa(1)
.build();
const serverRequest = new ServerRequestBuilder()
.withData(ttxRequest)
Expand All @@ -1184,6 +1196,42 @@ describe('33acrossBidAdapter:', function () {
});
});

context('when coppa is enabled', function() {
it('returns corresponding server requests with coppa: 1', function() {
sandbox.stub(config, 'getConfig').withArgs('coppa').returns(true);

const ttxRequest = new TtxRequestBuilder()
.withBanner()
.withProduct()
.withCoppa(1)
.build();
const serverRequest = new ServerRequestBuilder()
.withData(ttxRequest)
.build();
const [ builtServerRequest ] = spec.buildRequests(bidRequests, bidderRequest);

validateBuiltServerRequest(builtServerRequest, serverRequest);
});
});

context('when coppa is not enabled', function() {
it('returns corresponding server requests with coppa: 0', function() {
sandbox.stub(config, 'getConfig').withArgs('coppa').returns(false);
Copy link

@carlosfelix carlosfelix Jun 23, 2023

Choose a reason for hiding this comment

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

most likely getConfig('coppa') will return undefined, that will be the common scenario, so I think you could adjust this spec to cover this case as well.. my recommendation is the following.

e.g.

context('when coppa is NOT enabled', function() {
      it('returns corresponding server requests with coppa: 0', function() {
        [false, undefined].forEach((value)=> {
          sandbox.stub(config, 'getConfig').withArgs('coppa').returns(value);
           // the rest of the spec
        })

Although this "undefined" case scenario might not be needed if we only add coppa to the request when the value is different than undefined, like I am suggesting in the other comment.


const ttxRequest = new TtxRequestBuilder()
.withBanner()
.withProduct()
.withCoppa(0)
.build();
const serverRequest = new ServerRequestBuilder()
.withData(ttxRequest)
.build();
const [ builtServerRequest ] = spec.buildRequests(bidRequests, bidderRequest);

validateBuiltServerRequest(builtServerRequest, serverRequest);
});
});

context('when refererInfo values are available', function() {
context('when refererInfo.page is defined', function() {
it('returns corresponding server requests with site.page set', function() {
Expand Down Expand Up @@ -1780,12 +1828,14 @@ describe('33acrossBidAdapter:', function () {
.withProduct('siab')
.withBanner()
.withVideo()
.withCoppa(1)
.build();

const req2 = new TtxRequestBuilder('sample33xGUID123456780')
.withProduct('siab')
.withBanner()
.withVideo()
.withCoppa(1)
.build();

req2.imp[0].id = 'b3';
Expand All @@ -1794,6 +1844,7 @@ describe('33acrossBidAdapter:', function () {
.withProduct('inview')
.withBanner()
.withVideo()
.withCoppa(1)
.build();

req3.imp[0].id = 'b4';
Expand Down
44 changes: 43 additions & 1 deletion test/spec/modules/33acrossIdSystem_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { thirthyThreeAcrossIdSubmodule } from 'modules/33acrossIdSystem.js';
import * as utils from 'src/utils.js';

import { server } from 'test/mocks/xhr.js';
import { uspDataHandler } from 'src/adapterManager.js';
import { uspDataHandler, coppaDataHandler } from 'src/adapterManager.js';

describe('33acrossIdSystem', () => {
describe('name', () => {
Expand Down Expand Up @@ -157,6 +157,48 @@ describe('33acrossIdSystem', () => {
});
});

context('when coppa is enabled', () => {
it('should call endpoint with an enabled coppa signal', () => {
const completeCallback = () => {};
const { callback } = thirthyThreeAcrossIdSubmodule.getId({
params: {
pid: '12345'
}
});

sinon.stub(coppaDataHandler, 'getCoppa').returns(true);

callback(completeCallback);

const [request] = server.requests;

expect(request.url).to.contain('coppa=1');

coppaDataHandler.getCoppa.restore();
});
});

context('when coppa is not enabled', () => {
it('should call endpoint with coppa signal not enabled', () => {
const completeCallback = () => {};
const { callback } = thirthyThreeAcrossIdSubmodule.getId({
params: {
pid: '12345'
}
});

sinon.stub(coppaDataHandler, 'getCoppa').returns(false);

callback(completeCallback);

const [request] = server.requests;

expect(request.url).to.contain('coppa=0');

coppaDataHandler.getCoppa.restore();
});
});

context('when the partner ID is not given', () => {
it('should log an error', () => {
const logErrorSpy = sinon.spy(utils, 'logError');
Expand Down