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
Various performance improvements and bugfixes. #7465
Conversation
Imporves performance for Archiving and Range dates. Makes all kind of reports faster as well. Fixed bugs in labelFilter, reports total calculation and more.
@@ -120,6 +121,7 @@ private function getGenericFiltersHavingDefaultValues() | |||
foreach ($filters as $index => $filter) { | |||
if ($filter[0] === 'Sort') { | |||
$filters[$index][1]['filter_sort_column'] = array('string', $this->report->getDefaultSortColumn()); | |||
$filters[$index][1]['filter_sort_order'] = array('string', $this->report->getDefaultSortOrder()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reports need to be able to specify a default sort order. Eg when defaultSortOrder should be label, asc
@@ -90,6 +90,6 @@ | |||
<reportTotal> | |||
<nb_visits>8</nb_visits> | |||
<nb_hits>13</nb_hits> | |||
<exit_nb_visits>1</exit_nb_visits> | |||
<exit_nb_visits>2</exit_nb_visits> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another example where one can see that it fixes report total calculation as mentioned before.
Looks good to me 👍 merging |
Various performance improvements and bugfixes.
refs #4768 #7388 #6763
Improves performance for
Fixed bugs in LabelFilter, ReportsTotalCalculation, Sort and more.
A new feature is that we do now actually sort API output by
nb_visits, labels
by default which was often not the case. Also we do now sort by labels if both columns do not have a value. Eg if two rows do not havenb_visits
or any otherfilter_sort_column
then the order was random. We do now sort by label in such a case.I didn't create a pull request for each fix separately since they only shows after tweaking some code and where not really visible before.
A few UI tests fail since we do now sort by label if two rows have both no value set. See eg http://builds-artifacts.piwik.org/ui-tests.datatable_tweaks/11069.7/screenshot-diffs/singlediff.html?processed=../processed-ui-screenshots/PivotByDimension_pivoted.png&expected=PivotByDimension_pivoted.png&github=PivotByDimension_pivoted.png
For more information see my comments in the code. It would be kinda nice to have this in 2.12.0 since it brings many performance improvements.