From 141e8d0bf3d06fa488c9ebb53420a050ab02cb80 Mon Sep 17 00:00:00 2001
From: Guy Sartorelli <36352093+GuySartorelli@users.noreply.github.com>
Date: Fri, 11 Mar 2022 09:14:06 +1300
Subject: [PATCH] ENH: Respect sort and limit arguments (#158)

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.
---
 src/Reports/PagesDueForReviewReport.php       | 12 +++++++++-
 .../PagesWithoutReviewScheduleReport.php      | 22 +++++++++++--------
 tests/php/ContentReviewReportTest.php         | 12 +++++-----
 3 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/src/Reports/PagesDueForReviewReport.php b/src/Reports/PagesDueForReviewReport.php
index fc0bd02..cf4bfc3 100644
--- a/src/Reports/PagesDueForReviewReport.php
+++ b/src/Reports/PagesDueForReviewReport.php
@@ -148,16 +148,26 @@ public function columns()
 
     /**
      * @param array $params
+     * @param array|string|null $sort
+     * @param int|null $limit
      *
      * @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
index 0f7b402..e471d1a 100644
--- a/src/Reports/PagesWithoutReviewScheduleReport.php
+++ b/src/Reports/PagesWithoutReviewScheduleReport.php
@@ -104,10 +104,12 @@ public function columns()
 
     /**
      * @param array $params
+     * @param array|string|null $sort
+     * @param int|null $limit
      *
      * @return SS_List
      */
-    public function sourceRecords($params = [])
+    public function sourceRecords($params = [], $sort = null, $limit = null)
     {
         Versioned::set_stage(Versioned::DRAFT);
 
@@ -125,16 +127,18 @@ public function sourceRecords($params = [])
             ));
         }
 
-        $records->sort("ParentID");
-        $records = $records->toArray();
+        // Apply sort and limit if appropriate.
+        if ($sort !== null) {
+            $records = $records->sort($sort);
+        }
+        if ($limit !== null) {
+            $records = $records->limit($limit);
+        }
 
         // 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);
 
diff --git a/tests/php/ContentReviewReportTest.php b/tests/php/ContentReviewReportTest.php
index 5179657..29628e2 100644
--- a/tests/php/ContentReviewReportTest.php
+++ b/tests/php/ContentReviewReportTest.php
@@ -87,11 +87,11 @@ public function testPagesWithoutReviewScheduleReport()
 
         $results = $report->sourceRecords();
 
-        $this->assertEquals([
-            "Home",
-            "About Us",
-            "Page without review date",
-            "Page owned by group",
-        ], $results->column("Title"));
+        $this->assertListEquals([
+            ['Title' => 'Home'],
+            ['Title' => 'About Us'],
+            ['Title' => 'Page without review date'],
+            ['Title' => 'Page owned by group'],
+        ], $results);
     }
 }