Skip to content
Extraits de code Groupes Projets
Valider d7b7fa2c rédigé par Benaka's avatar Benaka
Parcourir les fichiers

Merge pull request #7877 from piwik/7854 

Fixes #7854, make sure to apply the limit filter on sites in groups instead of just groups and top level sites in all website dashboard.
parents 62f48382 fd88b281
Aucune branche associée trouvée
Aucune étiquette associée trouvée
Aucune requête de fusion associée trouvée
Masquer les modifications d'espaces
En ligne Côte à côte
plugins/MultiSites/Dashboard.php
+ 50
− 46
Voir le fichier @ d7b7fa2c
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
*/ */
namespace Piwik\Plugins\MultiSites; namespace Piwik\Plugins\MultiSites;
use Piwik\API\DataTablePostProcessor;
use Piwik\API\ResponseBuilder; use Piwik\API\ResponseBuilder;
use Piwik\Config; use Piwik\Config;
use Piwik\Metrics\Formatter; use Piwik\Metrics\Formatter;
...@@ -15,6 +16,7 @@ use Piwik\Period; ...@@ -15,6 +16,7 @@ use Piwik\Period;
use Piwik\DataTable; use Piwik\DataTable;
use Piwik\DataTable\Row; use Piwik\DataTable\Row;
use Piwik\DataTable\Row\DataTableSummaryRow; use Piwik\DataTable\Row\DataTableSummaryRow;
use Piwik\Plugin\Report;
use Piwik\Plugins\API\ProcessedReport; use Piwik\Plugins\API\ProcessedReport;
use Piwik\Site; use Piwik\Site;
use Piwik\View; use Piwik\View;
...@@ -49,6 +51,8 @@ class Dashboard ...@@ -49,6 +51,8 @@ class Dashboard
$pastData = $sites->getMetadata('pastData'); $pastData = $sites->getMetadata('pastData');
$sites->filter(function (DataTable $table) use ($pastData) { $sites->filter(function (DataTable $table) use ($pastData) {
$pastRow = null;
foreach ($table->getRows() as $row) { foreach ($table->getRows() as $row) {
$idSite = $row->getColumn('label'); $idSite = $row->getColumn('label');
$site = Site::getSite($idSite); $site = Site::getSite($idSite);
...@@ -61,11 +65,14 @@ class Dashboard ...@@ -61,11 +65,14 @@ class Dashboard
$pastRow = $pastData->getRowFromLabel($idSite); $pastRow = $pastData->getRowFromLabel($idSite);
if ($pastRow) { if ($pastRow) {
$pastRow->setColumn('label', $site['name']); $pastRow->setColumn('label', $site['name']);
$pastData->setLabelsHaveChanged();
} }
} }
} }
if ($pastData && $pastRow) {
$pastData->setLabelsHaveChanged();
}
}); });
$this->setSitesTable($sites); $this->setSitesTable($sites);
...@@ -73,20 +80,20 @@ class Dashboard ...@@ -73,20 +80,20 @@ class Dashboard
public function setSitesTable(DataTable $sites) public function setSitesTable(DataTable $sites)
{ {
$this->numSites = $sites->getRowsCount();
$this->sitesByGroup = $this->moveSitesHavingAGroupIntoSubtables($sites); $this->sitesByGroup = $this->moveSitesHavingAGroupIntoSubtables($sites);
$this->rememberNumberOfSites();
} }
public function getSites($request, $limit) 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); $this->makeSitesFlatAndApplyGenericFilters($this->sitesByGroup, $request);
$sitesFlat = $this->makeSitesFlat($sitesExpanded); $sites = $this->convertDataTableToArrayAndApplyQueuedFilters($this->sitesByGroup, $request);
$sitesFlat = $this->applyLimitIfNeeded($sitesFlat, $limit); $sites = $this->enrichValues($sites);
$sitesFlat = $this->enrichValues($sitesFlat);
return $sitesFlat; return $sites;
} }
public function getTotals() public function getTotals()
...@@ -107,7 +114,11 @@ class Dashboard ...@@ -107,7 +114,11 @@ class Dashboard
public function search($pattern) public function search($pattern)
{ {
$this->nestedSearch($this->sitesByGroup, $pattern); $this->nestedSearch($this->sitesByGroup, $pattern);
$this->rememberNumberOfSites();
}
private function rememberNumberOfSites()
{
$this->numSites = $this->sitesByGroup->getRowsCountRecursive(); $this->numSites = $this->sitesByGroup->getRowsCountRecursive();
} }
...@@ -153,19 +164,13 @@ class Dashboard ...@@ -153,19 +164,13 @@ class Dashboard
return $lastPeriod; return $lastPeriod;
} }
private function convertDataTableToArrayAndApplyFilters(DataTable $table, $request) private function convertDataTableToArrayAndApplyQueuedFilters(DataTable $table, $request)
{ {
$request['serialize'] = 0; $request['serialize'] = 0;
$request['expanded'] = 1; $request['expanded'] = 0;
$request['totals'] = 0; $request['totals'] = 0;
$request['format_metrics'] = 1; $request['format_metrics'] = 1;
$request['disable_generic_filters'] = 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';
}
$responseBuilder = new ResponseBuilder('php', $request); $responseBuilder = new ResponseBuilder('php', $request);
$rows = $responseBuilder->getResponse($table, 'MultiSites', 'getAll'); $rows = $responseBuilder->getResponse($table, 'MultiSites', 'getAll');
...@@ -234,7 +239,8 @@ class Dashboard ...@@ -234,7 +239,8 @@ class Dashboard
/** /**
* Makes sure to not have any subtables anymore. * Makes sure to not have any subtables anymore.
* So if $sites is *
* So if $table is
* array( * array(
* site1 * site1
* site2 * site2
...@@ -257,41 +263,39 @@ class Dashboard ...@@ -257,41 +263,39 @@ class Dashboard
* site7 * site7
* ) * )
* *
* @param $sites * in a sorted order
* @return array *
* @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) { // filter_sort_column does not work correctly is a bug in MultiSites.getAll
if (!empty($site['subtable'])) { if (!empty($request['filter_sort_column']) && $request['filter_sort_column'] === 'nb_pageviews') {
if (isset($site['idsubdatatable'])) { $request['filter_sort_column'] = 'Actions_nb_pageviews';
unset($site['idsubdatatable']); } elseif (!empty($request['filter_sort_column']) && $request['filter_sort_column'] === 'revenue') {
} $request['filter_sort_column'] = 'Goal_revenue';
$subtable = $site['subtable'];
unset($site['subtable']);
$flatSites[] = $site;
foreach ($subtable as $siteWithinGroup) {
$flatSites[] = $siteWithinGroup;
}
} else {
$flatSites[] = $site;
}
} }
return $flatSites; // make sure no limit filter is applied, we will do this manually
} $table->disableFilter('Limit');
private function applyLimitIfNeeded($sites, $limit) // this will apply the sort filter
{ /** @var DataTable $table */
// why do we need to apply a limit again? because we made sitesFlat and it may contain many more sites now $genericFilter = new DataTablePostProcessor('MultiSites', 'getAll', $request);
if ($limit > 0) { $table = $genericFilter->applyGenericFilters($table);
$sites = array_slice($sites, 0, $limit);
}
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) private function enrichValues($sites)
......
plugins/MultiSites/DataTable/Filter/NestedSitesLimiter.php 0 → 100644
+ 138
− 0
Voir le fichier @ d7b7fa2c
<?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
plugins/MultiSites/tests/Integration/DashboardTest.php
+ 95
− 2
Voir le fichier @ d7b7fa2c
...@@ -138,6 +138,24 @@ class DashboardTest extends IntegrationTestCase ...@@ -138,6 +138,24 @@ class DashboardTest extends IntegrationTestCase
$this->assertSame(4, $this->dashboard->getNumSites()); $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() public function test_getSites_shouldReturnAnArrayOfSites()
{ {
$this->setSitesTable(8); $this->setSitesTable(8);
...@@ -156,6 +174,81 @@ class DashboardTest extends IntegrationTestCase ...@@ -156,6 +174,81 @@ class DashboardTest extends IntegrationTestCase
$this->assertEquals($expectedSites, $this->dashboard->getSites(array(), $limit = 4)); $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() public function test_getSites_WithGroup_shouldApplyALimitAndKeepSitesWithinGroup()
{ {
$sites = $this->setSitesTable(20); $sites = $this->setSitesTable(20);
...@@ -172,8 +265,8 @@ class DashboardTest extends IntegrationTestCase ...@@ -172,8 +265,8 @@ class DashboardTest extends IntegrationTestCase
$expectedSites = array ( $expectedSites = array (
array ( array (
'label' => 'group1', 'label' => 'group1', // do not count group into the limit
'nb_visits' => 50, // there are 5 matching sites having that group, we only return 4, still result is correct! 'nb_visits' => 50, // there are 5 matching sites having that group, we only return 3, still result is correct!
'isGroup' => 1, 'isGroup' => 1,
), array ( ), array (
'label' => 'Site1', 'label' => 'Site1',
......
0% ou .
You are about to add 0 people to the discussion. Proceed with caution.
Terminez d'abord l'édition de ce message.
Veuillez vous inscrire ou vous pour commenter