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

Jump to bottom

Media.net Adapter Improvements #2634

Merged
merged 4 commits into from
Jun 8, 2018
Merged

Media.net Adapter Improvements #2634

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d746d01
Media.net adapter: passing slot visibility to server
binoy-chitale May 25, 2018
06a82aa
Media.net adapter: changed slot visibility logic and removed mocks on…
binoy-chitale Jun 4, 2018
4e7a59a
Media.net adapter: refactored slot-visibility tests to use beforeEach…
binoy-chitale Jun 8, 2018
79560e8
create only one sandbox and restore for medianet
snapwich Jun 8, 2018
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
33 changes: 24 additions & 9 deletions modules/medianetBidAdapter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,11 @@ function getCoordinates(id) {
let coordinates = {};
coordinates.top_left = {
y: rect.top,
x: rect.left,
x: rect.left
};
coordinates.bottom_right = {
y: rect.bottom,
x: rect.right,
x: rect.right
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

To get the distance from the top left corner of the document (as opposed to viewport) this code should be

    coordinates.top_left = {
      y: rect.top + window.pageYOffset,
      x: rect.left + window.pageXOffset,
    };
    coordinates.bottom_right = {
      y: rect.bottom + window.pageYOffset,
      x: rect.right + window.pageXOffset,
    };

return coordinates
}
Expand All @@ -110,7 +110,7 @@ function extParams(params, gdpr) {
if (ext.gdpr_applies) {
ext.gdpr_consent_string = gdpr.consentString || '';
}
let windowSize = getWindowSize();
let windowSize = spec.getWindowSize();
if (windowSize.w !== -1 && windowSize.h !== -1) {
ext.screen = windowSize;
}
Expand Down Expand Up @@ -138,10 +138,10 @@ function slotParams(bidRequest) {
}
const coordinates = getCoordinates(bidRequest.adUnitCode);
if (coordinates) {
params.ext.coordinates = coordinates;
let viewability = getSlotVisibility(coordinates.top_left, getMinSize(params.banner));
params.ext.viewability = viewability;
if (viewability > 0.5) {
let normCoordinates = normalizeCoordinates(coordinates);
params.ext.coordinates = normCoordinates;
params.ext.viewability = getSlotVisibility(coordinates.top_left, getMinSize(params.banner));
if (getSlotVisibility(normCoordinates.top_left, getMinSize(params.banner)) > 0.5) {
params.ext.visibility = SLOT_VISIBILITY.ABOVE_THE_FOLD;
} else {
params.ext.visibility = SLOT_VISIBILITY.BELOW_THE_FOLD;
Expand All @@ -159,7 +159,7 @@ function getMinSize(sizes) {

function getSlotVisibility(topLeft, size) {
let maxArea = size.w * size.h;
let windowSize = getWindowSize();
let windowSize = spec.getWindowSize();
let bottomRight = {
x: topLeft.x + size.w,
y: topLeft.y + size.h
Expand All @@ -181,6 +181,19 @@ function getOverlapArea(topLeft1, bottomRight1, topLeft2, bottomRight2) {
return ((Math.min(bottomRight1.x, bottomRight2.x) - Math.max(topLeft1.x, topLeft2.x)) * (Math.min(bottomRight1.y, bottomRight2.y) - Math.max(topLeft1.y, topLeft2.y)));
}

function normalizeCoordinates(coordinates) {
return {
top_left: {
x: coordinates.top_left.x + window.pageXOffset,
y: coordinates.top_left.y + window.pageYOffset,
},
bottom_right: {
x: coordinates.bottom_right.x + window.pageXOffset,
y: coordinates.bottom_right.y + window.pageYOffset,
}
}
}

function generatePayload(bidRequests, bidderRequests) {
return {
site: siteDetails(bidRequests[0].params.site),
Expand Down Expand Up @@ -280,6 +293,8 @@ export const spec = {
if (syncOptions.pixelEnabled) {
return filterUrlsByType(cookieSyncUrls, 'image');
}
}
},

getWindowSize,
};
registerBidder(spec);
36 changes: 17 additions & 19 deletions test/spec/modules/medianetBidAdapter_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -501,12 +501,8 @@ describe('Media.net bid adapter', () => {

describe('buildRequests', () => {
let sandbox;
let mock;
before(() => {
sandbox = sinon.sandbox.create();
mock = sinon.mock(window);
window.innerWidth = 1000;
window.innerHeight = 1000;
let documentStub = sandbox.stub(document, 'getElementById');
let boundingRect = {
top: 50,
Expand All @@ -520,10 +516,14 @@ describe('Media.net bid adapter', () => {
documentStub.withArgs('div-gpt-ad-1460505748561-0').returns({
getBoundingClientRect: () => boundingRect
});
let windowSizeStub = sandbox.stub(spec, 'getWindowSize');
windowSizeStub.returns({
w: 1000,
h: 1000
});
});
after(() => {
sandbox.restore();
mock.restore();
});
it('should build valid payload on bid', () => {
let requestObj = spec.buildRequests(VALID_BID_REQUEST, VALID_AUCTIONDATA);
Expand Down Expand Up @@ -566,17 +566,16 @@ describe('Media.net bid adapter', () => {
});

describe('slot visibility', () => {
let mock;
let sandbox = sinon.sandbox.create();
Copy link
Collaborator

@snapwich snapwich Jun 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

You should just put the sandbox creation in the outermost describe block's beforeEach and the restore in its afterEach. Doing this multiple times in different nested-levels of describe make it difficult to see if the sandbox is being properly cleaned up.

edit - just edited to say that they should be in a beforeEach and afterEach as well, as opposed to before and after which only run once per describe block.

before(() => {
mock = sinon.mock(window);
window.innerWidth = 1000;
window.innerHeight = 1000;
});
after(() => {
mock.restore();
let windowSizeStub = sandbox.stub(spec, 'getWindowSize');
windowSizeStub.returns({
w: 1000,
h: 1000
});
});
after(() => sandbox.restore());
it('slot visibility should be 2 and ratio 0 when ad unit is BTF', () => {
let sandbox = sinon.sandbox.create();
let documentStub = sandbox.stub(document, 'getElementById');
let boundingRect = {
top: 1010,
Expand All @@ -590,14 +589,14 @@ describe('Media.net bid adapter', () => {
documentStub.withArgs('div-gpt-ad-1460505748561-0').returns({
getBoundingClientRect: () => boundingRect
});

let bidReq = spec.buildRequests(VALID_BID_REQUEST, VALID_AUCTIONDATA);
let data = JSON.parse(bidReq.data);
expect(data.imp[0].ext.visibility).to.equal(2);
expect(data.imp[0].ext.viewability).to.equal(0);
sandbox.restore();
documentStub.restore();
Copy link
Collaborator

@snapwich snapwich Jun 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Since the documentStub was created with the sandbox it should be cleaned up by the sandbox automatically by the sandbox.restore() in the afterEach. Doing documentStub.restore() might double restore, which can do weird things.

});
it('slot visibility should be 2 and ratio < 0.5 when ad unit is partially inside viewport', () => {
let sandbox = sinon.sandbox.create();
let documentStub = sandbox.stub(document, 'getElementById');
let boundingRect = {
top: 990,
Expand All @@ -615,10 +614,9 @@ describe('Media.net bid adapter', () => {
let data = JSON.parse(bidReq.data);
expect(data.imp[0].ext.visibility).to.equal(2);
expect(data.imp[0].ext.viewability).to.equal(100 / 75000);
sandbox.restore();
documentStub.restore();
});
it('slot visibility should be 1 and ratio > 0.5 when ad unit mostly in viewport', () => {
let sandbox = sinon.sandbox.create();
let documentStub = sandbox.stub(document, 'getElementById');
let boundingRect = {
top: 800,
Expand All @@ -636,7 +634,7 @@ describe('Media.net bid adapter', () => {
let data = JSON.parse(bidReq.data);
expect(data.imp[0].ext.visibility).to.equal(1);
expect(data.imp[0].ext.viewability).to.equal(40000 / 75000);
sandbox.restore();
documentStub.restore();
});
it('co-ordinates should not be sent and slot visibility should be 0 when ad unit is not present', () => {
let bidReq = spec.buildRequests(VALID_BID_REQUEST, VALID_AUCTIONDATA);
Expand Down Expand Up @@ -679,7 +677,7 @@ describe('Media.net bid adapter', () => {
it('should not push response if no-bid', () => {
let validBids = [];
let bids = spec.interpretResponse(SERVER_RESPONSE_NOBID, []);
expect(bids).to.deep.equal(validBids)
expect(bids).to.deep.equal(validBids);
});
});
});