Skip to content

Commit e1eac19

Browse files
Merge pull request #37 from hammerstonedev/af-with-sum
Fix selects that include bindings
2 parents ea5bae0 + e2621f7 commit e1eac19

File tree

4 files changed

+59
-3
lines changed

4 files changed

+59
-3
lines changed

src/FastPaginate.php

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
use Closure;
99
use Illuminate\Database\Query\Expression;
10-
use Illuminate\Pagination\Paginator;
1110

1211
class FastPaginate
1312
{
@@ -55,7 +54,11 @@ protected function paginate(string $paginationMethod, Closure $paginatorOutput)
5554
$key = $model->getKeyName();
5655
$table = $model->getTable();
5756

58-
$innerSelectColumns = FastPaginate::getInnerSelectColumns($this);
57+
try {
58+
$innerSelectColumns = FastPaginate::getInnerSelectColumns($this);
59+
} catch (QueryIncompatibleWithFastPagination $e) {
60+
return $this->{$paginationMethod}($perPage, $columns, $pageName, $page);
61+
}
5962

6063
// This is the copy of the query that becomes
6164
// the inner query that selects keys only.
@@ -72,7 +75,7 @@ protected function paginate(string $paginationMethod, Closure $paginatorOutput)
7275
// Get the key values from the records on the current page without mutating them.
7376
$ids = $paginator->getCollection()->map->getRawOriginal($key)->toArray();
7477

75-
if ($model->getKeyType() === 'int') {
78+
if (in_array($model->getKeyType(), ['int', 'integer'])) {
7679
$this->query->whereIntegerInRaw("$table.$key", $ids);
7780
} else {
7881
$this->query->whereIn("$table.$key", $ids);
@@ -90,6 +93,8 @@ protected function paginate(string $paginationMethod, Closure $paginatorOutput)
9093
/**
9194
* @param $builder
9295
* @return array
96+
*
97+
* @throws QueryIncompatibleWithFastPagination
9398
*/
9499
public static function getInnerSelectColumns($builder)
95100
{
@@ -123,6 +128,11 @@ public static function getInnerSelectColumns($builder)
123128
// Otherwise we don't.
124129
return false;
125130
})
131+
->each(function ($column) {
132+
if (str_contains($column, '?')) {
133+
throw new QueryIncompatibleWithFastPagination;
134+
}
135+
})
126136
->prepend("$table.$key")
127137
->unique()
128138
->values()
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
/**
3+
* @author Aaron Francis <aarondfrancis@gmail.com|https://twitter.com/aarondfrancis>
4+
*/
5+
6+
namespace Hammerstone\FastPaginate;
7+
8+
class QueryIncompatibleWithFastPagination extends \Exception
9+
{
10+
}

tests/Integration/BaseTest.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ protected function setUp(): void
4040
$table->id();
4141
$table->integer('user_id');
4242
$table->string('name');
43+
$table->integer('views');
4344
});
4445

4546
Schema::create('notifications', function (Blueprint $table) {
@@ -62,6 +63,7 @@ protected function setUp(): void
6263
DB::table('posts')->insert([[
6364
'name' => "Post $i",
6465
'user_id' => $i,
66+
'views' => 1,
6567
]]);
6668

6769
DB::table('notifications')->insert([[

tests/Integration/BuilderTest.php

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,4 +351,38 @@ public function basic_simple_test_from_relation()
351351
$this->assertFalse($results->hasMorePages());
352352
$this->assertEquals(1, $results->currentPage());
353353
}
354+
355+
/** @test */
356+
public function with_sum_has_the_correct_number_of_parameters()
357+
{
358+
$queries = $this->withQueriesLogged(function () use (&$fast, &$regular) {
359+
$fast = User::query()
360+
->withSum([
361+
'posts as views_count' => function ($query) {
362+
$query->where('views', '>', 0);
363+
},
364+
], 'views')
365+
->orderBy('views_count')
366+
->fastPaginate();
367+
368+
$regular = User::query()
369+
->withSum([
370+
'posts as views_count' => function ($query) {
371+
$query->where('views', '>', 0);
372+
},
373+
], 'views')
374+
->orderBy('views_count')
375+
->paginate();
376+
});
377+
378+
$this->assertEquals($queries[0]['query'], $queries[2]['query']);
379+
$this->assertEquals($queries[0]['bindings'], $queries[2]['bindings']);
380+
381+
$this->assertEquals($queries[1]['query'], $queries[3]['query']);
382+
$this->assertEquals($queries[1]['bindings'], $queries[3]['bindings']);
383+
384+
$this->assertEquals($fast->toArray(), $regular->toArray());
385+
386+
$this->assertEquals(get_class($fast), get_class($regular));
387+
}
354388
}

0 commit comments

Comments
 (0)