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

Sharethrough: Change to using a closure for the callback from ajax to preserve bidObj #1108

Merged
merged 9 commits into from
May 5, 2017
Merged

Sharethrough: Change to using a closure for the callback from ajax to preserve bidObj #1108

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7c545cd
Change to using a closure for the callback from ajax to preserve bidObj
nuclearghost Apr 4, 2017
26bc7bb
fix jslint errors
rizhang Apr 6, 2017
8899ae5
update version
rizhang Apr 6, 2017
d797592
fix specs
rizhang Apr 6, 2017
ab68ddc
put entire callback inside try catch
rizhang Apr 26, 2017
9415adf
tests uses fake server
rizhang Apr 26, 2017
c6d8ea2
Merge branch 'master' into callback_closure
rizhang Apr 26, 2017
6ee0c4d
removed exposing callback
rizhang May 3, 2017
f130d09
Merge branch 'callback_closure' of github.com:sharethrough/Prebid.js …
rizhang May 3, 2017
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
24 changes: 12 additions & 12 deletions src/adapters/sharethrough.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ var bidfactory = require('../bidfactory.js');
var ajax = require('../ajax.js').ajax;

const STR_BIDDER_CODE = "sharethrough";
const STR_VERSION = "1.1.0";
const STR_VERSION = "1.2.0";

var SharethroughAdapter = function SharethroughAdapter() {

Expand All @@ -22,10 +22,16 @@ var SharethroughAdapter = function SharethroughAdapter() {
const bidRequest = bids[i];
str.placementCodeSet[bidRequest.placementCode] = bidRequest;
const scriptUrl = _buildSharethroughCall(bidRequest);
str.ajax(scriptUrl, $$PREBID_GLOBAL$$.strcallback, undefined, {withCredentials: true});
str.ajax(scriptUrl, _createCallback(bidRequest), undefined, {withCredentials: true});
}
}

function _createCallback(bidRequest) {
return (bidResponse) => {
_strcallback(bidRequest, bidResponse);
};
}

function _buildSharethroughCall(bid) {
const pkey = utils.getBidIdParameter('pkey', bid.params);

Expand All @@ -39,17 +45,10 @@ var SharethroughAdapter = function SharethroughAdapter() {
return url;
}

$$PREBID_GLOBAL$$.strcallback = function(bidResponse) {
function _strcallback(bidObj, bidResponse) {
try {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this wasn't a change in this PR, but I'd suggest moving the try { above the JSON.parse() so any errors parsing the JSON are caught so the bids are added to the bidManager as errors.

bidResponse = JSON.parse(bidResponse);
} catch (e) {
utils.logError(e);
return;
}

const bidId = bidResponse.bidId;
const bidObj = utils.getBidRequest(bidId);
try {
const bidId = bidResponse.bidId;
const bid = bidfactory.createBid(1, bidObj);
bid.bidderCode = STR_BIDDER_CODE;
bid.cpm = bidResponse.creatives[0].cpm;
Expand Down Expand Up @@ -84,7 +83,7 @@ var SharethroughAdapter = function SharethroughAdapter() {
} catch (e) {
_handleInvalidBid(bidObj);
}
};
}

function _handleInvalidBid(bidObj) {
const bid = bidfactory.createBid(2, bidObj);
Expand All @@ -102,6 +101,7 @@ var SharethroughAdapter = function SharethroughAdapter() {
return {
callBids: _callBids,
str : str,
callback: _strcallback
Copy link
Collaborator

Choose a reason for hiding this comment

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

no need to expose callback any longer

};
};

Expand Down
30 changes: 15 additions & 15 deletions test/spec/adapters/sharethrough_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,17 +63,8 @@ describe('sharethrough adapter', () => {

sinon.assert.calledTwice(adapter.str.ajax);

expect(firstBidUrl).to.contain(adapter.str.STR_BTLR_HOST + '/header-bid/v1?bidId=bidId1&placement_key=aaaa1111&hbVersion=%24prebid.version%24&strVersion=1.1.0&hbSource=prebid&');
expect(secondBidUrl).to.contain(adapter.str.STR_BTLR_HOST + '/header-bid/v1?bidId=bidId2&placement_key=bbbb2222&hbVersion=%24prebid.version%24&strVersion=1.1.0&hbSource=prebid&');
});

});

describe('strcallback', () => {

it('should exist and be a function', () => {
let shit = sandbox.stub(pbjs, 'strcallback');
expect(pbjs.strcallback).to.exist.and.to.be.a('function');
expect(firstBidUrl).to.contain(adapter.str.STR_BTLR_HOST + '/header-bid/v1?bidId=bidId1&placement_key=aaaa1111&hbVersion=%24prebid.version%24&strVersion=1.2.0&hbSource=prebid&');
expect(secondBidUrl).to.contain(adapter.str.STR_BTLR_HOST + '/header-bid/v1?bidId=bidId2&placement_key=bbbb2222&hbVersion=%24prebid.version%24&strVersion=1.2.0&hbSource=prebid&');
});

});
Expand All @@ -82,16 +73,18 @@ describe('sharethrough adapter', () => {

let firstBid;
let secondBid;
let server;

beforeEach(() => {
sandbox.stub(bidManager, 'addBidResponse');
server = sinon.fakeServer.create();

pbjs._bidsRequested.push(bidderRequest);
adapter.str.placementCodeSet['foo'] = {};
adapter.str.placementCodeSet['bar'] = {};
// respond

let bidderReponse1 = {
let bidderResponse1 = {
"adserverRequestId": "40b6afd5-6134-4fbb-850a-bb8972a46994",
"bidId": "bidId1",
"creatives": [
Expand All @@ -104,7 +97,7 @@ describe('sharethrough adapter', () => {
"stxUserId": ""
};

let bidderReponse2 = {
let bidderResponse2 = {
"adserverRequestId": "40b6afd5-6134-4fbb-850a-bb8972a46994",
"bidId": "bidId2",
"creatives": [
Expand All @@ -117,13 +110,20 @@ describe('sharethrough adapter', () => {
"stxUserId": ""
};

pbjs.strcallback(JSON.stringify(bidderReponse1));
pbjs.strcallback(JSON.stringify(bidderReponse2));
server.respondWith(/aaaa1111/,JSON.stringify(bidderResponse1));
server.respondWith(/bbbb2222/,JSON.stringify(bidderResponse2));
adapter.callBids(bidderRequest);

server.respond();

firstBid = bidManager.addBidResponse.firstCall.args[1];
secondBid = bidManager.addBidResponse.secondCall.args[1];
});

afterEach(() => {
server.restore();
});

it('should add a bid object for each bid', () => {
sinon.assert.calledTwice(bidManager.addBidResponse);
});
Expand Down