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

Handle sorting (Meilisearch) #537

Merged
merged 4 commits into from
Jan 14, 2022
Merged

Conversation

mgralikowski
Copy link
Contributor

Sorting in the meilisearch is for a long time and works very good so there is no reason to not implement this functionality.

https://docs.meilisearch.com/reference/features/search_parameters.html#sort

$query->orderBy('date', 'desc');
$query->orderBy('price', 'asc');

By this query, we sort results first by date (newest) and next by price (cheapest). Sorting works like in MySQL.

@mgralikowski mgralikowski changed the title Handle sorting Handle sorting (Meilisearch) Oct 5, 2021
@taylorotwell
Copy link
Member

Thanks for your pull request to Laravel!

Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include.

If possible, please consider releasing your code as a package so that the community can still take advantage of your contributions!

If you feel absolutely certain that this code corrects a bug in the framework, please "@" mention me in a follow-up comment with further explanation so that GitHub will send me a notification of your response.

@mgralikowski
Copy link
Contributor Author

@taylorotwell

It doesn't make sense. It is not a new functionality but it is an implementation already existed method added in accepted PR #123

@mgralikowski mgralikowski mentioned this pull request Oct 9, 2021
@lkmadushan
Copy link
Contributor

lkmadushan commented Oct 12, 2021

@mgralikowski @taylorotwell we are also facing this issue. ordering doesn't work out of the box with meilisearch when using scout.

@francoism90
Copy link

Would it be possible to still merge this?

@mgralikowski
Copy link
Contributor Author

Another reason to add sorting:

laravel/nova-issues#3480

If you search resources by text, you are not able to sort them anymore for example by date.

@mgralikowski
Copy link
Contributor Author

@taylorotwell come on.

It does not add any complexity, it is just a one, protected method...

There is a whole article about sorting:
https://docs.meilisearch.com/reference/features/sorting.html#configuring-meilisearch-for-sorting-at-search-time

@crynobone
Copy link
Member

crynobone commented Nov 29, 2021

If you search resources by text, you are not able to sort them anymore for example by date.

this PR will have zero effect on Laravel Nova, the original limitation still persists.

@mgralikowski
Copy link
Contributor Author

The issue with Nova is exactly what my PR resolves - a combination of full-text searching/filtering with sorting at the same moment. In the Nova, we want to find rows by phrase and in the second step - sort them by date - for example.

I have not analyzed the code base of Nova, so I am sure it will not resolve the issue automatically, this is only the first step and basically for Meilisearch only.

@taylorotwell taylorotwell merged commit 3c16044 into laravel:9.x Jan 14, 2022
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.

5 participants