Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[9.x] Fixes pagination count when Laravel\Scout\Builder contains custom query callback #469

Merged
merged 6 commits into from
Apr 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
"require-dev": {
"meilisearch/meilisearch-php": "^0.17",
"mockery/mockery": "^1.0",
"orchestra/testbench": "^6.17",
"phpunit/phpunit": "^9.3"
},
"autoload": {
Expand Down
13 changes: 11 additions & 2 deletions phpunit.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,17 @@
stopOnFailure="false"
>
<testsuites>
<testsuite name="Package Test Suite">
<directory suffix=".php">./tests/</directory>
<testsuite name="Unit">
<directory suffix="Test.php">./tests/Unit</directory>
</testsuite>
<testsuite name="Feature">
<directory suffix="Test.php">./tests/Feature</directory>
</testsuite>
</testsuites>

crynobone marked this conversation as resolved.
Show resolved Hide resolved
<php>
<env name="APP_ENV" value="testing"/>
<env name="APP_KEY" value="AckfSECXIvnK5r28GVIWUAxmbBSjTsmF"/>
<env name="DB_CONNECTION" value="testing"/>
</php>
</phpunit>
23 changes: 21 additions & 2 deletions src/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ public function paginate($perPage = null, $pageName = 'page', $page = null)

$paginator = Container::getInstance()->makeWith(LengthAwarePaginator::class, [
'items' => $results,
'total' => $engine->getTotalCount($rawResults),
'total' => $this->getTotalCount($rawResults),
'perPage' => $perPage,
'currentPage' => $page,
'options' => [
Expand Down Expand Up @@ -352,7 +352,7 @@ public function paginateRaw($perPage = null, $pageName = 'page', $page = null)

$paginator = Container::getInstance()->makeWith(LengthAwarePaginator::class, [
'items' => $results,
'total' => $engine->getTotalCount($results),
'total' => $this->getTotalCount($results),
'perPage' => $perPage,
'currentPage' => $page,
'options' => [
Expand All @@ -364,6 +364,25 @@ public function paginateRaw($perPage = null, $pageName = 'page', $page = null)
return $paginator->appends('query', $this->query);
}

/**
* Get the total number of results from the Scout engine, or fallback to query builder.
*
* @param mixed $results
* @return int
*/
protected function getTotalCount($results)
{
$engine = $this->engine();

if (is_null($this->queryCallback)) {
return $engine->getTotalCount($results);
}

return $this->model->queryScoutModelsByIds(
$this, $engine->mapIds($results)->all()
)->toBase()->getCountForPagination();
}

/**
* Get the engine that should handle the query.
*
Expand Down
89 changes: 89 additions & 0 deletions tests/Feature/BuilderTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
<?php

namespace Laravel\Scout\Tests\Feature;

use Illuminate\Database\Eloquent\Factories\Sequence;
use Illuminate\Foundation\Auth\User;
use Illuminate\Foundation\Testing\WithFaker;
use Laravel\Scout\EngineManager;
use Laravel\Scout\Engines\MeiliSearchEngine;
use Laravel\Scout\ScoutServiceProvider;
use Laravel\Scout\Tests\Fixtures\SearchableUserModel;
use Mockery as m;
use Orchestra\Testbench\Factories\UserFactory;
use Orchestra\Testbench\TestCase;

class BuilderTest extends TestCase
{
use WithFaker;

protected function getPackageProviders($app)
{
return [ScoutServiceProvider::class];
}

protected function defineEnvironment($app)
{
$app->make('config')->set('scout.driver', 'fake');
}

protected function defineDatabaseMigrations()
{
$this->setUpFaker();
$this->loadLaravelMigrations();

UserFactory::new()->count(50)->state(new Sequence(function () {
return ['name' => 'Laravel '.$this->faker()->name()];
}))->create();

UserFactory::new()->times(50)->create();
}

public function test_it_can_paginate_without_custom_query_callback()
{
$this->prepareScoutSearchMockUsing('Laravel');

$paginator = SearchableUserModel::search('Laravel')->paginate();

$this->assertSame(50, $paginator->total());
$this->assertSame(4, $paginator->lastPage());
$this->assertSame(15, $paginator->perPage());
}

public function test_it_can_paginate_with_custom_query_callback()
{
$this->prepareScoutSearchMockUsing('Laravel');

$paginator = SearchableUserModel::search('Laravel')->query(function ($builder) {
return $builder->where('id', '<', 11);
})->paginate();

$this->assertSame(10, $paginator->total());
$this->assertSame(1, $paginator->lastPage());
$this->assertSame(15, $paginator->perPage());
}

protected function prepareScoutSearchMockUsing($searchQuery)
{
$engine = m::mock('MeiliSearch\Client');
$indexes = m::mock('MeiliSearch\Endpoints\Indexes');

$manager = $this->app->make(EngineManager::class);
$manager->extend('fake', function () use ($engine) {
return new MeiliSearchEngine($engine);
});

$query = User::where('name', 'like', $searchQuery.'%');

$engine->shouldReceive('index')->with('users')->andReturn($indexes);
$indexes->shouldReceive('rawSearch')->with($searchQuery, ['limit' => 15])->andReturn([
'hits' => $query->get()->transform(function ($result) {
return [
'id' => $result->getKey(),
'name' => $result->name,
];
}),
'nbHits' => $query->count(),
]);
}
}
16 changes: 2 additions & 14 deletions tests/SearchableTest.php → tests/Feature/SearchableTest.php
Original file line number Diff line number Diff line change
@@ -1,19 +1,14 @@
<?php

namespace Laravel\Scout\Tests;
namespace Laravel\Scout\Tests\Feature;

use Illuminate\Database\Eloquent\Builder;
use Laravel\Scout\Tests\Fixtures\SearchableModel;
use Mockery as m;
use PHPUnit\Framework\TestCase;
use Orchestra\Testbench\TestCase;

class SearchableTest extends TestCase
{
protected function tearDown(): void
{
m::close();
}

public function test_searchable_using_update_is_called_on_collection()
{
$collection = m::mock();
Expand Down Expand Up @@ -84,10 +79,3 @@ public function newQuery()
return $mock;
}
}

namespace Laravel\Scout;

function config($arg)
{
return false;
}
13 changes: 13 additions & 0 deletions tests/Fixtures/SearchableUserModel.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?php

namespace Laravel\Scout\Tests\Fixtures;

use Illuminate\Foundation\Auth\User as Model;
use Laravel\Scout\Searchable;

class SearchableUserModel extends Model
{
use Searchable;

protected $table = 'users';
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php

namespace Laravel\Scout\Tests;
namespace Laravel\Scout\Tests\Unit;

use Algolia\AlgoliaSearch\SearchClient;
use Illuminate\Database\Eloquent\Collection;
Expand Down
2 changes: 1 addition & 1 deletion tests/BuilderTest.php → tests/Unit/BuilderTest.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php

namespace Laravel\Scout\Tests;
namespace Laravel\Scout\Tests\Unit;

use Illuminate\Database\Eloquent\Collection;
use Illuminate\Pagination\Paginator;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php

namespace Laravel\Scout\Tests;
namespace Laravel\Scout\Tests\Unit;

use Illuminate\Database\Eloquent\Collection;
use Laravel\Scout\Jobs\MakeSearchable;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php

namespace Laravel\Scout\Tests;
namespace Laravel\Scout\Tests\Unit;

use Illuminate\Database\Eloquent\Collection;
use Illuminate\Support\LazyCollection;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php

namespace Laravel\Scout\Tests;
namespace Laravel\Scout\Tests\Unit;

use Illuminate\Support\Facades\Config;
use Laravel\Scout\ModelObserver;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php

namespace Laravel\Scout\Tests;
namespace Laravel\Scout\Tests\Unit;

use Illuminate\Database\Eloquent\Builder;
use Laravel\Scout\SearchableScope;
Expand Down