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.
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).
review
Excellent. One clarification...
_paq.push(["setDomains", ...
can accept paths?parse_url
to isolate the host from the pathOK. 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:
public function getJavascriptTag($idSite, $piwikUrl = '', $mergeSubdomains = false, $groupPageTitlesByDomain = false, $mergeAliasUrls = false, $newParam1 = false, $newParam2 = false, $newParam3 = false, $newParam4 = false);
Alternatively, the function could be defined more like:
public function getJavascriptTag($idSite, $piwikUrl = '', $options = array());
...where $options is an array built by the API client that includes keys like mergeSubdomains, groupPageTitlesByDomain, and mergeAliasUrls.
Better use named parameters because the web API will accept named parameters in any order and because they are all optional it looks good!
Updated:
params
variable of jsTrackingGenerator.js (line 190). In theory, it should be possible to substitute an API call for lines 216 to 270. That would also be the easiest way to thoroughly test the new code, but I don't know how you normally make API calls in JS.