From 6752462ba6d097437fd9be371444ff358cb4f2a3 Mon Sep 17 00:00:00 2001 From: Simon Barrett Date: Mon, 7 Sep 2026 14:59:41 +0100 Subject: [PATCH] Group the table search so it narrows a report instead of widening it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TableBuilder::rowsQuery() appended each searchable column as a bare top-level orWhere, after everything query() returned and after the model's global scopes. The predicate became WHERE (report constraints) AND ... OR column LIKE '%term%' so any report whose query() carried real constraints discarded them the moment someone typed in the search box. Nothing inside query() could prevent it: wrapping that body in a nested where() does not help, because the orWhere sits outside that group too. Measured in the consuming app against production data: a health and safety report returning 115 rows returned 32,215 — the entire records table — when searched for "a", and searching "Fire" returned 71 courses that were not health and safety, including "Fire up your Facebook page". The loop is now wrapped in its own where() group, so search composes as AND (a OR b) and every report using TableBuilder becomes correct without change. Refs ACT-Training/people#3515 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01MfdXn5v557zRZ9F6bYcjAd --- src/TableBuilder.php | 47 +++++++------ tests/TableBuilderSearchScopingTest.php | 87 +++++++++++++++++++++++++ 2 files changed, 114 insertions(+), 20 deletions(-) create mode 100644 tests/TableBuilderSearchScopingTest.php diff --git a/src/TableBuilder.php b/src/TableBuilder.php index 1d9b931..5a9da10 100755 --- a/src/TableBuilder.php +++ b/src/TableBuilder.php @@ -93,27 +93,34 @@ public function rowsQuery() }); if ($this->searchBy && $this->searchBy !== '') { - foreach ($this->getSearchableColumns() as $column) { - // Explode the column to separate relationships and the actual column name - $parts = explode('.', $column); - - // Extract the actual column name from the end of the array - $actualColumnName = array_pop($parts); - - // If there are relationships defined (implied by remaining elements in $parts) - if (! empty($parts)) { - // Build the relationship string from the remaining $parts - $relationshipPath = implode('.', $parts); - - // Use a closure to apply the search condition on the related model - $query->orWhereHas($relationshipPath, function ($query) use ($actualColumnName) { - $query->where($actualColumnName, 'like', '%'.$this->searchBy.'%'); - }); - } else { - // If there are no relationships, directly apply the search condition on the current model - $query->orWhere($actualColumnName, 'like', '%'.$this->searchBy.'%'); + // Grouped, so the searchable columns OR against each other and AND against + // everything else. Left ungrouped these were bare top-level orWheres applied + // after query(), which meant any report with real constraints discarded them + // as soon as someone typed — search widened the result set instead of + // narrowing it, and no report could defend against it from inside query(). + $query->where(function ($query) { + foreach ($this->getSearchableColumns() as $column) { + // Explode the column to separate relationships and the actual column name + $parts = explode('.', $column); + + // Extract the actual column name from the end of the array + $actualColumnName = array_pop($parts); + + // If there are relationships defined (implied by remaining elements in $parts) + if (! empty($parts)) { + // Build the relationship string from the remaining $parts + $relationshipPath = implode('.', $parts); + + // Use a closure to apply the search condition on the related model + $query->orWhereHas($relationshipPath, function ($query) use ($actualColumnName) { + $query->where($actualColumnName, 'like', '%'.$this->searchBy.'%'); + }); + } else { + // If there are no relationships, directly apply the search condition on the current model + $query->orWhere($actualColumnName, 'like', '%'.$this->searchBy.'%'); + } } - } + }); } $dottedFilterValue = Arr::dot($this->filterValues); diff --git a/tests/TableBuilderSearchScopingTest.php b/tests/TableBuilderSearchScopingTest.php new file mode 100644 index 0000000..2552a6c --- /dev/null +++ b/tests/TableBuilderSearchScopingTest.php @@ -0,0 +1,87 @@ +where('category', 'health-and-safety'); + } + + public function columns(): array + { + return [ + Column::make('Title', 'title')->searchable(), + ]; + } + + public function filters(): array + { + return []; + } +} + +beforeEach(function () { + Schema::create('search_scoping_records', function ($table) { + $table->id(); + $table->string('category'); + $table->string('title'); + }); + + SearchScopingRecord::insert([ + ['category' => 'health-and-safety', 'title' => 'Fire Safety Awareness'], + ['category' => 'health-and-safety', 'title' => 'Manual Handling'], + ['category' => 'marketing', 'title' => 'Fire up your Facebook page'], + ['category' => 'marketing', 'title' => 'Content strategy'], + ]); +}); + +it('returns only the constrained rows when nothing is searched', function () { + $table = new SearchScopingTable; + + expect($table->rowsQuery()->pluck('title')->all()) + ->toEqualCanonicalizing(['Fire Safety Awareness', 'Manual Handling']); +}); + +it('keeps the query constraints when a search term is typed', function () { + $table = new SearchScopingTable; + $table->searchBy = 'Fire'; + + // Without grouping this returns the marketing row too, because the search ORs + // past `category = health-and-safety`. + expect($table->rowsQuery()->pluck('title')->all()) + ->toEqual(['Fire Safety Awareness']); +}); + +it('never returns more rows with a search term than without one', function () { + $unsearched = (new SearchScopingTable)->rowsQuery()->count(); + + $table = new SearchScopingTable; + $table->searchBy = 'a'; + + expect($table->rowsQuery()->count())->toBeLessThanOrEqual($unsearched); +});