Skip to content

Commit

Permalink
fix(UX): Improve search relevance for link field (backport #22729) (#…
Browse files Browse the repository at this point in the history
…22966)

* fix: Improve search relevance for search_link

When `locate` returns 0 it's shown on top instead it should be shown
last or not shown at all.

This is math hack to avoid using any complex SQL functionality which
isn't allowed in DB query.

(cherry picked from commit 3144422)

* fix: Give idx higher preference than meta order

Meta order in most cases is default "modified" which doesn't quite help.

idx is # of times a document is referred to somewhere else, which is
more likely to be relevant.

(cherry picked from commit fec7759)

* fix: Make search_link query postgres compatible

(cherry picked from commit 55a4449)

---------

Co-authored-by: Ankush Menat <ankush@frappe.io>
  • Loading branch information
mergify[bot] and ankush committed Oct 30, 2023
1 parent e2016fe commit abc3cca
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 10 deletions.
18 changes: 10 additions & 8 deletions frappe/desk/search.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,16 +188,18 @@ def search_widget(

order_by_based_on_meta = get_order_by(doctype, meta)
# 2 is the index of _relevance column
order_by = f"{order_by_based_on_meta}, `tab{doctype}`.idx desc"
order_by = f"`tab{doctype}`.idx desc, {order_by_based_on_meta}"

if not meta.translated_doctype:
formatted_fields.append(
"""locate({_txt}, `tab{doctype}`.`name`) as `_relevance`""".format(
_txt=frappe.db.escape((txt or "").replace("%", "").replace("@", "")),
doctype=doctype,
)
)
order_by = f"_relevance, {order_by}"
_txt = frappe.db.escape((txt or "").replace("%", "").replace("@", ""))
_relevance = f"(1 / nullif(locate({_txt}, `tab{doctype}`.`name`), 0))"
formatted_fields.append(f"""{_relevance} as `_relevance`""")
# Since we are sorting by alias postgres needs to know number of column we are sorting
if frappe.db.db_type == "mariadb":
order_by = f"ifnull(_relevance, -9999) desc, {order_by}"
elif frappe.db.db_type == "postgres":
# Since we are sorting by alias postgres needs to know number of column we are sorting
order_by = f"{len(formatted_fields)} desc nulls last, {order_by}"

ignore_permissions = (
True
Expand Down
22 changes: 20 additions & 2 deletions frappe/tests/test_search.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# License: MIT. See LICENSE

import re
from functools import partial

import frappe
from frappe.app import make_form_dict
Expand Down Expand Up @@ -132,15 +133,32 @@ def test_validate_and_sanitize_search_inputs(self):

def test_reference_doctype(self):
"""search query methods should get reference_doctype if they want"""
search_link(
results = test_search(
doctype="User",
txt="",
filters=None,
page_length=20,
reference_doctype="ToDo",
query="frappe.tests.test_search.query_with_reference_doctype",
)
self.assertListEqual(frappe.response["results"], [])
self.assertListEqual(results, [])

def test_search_relevance(self):
search = partial(test_search, doctype="Language", filters=None, page_length=10)
for row in search(txt="e"):
self.assertTrue(row["value"].startswith("e"))

for row in search(txt="es"):
self.assertIn("es", row["value"])

# Assume that "es" is used at least 10 times, it should now be first
frappe.db.set_value("Language", "es", "idx", 10)
self.assertEqual("es", search(txt="es")[0]["value"])


def test_search(*args, **kwargs):
search_link(*args, **kwargs)
return frappe.response["results"]


@frappe.validate_and_sanitize_search_inputs
Expand Down

0 comments on commit abc3cca

Please sign in to comment.