-
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
Updates to PubWise Analytics - Enable config, UTM & Debug/Logging Upd… #1409
Conversation
@matthewlane @protonate @mkendall07 Not to be pushy, but if this could make it into a 0.26.x patch that is upcoming I would be grateful and I can send cookies and cupcakes. Either way, appreciate it! |
Merge conflicts have been resolved. The "default" url will have a valid response in the next 24-48 hours when the API endpoint is updated. I also fixed up the ESLint errors. |
…eAnalyticsUpdate22
…js into pubwiseAnalyticsUpdate22
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.
@GLStephen Added some comments.
I tried to enable analytics with hello_world test page.
pbjs.enableAnalytics({
provider: 'pubwise',
options: {
site: 'test-test-test-test',
endpoint: 'https://api.pubwise.io/api/v4/event/default/',
}
});
But getting 404 on your endpoint.
modules/pubwiseAnalyticsAdapter.js
Outdated
let pwAnalyticsEnabled = false; | ||
let utmKeys = {'utm_source': '', 'utm_medium': '', 'utm_campaign': '', 'utm_term': '', 'utm_content': ''}; | ||
|
||
let getParameterByName = function (name) { |
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.
This function is already defined in utils. Use that one.
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.
Done
modules/pubwiseAnalyticsAdapter.js
Outdated
} | ||
|
||
function sendEvent(eventType, data) { | ||
utils.logInfo(analyticsName + 'Event ' + eventType + ' ' + pwAnalyticsEnabled, data); |
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.
Use backticks here and other places in this file.
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.
Done
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.
Hey, I think you misunderstood me. I meant use template literals.
Check here https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals
e.g: utils.logWarn(`Test string ${variable}.`);
modules/pubwiseAnalyticsAdapter.js
Outdated
function enrichWithMetrics(dataBag) { | ||
try { | ||
dataBag['pw_version'] = pubwiseVersion; | ||
dataBag['pbjs_version'] = pbjs.version; |
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.
Use
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.
Done
@jaiminpanchal27 I took another look and getParameterByName is not exported and is not used anywhere else currently |
Oops.. Sorry for that. I will update utils and let you know. Also i replied to you on the backtick change. |
@jaiminpanchal27 I pushed up a version with the required Utils change. If that will block us from getting into the next release then I'd rather duplicate the function. If you want me to pull your change later I can do that too. However, as it stands for the adpater to work I had to include that change in my update. Also, the staging "default" API endpoint will be updated and this can be released with that endpoint 404ing. I've added valid test config data to the analytics adapter header comments. That config will work. The "default" endpoint is a "canary" for us to detect misconfigured sites. In our next update it will have info about how to fix the endpoint config, etc. Just have to deal with releases being out of phase. |
This line was added in prebid#1409, removing this then I'll merge
* Added PolluxNetwork Bid Adapter Added module, test spec and integration example for Pollux Network Bid Adapter * Update Pollux domain Update Pollux default domain on prebid adapter * Export getParameterByName method On Utils.js make getParameterByName method public * Executed changes requested by @jaiminpanchal27 on 2017-08-01 Moved zone_728x90.html to integrationExamples/gpt/pollux_zone_728x90.html; Added bidRequest as second parameter to bidfactory.createBid() on Pollux Bid Adapter; Added more test cases to increase test coverage (at least 85%); Review Ref: - #1431 (review) * Fixed Eslint errors on commit f745fe1 * Executed changes requested on PR#1431 review #54993573 - Removed $$PREBID_GLOBAL$$ public vars in unit test; - Moved stubs creation and its restoration to beforeEach and afterEach hooks in unit test; - Exposed polluxHandler method on polluxBidAdapter. * Remove redundant export This line was added in #1409, removing this then I'll merge
prebid#1409) * Updates to Build Process to Support Building DIST and DEV Together * Updates to PubWise Analytics - Enable config, UTM & Debug/Logging Updates * Fixing ESLint Errors * Updates for ESLint which is now properly installed * Proper ESLint Fix Commit without Erroneous Files * File Cleanup * Additional File Cleanup to get Back to Upstream Master * Updates for Comments from jaiminpanchal27 * Adding by in ParameterByName * Updates w/ Utils
* Added PolluxNetwork Bid Adapter Added module, test spec and integration example for Pollux Network Bid Adapter * Update Pollux domain Update Pollux default domain on prebid adapter * Export getParameterByName method On Utils.js make getParameterByName method public * Executed changes requested by @jaiminpanchal27 on 2017-08-01 Moved zone_728x90.html to integrationExamples/gpt/pollux_zone_728x90.html; Added bidRequest as second parameter to bidfactory.createBid() on Pollux Bid Adapter; Added more test cases to increase test coverage (at least 85%); Review Ref: - prebid#1431 (review) * Fixed Eslint errors on commit f745fe1 * Executed changes requested on PR#1431 review #54993573 - Removed $$PREBID_GLOBAL$$ public vars in unit test; - Moved stubs creation and its restoration to beforeEach and afterEach hooks in unit test; - Exposed polluxHandler method on polluxBidAdapter. * Remove redundant export This line was added in prebid#1409, removing this then I'll merge
* Added PolluxNetwork Bid Adapter Added module, test spec and integration example for Pollux Network Bid Adapter * Update Pollux domain Update Pollux default domain on prebid adapter * Export getParameterByName method On Utils.js make getParameterByName method public * Executed changes requested by @jaiminpanchal27 on 2017-08-01 Moved zone_728x90.html to integrationExamples/gpt/pollux_zone_728x90.html; Added bidRequest as second parameter to bidfactory.createBid() on Pollux Bid Adapter; Added more test cases to increase test coverage (at least 85%); Review Ref: - #1431 (review) * Fixed Eslint errors on commit f745fe1 * Executed changes requested on PR#1431 review #54993573 - Removed $$PREBID_GLOBAL$$ public vars in unit test; - Moved stubs creation and its restoration to beforeEach and afterEach hooks in unit test; - Exposed polluxHandler method on polluxBidAdapter. * Remove redundant export This line was added in #1409, removing this then I'll merge * Update Pollux Adapter to v1.0 * Changes requested on Pollux Adapter pull request #1694 review #74933409 * Changes requested on Pollux Adapter pull request #1694 review #75505070 Rmoved parameter bidderCode from bid responses * Fixed breaking changes to serverResponse in interpretResponse method Parameter serverResponse of method interpretResponse in bid adapter changed from array of bids to an object, where bids are now nested within its parameter body. Plus a refactor of var declaration and log messages. * Fix lint errors on push for commit cc653a
Type of change
This is an update to PubWise.io Analytics Adapter to support UTM parameters and improve loading when enableAnalytics is called in all supported places.
New Features
UTM Tracking
PubWise analytics now tracks UTM parameters and allows segments to be built on campaign in the PubWise UI, this is the foundation for additional functionality in the "Pro" version of PubWise analytics that will allow advanced segmentation
Updates