@mnapoli opened this issue on August 3rd 2015

Fixes #7978, replaces #8050

Problems: - Excluded User Agents is a comma-separated list, but user agents can contain commas - we want to let users wrap user agents in quotes (see https://github.com/piwik/piwik/issues/7978#issuecomment-121509999) which is currently not supported - the inline help for Excluded User Agents doesn't say how to format user agent strings (#7978) - other fields in the page ask for one item per line: - it's not consistent with Excluded User Agents (which is comma-separated), so it's confusing - in the code it's actually comma-separated, so it's clearly misleading and broken for some users

These problems apply to global and site-specific settings.

Goals: - [x] consistent formatting for all fields (and both global and site-specific settings) - [x] excluded user agents - [x] excluded query parameters - [x] excluded IPs - [x] it needs to be "one per line" because of user agents (that can contain commas) - [x] enclosing quotes need to be accepted (and trimmed) for user agents - [x] field helps need to match the actual behavior - [x] migration script - [x] tests - [x] update changelog regarding API changes

Notes: - enclosing quotes for user agents are trimmed (for the other fields I didn't think it would make sense) - the help text for user agents has also been updated (only the english version), it now explicitly says that user agents should be separated by new lines. What is the best course of action to ensure translations are updated? New translation key? - The following API methods are now returning an array of strings instead of a list encoded in CSV: - SitesManager.getExcludedIpsGlobal - SitesManager.getExcludedQueryParametersGlobal - SitesManager.getExcludedUserAgentsGlobal - The following API methods now accept values separated by new lines instead of commas: - SitesManager.setGlobalExcludedIps - SitesManager.setGlobalExcludedQueryParameters - SitesManager.setGlobalExcludedUserAgents

@mnapoli commented on August 3rd 2015

Tests are OK, the only fails are for UI screenshots where the translation has been changed: http://builds-artifacts.piwik.org/piwik/piwik/bug-7978/14619/

@tsteur commented on August 13th 2015

Is this PR similar to #8368 ? Also needs a rebase.

Re HTTP API break maybe @mattab can have a look

@mattab commented on August 13th 2015

@mnapoli can you target this PR for 3.0 branch? we will break the API there and keep 2.15.0 without API breakage :+1:

@mnapoli commented on August 13th 2015

Is this PR similar to #8368?

Not really it's a more global refactoring and improvement to fix 4 different issues (see the PR description).

Will target 3.0.

@tsteur commented on August 13th 2015

So we still need to merge #8368 on top?

@mnapoli commented on August 13th 2015

No, this PR fixes #7978.

@mnapoli commented on August 17th 2015

Closed in favor of #8586 that is targetting 3.0.

This issue was closed on August 17th 2015
Powered by GitHub Issue Mirror