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

Jump to bottom

Websites manager: Add possibility to exclude URL parameters matching a regular expressions #10846

Merged
merged 2 commits into from Nov 30, 2016
Merged

Websites manager: Add possibility to exclude URL parameters matching a regular expressions #10846

merged 2 commits into from Nov 30, 2016

Conversation

pafgoncalves
Copy link
Contributor

Allows to use regular expressions in the exclude parameters option.
Doesn't break compatibility if the parameter is not a regular expression, doing a strict compare.
"/.*/" can be used to exclude all parameters.

@pafgoncalves
Add regular expressions to the exclude parameters
645accf 
Allows to use regular expressions in the exclude parameters option.
Doesn't break compatibility if the parameter is not a regular expression, doing a strict compare.
"/.*/" can be used to exclude all parameters.
@pafgoncalves pafgoncalves mentioned this pull request Nov 9, 2016
Open
@mattab mattab added the Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. label Nov 11, 2016
@mattab mattab added this to the 3.0.0-b4 milestone Nov 11, 2016
@mattab
Copy link
Member

mattab commented Nov 11, 2016

Thanks @pafgoncalves for the PR! That'd be a useful new feature indeed. Here is my review:

  • in the UI it would be useful to mention that the excluding parameters list supports regular expression and give an example eg. "You can also use a regular expression such as XYZ". What would be a good XYZ realistic regex example?
  • could you maybe add a unit test in UrlHelperTest file, to ensure that in the future this feature is never broken and always works? Unfortunately there was no test already for the
  • ideally the integration test in tests/PHPUnit/Integration/Tracker/ActionTest.php would also have new test cases with regex (in getTestUrls)

we're looking forward to merging this after these few changes!

Changed the UI texts to explain that the excluding parameters list su…
d92319c 
…pports regular expressions.

Added unit tests.
@pafgoncalves
Copy link
Contributor Author

I have made the requested changes.
See if it's ok.

@mattab
Copy link
Member

mattab commented Nov 30, 2016

@pafgoncalves Thanks for the follow up and this great PR 👍 Would love to see you contribute more to Piwik as this is fantastic contribution.

@mattab mattab merged commit fa2fca9 into matomo-org:3.x-dev Nov 30, 2016
@mattab mattab changed the title Add regular expressions to the exclude parameters Websites manager: Add possibility to exclude URL parameters matching a regular expressions Nov 30, 2016
@mattab mattab added the Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. label Nov 30, 2016
@mattab mattab mentioned this pull request Nov 30, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review.
Projects
None yet
Milestone
3.0.0-b4
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@pafgoncalves @mattab