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.

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

Test Bid + Crid #1

Merged
merged 4 commits into from
Dec 11, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 13 additions & 10 deletions modules/33acrossBidAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ function _createBidResponse(response) {
height: response.seatbid[0].bid[0].h,
ad: response.seatbid[0].bid[0].adm,
ttl: response.seatbid[0].bid[0].ttl || 60,
creativeId: response.seatbid[0].bid[0].ext.rp.advid,
creativeId: response.seatbid[0].bid[0].crid,
currency: response.cur,
netRevenue: true
}
Expand All @@ -27,6 +27,9 @@ function _createServerRequest(bidRequest) {
const ttxRequest = {};
const params = bidRequest.params;

/*
* Infer data for the request payload
*/
ttxRequest.imp = [];
ttxRequest.imp[0] = {
banner: {
Expand All @@ -38,22 +41,27 @@ function _createServerRequest(bidRequest) {
}
}
}

ttxRequest.site = { id: params.siteId };

// Go ahead send the bidId in request to 33exchange so it's kept track of in the bid response and
// therefore in ad targetting process
ttxRequest.id = bidRequest.bidId;
// Finally, set the openRTB 'test' param if this is to be a test bid
if (params.test === 1) {
ttxRequest.test = 1;
}

/*
* Now construt the full server request
*/
const options = {
contentType: 'application/json',
withCredentials: false
};

// Allow the ability to configure the HB endpoint for testing purposes.
const ttxSettings = config.getConfig('ttxSettings');
const url = (ttxSettings && ttxSettings.url) || END_POINT;

// Return the server request
return {
'method': 'POST',
'url': url,
Expand Down Expand Up @@ -93,12 +101,7 @@ function isBidRequestValid(bid) {
return false;
}

if ((typeof bid.params.site === 'undefined' || typeof bid.params.site.id === 'undefined') &&
(typeof bid.params.siteId === 'undefined')) {
return false;
}

if (typeof bid.params.productId === 'undefined') {
if (typeof bid.params.siteId === 'undefined' || typeof bid.params.productId === 'undefined') {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Still not allowing test params to be passed via site object because:

  1. The test bid feature makes this redundant
  2. The test endpoint can now be set in pbjs.config object as follows:
     pbjs.setConfig({
           userSync: {
               iframeEnabled: true, 
               syncDelay: 3000
           },
           ttxSettings: {
               url: 'https://staging-ssc.33across.com/api/v1/hb',
               syncUrl: 'https://staging-de.tynt.com/deb/v2?m=xch'
           }
       });
    

return false;
}

Expand Down
192 changes: 109 additions & 83 deletions test/spec/modules/33acrossBidAdapter_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ const { userSync } = require('../../../src/userSync');
const { config } = require('../../../src/config');

const { expect } = require('chai');

const {
isBidRequestValid,
buildRequests,
Expand Down Expand Up @@ -44,78 +43,71 @@ describe('33acrossBidAdapter:', function () {
});

describe('isBidRequestValid:', function () {
context('valid bid request:', function () {
Copy link
Collaborator Author

@curlyblueeagle curlyblueeagle Nov 20, 2017

Choose a reason for hiding this comment

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

Had to remove wrapping context since doing so failed nesting rules in linter

Choose a reason for hiding this comment

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

Interesting. Something along the lines of "there aren't enough test cases in this context / describe", or something like that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh no, 'twas too many nested calls. Their rules allowed a max of 3

it('returns true when bidder, params.siteId, params.productId are set', function() {
const validBid = {
bidder: BIDDER_CODE,
params: {
siteId: SITE_ID,
productId: PRODUCT_ID
}
it('returns true when valid bid request is sent', function() {
const validBid = {
bidder: BIDDER_CODE,
params: {
siteId: SITE_ID,
productId: PRODUCT_ID
}
}

expect(isBidRequestValid(validBid)).to.be.true;
})
expect(isBidRequestValid(validBid)).to.be.true;
});

context('valid test bid request:', function () {
it('returns true when bidder, params.site.id, params.productId are set', function() {
const validBid = {
bidder: BIDDER_CODE,
params: {
site: {
id: SITE_ID
},
productId: PRODUCT_ID
}
it('returns true when valid test bid request is sent', function() {
const validBid = {
bidder: BIDDER_CODE,
params: {
siteId: SITE_ID,
productId: PRODUCT_ID,
test: 1
}
}

expect(isBidRequestValid(validBid)).to.be.true;
});
expect(isBidRequestValid(validBid)).to.be.true;
});

context('invalid bid request:', function () {
it('returns false when bidder not set to "33across"', function () {
const invalidBid = {
bidder: 'foo',
params: {
siteId: SITE_ID,
productId: PRODUCT_ID
}
it('returns false when bidder not set to "33across"', function () {
const invalidBid = {
bidder: 'foo',
params: {
siteId: SITE_ID,
productId: PRODUCT_ID
}
}

expect(isBidRequestValid(invalidBid)).to.be.false;
});
expect(isBidRequestValid(invalidBid)).to.be.false;
});

it('returns false when params not set', function() {
const invalidBid = {
bidder: 'foo'
}
it('returns false when params not set', function() {
const invalidBid = {
bidder: 'foo'
}

expect(isBidRequestValid(invalidBid)).to.be.false;
});
expect(isBidRequestValid(invalidBid)).to.be.false;
});

it('returns false when params.siteId or params.site.id not set', function() {
const invalidBid = {
bidder: 'foo',
params: {
productId: PRODUCT_ID
}
it('returns false when site ID is not set in params', function() {
const invalidBid = {
bidder: 'foo',
params: {
productId: PRODUCT_ID
}
}

expect(isBidRequestValid(invalidBid)).to.be.false;
});
expect(isBidRequestValid(invalidBid)).to.be.false;
});

it('returns false when params.productId not set', function() {
const invalidBid = {
bidder: 'foo',
params: {
siteId: SITE_ID
}
it('returns false when product ID not set in params', function() {
const invalidBid = {
bidder: 'foo',
params: {
siteId: SITE_ID
}
}

expect(isBidRequestValid(invalidBid)).to.be.false;
});
expect(isBidRequestValid(invalidBid)).to.be.false;
});
});

Expand Down Expand Up @@ -148,40 +140,89 @@ describe('33acrossBidAdapter:', function () {
},
id: 'b1'
};
const serverRequest = {
'method': 'POST',
'url': END_POINT,
'data': JSON.stringify(ttxRequest),
'options': {
'contentType': 'application/json',
'withCredentials': false
}
}
const builtServerRequests = buildRequests(this.bidRequests);
expect(builtServerRequests).to.deep.equal([ serverRequest ]);
expect(builtServerRequests.length).to.equal(1);
});

it('returns corresponding test server requests for each valid bidRequest', function() {
this.sandbox.stub(config, 'getConfig', () => {
return {
'url': 'https://foo.com/hb/'
}
});

const ttxRequest = {
imp: [ {
banner: {
format: [
{
w: 300,
h: 250,
ext: { }
},
{
w: 728,
h: 90,
ext: { }
}
]
},
ext: {
ttx: {
prod: PRODUCT_ID
}
}
} ],
site: {
id: SITE_ID
},
id: 'b1'
};
const serverRequest = {
method: 'POST',
url: END_POINT,
url: 'https://foo.com/hb/',
data: JSON.stringify(ttxRequest),
options: {
contentType: 'application/json',
withCredentials: false
}
}
};

const builtServerRequests = buildRequests(this.bidRequests);
expect(builtServerRequests).to.deep.equal([ serverRequest ]);
expect(builtServerRequests.length).to.equal(1);
});

it('returns corresponding test server requests for each valid bidRequest', function() {
it('returns corresponding test server requests for each valid test bidRequest', function() {
this.sandbox.stub(config, 'getConfig', () => {
return {
'url': 'https://foo.com/hb/'
}
});

this.bidRequests[0].params.test = 1;
const ttxRequest = {
imp: [ {
banner: {
format: [
{
w: 300,
h: 250,
ext: {}
ext: { }
},
{
w: 728,
h: 90,
ext: {}
ext: { }
}
]
},
Expand All @@ -194,7 +235,8 @@ describe('33acrossBidAdapter:', function () {
site: {
id: SITE_ID
},
id: 'b1'
id: 'b1',
test: 1
};
const serverRequest = {
method: 'POST',
Expand Down Expand Up @@ -268,11 +310,7 @@ describe('33acrossBidAdapter:', function () {
bid: [ {
id: '1',
adm: '<html><h3>I am an ad</h3></html>',
ext: {
rp: {
advid: 1
}
},
crid: 1,
h: 250,
w: 300,
price: 0.0938
Expand Down Expand Up @@ -322,23 +360,15 @@ describe('33acrossBidAdapter:', function () {
bid: [ {
id: '1',
adm: '<html><h3>I am an ad</h3></html>',
ext: {
rp: {
advid: 1
}
},
crid: 1,
h: 250,
w: 300,
price: 0.0940
},
{
id: '2',
adm: '<html><h3>I am an ad</h3></html>',
ext: {
rp: {
advid: 2
}
},
crid: 2,
h: 250,
w: 300,
price: 0.0938
Expand All @@ -349,11 +379,7 @@ describe('33acrossBidAdapter:', function () {
bid: [ {
id: '3',
adm: '<html><h3>I am an ad</h3></html>',
ext: {
rp: {
advid: 3
}
},
crid: 3,
h: 250,
w: 300,
price: 0.0938
Expand All @@ -373,7 +399,7 @@ describe('33acrossBidAdapter:', function () {
creativeId: 1,
currency: 'USD',
netRevenue: true
}
};

expect(interpretResponse({ body: serverResponse }, this.serverRequest)).to.deep.equal([ bidResponse ]);
});
Expand Down