ENH: Respect sort and limit arguments

These parameters are defined in the PHPDocs for `Report` and are technically part of the method signature. They should be respected and in the case of the new default limit in silverstripe/silverstripe-reports#139 this could have performance ramifications for large datasets.
GuySartorelli · Mar 7, 2022 · 9bdd084 · 9bdd084
1 parent 46a637a
commit 9bdd084
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 9 deletions.
diff --git a/src/Reports/PagesDueForReviewReport.php b/src/Reports/PagesDueForReviewReport.php
@@ -151,13 +151,21 @@ public function columns()
      *
      * @return SS_List
      */
-    public function sourceRecords($params = [])
+    public function sourceRecords($params = [], $sort = null, $limit = null)
     {
         Versioned::set_stage(Versioned::DRAFT);
 
         $records = SiteTree::get();
         $compatibility = ContentReviewCompatability::start();
 
+        // Apply sort and limit if appropriate.
+        if ($sort !== null) {
+            $records = $records->sort($sort);
+        }
+        if ($limit !== null) {
+            $records = $records->limit($limit);
+        }
+
         if (empty($params['ReviewDateBefore']) && empty($params['ReviewDateAfter'])) {
             // If there's no review dates set, default to all pages due for review now
             $records = $records->where(

diff --git a/src/Reports/PagesWithoutReviewScheduleReport.php b/src/Reports/PagesWithoutReviewScheduleReport.php
@@ -107,7 +107,7 @@ public function columns()
      *
      * @return SS_List
      */
-    public function sourceRecords($params = [])
+    public function sourceRecords($params = [], $sort = null, $limit = null)
     {
         Versioned::set_stage(Versioned::DRAFT);
 
@@ -125,16 +125,19 @@ public function sourceRecords($params = [])
             ));
         }
 
-        $records->sort("ParentID");
+        if ($sort !== null) {
+            $sort = 'ParentID';
+        }
+        $records = $records->sort($sort);
+        if ($limit !== null) {
+            $records = $records->limit($limit);
+        }
         $records = $records->toArray();
 
         // Trim out calculated values
-        $list = ArrayList::create();
-        foreach ($records as $record) {
-            if (!$this->hasReviewSchedule($record)) {
-                $list->push($record);
-            }
-        }
+        $list = $records->filterByCallback(function($record) {
+            return !$this->hasReviewSchedule($record);
+        });
 
         ContentReviewCompatability::done($compatibility);