Adomik Analytics Adapter #1536
Adomik Analytics Adapter #1536
Conversation
|
Hi @dbemiller, I closed the previous pull request for greater clarity. Regards, Yann. |
modules/adomikAnalyticsAdapter.js
Outdated
| track({ eventType, args }) { | ||
| switch (eventType) { | ||
| case auctionInit: | ||
| currentContext = { |
| describe('Adomik Prebid Analytic', function () { | ||
| describe('enableAnalytics', function () { | ||
| it('should catch all events', function () { | ||
| sinon.spy(adomikAnalytics, 'track'); |
There was a problem hiding this comment.
spies need to be initialized & restored in beforeEach and afterEach blocks.
| events.emit(constants.EVENTS.BID_TIMEOUT, {}); | ||
| events.emit(constants.EVENTS.AUCTION_END, {}); | ||
|
|
||
| sinon.assert.callCount(adomikAnalytics.track, 6); |
There was a problem hiding this comment.
This doesn't really test your code at all. The prebid-core code is responsible for making sure that track is called when events are emitted.
Your tests need to make sure that the track implementation does what it's supposed to.
modules/adomikAnalyticsAdapter.js
Outdated
| splittedUrl.forEach((split, i) => { | ||
| const partUrl = `${split}&id=${currentContext.id}&part=${i}&on=${splittedUrl.length - 1}`; | ||
| const img = new Image(1, 1); | ||
| img.src = 'http://' + currentContext.url + '/?q=' + partUrl; |
There was a problem hiding this comment.
Are you sure that this makes a request on all the browsers you care about?
I'm cool with it if you are... but most people would use XMLHttpRequest. I'm not sure if anything in the HTML spec guarantees that the browser makes the request just by setting the src on an image, so it might throw off your stats.
|
Hi @dbemiller, I updated the code to be consistent with other adapters. Tell me if there's something wrong. Regards, Yann. |
Type of change
Description of change
New Adomik analytics adapter to integrate our script in Prebid.js