algolia / scout-extended

Scout Extended: The Full Power of Algolia in Laravel
https://www.algolia.com/doc/framework-integration/laravel/getting-started/introduction-to-scout-extended/
MIT License
395 stars 85 forks source link

[BUG] - empty whereIn array results in condition ignored #265

Closed VinceG closed 3 years ago

VinceG commented 3 years ago
Q A
Bug fix? Yes
New feature? No
BC breaks? no
Related Issue Fix #200
Need Doc update no

Describe your change

This will add an empty condition to the whereIn method to make sure it'll add the condition only if the array is not empty. if it's empty it'll add a false condition to make sure the condition will stop ALL records from being returned.

What problem is this fixing?

Originally passing an empty array to whereIn resulted in ignoring the entire condition, this results in unexpected search results where a user might see more results than they should.

For example

// Returns all records matching the query ignores the id condition
// it'll return all results matching the search query and ignores completely the whereIn condition
$query = App\Models\Areas\Area::search('santa')->whereIn('author_id', [])->get();

With this fix, it'll do exactly what Laravel does with its query builder where if the array is empty it'll append a false condition.

// Returns all records matching the query appends 0 = 1 similar to how laravel handles the whereIn with empty array
// https://github.com/laravel/framework/blob/8.x/src/Illuminate/Database/Query/Grammars/Grammar.php#L271
$query = App\Models\Areas\Area::search('santa')->whereIn('id', [])->get();

added a corresponding test to make sure everything passes with this change.

VinceG commented 3 years ago

Looks like the tests are failing but not related to this PR.

DevinCodes commented 3 years ago

I re-ran the tests so that the credentials work properly (sorry for the inconvenience on that).

It seems like there are some PHPStan errors that make the CI fail, but this is unexpected. There's an issue on the PHPStan repo that describes the same situation we're encountering. We have to wait until this commit is released, or ignore the errors for now through the phpstan.neon.dist file.

DevinCodes commented 3 years ago

@VinceG Let's unblock this, and ignore the PHPStan errors for now. I'll make sure keep track of this in the coming days/weeks.

Can you please ignore the errors for now by adding the following to the phpstan.neon.dist file, under the ignoreErrors key? Thank you in advance!

- '#Cannot call method withScoutMetadata\(\) on class-string\|object.#'
- '#Cannot call method getScoutKey\(\) on class-string\|object.#'
- '#Cannot call method toSearchableArray\(\) on class-string\|object.#'
VinceG commented 3 years ago

@DevinCodes Done.

DevinCodes commented 3 years ago

This fix is released in v1.14.0 🎉

Thank you again for your contribution!