diff --git a/plugins/MultiSites/Dashboard.php b/plugins/MultiSites/Dashboard.php
index 623e51a9b16e30edbef1686a3c09c4922e7cd567..c757858504e3fd5a0bf690fe6763a8b345a93880 100644
--- a/plugins/MultiSites/Dashboard.php
+++ b/plugins/MultiSites/Dashboard.php
@@ -8,6 +8,7 @@
*/
namespace Piwik\Plugins\MultiSites;
+use Piwik\API\DataTablePostProcessor;
use Piwik\API\ResponseBuilder;
use Piwik\Config;
use Piwik\Metrics\Formatter;
@@ -15,6 +16,7 @@ use Piwik\Period;
use Piwik\DataTable;
use Piwik\DataTable\Row;
use Piwik\DataTable\Row\DataTableSummaryRow;
+use Piwik\Plugin\Report;
use Piwik\Plugins\API\ProcessedReport;
use Piwik\Site;
use Piwik\View;
@@ -49,6 +51,8 @@ class Dashboard
$pastData = $sites->getMetadata('pastData');
$sites->filter(function (DataTable $table) use ($pastData) {
+ $pastRow = null;
+
foreach ($table->getRows() as $row) {
$idSite = $row->getColumn('label');
$site = Site::getSite($idSite);
@@ -61,11 +65,14 @@ class Dashboard
$pastRow = $pastData->getRowFromLabel($idSite);
if ($pastRow) {
$pastRow->setColumn('label', $site['name']);
- $pastData->setLabelsHaveChanged();
}
}
}
+ if ($pastData && $pastRow) {
+ $pastData->setLabelsHaveChanged();
+ }
+
});
$this->setSitesTable($sites);
@@ -73,20 +80,20 @@ class Dashboard
public function setSitesTable(DataTable $sites)
{
- $this->numSites = $sites->getRowsCount();
$this->sitesByGroup = $this->moveSitesHavingAGroupIntoSubtables($sites);
+ $this->rememberNumberOfSites();
}
public function getSites($request, $limit)
{
- $request['filter_limit'] = $limit;
+ $request['filter_limit'] = $limit;
+ $request['filter_offset'] = isset($request['filter_offset']) ? $request['filter_offset'] : 0;
- $sitesExpanded = $this->convertDataTableToArrayAndApplyFilters($this->sitesByGroup, $request);
- $sitesFlat = $this->makeSitesFlat($sitesExpanded);
- $sitesFlat = $this->applyLimitIfNeeded($sitesFlat, $limit);
- $sitesFlat = $this->enrichValues($sitesFlat);
+ $this->makeSitesFlatAndApplyGenericFilters($this->sitesByGroup, $request);
+ $sites = $this->convertDataTableToArrayAndApplyQueuedFilters($this->sitesByGroup, $request);
+ $sites = $this->enrichValues($sites);
- return $sitesFlat;
+ return $sites;
}
public function getTotals()
@@ -107,7 +114,11 @@ class Dashboard
public function search($pattern)
{
$this->nestedSearch($this->sitesByGroup, $pattern);
+ $this->rememberNumberOfSites();
+ }
+ private function rememberNumberOfSites()
+ {
$this->numSites = $this->sitesByGroup->getRowsCountRecursive();
}
@@ -153,19 +164,13 @@ class Dashboard
return $lastPeriod;
}
- private function convertDataTableToArrayAndApplyFilters(DataTable $table, $request)
+ private function convertDataTableToArrayAndApplyQueuedFilters(DataTable $table, $request)
{
$request['serialize'] = 0;
- $request['expanded'] = 1;
+ $request['expanded'] = 0;
$request['totals'] = 0;
$request['format_metrics'] = 1;
-
- // filter_sort_column does not work correctly is a bug in MultiSites.getAll
- if (!empty($request['filter_sort_column']) && $request['filter_sort_column'] === 'nb_pageviews') {
- $request['filter_sort_column'] = 'Actions_nb_pageviews';
- } elseif (!empty($request['filter_sort_column']) && $request['filter_sort_column'] === 'revenue') {
- $request['filter_sort_column'] = 'Goal_revenue';
- }
+ $request['disable_generic_filters'] = 1;
$responseBuilder = new ResponseBuilder('php', $request);
$rows = $responseBuilder->getResponse($table, 'MultiSites', 'getAll');
@@ -234,7 +239,8 @@ class Dashboard
/**
* Makes sure to not have any subtables anymore.
- * So if $sites is
+ *
+ * So if $table is
* array(
* site1
* site2
@@ -257,41 +263,39 @@ class Dashboard
* site7
* )
*
- * @param $sites
- * @return array
+ * in a sorted order
+ *
+ * @param DataTable $table
+ * @param array $request
*/
- private function makeSitesFlat($sites)
+ private function makeSitesFlatAndApplyGenericFilters(DataTable $table, $request)
{
- $flatSites = array();
+ // we handle limit here as we have to apply sort filter, then make sites flat, then apply limit filter.
+ $filterOffset = $request['filter_offset'];
+ $filterLimit = $request['filter_limit'];
+ unset($request['filter_offset']);
+ unset($request['filter_limit']);
- foreach ($sites as $site) {
- if (!empty($site['subtable'])) {
- if (isset($site['idsubdatatable'])) {
- unset($site['idsubdatatable']);
- }
-
- $subtable = $site['subtable'];
- unset($site['subtable']);
- $flatSites[] = $site;
- foreach ($subtable as $siteWithinGroup) {
- $flatSites[] = $siteWithinGroup;
- }
- } else {
- $flatSites[] = $site;
- }
+ // filter_sort_column does not work correctly is a bug in MultiSites.getAll
+ if (!empty($request['filter_sort_column']) && $request['filter_sort_column'] === 'nb_pageviews') {
+ $request['filter_sort_column'] = 'Actions_nb_pageviews';
+ } elseif (!empty($request['filter_sort_column']) && $request['filter_sort_column'] === 'revenue') {
+ $request['filter_sort_column'] = 'Goal_revenue';
}
- return $flatSites;
- }
+ // make sure no limit filter is applied, we will do this manually
+ $table->disableFilter('Limit');
- private function applyLimitIfNeeded($sites, $limit)
- {
- // why do we need to apply a limit again? because we made sitesFlat and it may contain many more sites now
- if ($limit > 0) {
- $sites = array_slice($sites, 0, $limit);
- }
+ // this will apply the sort filter
+ /** @var DataTable $table */
+ $genericFilter = new DataTablePostProcessor('MultiSites', 'getAll', $request);
+ $table = $genericFilter->applyGenericFilters($table);
- return $sites;
+ // make sure from now on the sites will be no longer sorted, they were already sorted
+ $table->disableFilter('Sort');
+
+ // make sites flat and limit
+ $table->filter('Piwik\Plugins\MultiSites\DataTable\Filter\NestedSitesLimiter', array($filterOffset, $filterLimit));
}
private function enrichValues($sites)
diff --git a/plugins/MultiSites/DataTable/Filter/NestedSitesLimiter.php b/plugins/MultiSites/DataTable/Filter/NestedSitesLimiter.php
new file mode 100644
index 0000000000000000000000000000000000000000..7151dcb8c75faa72bb30718d8bc66427201156d2
--- /dev/null
+++ b/plugins/MultiSites/DataTable/Filter/NestedSitesLimiter.php
@@ -0,0 +1,138 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ *
+ */
+namespace Piwik\Plugins\MultiSites\DataTable\Filter;
+
+use Piwik\DataTable\BaseFilter;
+use Piwik\DataTable\Row;
+use Piwik\DataTable;
+
+/**
+ * Makes sure to not have any subtables anymore and applies the limit to the flattened table.
+ *
+ * So if $table is
+ * array(
+ * site1
+ * group2
+ * subtable => site3
+ * site4
+ * site5
+ * site6
+ * site7
+ * )
+ *
+ * it will format this to
+ *
+ * array(
+ * site1
+ * group2
+ * site3
+ * site4
+ * site5
+ * site6
+ * site7
+ * )
+ *
+ * and then apply the limit filter.
+ *
+ * Each group will not count into the limit/offset. This way, if one requests a limit of 50 sites,
+ * we make sure to return 50 sites.
+ *
+ * @param $sites
+ * @return array
+ */
+class NestedSitesLimiter extends BaseFilter
+{
+ private $offset = 0;
+ private $limit = 0;
+ /**
+ * @var Row[]
+ */
+ private $rows = array();
+
+ /**
+ * Constructor.
+ *
+ * @param DataTable $table The table to eventually filter.
+ */
+ public function __construct($table, $offset, $limit)
+ {
+ parent::__construct($table);
+ $this->offset = $offset;
+ $this->limit = $limit;
+ }
+
+ /**
+ * @param DataTable $table
+ */
+ public function filter($table)
+ {
+ $numRows = 0;
+ $lastGroupFromPreviousPage = null;
+
+ foreach ($table->getRows() as $row) {
+
+ $this->addRowIfNeeded($row, $numRows);
+ $numRows++;
+
+ $subtable = $row->getSubtable();
+ if ($subtable) {
+ if (!$this->hasRows()) {
+ $lastGroupFromPreviousPage = $row;
+ }
+ foreach ($subtable->getRows() as $subRow) {
+ $this->addRowIfNeeded($subRow, $numRows);
+ $numRows++;
+ }
+ $row->removeSubtable();
+ }
+
+ if ($this->hasNumberOfRequestedRowsFound()) {
+ break;
+ }
+ }
+
+ $this->prependGroupIfFirstSiteBelongsToAGroupButGroupIsMissingInRows($lastGroupFromPreviousPage);
+
+ $table->setRows($this->rows);
+ }
+
+ private function hasNumberOfRequestedRowsFound()
+ {
+ return count($this->rows) >= $this->limit;
+ }
+
+ private function hasRows()
+ {
+ return count($this->rows) !== 0;
+ }
+
+ private function addRowIfNeeded(Row $row, $numRows)
+ {
+ $inOffset = $numRows >= $this->offset;
+
+ if ($inOffset && !$this->hasNumberOfRequestedRowsFound()) {
+ $this->rows[] = $row;
+ }
+ }
+
+ /**
+ * @param Row $lastGroupFromPreviousPage
+ */
+ private function prependGroupIfFirstSiteBelongsToAGroupButGroupIsMissingInRows($lastGroupFromPreviousPage)
+ {
+ if ($lastGroupFromPreviousPage && !empty($this->rows)) {
+ // the result starts with a row that does belong to a group, we make sure to show this group before that site
+ $group = reset($this->rows)->getMetadata('group');
+ if ($group && $lastGroupFromPreviousPage->getColumn('label') === $group) {
+ array_unshift($this->rows, $lastGroupFromPreviousPage);
+ // we do not remove the last item as it could result in errors, instead we show $limit+1 entries
+ }
+ }
+ }
+}
\ No newline at end of file
diff --git a/plugins/MultiSites/tests/Integration/DashboardTest.php b/plugins/MultiSites/tests/Integration/DashboardTest.php
index 33d671cbb2f352e75d33a7a2fec9f13c9dfe6b09..525cd95941ab9554522cb391c9ab7724227ebd7c 100644
--- a/plugins/MultiSites/tests/Integration/DashboardTest.php
+++ b/plugins/MultiSites/tests/Integration/DashboardTest.php
@@ -138,6 +138,24 @@ class DashboardTest extends IntegrationTestCase
$this->assertSame(4, $this->dashboard->getNumSites());
}
+ public function test_getNumSites_ShouldCountGroupsIntoResult()
+ {
+ $sites = $this->setSitesTable(20);
+
+ $this->setGroupForSiteId($sites, $siteId = 1, 'group1');
+ $this->setGroupForSiteId($sites, $siteId = 2, 'group2');
+ $this->setGroupForSiteId($sites, $siteId = 3, 'group1');
+ $this->setGroupForSiteId($sites, $siteId = 4, 'group4');
+ $this->setGroupForSiteId($sites, $siteId = 15, 'group1');
+ $this->setGroupForSiteId($sites, $siteId = 16, 'group1');
+ $this->setGroupForSiteId($sites, $siteId = 18, 'group1');
+ $this->setGroupForSiteId($sites, $siteId = 6, 'group4');
+ $this->dashboard->setSitesTable($sites);
+
+ // 3 different groups
+ $this->assertSame(20 + 3, $this->dashboard->getNumSites());
+ }
+
public function test_getSites_shouldReturnAnArrayOfSites()
{
$this->setSitesTable(8);
@@ -156,6 +174,81 @@ class DashboardTest extends IntegrationTestCase
$this->assertEquals($expectedSites, $this->dashboard->getSites(array(), $limit = 4));
}
+ public function test_getSites_ShouldApplyLimitCorrectIfThereAreLessFirstLevelRowsThenLimit()
+ {
+ $sites = $this->setSitesTable(8);
+
+ $this->setGroupForSiteId($sites, $siteId = 1, 'group1');
+ $this->setGroupForSiteId($sites, $siteId = 2, 'group2');
+ $this->setGroupForSiteId($sites, $siteId = 3, 'group1');
+ $this->setGroupForSiteId($sites, $siteId = 4, 'group4');
+ $this->setGroupForSiteId($sites, $siteId = 5, 'group4');
+ $this->setGroupForSiteId($sites, $siteId = 6, 'group4');
+ $this->setGroupForSiteId($sites, $siteId = 7, 'group4');
+ $this->dashboard->setSitesTable($sites);
+
+ $expectedSites = array(
+ array ('label' => 'group1',
+ 'nb_visits' => 20,
+ 'isGroup' => 1,
+ ), array ('label' => 'Site1',
+ 'nb_visits' => 10,
+ 'group' => 'group1',
+ ), array ('label' => 'Site3',
+ 'nb_visits' => 10,
+ 'group' => 'group1',
+ ), array ('label' => 'group2',
+ 'nb_visits' => 10,
+ 'isGroup' => 1,
+ ), array ('label' => 'Site2',
+ 'nb_visits' => 10,
+ 'group' => 'group2',
+ ), array ('label' => 'Site8',
+ 'nb_visits' => 10,
+ ),
+ );
+
+ // there will be 4 first level entries (group1, group2, group4 and site8), offset is 5, limit is 6.
+ // See https://github.com/piwik/piwik/issues/7854 before there was no site returned since 5 > 4 first level entries
+
+ $this->assertEquals($expectedSites, $this->dashboard->getSites(array('filter_offset' => 5), $limit = 6));
+ }
+
+ public function test_getSites_ShouldReturnOneMoreGroup_IfFirstSiteBelongsToAGroupButGroupWouldBeNormallyNotInResult()
+ {
+ $sites = $this->setSitesTable(8);
+
+ $this->setGroupForSiteId($sites, $siteId = 1, 'group1');
+ $this->setGroupForSiteId($sites, $siteId = 2, 'group2');
+ $this->setGroupForSiteId($sites, $siteId = 3, 'group1');
+ $this->setGroupForSiteId($sites, $siteId = 4, 'group4');
+ $this->setGroupForSiteId($sites, $siteId = 5, 'group4');
+ $this->setGroupForSiteId($sites, $siteId = 6, 'group4');
+ $this->setGroupForSiteId($sites, $siteId = 7, 'group4');
+ $this->dashboard->setSitesTable($sites);
+
+ $expectedSites = array(
+ array ('label' => 'group4', // this group should be the added group, that's why there are 5 entries
+ 'nb_visits' => 40,
+ 'isGroup' => 1,
+ ), array ('label' => 'Site6',
+ 'nb_visits' => 10,
+ 'group' => 'group4',
+ ), array ('label' => 'Site7',
+ 'nb_visits' => 10,
+ 'group' => 'group4',
+ ), array ('label' => 'group1',
+ 'nb_visits' => 20,
+ 'isGroup' => 1,
+ ), array ('label' => 'Site1',
+ 'nb_visits' => 10,
+ 'group' => 'group1',
+ )
+ );
+
+ $this->assertEquals($expectedSites, $this->dashboard->getSites(array('filter_offset' => 3), $limit = 4));
+ }
+
public function test_getSites_WithGroup_shouldApplyALimitAndKeepSitesWithinGroup()
{
$sites = $this->setSitesTable(20);
@@ -172,8 +265,8 @@ class DashboardTest extends IntegrationTestCase
$expectedSites = array (
array (
- 'label' => 'group1',
- 'nb_visits' => 50, // there are 5 matching sites having that group, we only return 4, still result is correct!
+ 'label' => 'group1', // do not count group into the limit
+ 'nb_visits' => 50, // there are 5 matching sites having that group, we only return 3, still result is correct!
'isGroup' => 1,
), array (
'label' => 'Site1',