Skip to content

Commit

Permalink
feat: search in translated title, if we show title (#17828) (#18395)
Browse files Browse the repository at this point in the history
* refactor: use meta.translated_doctype

* refactor: get_title_field_query

* feat: search in title, if we show title

* refactor: build_for_autosuggest

* style: black

* fix: don't order translated doctypes by untranslated relevance

* feat: match all fields for translated doctypes

* feat: translate all fields in description, remove redundant title

* refactor: title in link

* fix: show name in description for title links

(cherry picked from commit 3d17e15)

Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com>
  • Loading branch information
mergify[bot] and barredterra committed Oct 12, 2022
1 parent 7c9ee6d commit 73758d7
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 67 deletions.
5 changes: 3 additions & 2 deletions frappe/core/doctype/doctype/doctype.json
Expand Up @@ -320,7 +320,8 @@
"depends_on": "eval:!doc.istable",
"fieldname": "title_field",
"fieldtype": "Data",
"label": "Title Field"
"label": "Title Field",
"mandatory_depends_on": "eval:doc.show_title_field_in_link"
},
{
"depends_on": "eval:!doc.istable",
Expand Down Expand Up @@ -687,7 +688,7 @@
"link_fieldname": "reference_doctype"
}
],
"modified": "2022-08-24 06:42:27.779699",
"modified": "2022-09-02 12:05:59.589751",
"modified_by": "Administrator",
"module": "Core",
"name": "DocType",
Expand Down
106 changes: 43 additions & 63 deletions frappe/desk/search.py
Expand Up @@ -8,7 +8,6 @@
import frappe
from frappe import _, is_whitelisted
from frappe.permissions import has_permission
from frappe.translate import get_translated_doctypes
from frappe.utils import cint, cstr, unique


Expand Down Expand Up @@ -150,10 +149,6 @@ def search_widget(
filters = []
or_filters = []

translated_doctypes = frappe.cache().hget(
"translated_doctypes", "doctypes", get_translated_doctypes
)

# build from doctype
if txt:
field_types = [
Expand All @@ -175,7 +170,7 @@ def search_widget(

for f in search_fields:
fmeta = meta.get_field(f.strip())
if (doctype not in translated_doctypes) and (
if not meta.translated_doctype and (
f == "name" or (fmeta and fmeta.fieldtype in field_types)
):
or_filters.append([doctype, f.strip(), "like", f"%{txt}%"])
Expand All @@ -191,26 +186,25 @@ def search_widget(
fields = list(set(fields + json.loads(filter_fields)))
formatted_fields = [f"`tab{meta.name}`.`{f.strip()}`" for f in fields]

title_field_query = get_title_field_query(meta)

# Insert title field query after name
if title_field_query:
formatted_fields.insert(1, title_field_query)

# find relevance as location of search term from the beginning of string `name`. used for sorting results.
formatted_fields.append(
"""locate({_txt}, `tab{doctype}`.`name`) as `_relevance`""".format(
_txt=frappe.db.escape((txt or "").replace("%", "").replace("@", "")),
doctype=doctype,
)
)
if meta.show_title_field_in_link:
formatted_fields.insert(1, f"`tab{meta.name}`.{meta.title_field} as `label`")

# In order_by, `idx` gets second priority, because it stores link count
from frappe.model.db_query import get_order_by

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

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}"

ptype = "select" if frappe.only_has_select_perm(doctype) else "read"
ignore_permissions = (
Expand All @@ -219,29 +213,29 @@ def search_widget(
else (cint(ignore_user_permissions) and has_permission(doctype, ptype=ptype))
)

if doctype in translated_doctypes:
page_length = None

values = frappe.get_list(
doctype,
filters=filters,
fields=formatted_fields,
or_filters=or_filters,
limit_start=start,
limit_page_length=page_length,
limit_page_length=None if meta.translated_doctype else page_length,
order_by=order_by,
ignore_permissions=ignore_permissions,
reference_doctype=reference_doctype,
as_list=not as_dict,
strict=False,
)

if doctype in translated_doctypes:
if meta.translated_doctype:
# Filtering the values array so that query is included in very element
values = (
v
for v in values
if re.search(f"{re.escape(txt)}.*", _(v.name if as_dict else v[0]), re.IGNORECASE)
result
for result in values
if any(
re.search(f"{re.escape(txt)}.*", _(cstr(value)) or "", re.IGNORECASE)
for value in (result.values() if as_dict else result)
)
)

# Sorting the values array so that relevant results always come first
Expand All @@ -250,12 +244,14 @@ def search_widget(
values = sorted(values, key=lambda x: relevance_sorter(x, txt, as_dict))

# remove _relevance from results
if as_dict:
for r in values:
r.pop("_relevance")
frappe.response["values"] = values
else:
frappe.response["values"] = [r[:-1] for r in values]
if not meta.translated_doctype:
if as_dict:
for r in values:
r.pop("_relevance")
else:
values = [r[:-1] for r in values]

frappe.response["values"] = values


def get_std_fields_list(meta, key):
Expand All @@ -275,39 +271,23 @@ def get_std_fields_list(meta, key):
return sflist


def get_title_field_query(meta):
title_field = meta.title_field if meta.title_field else None
show_title_field_in_link = (
meta.show_title_field_in_link if meta.show_title_field_in_link else None
)
field = None

if title_field and show_title_field_in_link:
field = f"`tab{meta.name}`.{title_field} as `label`"

return field

def build_for_autosuggest(res: list[tuple], doctype: str) -> list[dict]:
def to_string(parts):
return ", ".join(
unique(_(cstr(part)) if meta.translated_doctype else cstr(part) for part in parts if part)
)

def build_for_autosuggest(res, doctype):
results = []
meta = frappe.get_meta(doctype)
if not (meta.title_field and meta.show_title_field_in_link):
for r in res:
r = list(r)
results.append({"value": r[0], "description": ", ".join(unique(cstr(d) for d in r[1:] if d))})

if meta.show_title_field_in_link:
for item in res:
item = list(item)
label = item[1] # use title as label
item[1] = item[0] # show name in description instead of title
del item[2] # remove redundant title ("label") value
results.append({"value": item[0], "label": label, "description": to_string(item[1:])})
else:
title_field_exists = meta.title_field and meta.show_title_field_in_link
_from = 2 if title_field_exists else 1 # to exclude title from description if title_field_exists
for r in res:
r = list(r)
results.append(
{
"value": r[0],
"label": r[1] if title_field_exists else None,
"description": ", ".join(unique(cstr(d) for d in r[_from:] if d)),
}
)
results.extend({"value": item[0], "description": to_string(item[1:])} for item in res)

return results

Expand Down Expand Up @@ -383,7 +363,7 @@ def get_user_groups():
def get_link_title(doctype, docname):
meta = frappe.get_meta(doctype)

if meta.title_field and meta.show_title_field_in_link:
if meta.show_title_field_in_link:
return frappe.db.get_value(doctype, docname, meta.title_field)

return docname
12 changes: 10 additions & 2 deletions frappe/public/js/frappe/form/controls/link.js
Expand Up @@ -89,10 +89,13 @@ frappe.ui.form.ControlLink = class ControlLink extends frappe.ui.form.ControlDat
is_translatable() {
return in_list(frappe.boot?.translated_doctypes || [], this.get_options());
}
is_title_link() {
return in_list(frappe.boot.link_title_doctypes, this.get_options());
}
async set_link_title(value) {
const doctype = this.get_options();

if (!doctype || !in_list(frappe.boot.link_title_doctypes, doctype)) {
if (!doctype || !this.is_title_link()) {
this.translate_and_set_input_value(value, value);
return;
}
Expand Down Expand Up @@ -207,7 +210,12 @@ frappe.ui.form.ControlLink = class ControlLink extends frappe.ui.form.ControlDat

let _label = me.get_translated(d.label);
let html = d.html || "<strong>" + _label + "</strong>";
if (d.description && d.value !== d.description) {
if (
d.description &&
// for title links, we want to inlude the value in the description
// because it will not visible otherwise
(me.is_title_link() || d.value !== d.description)
) {
html += '<br><span class="small">' + __(d.description) + "</span>";
}
return $("<li></li>")
Expand Down

0 comments on commit 73758d7

Please sign in to comment.