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

Fix search ordered by best match #3706

Merged
merged 5 commits into from
Apr 14, 2019

Conversation

Jakdaw
Copy link
Contributor

@Jakdaw Jakdaw commented Apr 12, 2019

What type of PR is this? (check all applicable)

  • Bug Fix

Description

Make searches order by best-match (unless something else is explicitly requested).

Related Tickets & Documents

See discussion at:

https://discuss.redash.io/t/searching-for-queries-after-upgrade-to-v5/2590/6

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

Jakdaw and others added 5 commits March 27, 2019 12:05
Update from getredash/redash
this already request it explicitly and it breaks the search
function that wants to order by best match.
…re's

no search term, rather than when there is one and we should use the
best-match ordering.
Copy link
Member

@arikfr arikfr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @Jakdaw!

We should probably improve test coverage around this area to make sure this doesn't happen again.

@arikfr arikfr merged commit af168c6 into getredash:master Apr 14, 2019
@jezdez
Copy link
Member

jezdez commented Apr 15, 2019

I'm not really following this code anymore (and I wrote it), so maybe it would make sense to refactor the order_results function into something that is easier to understand?

harveyrendell pushed a commit to pushpay/redash that referenced this pull request Nov 14, 2019
* Don't force an order by created date - any usecases that want
this already request it explicitly and it breaks the search
function that wants to order by best match.

* Fix the logic so that we fall back to a default search order when there's
no search term, rather than when there is one and we should use the
best-match ordering.

* Remove accidentially added blank line
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants