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

Row evolution data export fails when filter_limit is supplied #5397

Closed
mgazdzik opened this issue Jul 1, 2014 · 7 comments
Closed

Row evolution data export fails when filter_limit is supplied #5397

mgazdzik opened this issue Jul 1, 2014 · 7 comments
Labels
Bug For errors / faults / flaws. Critical Indicates the severity of an issue is very critical and the issue has a very high priority.
Milestone

Comments

@mgazdzik
Copy link
Contributor

mgazdzik commented Jul 1, 2014

There seems to be a bug with exporting data from row evolution for labels in action report, which have bounce rate = 100%.

Steps to reproduce:

Tasks

  • write unit or integration test reproducing this issue
  • fix bug
@mattab
Copy link
Member

mattab commented Jul 1, 2014

It seems to affect pages URLs that contain a comma character (and maybe other characters are affected?)

@quba
Copy link
Contributor

quba commented Jul 3, 2014

@tsteur
Copy link
Member

tsteur commented Jul 7, 2014

@ kuba.cc suggestion seems to be the actual issue see ResponseBuilder. I'm gonna try to move the label filter upwards just to see which test fail afterwards...

@tsteur
Copy link
Member

tsteur commented Jul 7, 2014

In df3427b: refs #5397 wondering which tests will fail when applying the label filter before limit filter & co

@tsteur
Copy link
Member

tsteur commented Jul 7, 2014

Tests fail after moving the label filter, see https://travis-ci.org/piwik/piwik/jobs/29283168

I think one problem is that "SafeDecodeLabel" ( https://github.com/piwik/piwik/blob/master/core/API/ResponseBuilder.php#L319 ) is no longer executed before the LabelFilter meaning the tests should be updated meaning we would change the API.

We could recommend to always use filter_limit = -1 when a label filter is used but doesn't sound very intuitive. Especially our export links in the UI would have to take care of this as well :( A hack to always set the filter_limit to -1 when a label filter is used results in even more problems in the end.

So basically to keep the API the same we'd have to "undo" the SafeDecodeLabel of the label url parameter and then compare. This could work

@tsteur
Copy link
Member

tsteur commented Jul 7, 2014

Couldn't really fix it without breaking anything. Maybe someone else wants to have a look?

@mgazdzik mgazdzik added this to the 2.5.0 - Piwik 2.5.0 milestone Jul 8, 2014
@diosmosis diosmosis changed the title Row evolution data export show no data for page URLs containing a comma Row evolution data export fails when filter_limit is supplied (was: Row evolution data export show no data for page URLs containing a comma) Jul 10, 2014
@diosmosis
Copy link
Member

Changing title since the bug is only present when a filter_limit is present.

@mattab mattab changed the title Row evolution data export fails when filter_limit is supplied (was: Row evolution data export show no data for page URLs containing a comma) Row evolution data export fails when filter_limit is supplied Jul 10, 2014
diosmosis pushed a commit that referenced this issue Jul 11, 2014
…lFilter: ignore filter_limit and filter_truncate when label query param used since it makes no sense to use them w/ LabelFilter.
@mattab mattab closed this as completed Jul 11, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws. Critical Indicates the severity of an issue is very critical and the issue has a very high priority.
Projects
None yet
Development

No branches or pull requests

5 participants