From 375c31904f71b93e8d1d1e3d760ad02539621d44 Mon Sep 17 00:00:00 2001 From: GuySartorelli <36352093+GuySartorelli@users.noreply.github.com> Date: Mon, 31 Jan 2022 17:20:11 +1300 Subject: [PATCH] ENH Add configuration to cap report counts for large datasets. (#139) 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...) --- README.md | 10 +++++++++ code/Report.php | 41 ++++++++++++++++++++++++++++++++-- code/ReportAdmin.php | 2 +- tests/ReportTest.php | 16 +++++++++++++ tests/ReportTest/FakeTest3.php | 15 +++++++++++++ 5 files changed, 81 insertions(+), 3 deletions(-) create mode 100644 tests/ReportTest/FakeTest3.php diff --git a/README.md b/README.md index 37482647..0f680828 100644 --- a/README.md +++ b/README.md @@ -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) diff --git a/code/Report.php b/code/Report.php index f1dad08f..fa783d0c 100644 --- a/code/Report.php +++ b/code/Report.php @@ -25,6 +25,7 @@ use SilverStripe\ORM\ArrayList; 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; @@ -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. * @@ -216,18 +225,46 @@ class Report extends ViewableData /** * 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. diff --git a/code/ReportAdmin.php b/code/ReportAdmin.php index b18a1d9f..95faebd6 100644 --- a/code/ReportAdmin.php +++ b/code/ReportAdmin.php @@ -242,7 +242,7 @@ class ReportAdmin extends LeftAndMain implements PermissionProvider )); $columns->setFieldFormatting(array( - 'title' => '$value ($Count)' + 'title' => '$value ($CountForOverview)' )); $gridField->addExtraClass('all-reports-gridfield'); $fields->push($gridField); diff --git a/tests/ReportTest.php b/tests/ReportTest.php index 2a04ffb2..07168461 100644 --- a/tests/ReportTest.php +++ b/tests/ReportTest.php @@ -114,4 +114,20 @@ class ReportTest extends SapphireTest $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()); + } } diff --git a/tests/ReportTest/FakeTest3.php b/tests/ReportTest/FakeTest3.php new file mode 100644 index 00000000..b327303c --- /dev/null +++ b/tests/ReportTest/FakeTest3.php @@ -0,0 +1,15 @@ +