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

Sort study search results #12088

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

dignissimus
Copy link
Contributor

@dignissimus dignissimus commented Dec 23, 2022

Resolves #12060


The code looks alright, but this is blocked on testing with elastic search. I'm marking this as ready for review but this PR requires testing.

@dignissimus dignissimus marked this pull request as ready for review December 31, 2022 12:11
@kraktus
Copy link
Member

kraktus commented Mar 4, 2023

Just saw that Lila-gitpod now support ES, that can be option to test your patch if you don't want to set it up locally: https://github.com/lichess-org/lila-gitpod/blob/657b5f9e91a8284b6bb6e8ad49b30d847014a4fe/docs/optional-services.md

@kraktus kraktus self-assigned this Aug 31, 2023
ethanwater

This comment was marked as spam.

@fitztrev
Copy link
Member

fitztrev commented Nov 9, 2023

Pulled this down to test it. I wasn't able to rebase so I ended up just reapplying the changes to a new branch. Here's my branch with @dignissimus's changes re-applied against the latest master.

Created a couple studies and I see the dropdown on the Study search result page.

At least in my environment, when a user hearts a study, the likes value isn't incrementing in elasticsearch. And some of the other orderBys don't seem to have an effect yet.

If anyone is going to test in gitpod, this will show you the data in elasticsearch:

docker compose exec lila bash -c "curl 'http://elasticsearch:9200/study/_search?pretty=true&q=*:*'"

@kraktus
Copy link
Member

kraktus commented Nov 13, 2023

Maybe (probably in fact), need to change things in https://github.com/lichess-org/lila-search too

@ijm8710
Copy link

ijm8710 commented Feb 25, 2025

Is there any way to bump this :)

@lenguyenthanh
Copy link
Member

the approach in this pr doesn't work (at least for now) because we have to sort the results from lila-search first.

@ijm8710
Copy link

ijm8710 commented Feb 25, 2025

the approach in this pr doesn't work (at least for now) because we have to sort the results from lila-search first.

Ok but is there any way you think someone may take on with a diff approach.

Don't want to be greedy as I'm not adept enough to fix this myself, but seems like it's been stuck for some time and to have a workaround to surface results in reverse heart order seems like a feasible endeavor.

Not asking for this to be done tomorrow, but you think there's any way we can kickstart it to be given another look by someone in next few months or at least this year?

@lenguyenthanh
Copy link
Member

This is definitely a high reward one, but it evolves different part of the system. It's on my list for a long time but haven't found time/motivation to work on this. will try to work on this soonis (but unfortunately can't have any promise).

@ijm8710
Copy link

ijm8710 commented Feb 25, 2025

Thanks! If it's not in next couple months I get it but if there any way to prioritize for next 6 month I do agree it would be high reward

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.

Provide a way to rank study searches
6 participants