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 parameters to getJavascriptTag API call #127
Add parameters to getJavascriptTag API call #127
Conversation
Thank you for the pull request. Can I ask why you made such change? |
WP-Piwik (WordPress plugin) uses this API call to get the JS tracking code for placement on WordPress sites. I want to "Track visitors across all subdomains of..." my site, but couldn't upgrade WP-Piwik (in a scalable way at least) unless the Piwik API supported the same options. While I only needed the one option (mergeSubdomains), I figured I'd implement all three while I was messing with that part of the codebase. |
If it isn't immediately obvious, this dovetails with improvements I made to the PiwikTracker API. I use PiwikTracker to set the cookie in my ShortURL processor and then pick the user back up in WP-Piwik (once I add support for wildcard domain cookies). |
+1 Good one! |
review
|
Excellent. One clarification...
|
|
OK. I've applied the requested changes and (manually) tested the code. I might as well add the 4 advanced parameters from the Tracking Code page while I'm in this part of the code. From your viewpoint, is the best approach to continue to expand the function's parameters:
Alternatively, the function could be defined more like:
...where $options is an array built by the API client that includes keys like mergeSubdomains, groupPageTitlesByDomain, and mergeAliasUrls. |
…e Piwik Administrative interface (Tracking Code section) by adding three boolean parameters: - mergeSubdomains: Track visitors across all subdomains of <site name> - prependDomain: Prepend the site domain to the page title when tracking - hideAlias: In the "Outlinks" report, hide clicks to known alias URLs of <site name>
…verification, but leaves parsing to separate the host (required) from the path (unwanted). Renames several variables to more closely mirror jsTrackingGenerator.js.
…ManagerAPI. Added "use" SitesManagerAPI to Piwik.php.
Better use named parameters because the web API will accept named parameters in any order and because they are all optional it looks good! |
…-end logic to more closely mirror jsTrackingGenerator.js.
Updated:
|
Add parameters to getJavascriptTag API call Fixes #4247
This pull request brings the getJavascriptTag API call up to parity with the Piwik Administrative interface (Tracking Code section) by adding three boolean parameters:
Please note that the new code makes database calls. I'm not certain how you normally sanitize user input so I have applied
Common::sanitizeInputValue()
to the variables that could be used to inject SQL. Please provide guidance if you prefer a different strategy.