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

Desktop vs mobile fix #104

Closed
wants to merge 2 commits into from
Closed

Desktop vs mobile fix #104

wants to merge 2 commits into from

Conversation

clearcode
Copy link

Original issue is: on User settings report, after clicking evolution row for desktop vs mobile report user gets notice "No data for this graph" (checked at demo.piwik.org a minute ago). After debuging I found out that in case of Piwik_DataTable_Array after inserting empty rows, object is populated with separate index for fields. When evolution graph tries to filter out unnecesary columns, it passes original label, which is i.e. Desktop. In normal case (when there is no index) filtering is performed using actual labels (translated). On the other side, when this additional index is present ,it is used to filter out not needed rows. This leads to situation when datatable is being filtered to leave only translated label, but search is performed on untranslated index and this leads to returning of empty datatable. I figured out that the best way to avoid this situation is to block adding empty rows, and therefore additional index is not created. Maybe this isn't the best solution possible, but I'm not quite sure what is this additional index used for in other cases and this fixes Mobile vs Desktop evolution row report.

@mattab
Copy link
Member

mattab commented Sep 6, 2013

Tracked in Trac at: http://dev.piwik.org/trac/ticket/4132#ticket

mattab added a commit that referenced this pull request Sep 10, 2013
@mattab mattab closed this Sep 10, 2013
@mattab
Copy link
Member

mattab commented Sep 10, 2013

Fixed in
a80d800
test in 2c79c31#L1L165

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants