Skip to content

Commit

Permalink
ENH Add configuration to cap report counts for large datasets. (#139)
Browse files Browse the repository at this point in the history
Opted not to return the result from `getCount()` directly since the PHPDoc says this returns an int (even though it technically already returns a string if the report isn't set up correctly...)
  • Loading branch information
GuySartorelli authored Jan 31, 2022
1 parent 0cb2654 commit 375c319
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 3 deletions.
10 changes: 10 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,16 @@ The reports section will not show up in the CMS if:
* There are no reports to show
* The logged in user does not have permission to view any reports

For large datasets, the reports section may take a long time to load, since each report is getting a count of the items it contains to display next to the title.

To mitigate this issue, there is a cap on the number of items that will be counted per report. This is set at 10,000 items by default, but can be configured using the `limit_count_in_overview` configuration variable. Setting this to `null` will result in showing the actual count regardless of how many items there are.

```yml
SilverStripe\Reports\Report:
limit_count_in_overview: 500
```
Note that some reports may have overridden the `getCount` method, and for those reports this may not apply.

## Links ##

* [License](./LICENSE)
41 changes: 39 additions & 2 deletions code/Report.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
use SilverStripe\ORM\CMSPreviewable;
use SilverStripe\ORM\DataList;
use SilverStripe\ORM\DataQuery;
use SilverStripe\ORM\Limitable;
use SilverStripe\ORM\SS_List;
use SilverStripe\Security\Member;
use SilverStripe\Security\Permission;
Expand Down Expand Up @@ -105,6 +106,14 @@ class Report extends ViewableData
SideReportWrapper::class,
];

/**
* The maximum number of items to include in the count in the reports overview
*
* @config
* @var int|null
*/
private static $limit_count_in_overview = 10000;

/**
* Return the title of this report.
*
Expand Down Expand Up @@ -216,18 +225,46 @@ protected function sanitiseClassName($class)
/**
* counts the number of objects returned
* @param array $params - any parameters for the sourceRecords
* @param int|null $limit - the maximum number of records to count
* @return int
*/
public function getCount($params = array())
public function getCount($params = array(), $limit = null)
{
$sourceRecords = $this->sourceRecords($params, null, null);
$sourceRecords = $this->sourceRecords($params, null, $limit);
if (!$sourceRecords instanceof SS_List) {
user_error(static::class . "::sourceRecords does not return an SS_List", E_USER_NOTICE);
return "-1";
}
// Some reports may not use the $limit parameter in sourceRecords since it isn't actually
// used anywhere else - so make sure we limit record counts if possible.
if ($sourceRecords instanceof Limitable) {
$sourceRecords = $sourceRecords->limit($limit);
}
return $sourceRecords->count();
}

/**
* Counts the number of objects returned up to a configurable limit.
*
* Large datasets can cause performance issues for some reports if allowed to count all records.
* To mitigate this, you can set the limit_count_in_overview config variable to the maximum number
* of items you wish to count to. Counts will be limited to this value, and any counts that hit
* this limit will be displayed with a plus, e.g. "500+"
*
* The default is to have no limit.
*
* @return string
*/
public function getCountForOverview(): string
{
$limit = $this->config()->get('limit_count_in_overview');
$count = $this->getCount([], $limit);
if ($limit && $count == $limit) {
$count = "$count+";
}
return "$count";
}

/**
* Return an array of excluded reports. That is, reports that will not be included in
* the list of reports in report admin in the CMS.
Expand Down
2 changes: 1 addition & 1 deletion code/ReportAdmin.php
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ public function getEditForm($id = null, $fields = null)
));

$columns->setFieldFormatting(array(
'title' => '<a href=\"$Link\" class=\"grid-field__link-block\">$value ($Count)</a>'
'title' => '<a href=\"$Link\" class=\"grid-field__link-block\">$value ($CountForOverview)</a>'
));
$gridField->addExtraClass('all-reports-gridfield');
$fields->push($gridField);
Expand Down
16 changes: 16 additions & 0 deletions tests/ReportTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -114,4 +114,20 @@ public function testColumnLink()
$titleContent
);
}

public function testCountForOverview()
{
$report = new ReportTest\FakeTest3();

// Count is limited to 10000 by default
$this->assertEquals('10000+', $report->getCountForOverview());

// Count is limited as per configuration
Config::modify()->set(ReportTest\FakeTest3::class, 'limit_count_in_overview', 15);
$this->assertEquals('15+', $report->getCountForOverview());

// A null limit displays the full count
Config::modify()->set(ReportTest\FakeTest3::class, 'limit_count_in_overview', null);
$this->assertEquals('15000', $report->getCountForOverview());
}
}
15 changes: 15 additions & 0 deletions tests/ReportTest/FakeTest3.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php

namespace SilverStripe\Reports\Tests\ReportTest;

use SilverStripe\Dev\TestOnly;
use SilverStripe\ORM\ArrayList;
use SilverStripe\Reports\Report;

class FakeTest3 extends Report implements TestOnly
{
public function sourceRecords($params, $sort, $limit)
{
return new ArrayList(range(1, 15000));
}
}

0 comments on commit 375c319

Please sign in to comment.