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

Removed numericFilters from AlgoliaEngine. #837

Merged

Conversation

Boorinio
Copy link
Contributor

Hopefully fixes
#831
@driesvints

@taylorotwell
Copy link
Member

The docs say where is for numeric equality checks.

@Boorinio
Copy link
Contributor Author

The docs say where is for numeric equality checks.

Indeed, which is inconsistent with how meilisearch works (which supports everything). The docs also mention this

$orders = Order::search('Star Trek')->whereIn(
    'status', ['open', 'paid']
)->get();

Which currently doesn't work

@driesvints
Copy link
Member

@taylorotwell there is indeed a deeper underlying issue here with inconsistency between drivers. I feel like we have an opportunity here to align the different drivers more and at the same time make the Algolia search more powerful. Could we reconsider?

@driesvints driesvints reopened this Jun 17, 2024
@taylorotwell
Copy link
Member

I guess we'll give it a shot. Will have to roll it back if it breaks something.

@taylorotwell taylorotwell merged commit dadf49e into laravel:10.x Jun 17, 2024
27 checks passed
@taylorotwell
Copy link
Member

@driesvints we need to test this on a real Laravel application before it can be released.

@Boorinio
Copy link
Contributor Author

I guess we'll give it a shot. Will have to roll it back if it breaks something.

Thanks for merging this, if it does break something you can comment here, I will be happy to do a follow up!

@driesvints
Copy link
Member

@Boorinio unfortunately we had to revert this one as it's incompatible with https://github.com/algolia/scout-extended which still expects numericFilters. Can you re-attempt the PR to 11.x and also add an entry to the upgrade guide?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants