-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
SublimeBidAdapter: Update to version 0.5.1 #4977
Conversation
Hello, taking a look at this now. We do not allow adapters to fire pixels inside of non registered pixel events, so sending an event for every bidResponse is not allowed. Adapters should only be sending pixels on the allowed events like, onTimeout, onBidWon etc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see the comment and remove any pixel's which are fired outside of the allowed event hooks.
* Update sublimeBidAdapter to 0.5.1 * Add tests for private functions * Remove window.sublime * Update pixel name for bid event
e0feea2
to
e14b9e9
Compare
modules/sublimeBidAdapter.js
Outdated
const bidResponses = []; | ||
const response = serverResponse.body; | ||
|
||
sendEvent('dintres'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pixel fires outside of the pixel listen hooks is not allowed. please remove this.
modules/sublimeBidAdapter.js
Outdated
|
||
bidResponses.push(bidResponse); | ||
} | ||
sendEvent('bida', true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above, please remove
modules/sublimeBidAdapter.js
Outdated
sendEvent('bida', true); | ||
bidResponses.push(bidResponse); | ||
} else { | ||
sendEvent('dnobid'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove
modules/sublimeBidAdapter.js
Outdated
*/ | ||
function onTimeout(timeoutData) { | ||
log('Timeout from adapter', timeoutData); | ||
sendEvent('dbidtimeout', true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pixels are allowed onTimeout so you can leave this
* add prebid version of adapter * Feature/update sublime adapter (#21) * Update sublimeBidAdapter to 0.5.1 * Add tests for private functions * Remove window.sublime * Update pixel name for bid event * Remove pixels on non-event and add onBidWon * Incremente version of sublimeBidAdapter * Renamed pixel for timeout and introduce gvlid * Remove unnecessary params for sendEvent Co-Authored-By: fgcloutier <fg.cloutier@sublimeskinz.com> Co-authored-by: Gaby <gaby.hourlier@sublimeskinz.com> Co-authored-by: fgcloutier <fg.cloutier@sublimeskinz.com>
Hello @robertrmartinez, sorry for the delay! |
Oops, it seems that we have errors with the CI. |
CI should be fine now! |
Hi sorry for the delay, have been swamped. Will get this reviewed and merged tomorrow! |
Cool, thank you! Let me know if I missed anything! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome stuff.
Thanks for this!
* add prebid version of adapter * Feature/update sublime adapter (prebid#21) * Update sublimeBidAdapter to 0.5.1 * Add tests for private functions * Remove window.sublime * Update pixel name for bid event * Remove pixels on non-event and add onBidWon (prebid#22) * add prebid version of adapter * Feature/update sublime adapter (prebid#21) * Update sublimeBidAdapter to 0.5.1 * Add tests for private functions * Remove window.sublime * Update pixel name for bid event * Remove pixels on non-event and add onBidWon * Incremente version of sublimeBidAdapter * Renamed pixel for timeout and introduce gvlid * Remove unnecessary params for sendEvent Co-Authored-By: fgcloutier <fg.cloutier@sublimeskinz.com> Co-authored-by: Gaby <gaby.hourlier@sublimeskinz.com> Co-authored-by: fgcloutier <fg.cloutier@sublimeskinz.com> * Remove trailing-space * Fix version in tests Co-authored-by: Gaby <gaby.hourlier@sublimeskinz.com> Co-authored-by: fgcloutier <fg.cloutier@sublimeskinz.com>
Type of change
Description of change
Hello, we made changes on our adapter!
We modified the code style, added some pixels to have more visibility on our adapter behaviour and made debug easier.