Sti3bas
commented
1 year ago @seivad can you please provide the full stack trace?
Maybe you could even provide a minimal reproducible repo?
Closed seivad closed 1 year ago
Sti3bas
commented
1 year ago @seivad can you please provide the full stack trace?
Maybe you could even provide a minimal reproducible repo?
cbaconnier
commented
1 year ago First of all, thanks for the library! Implementing laravel-scout in a existing project would have been a pain, but luckily with your library it seems we don't even have to update our tests :heart:
Here a reproducible example:
git clone git@github.com:cbaconnier/laravel-scout-array-bug.git
cd laravel-scout-array-bug
composer install
cp .env.example .env
php artisan key:generate
php artisan testFrom my analysis I see few issues
First, there is this index string passed in parameters. It contains users_index and it's passed in our callback function.
But neither Algolia or Meilisearch have it.
https://github.com/Sti3bas/laravel-scout-array-driver/blob/6e103a237b064110279226889d048d596786bdeb/src/Engines/ArrayEngine.php#L112-L122
Then, when we use the callback, the first argument will be the underlying engine ( Algolia\AlgoliaSearch\SearchIndex or Meilisearch\Endpoints\Indexes )
They probably wont work the same and testing will probably be painful.
I'm not sure this callback is used elsewhere in this library. But if it was only attended to use with this search, my suggestion would be to remove the callback call.
By removing the callback, we can still use the typing instead of having Indexes|ArrayEngine (or use phpdoc) and we wont need to add a new search method in the store and yada yada...
Also, as a more extended feature, it could be interesting to store the callback and test it;
Search::assertSearchedWithCallback(function(Indexes $meiliSearch, string $query, array $options){
return true;
}); // Hey @cbaconnier,
Thanks for putting an effort into debugging this issue and providing a detailed description. Really appreciated!
It seems that I overlooked or missed the coverage of the search functionality, most likely because I've used the JS version of Algolia's search client in my project while I was creating this library.
I would like to fix this issue by releasing a new version which would also provide an additional features like faking the search results and adding new search assertions.
The API would be something like this:
Search::fakeSearch(User::class, 'john', [
User::factory()->create(['name' => 'John Doe']),
User::factory()->create(['name' => 'John Cook']),
]);
$response = $this->get('/search/john');
$response->assertStatus(200);
$response->assertSee('John Doe');
$response->assertSee('John Cook');
Search::assertSearchedFor(User::class, 'john', function(MockInterface $meilisearch) {
// an optional closure to make sure that the correct methods were called and correct options were passed to underlying engine using Mockery spies
$meilisearch->shouldHaveReceived('search')->with('john', [
'perPage' => null,
'sort' => ['name:asc'],
'filter' => 'email IS NOT NULL'
]);
});I have an MVP ready, just need to cleanup/polish the code, but would like to hear your thoughts on this first!
cbaconnier
commented
1 year ago Hey @Sti3bas,
That's by far a better idea that I was thinking of. I will close my PR.
Thank you very much for the time you have spent on this, I'm looking forward to update my tests with your solution 😄
Hey mate,
Based on the Laravel Scout example, I am using Meilisearch and using a callback function to narrow results down within the controller. For testing I am trying to use your package but it is complaining about the ->search() function you need to return with based on the docs.
Doc example:
and my terminal error
Error: Call to undefined method Sti3bas\ScoutArray\ArrayStore::search() in...I can see you've implemented the search() method which calls performSearch() but so I'm not sure why it can't find that function during the test. Any ideas would be great so I can test these controllers that utilise Scout.