From 1a2d13417c1c9893ad4045509adeb94cc33d7f9e Mon Sep 17 00:00:00 2001 From: michalsn Date: Sat, 28 Jun 2025 19:47:26 +0200 Subject: [PATCH] fix: ensure beforeFind model events affect both count and results in pagination --- system/BaseModel.php | 39 +++++++++++++- tests/_support/Models/UserWithEventsModel.php | 44 ++++++++++++++++ tests/system/Models/PaginateModelTest.php | 51 +++++++++++++++++++ user_guide_src/source/changelogs/v4.6.2.rst | 1 + 4 files changed, 134 insertions(+), 1 deletion(-) create mode 100644 tests/_support/Models/UserWithEventsModel.php diff --git a/system/BaseModel.php b/system/BaseModel.php index ca0e20c32829..dbdd10b0e743 100644 --- a/system/BaseModel.php +++ b/system/BaseModel.php @@ -1280,12 +1280,49 @@ public function paginate(?int $perPage = null, string $group = 'default', ?int $ } $page = $page >= 1 ? $page : $pager->getCurrentPage($group); + + // Get the tempPager to estimate required variables, use dummy count of 1 + $tempPager = $pager->store($group, $page, $perPage, 1, $segment); + $perPage = $tempPager->getPerPage($group); + $offset = ($page - 1) * $perPage; + + if ($this->tempAllowCallbacks) { + // Call the before event and check for a return + $eventData = $this->trigger('beforeFind', [ + 'method' => 'findAll', + 'limit' => $perPage, + 'offset' => $offset, + 'singleton' => false, + ]); + + if (isset($eventData['returnData']) && $eventData['returnData'] === true) { + return $eventData['data']; + } + } + // Store it in the Pager library, so it can be paginated in the views. $this->pager = $pager->store($group, $page, $perPage, $this->countAllResults(false), $segment); $perPage = $this->pager->getPerPage($group); $offset = ($pager->getCurrentPage($group) - 1) * $perPage; - return $this->findAll($perPage, $offset); + // Backup since it will be reset in the findAll method + $tempAllowCallbacks = $this->tempAllowCallbacks; + + $data = $this->allowCallbacks(false)->findAll($perPage, $offset); + + if ($tempAllowCallbacks) { + $eventData = $this->trigger('afterFind', [ + 'data' => $data, + 'limit' => $perPage, + 'offset' => $offset, + 'method' => 'findAll', + 'singleton' => false, + ]); + + return $eventData['data']; + } + + return $data; } /** diff --git a/tests/_support/Models/UserWithEventsModel.php b/tests/_support/Models/UserWithEventsModel.php new file mode 100644 index 000000000000..d2b451fed75d --- /dev/null +++ b/tests/_support/Models/UserWithEventsModel.php @@ -0,0 +1,44 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace Tests\Support\Models; + +use CodeIgniter\Model; + +/** + * @method int affectedRows() + */ +class UserWithEventsModel extends Model +{ + protected $table = 'user'; + protected $allowedFields = [ + 'name', + 'email', + 'country', + 'deleted_at', + ]; + protected $returnType = 'object'; + protected $useSoftDeletes = true; + protected $dateFormat = 'datetime'; + public $name = ''; + public $email = ''; + public $country = ''; + protected $beforeFind = ['onlyUS']; + + protected function onlyUS(array $data): array + { + $this->where('country', 'US'); + + return $data; + } +} diff --git a/tests/system/Models/PaginateModelTest.php b/tests/system/Models/PaginateModelTest.php index b7e4661f83ec..ea2551780472 100644 --- a/tests/system/Models/PaginateModelTest.php +++ b/tests/system/Models/PaginateModelTest.php @@ -15,6 +15,7 @@ use PHPUnit\Framework\Attributes\Group; use Tests\Support\Models\UserModel; +use Tests\Support\Models\UserWithEventsModel; use Tests\Support\Models\ValidModel; /** @@ -109,4 +110,54 @@ public function testMultiplePager(): void $this->assertStringContainsString('?page_user=3"', $pager->links('user')); $this->assertStringContainsString('?page_user=4"', $pager->links('user')); } + + public function testPaginateWithBeforeFindEvents(): void + { + $this->createModel(UserWithEventsModel::class); + + $this->seedPaginateEventModel(); + + // Test pagination - beforeFind event should filter to only US users + $data = $this->model->paginate(2); + + // Should only get US users in results + $this->assertCount(2, $data); + $this->assertSame(3, $this->model->pager->getDetails()['total']); + $this->assertSame(2, $this->model->pager->getPageCount()); + + // Verify all returned users are from US + foreach ($data as $user) { + $this->assertSame('US', $user->country); + } + } + + public function testPaginateWithBeforeFindEventsAndDisabledCallbacks(): void + { + $this->createModel(UserWithEventsModel::class); + + $this->seedPaginateEventModel(); + + $data = $this->model->allowCallbacks(false)->paginate(2); + + // Should get all users + $this->assertCount(2, $data); + $this->assertSame(9, $this->model->pager->getDetails()['total']); + + // Should have users from different countries + $countries = array_unique(array_column($data, 'country')); + $this->assertGreaterThan(1, count($countries)); + } + + private function seedPaginateEventModel(): void + { + $testData = [ + ['name' => 'Jean', 'email' => 'jean@test.com', 'country' => 'France'], + ['name' => 'Marie', 'email' => 'marie@test.com', 'country' => 'France'], + ['name' => 'John', 'email' => 'john@test.com', 'country' => 'US'], + ['name' => 'Hans', 'email' => 'hans@test.com', 'country' => 'Germany'], + ['name' => 'Luigi', 'email' => 'luigi@test.com', 'country' => 'Italy'], + ]; + + $this->model->insertBatch($testData); + } } diff --git a/user_guide_src/source/changelogs/v4.6.2.rst b/user_guide_src/source/changelogs/v4.6.2.rst index f39a0c46cf81..e6e95895ecb2 100644 --- a/user_guide_src/source/changelogs/v4.6.2.rst +++ b/user_guide_src/source/changelogs/v4.6.2.rst @@ -39,6 +39,7 @@ Bugs Fixed - **Database:** Fixed a bug where ``when()`` and ``whenNot()`` in ``ConditionalTrait`` incorrectly evaluated certain falsy values (such as ``[]``, ``0``, ``0.0``, and ``'0'``) as truthy, causing callbacks to be executed unexpectedly. These methods now cast the condition to a boolean using ``(bool)`` to ensure consistent behavior with PHP's native truthiness. - **Database:** Fixed encapsulation violation in ``BasePreparedQuery`` when accessing ``BaseConnection::transStatus`` protected property. - **Email:** Fixed a bug where ``Email::getHostname()`` failed to use ``$_SERVER['SERVER_ADDR']`` when ``$_SERVER['SERVER_NAME']`` was not set. +- **Model:** Fixed a bug in ``BaseModel::paginate()`` where ``beforeFind`` events were not applied to the count query, causing inconsistent pagination metadata. The total count now correctly reflects the same filters applied to the paginated results. - **Security:** Fixed a bug where the ``sanitize_filename()`` function from the Security helper would throw an error when used in CLI requests. - **Session:** Fixed a bug where using the ``DatabaseHandler`` with an unsupported database driver (such as ``SQLSRV``, ``OCI8``, or ``SQLite3``) did not throw an appropriate error. - **URI:** Fixed a bug in ``URI::getAuthority()`` where schemes without defined default ports (like ``rtsp://``) would cause issues due to missing array key handling.