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.

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

Add PubWise Analytics #1151

Merged
merged 2 commits into from
May 15, 2017
Merged

Add PubWise Analytics #1151

merged 2 commits into from
May 15, 2017

Conversation

GLStephen
Copy link
Collaborator

@GLStephen GLStephen commented Apr 24, 2017

Type of change

  • [ X ] Other - Analytics Adapter

Description of change

This is the analytics adapter for http://PubWise.io. PubWise.io provides a full featured analytics package for Prebid.js. In addition to the analytics package PubWise Includes the ability to fully manage configuration, deployment and optimization of header bidding. The PubWise analytics package is free for all sites of any size and integrates fully with our monetization platform. PubWise also supports advanced reporting features such as discrepancy reporting all the way to demand partner and machine learning based optimization of header bidding configuration.

Copy link
Collaborator

@matthewlane matthewlane left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, a few comments for your review below


export default pubwiseAnalytics;


Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick: extra blank line not needed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll remove that when I fix the above.

if (args !== undefined && args.config !== undefined) {
target_site = args.config.site;
}
if (args !== undefined && args.config !== undefined) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This line is the same as line 43, is that on purpose? When running tests on this PR there are a few WARN [web-server]: 404: /undefined warnings in the console, which may be coming from line 47. Adding && args.config.endpoint before the closing ) on 46 stops those warnings, but I may be misunderstanding the intention here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The config, and sometimes args, is not available in future calls of _track after the initial call of enable analytics. Overriding/extending enable led to other issues specifically multiple analytics adapters would no longer load. Since I have not been able to fix either of those other issues globally this was my compromise. I will clean up the logic here and look into the WARN [web-server]: 404: /undefined issue.

@GLStephen
Copy link
Collaborator Author

I have updated the analytics adapter based on feedback as well as removed a bit of event filtering since we've updated the endpoint to handle determination of event handling at the API endpoint.

@GLStephen
Copy link
Collaborator Author

Downloader addition is here prebid/prebid.github.io#242 Thanks!

@GLStephen
Copy link
Collaborator Author

@matthewlane @protonate Any idea what version this might land in? 24? Just trying to plan ahead. Thanks!

@mkendall07
Copy link
Member

LGTM.

@mkendall07 mkendall07 merged commit 0b9dc7b into prebid:master May 15, 2017
bfmdeploy pushed a commit to bfmdeploy/Prebid.js that referenced this pull request May 19, 2017
* PubWise Analytics

* Updates based on Feedback
mkendall07 pushed a commit that referenced this pull request May 19, 2017
* Added ad unit size to bid request

* Fixed lint errors

* Added ad unit size to bid request

* Prebid 0.23.1 Release

* Add trafficSourceCode + test (#1184)

* pre release version bump

* inclusion of popular Nordic ad sizes to default size list (#1168)

inclusion of popularNordic ad sizes to default size list

Removed redundant 104 size and added size 32 instead

Fixed trailing comma

* Add PubWise Analytics (#1151)

* PubWise Analytics

* Updates based on Feedback

* add new rp_secure param to rubicon adapter (#1190)

* Add type conversion into PrebidServer to handle inconsistent types. (#1195)

* Add type conversion into prebxdserver to handle inconsistent types.

* Only check for params that exist.

* adding test

* fix string conversion and add unit tests

* fix for code review

* add debug output

* remove accidental commit

* newline

* Add dynamic bidfloor parameter to Smart Adserver Adapter (#1194)

* Smart AdServer adapter

Add Smart AdServer adapter with tests

* fix not supported method

Replace startsWith which is not supported in all browser version by
lastIndexOf.

* Issue with optional parameter

Fix issue when no targeting is specified and remove "undefined" value
passed in url

* Add dynamic bidfloor option in the SmartAdServer prebid call.

* Bug fix: bids served by secure creatives does not get pushed into _winningBids (#1192)

* Upgrade linters to ESLint with stricter code style (#1111)

* Upgraded to ESLint with stricter code style, for both sources and tests
* Updated some dependencies and the yarn.lock file

* Add Support for DigiTrust in Rubicon Adapter (#1201)

* Add support for DigiTrust

* Add rubicon tests covering digitrust failures

* Remove whitespace in Rubicon adapter

* HIRO Media: Remove batching mechanism and use AJAX instead of JSONP (#1133)

* Remove batching mechanism and use AJAX instead of JSONP

* Fix `undefined` value checks

* Rename secureCreatives file and lint (#1203)

* Rename secureCreatives file and lint

* Updated package script for linting

* Use 'gulp run-tests' in package script for testing

* updated tag (#1212)

* Common user-sync (#1144)

* Changed “bidRequest” to “bid” for clarity
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants