From aa6520b5b010ac777b0eb054b2fc5752649fcd0f Mon Sep 17 00:00:00 2001 From: codescientist703 Date: Fri, 18 Jun 2021 13:53:57 +0530 Subject: [PATCH 1/7] fix: add sorting for paren tree doctype to appear above child --- frappe/desk/search.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frappe/desk/search.py b/frappe/desk/search.py index 040a8c2118..be62b9699d 100644 --- a/frappe/desk/search.py +++ b/frappe/desk/search.py @@ -167,6 +167,9 @@ def search_widget(doctype, txt, query=None, searchfield=None, start=0, as_list=not as_dict, strict=False) + # Bring Parent doctype to top and child doctype below it using sorting + values = sorted(values, key=lambda x: ((x[1] is not None), (x[0]))) + if doctype in UNTRANSLATED_DOCTYPES: values = tuple([v for v in list(values) if re.search(re.escape(txt)+".*", (_(v.name) if as_dict else _(v[0])), re.IGNORECASE)]) From 85bbcaaac41bbbf08b0475596e17479fb60b770d Mon Sep 17 00:00:00 2001 From: codescientist703 Date: Wed, 23 Jun 2021 13:13:51 +0530 Subject: [PATCH 2/7] fix: added sorting and filtering logic for relevant input results --- frappe/desk/search.py | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/frappe/desk/search.py b/frappe/desk/search.py index be62b9699d..64ae7f7b7a 100644 --- a/frappe/desk/search.py +++ b/frappe/desk/search.py @@ -167,11 +167,20 @@ def search_widget(doctype, txt, query=None, searchfield=None, start=0, as_list=not as_dict, strict=False) - # Bring Parent doctype to top and child doctype below it using sorting - values = sorted(values, key=lambda x: ((x[1] is not None), (x[0]))) + # Filtering the values array so that query is included in very element + values = tuple( + [ + v for v in list(values) + if re.search( + re.escape(txt) + ".*", (_(v.name) if as_dict else _(v[0])), re.IGNORECASE + ) + ] + ) - if doctype in UNTRANSLATED_DOCTYPES: - values = tuple([v for v in list(values) if re.search(re.escape(txt)+".*", (_(v.name) if as_dict else _(v[0])), re.IGNORECASE)]) + # Sorting the values array so that relevant results always come first + # This will first bring elements on top in which query is a prefix of element + # Then it will bring the rest of the elements and sort them in lexicographical order + values = sorted(values, key=lambda x: sorting_comparator(x, txt, as_dict)) # remove _relevance from results if as_dict: @@ -211,6 +220,13 @@ def scrub_custom_query(query, key, txt): query = query.replace('%s', ((txt or '') + '%')) return query +def sorting_comparator(key, query, as_dict): + value = (_(key.name) if as_dict else _(key[0])) + return ( + value.lower().startswith(query.lower()) is False, + value + ) + @wrapt.decorator def validate_and_sanitize_search_inputs(fn, instance, args, kwargs): kwargs.update(dict(zip(fn.__code__.co_varnames, args))) From ecfa8c843f13e6ab7abb629a51a8ce7c44df386e Mon Sep 17 00:00:00 2001 From: codescientist703 Date: Wed, 7 Jul 2021 17:33:59 +0530 Subject: [PATCH 3/7] fix: child results should appear for parent search query --- frappe/desk/search.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/frappe/desk/search.py b/frappe/desk/search.py index 64ae7f7b7a..b0e9592c72 100644 --- a/frappe/desk/search.py +++ b/frappe/desk/search.py @@ -167,15 +167,16 @@ def search_widget(doctype, txt, query=None, searchfield=None, start=0, as_list=not as_dict, strict=False) - # Filtering the values array so that query is included in very element - values = tuple( - [ - v for v in list(values) - if re.search( - re.escape(txt) + ".*", (_(v.name) if as_dict else _(v[0])), re.IGNORECASE - ) - ] - ) + if doctype in UNTRANSLATED_DOCTYPES: + # Filtering the values array so that query is included in very element + values = tuple( + [ + v for v in list(values) + if re.search( + re.escape(txt) + ".*", (_(v.name) if as_dict else _(v[0])), re.IGNORECASE + ) + ] + ) # Sorting the values array so that relevant results always come first # This will first bring elements on top in which query is a prefix of element From ba062adca84dfecb0e129c292b30303156213db4 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 8 Jul 2021 13:36:25 +0530 Subject: [PATCH 4/7] refactor(search): Improvements in search_widget, search_link APIs * Minor perf enhancements * Renamed sorting_comparator to relevance_sorter --- frappe/desk/search.py | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/frappe/desk/search.py b/frappe/desk/search.py index b0e9592c72..3cce80a1a0 100644 --- a/frappe/desk/search.py +++ b/frappe/desk/search.py @@ -169,19 +169,17 @@ def search_widget(doctype, txt, query=None, searchfield=None, start=0, if doctype in UNTRANSLATED_DOCTYPES: # Filtering the values array so that query is included in very element - values = tuple( - [ - v for v in list(values) - if re.search( - re.escape(txt) + ".*", (_(v.name) if as_dict else _(v[0])), re.IGNORECASE - ) - ] + values = ( + v for v in values + if re.search( + f"{re.escape(txt)}.*", _(v.name if as_dict else v[0]), re.IGNORECASE ) + ) # Sorting the values array so that relevant results always come first # This will first bring elements on top in which query is a prefix of element # Then it will bring the rest of the elements and sort them in lexicographical order - values = sorted(values, key=lambda x: sorting_comparator(x, txt, as_dict)) + values = sorted(values, key=lambda x: relevance_sorter(x, txt, as_dict)) # remove _relevance from results if as_dict: @@ -221,10 +219,10 @@ def scrub_custom_query(query, key, txt): query = query.replace('%s', ((txt or '') + '%')) return query -def sorting_comparator(key, query, as_dict): - value = (_(key.name) if as_dict else _(key[0])) +def relevance_sorter(key, query, as_dict): + value = _(key.name if as_dict else key[0]) return ( - value.lower().startswith(query.lower()) is False, + value.lower().startswith(query.lower()) == False, value ) From 79124873c3357086580edd53c56b0756b4c68e1b Mon Sep 17 00:00:00 2001 From: codescientist703 Date: Tue, 13 Jul 2021 18:43:34 +0530 Subject: [PATCH 5/7] fix: fixed sider recommendation --- frappe/desk/search.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/desk/search.py b/frappe/desk/search.py index 3cce80a1a0..f9b65fc98e 100644 --- a/frappe/desk/search.py +++ b/frappe/desk/search.py @@ -222,7 +222,7 @@ def scrub_custom_query(query, key, txt): def relevance_sorter(key, query, as_dict): value = _(key.name if as_dict else key[0]) return ( - value.lower().startswith(query.lower()) == False, + value.lower().startswith(query.lower()) is not True, value ) From b619c3d33044a0f1b4951a98924ace3fdeb7069f Mon Sep 17 00:00:00 2001 From: codescientist703 Date: Tue, 13 Jul 2021 20:24:24 +0530 Subject: [PATCH 6/7] test: added test for order of link --- frappe/tests/test_search.py | 56 ++++++++++++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/frappe/tests/test_search.py b/frappe/tests/test_search.py index 9ad02f49a6..fb99db2dc7 100644 --- a/frappe/tests/test_search.py +++ b/frappe/tests/test_search.py @@ -7,6 +7,48 @@ from frappe.desk.search import search_link from frappe.desk.search import search_widget class TestSearch(unittest.TestCase): + def setUp(self): + self.tree_doctype_name = 'Test Tree Order' + + # Create Tree doctype + self.tree_doc = frappe.get_doc({ + 'doctype': 'DocType', + 'name': self.tree_doctype_name, + 'module': 'Custom', + 'custom': 1, + 'is_tree': 1, + 'autoname': 'field:random', + 'fields': [{ + 'fieldname': 'random', + 'label': 'Random', + 'fieldtype': 'Data' + }] + }).insert() + self.tree_doc.search_fields = 'parent_test_tree_order' + self.tree_doc.save() + + # Create root for the tree doctype + self.parent_doctype_name = 'All Territories' + frappe.get_doc(doctype=self.tree_doctype_name, random=self.parent_doctype_name, + is_group=1).insert() + + # Create children for the root + self.child_doctypes_names = ['USA', 'India', 'Russia', 'China'] + self.child_doctype_list = [] + for child_name in self.child_doctypes_names: + temp = frappe.get_doc(doctype=self.tree_doctype_name, random=child_name, + parent_test_tree_order=self.parent_doctype_name) + temp.insert() + self.child_doctype_list.append(temp) + + def tearDown(self): + # Deleting all the created doctype + for child_doctype in self.child_doctype_list: + child_doctype.delete() + frappe.delete_doc(self.tree_doctype_name, self.parent_doctype_name, + force=1, ignore_permissions=True, for_reload=True) + self.tree_doc.delete() + def test_search_field_sanitizer(self): # pass search_link('DocType', 'User', query=None, filters=None, page_length=20, searchfield='name') @@ -38,6 +80,18 @@ class TestSearch(unittest.TestCase): search_link, 'DocType', 'Customer', query=None, filters=None, page_length=20, searchfield=';') + def test_link_field_order(self): + # Making a request to the search_link with the tree doctype + search_link(doctype=self.tree_doctype_name, txt='all', query=None, + filters=None, page_length=20, searchfield=None) + result = frappe.response['results'] + + # Check whether the result is sorted or not + self.assertEquals(self.parent_doctype_name, result[0]['value']) + + # Check whether searching for parent also list out children + self.assertEquals(len(result), len(self.child_doctypes_names) + 1) + #Search for the word "pay", part of the word "pays" (country) in french. def test_link_search_in_foreign_language(self): try: @@ -80,4 +134,4 @@ class TestSearch(unittest.TestCase): @frappe.validate_and_sanitize_search_inputs def get_data(doctype, txt, searchfield, start, page_len, filters): - return [doctype, txt, searchfield, start, page_len, filters] \ No newline at end of file + return [doctype, txt, searchfield, start, page_len, filters] From 6f17e3d45fa90b9aec60b6b0c5403c57e707f9df Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 14 Jul 2021 12:02:33 +0530 Subject: [PATCH 7/7] perf(test_search): Setup and teardown tests only if needed --- frappe/tests/test_search.py | 99 ++++++++++++++++++++++--------------- 1 file changed, 60 insertions(+), 39 deletions(-) diff --git a/frappe/tests/test_search.py b/frappe/tests/test_search.py index fb99db2dc7..afb584ea15 100644 --- a/frappe/tests/test_search.py +++ b/frappe/tests/test_search.py @@ -1,4 +1,4 @@ -# Copyright (c) 2018, Frappe Technologies Pvt. Ltd. and Contributors +# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors # MIT License. See license.txt import unittest @@ -6,48 +6,15 @@ import frappe from frappe.desk.search import search_link from frappe.desk.search import search_widget + class TestSearch(unittest.TestCase): def setUp(self): - self.tree_doctype_name = 'Test Tree Order' - - # Create Tree doctype - self.tree_doc = frappe.get_doc({ - 'doctype': 'DocType', - 'name': self.tree_doctype_name, - 'module': 'Custom', - 'custom': 1, - 'is_tree': 1, - 'autoname': 'field:random', - 'fields': [{ - 'fieldname': 'random', - 'label': 'Random', - 'fieldtype': 'Data' - }] - }).insert() - self.tree_doc.search_fields = 'parent_test_tree_order' - self.tree_doc.save() - - # Create root for the tree doctype - self.parent_doctype_name = 'All Territories' - frappe.get_doc(doctype=self.tree_doctype_name, random=self.parent_doctype_name, - is_group=1).insert() - - # Create children for the root - self.child_doctypes_names = ['USA', 'India', 'Russia', 'China'] - self.child_doctype_list = [] - for child_name in self.child_doctypes_names: - temp = frappe.get_doc(doctype=self.tree_doctype_name, random=child_name, - parent_test_tree_order=self.parent_doctype_name) - temp.insert() - self.child_doctype_list.append(temp) + if self._testMethodName == "test_link_field_order": + setup_test_link_field_order(self) def tearDown(self): - # Deleting all the created doctype - for child_doctype in self.child_doctype_list: - child_doctype.delete() - frappe.delete_doc(self.tree_doctype_name, self.parent_doctype_name, - force=1, ignore_permissions=True, for_reload=True) - self.tree_doc.delete() + if self._testMethodName == "test_link_field_order": + teardown_test_link_field_order(self) def test_search_field_sanitizer(self): # pass @@ -135,3 +102,57 @@ class TestSearch(unittest.TestCase): @frappe.validate_and_sanitize_search_inputs def get_data(doctype, txt, searchfield, start, page_len, filters): return [doctype, txt, searchfield, start, page_len, filters] + +def setup_test_link_field_order(TestCase): + TestCase.tree_doctype_name = 'Test Tree Order' + TestCase.child_doctype_list = [] + TestCase.child_doctypes_names = ['USA', 'India', 'Russia', 'China'] + TestCase.parent_doctype_name = 'All Territories' + + # Create Tree doctype + TestCase.tree_doc = frappe.get_doc({ + 'doctype': 'DocType', + 'name': TestCase.tree_doctype_name, + 'module': 'Custom', + 'custom': 1, + 'is_tree': 1, + 'autoname': 'field:random', + 'fields': [{ + 'fieldname': 'random', + 'label': 'Random', + 'fieldtype': 'Data' + }] + }).insert() + TestCase.tree_doc.search_fields = 'parent_test_tree_order' + TestCase.tree_doc.save() + + # Create root for the tree doctype + frappe.get_doc({ + "doctype": TestCase.tree_doctype_name, + "random": TestCase.parent_doctype_name, + "is_group": 1 + }).insert() + + # Create children for the root + for child_name in TestCase.child_doctypes_names: + temp = frappe.get_doc({ + "doctype": TestCase.tree_doctype_name, + "random": child_name, + "parent_test_tree_order": TestCase.parent_doctype_name + }).insert() + TestCase.child_doctype_list.append(temp) + +def teardown_test_link_field_order(TestCase): + # Deleting all the created doctype + for child_doctype in TestCase.child_doctype_list: + child_doctype.delete() + + frappe.delete_doc( + TestCase.tree_doctype_name, + TestCase.parent_doctype_name, + ignore_permissions=True, + force=True, + for_reload=True, + ) + + TestCase.tree_doc.delete()