From 702e8674f94fde353ac12885f5782ea73ef75b6f Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 11 Oct 2022 20:10:06 +0530 Subject: [PATCH 01/22] refactor: use file api to read import data (#18379) * refactor: use file api for reading data in import * fix: remove unused import Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com> Co-authored-by: Faris Ansari Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com> --- .github/workflows/ui-tests.yml | 1 + frappe/core/doctype/data_import/importer.py | 9 ++++++--- frappe/core/doctype/file/file.py | 1 - 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ui-tests.yml b/.github/workflows/ui-tests.yml index 788da98367..ab71726cc2 100644 --- a/.github/workflows/ui-tests.yml +++ b/.github/workflows/ui-tests.yml @@ -128,6 +128,7 @@ jobs: DB: mariadb - name: Verify yarn.lock + if: ${{ steps.check-build.outputs.build == 'strawberry' }} run: | cd ~/frappe-bench/apps/frappe yarn install --immutable --immutable-cache --check-cache diff --git a/frappe/core/doctype/data_import/importer.py b/frappe/core/doctype/data_import/importer.py index 57849e5cfc..ea90b24a6f 100644 --- a/frappe/core/doctype/data_import/importer.py +++ b/frappe/core/doctype/data_import/importer.py @@ -572,12 +572,15 @@ class ImportFile: ###### - def read_file(self, file_path): + def read_file(self, file_path: str): extn = os.path.splitext(file_path)[1][1:] file_content = None - with open(file_path, mode="rb") as f: - file_content = f.read() + + file_name = frappe.db.get_value("File", {"file_url": file_path}) + if file_name: + file = frappe.get_doc("File", file_name) + file_content = file.get_content() return file_content, extn diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index e1faf331d6..1518c72f95 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -422,7 +422,6 @@ class File(Document): return os.path.exists(self.get_full_path()) def get_content(self) -> bytes: - """Returns [`file_name`, `content`] for given file name `fname`""" if self.is_folder: frappe.throw(_("Cannot get file contents of a Folder")) From 23caa1f93ba18a9a9daac7eac491cd1993150bc0 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Tue, 11 Oct 2022 22:08:41 +0530 Subject: [PATCH 02/22] perf: Perform `db.set_value` with single query only (backport #18305) (#18349) * perf: single query `db.set_value` (cherry picked from commit cee2b50461e974c410024d9e2935bb65385bdbe3) * fix: better cache validation - Only delete a single doc if we know which doc changed - Drop all docs other wise (kinda bad, but this isn't used frequently, will fix when visiting entire caching system again) (cherry picked from commit bfa6a5fbdff932c8c3033d70682cd6a751095989) Co-authored-by: Ankush Menat --- frappe/database/database.py | 17 ++++------------- frappe/tests/test_db.py | 23 +++++++++-------------- frappe/tests/test_perf.py | 14 ++++++++++++++ 3 files changed, 27 insertions(+), 27 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index 40117fc5fa..02892a5d58 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -27,7 +27,6 @@ from frappe.database.utils import ( from frappe.exceptions import DoesNotExistError, ImplicitCommitError from frappe.model.utils.link_count import flush_local_link_count from frappe.query_builder.functions import Count -from frappe.query_builder.utils import DocType from frappe.utils import cast as cast_fieldtype from frappe.utils import get_datetime, get_table_name, getdate, now, sbool @@ -857,7 +856,7 @@ class Database: :param modified_by: Set this user as `modified_by`. :param update_modified: default True. Set as false, if you don't want to update the timestamp. :param debug: Print the query in the developer / js console. - :param for_update: Will add a row-level lock to the value that is being set so that it can be released on commit. + :param for_update: [DEPRECATED] This function now performs updates in single query, locking is not required. """ is_single_doctype = not (dn and dt != dn) to_update = field if isinstance(field, dict) else {field: val} @@ -879,19 +878,11 @@ class Database: frappe.clear_document_cache(dt, dt) else: - table = DocType(dt) - - if for_update: - docnames = tuple( - self.get_values(dt, dn, "name", debug=debug, for_update=for_update, pluck=True) - ) or (NullValue(),) - query = frappe.qb.update(table).where(table.name.isin(docnames)) - - for docname in docnames: - frappe.clear_document_cache(dt, docname) + query = frappe.qb.engine.build_conditions(table=dt, filters=dn, update=True) + if isinstance(dn, str): + frappe.clear_document_cache(dt, dn) else: - query = frappe.qb.engine.build_conditions(table=dt, filters=dn, update=True) # TODO: Fix this; doesn't work rn - gavin@frappe.io # frappe.cache().hdel_keys(dt, "document_cache") # Workaround: clear all document caches diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index f4965bb5a9..08fef66bd0 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -734,7 +734,7 @@ class TestDBSetValue(FrappeTestCase): frappe.db.get_value("ToDo", todo.name, ["modified", "modified_by"]), ) - def test_for_update(self): + def test_set_value(self): self.todo1.reload() with patch.object(Database, "sql") as sql_called: @@ -745,28 +745,23 @@ class TestDBSetValue(FrappeTestCase): f"{self.todo1.description}-edit by `test_for_update`", ) first_query = sql_called.call_args_list[0].args[0] - second_query = sql_called.call_args_list[1].args[0] - self.assertTrue(sql_called.call_count == 2) - self.assertTrue("FOR UPDATE" in first_query) if frappe.conf.db_type == "postgres": from frappe.database.postgres.database import modify_query - self.assertTrue(modify_query("UPDATE `tabToDo` SET") in second_query) + self.assertTrue(modify_query("UPDATE `tabToDo` SET") in first_query) if frappe.conf.db_type == "mariadb": - self.assertTrue("UPDATE `tabToDo` SET" in second_query) + self.assertTrue("UPDATE `tabToDo` SET" in first_query) def test_cleared_cache(self): self.todo2.reload() + frappe.get_cached_doc(self.todo2.doctype, self.todo2.name) # init cache - with patch.object(frappe, "clear_document_cache") as clear_cache: - frappe.db.set_value( - self.todo2.doctype, - self.todo2.name, - "description", - f"{self.todo2.description}-edit by `test_cleared_cache`", - ) - clear_cache.assert_called() + description = f"{self.todo2.description}-edit by `test_cleared_cache`" + + frappe.db.set_value(self.todo2.doctype, self.todo2.name, "description", description) + cached_doc = frappe.get_cached_doc(self.todo2.doctype, self.todo2.name) + self.assertEqual(cached_doc.description, description) def test_update_alias(self): args = (self.todo1.doctype, self.todo1.name, "description", "Updated by `test_update_alias`") diff --git a/frappe/tests/test_perf.py b/frappe/tests/test_perf.py index 5ce5d2dd5d..7108228f6c 100644 --- a/frappe/tests/test_perf.py +++ b/frappe/tests/test_perf.py @@ -50,6 +50,20 @@ class TestPerformance(FrappeTestCase): with self.assertQueryCount(0): frappe.get_meta("User") + def test_set_value_query_count(self): + frappe.db.set_value("User", "Administrator", "interest", "Nothing") + + with self.assertQueryCount(1): + frappe.db.set_value("User", "Administrator", "interest", "Nothing") + + with self.assertQueryCount(1): + frappe.db.set_value("User", {"user_type": "System User"}, "interest", "Nothing") + + with self.assertQueryCount(1): + frappe.db.set_value( + "User", {"user_type": "System User"}, {"interest": "Nothing", "bio": "boring person"} + ) + def test_controller_caching(self): get_controller("User") From 5eb803ca95f75d78a8d800b22e32ce4ed6d303fe Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Tue, 11 Oct 2022 19:06:00 +0530 Subject: [PATCH 03/22] fix: format currency value as float while grid upload (cherry picked from commit 95f23b3adb6e22c2e7853da8e0cb809b42c903b1) --- frappe/public/js/frappe/form/grid.js | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/public/js/frappe/form/grid.js b/frappe/public/js/frappe/form/grid.js index 02646a6687..8da073ff59 100644 --- a/frappe/public/js/frappe/form/grid.js +++ b/frappe/public/js/frappe/form/grid.js @@ -1011,6 +1011,7 @@ export default class Grid { Int: (val) => cint(val), Check: (val) => cint(val), Float: (val) => flt(val), + Currency: (val) => flt(val), }; // upload From a0f92537c54dbaaa3792d2d13796a86db3dd080a Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 12 Oct 2022 12:19:37 +0530 Subject: [PATCH 04/22] refactor: load balanced parallel tests without orchestrator (#18386) (#18390) * feat: dry run in test runner for debuggin * refactor: load balance TestRunner using # of tests (cherry picked from commit cfee53d573418bdc8b7bccbb8b5eb8575aaca789) Co-authored-by: Ankush Menat --- frappe/commands/utils.py | 17 +++++++++-- frappe/parallel_test_runner.py | 52 +++++++++++++++++++++++++++++++--- 2 files changed, 63 insertions(+), 6 deletions(-) diff --git a/frappe/commands/utils.py b/frappe/commands/utils.py index 38c45cd8ad..0f7ab79788 100644 --- a/frappe/commands/utils.py +++ b/frappe/commands/utils.py @@ -819,9 +819,16 @@ def run_tests( @click.option("--total-builds", help="Total number of builds", default=1) @click.option("--with-coverage", is_flag=True, help="Build coverage file") @click.option("--use-orchestrator", is_flag=True, help="Use orchestrator to run parallel tests") +@click.option("--dry-run", is_flag=True, default=False, help="Dont actually run tests") @pass_context def run_parallel_tests( - context, app, build_number, total_builds, with_coverage=False, use_orchestrator=False + context, + app, + build_number, + total_builds, + with_coverage=False, + use_orchestrator=False, + dry_run=False, ): with CodeCoverage(with_coverage, app): site = get_site(context) @@ -832,7 +839,13 @@ def run_parallel_tests( else: from frappe.parallel_test_runner import ParallelTestRunner - ParallelTestRunner(app, site=site, build_number=build_number, total_builds=total_builds) + ParallelTestRunner( + app, + site=site, + build_number=build_number, + total_builds=total_builds, + dry_run=dry_run, + ) @click.command( diff --git a/frappe/parallel_test_runner.py b/frappe/parallel_test_runner.py index 39a00235cb..905296c5f3 100644 --- a/frappe/parallel_test_runner.py +++ b/frappe/parallel_test_runner.py @@ -18,11 +18,12 @@ if click_ctx: class ParallelTestRunner: - def __init__(self, app, site, build_number=1, total_builds=1): + def __init__(self, app, site, build_number=1, total_builds=1, dry_run=False): self.app = app self.site = site self.build_number = frappe.utils.cint(build_number) or 1 self.total_builds = frappe.utils.cint(total_builds) + self.dry_run = dry_run self.setup_test_site() self.run_tests() @@ -31,6 +32,9 @@ class ParallelTestRunner: if not frappe.db: frappe.connect() + if self.dry_run: + return + frappe.flags.in_test = True frappe.clear_cache() frappe.utils.scheduler.disable_scheduler() @@ -64,6 +68,10 @@ class ParallelTestRunner: if not file_info: return + if self.dry_run: + print("running tests from", "/".join(file_info)) + return + frappe.set_user("Administrator") path, filename = file_info module = self.get_module(path, filename) @@ -108,12 +116,48 @@ class ParallelTestRunner: sys.exit(1) def get_test_file_list(self): + # Load balance based on total # of tests ~ each runner should get roughly same # of tests. test_list = get_all_tests(self.app) - split_size = frappe.utils.ceil(len(test_list) / self.total_builds) - # [1,2,3,4,5,6] to [[1,2], [3,4], [4,6]] if split_size is 2 - test_chunks = [test_list[x : x + split_size] for x in range(0, len(test_list), split_size)] + + test_counts = [self.get_test_count(test) for test in test_list] + test_chunks = split_by_weight(test_list, test_counts, chunk_count=self.total_builds) + return test_chunks[self.build_number - 1] + @staticmethod + def get_test_count(test): + """Get approximate count of tests inside a file""" + file_name = "/".join(test) + + with open(file_name) as f: + test_count = f.read().count("def test_") + + return test_count + + +def split_by_weight(work, weights, chunk_count): + """Roughly split work by respective weight while keep ordering.""" + expected_weight = sum(weights) // chunk_count + + chunks = [[] for _ in range(chunk_count)] + + chunk_no = 0 + chunk_weight = 0 + + for task, weight in zip(work, weights): + if chunk_weight > expected_weight: + chunk_weight = 0 + chunk_no += 1 + assert chunk_no < chunk_count + + chunks[chunk_no].append(task) + chunk_weight += weight + + assert len(work) == sum(len(chunk) for chunk in chunks) + assert len(chunks) == chunk_count + + return chunks + class ParallelTestResult(unittest.TextTestResult): def startTest(self, test): From e0d6d0b7b1af130e9ba15749b080fa5f8f99fc06 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 12 Oct 2022 12:28:58 +0530 Subject: [PATCH 05/22] fix: progress bar not disappearing (#18375) (#18392) * fix: progress bar not disappearing * style: format [skip ci] Co-authored-by: Ankush Menat (cherry picked from commit 729be707cb66e3d08e87ea3e845c701467f1758d) Co-authored-by: stephen --- frappe/public/js/frappe/socketio_client.js | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/frappe/public/js/frappe/socketio_client.js b/frappe/public/js/frappe/socketio_client.js index 6d01c19d42..9e290ede0b 100644 --- a/frappe/public/js/frappe/socketio_client.js +++ b/frappe/public/js/frappe/socketio_client.js @@ -42,16 +42,13 @@ frappe.socketio = { data.percent = (flt(data.progress[0]) / data.progress[1]) * 100; } if (data.percent) { - if (data.percent == 100) { - frappe.hide_progress(); - } else { - frappe.show_progress( - data.title || __("Progress"), - data.percent, - 100, - data.description - ); - } + frappe.show_progress( + data.title || __("Progress"), + data.percent, + 100, + data.description, + true + ); } }); From 7c9ee6d4a7a299216c1b5597d152275adeda3bdb Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 12 Oct 2022 15:30:14 +0530 Subject: [PATCH 06/22] fix: google calendar sync times (#18384) (#18394) (cherry picked from commit 77ae997b146274f562e49eb676e57d6b11d9658a) Co-authored-by: PeterG --- .../integrations/doctype/google_calendar/google_calendar.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/integrations/doctype/google_calendar/google_calendar.py b/frappe/integrations/doctype/google_calendar/google_calendar.py index 09ed012454..9f9aca1123 100644 --- a/frappe/integrations/doctype/google_calendar/google_calendar.py +++ b/frappe/integrations/doctype/google_calendar/google_calendar.py @@ -517,10 +517,10 @@ def google_calendar_to_repeat_on(start, end, recurrence=None): repeat_on = { "starts_on": get_datetime(start.get("date")) if start.get("date") - else parser.parse(start.get("dateTime")).utcnow(), + else parser.parse(start.get("dateTime")).astimezone().replace(tzinfo=None), "ends_on": get_datetime(end.get("date")) if end.get("date") - else parser.parse(end.get("dateTime")).utcnow(), + else parser.parse(end.get("dateTime")).astimezone().replace(tzinfo=None), "all_day": 1 if start.get("date") else 0, "repeat_this_event": 1 if recurrence else 0, "repeat_on": None, From 73758d7883a0f8bcffa50593d42bd0fbaa153225 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 12 Oct 2022 16:33:26 +0530 Subject: [PATCH 07/22] feat: search in translated title, if we show title (#17828) (#18395) * 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 3d17e1589e30e659985afa0ab70f3408bea08eb9) Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com> --- frappe/core/doctype/doctype/doctype.json | 5 +- frappe/desk/search.py | 106 +++++++----------- frappe/public/js/frappe/form/controls/link.js | 12 +- 3 files changed, 56 insertions(+), 67 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.json b/frappe/core/doctype/doctype/doctype.json index bfa91cea75..0b2ced43da 100644 --- a/frappe/core/doctype/doctype/doctype.json +++ b/frappe/core/doctype/doctype/doctype.json @@ -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", @@ -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", diff --git a/frappe/desk/search.py b/frappe/desk/search.py index 53a8c39ee5..c4a94074ba 100644 --- a/frappe/desk/search.py +++ b/frappe/desk/search.py @@ -8,7 +8,6 @@ import re 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 @@ -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 = [ @@ -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}%"]) @@ -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 = ( @@ -219,16 +213,13 @@ 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, @@ -236,12 +227,15 @@ def search_widget( 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 @@ -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): @@ -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 @@ -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 diff --git a/frappe/public/js/frappe/form/controls/link.js b/frappe/public/js/frappe/form/controls/link.js index 9a7cc0f2c4..076e343f22 100644 --- a/frappe/public/js/frappe/form/controls/link.js +++ b/frappe/public/js/frappe/form/controls/link.js @@ -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; } @@ -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 || "" + _label + ""; - 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 += '
' + __(d.description) + ""; } return $("
  • ") From c2f1c5744b269758ebce48b5fb8feac0488442c1 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 12 Oct 2022 22:12:58 +0530 Subject: [PATCH 08/22] fix: correct import for markupsafe.escape ref: https://github.com/pallets/jinja/issues/1626 --- frappe/tests/test_search.py | 22 ++++++++++++++++++++++ frappe/www/search.py | 4 ++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/frappe/tests/test_search.py b/frappe/tests/test_search.py index 1137c42ebb..82c2f4c567 100644 --- a/frappe/tests/test_search.py +++ b/frappe/tests/test_search.py @@ -3,8 +3,11 @@ import frappe +from frappe.app import make_form_dict from frappe.desk.search import get_names_for_mentions, search_link, search_widget from frappe.tests.utils import FrappeTestCase +from frappe.utils import set_request +from frappe.website.serve import get_response class TestSearch(FrappeTestCase): @@ -235,3 +238,22 @@ def teardown_test_link_field_order(TestCase): ) TestCase.tree_doc.delete() + + +class TestWebsiteSearch(FrappeTestCase): + def get(self, path, user="Guest"): + frappe.set_user(user) + set_request(method="GET", path=path) + make_form_dict(frappe.local.request) + response = get_response() + frappe.set_user("Administrator") + return response + + def test_basic_search(self): + + no_search = self.get("/search") + self.assertEqual(no_search.status_code, 200) + + response = self.get("/search?q=b") + self.assertEqual(response.status_code, 200) + self.assertIn("Search Results", response.get_data(as_text=True)) diff --git a/frappe/www/search.py b/frappe/www/search.py index d8a939cb15..8eac7b5cd6 100644 --- a/frappe/www/search.py +++ b/frappe/www/search.py @@ -1,4 +1,4 @@ -from jinja2 import utils +import markupsafe import frappe from frappe import _ @@ -10,7 +10,7 @@ from frappe.utils.global_search import web_search def get_context(context): context.no_cache = 1 if frappe.form_dict.q: - query = str(utils.escape(sanitize_html(frappe.form_dict.q))) + query = str(markupsafe.escape(sanitize_html(frappe.form_dict.q))) context.title = _("Search Results for") context.query = query context.route = "/search" From 6848ee9c5759d45f8c53c870b435c10217faaf66 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 13 Oct 2022 11:43:41 +0530 Subject: [PATCH 09/22] fix: explicitly set doctype in queries (#18403) (#18405) (cherry picked from commit 2f358dea038c9dfcacd0f6e5af10e9a291fde726) Co-authored-by: Dany Robert --- frappe/contacts/doctype/address/address.py | 3 ++- frappe/contacts/doctype/contact/contact.py | 3 ++- frappe/core/doctype/user/user.py | 1 + 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/frappe/contacts/doctype/address/address.py b/frappe/contacts/doctype/address/address.py index 42dbdd6177..5fe22eb7f2 100644 --- a/frappe/contacts/doctype/address/address.py +++ b/frappe/contacts/doctype/address/address.py @@ -228,11 +228,12 @@ def get_company_address(company): def address_query(doctype, txt, searchfield, start, page_len, filters): from frappe.desk.reportview import get_match_cond + doctype = "Address" link_doctype = filters.pop("link_doctype") link_name = filters.pop("link_name") condition = "" - meta = frappe.get_meta("Address") + meta = frappe.get_meta(doctype) for fieldname, value in filters.items(): if meta.get_field(fieldname) or fieldname in frappe.db.DEFAULT_COLUMNS: condition += f" and {fieldname}={frappe.db.escape(value)}" diff --git a/frappe/contacts/doctype/contact/contact.py b/frappe/contacts/doctype/contact/contact.py index a17f46216b..1ed4307d8d 100644 --- a/frappe/contacts/doctype/contact/contact.py +++ b/frappe/contacts/doctype/contact/contact.py @@ -210,8 +210,9 @@ def update_contact(doc, method): def contact_query(doctype, txt, searchfield, start, page_len, filters): from frappe.desk.reportview import get_match_cond + doctype = "Contact" if ( - not frappe.get_meta("Contact").get_field(searchfield) + not frappe.get_meta(doctype).get_field(searchfield) and searchfield not in frappe.db.DEFAULT_COLUMNS ): return [] diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 50f8697296..345be99a63 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -901,6 +901,7 @@ def reset_password(user): def user_query(doctype, txt, searchfield, start, page_len, filters): from frappe.desk.reportview import get_filters_cond, get_match_cond + doctype = "User" conditions = [] user_type_condition = "and user_type != 'Website User'" From 19a394e99fddf35fbfaf73be4c46ca6f0f6f7972 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 13 Oct 2022 12:23:07 +0530 Subject: [PATCH 10/22] test: drop/fix flaky tests --- .../event_producer/test_event_producer.py | 38 ------------------- 1 file changed, 38 deletions(-) diff --git a/frappe/event_streaming/doctype/event_producer/test_event_producer.py b/frappe/event_streaming/doctype/event_producer/test_event_producer.py index 70e7483f92..aac886c0ba 100644 --- a/frappe/event_streaming/doctype/event_producer/test_event_producer.py +++ b/frappe/event_streaming/doctype/event_producer/test_event_producer.py @@ -44,44 +44,6 @@ class TestEventProducer(FrappeTestCase): self.pull_producer_data() self.assertFalse(frappe.db.exists("ToDo", producer_doc.name)) - @run_only_if(db_type_is.MARIADB) - def test_multiple_doctypes_sync(self): - # TODO: This test is extremely flaky with Postgres. Rewrite this! - producer = get_remote_site() - - # insert todo and note in producer - producer_todo = insert_into_producer(producer, "test multiple doc sync") - producer_note1 = frappe._dict(doctype="Note", title="test multiple doc sync 1") - delete_on_remote_if_exists(producer, "Note", {"title": producer_note1["title"]}) - frappe.db.delete("Note", {"title": producer_note1["title"]}) - producer_note1 = producer.insert(producer_note1) - producer_note2 = frappe._dict(doctype="Note", title="test multiple doc sync 2") - delete_on_remote_if_exists(producer, "Note", {"title": producer_note2["title"]}) - frappe.db.delete("Note", {"title": producer_note2["title"]}) - producer_note2 = producer.insert(producer_note2) - - # update in producer - producer_todo["description"] = "test multiple doc update sync" - producer_todo = producer.update(producer_todo) - producer_note1["content"] = "testing update sync" - producer_note1 = producer.update(producer_note1) - - producer.delete("Note", producer_note2.name) - - self.pull_producer_data() - - # check inserted - self.assertTrue(frappe.db.exists("ToDo", producer_todo.name)) - - # check update - local_todo = frappe.get_doc("ToDo", producer_todo.name) - self.assertEqual(local_todo.description, producer_todo.description) - local_note1 = frappe.get_doc("Note", producer_note1.name) - self.assertEqual(local_note1.content, producer_note1.content) - - # check delete - self.assertFalse(frappe.db.exists("Note", producer_note2.name)) - def test_child_table_sync_with_dependencies(self): producer = get_remote_site() producer_user = frappe._dict( From cb866f921e46807e4338803be5ca1647ae6e9132 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Wed, 12 Oct 2022 22:29:30 +0530 Subject: [PATCH 11/22] fix(UI): Child table responsive (cherry picked from commit 9280a41e2737f1fea1160b2143bcdc827479fe23) --- frappe/public/js/frappe/form/grid.js | 28 ++++++++++++------------ frappe/public/js/frappe/form/grid_row.js | 18 ++++++++++----- frappe/public/scss/common/grid.scss | 15 +++++++++++++ 3 files changed, 41 insertions(+), 20 deletions(-) diff --git a/frappe/public/js/frappe/form/grid.js b/frappe/public/js/frappe/form/grid.js index 8da073ff59..2ffba97efa 100644 --- a/frappe/public/js/frappe/form/grid.js +++ b/frappe/public/js/frappe/form/grid.js @@ -52,9 +52,7 @@ export default class Grid { } allow_on_grid_editing() { - if (frappe.utils.is_xs()) { - return false; - } else if ((this.meta && this.meta.editable_grid) || !this.meta) { + if ((this.meta && this.meta.editable_grid) || !this.meta) { return true; } else { return false; @@ -66,17 +64,19 @@ export default class Grid {

    -
    -
    -
    -
    -
    - Grid Empty State - ${__("No Data")} +
    +
    +
    +
    +
    +
    + Grid Empty State + ${__("No Data")} +
    diff --git a/frappe/public/js/frappe/form/grid_row.js b/frappe/public/js/frappe/form/grid_row.js index 65f7be3dd1..4927f1e300 100644 --- a/frappe/public/js/frappe/form/grid_row.js +++ b/frappe/public/js/frappe/form/grid_row.js @@ -254,7 +254,7 @@ export default class GridRow { ).appendTo(this.row); this.row_index = $( - `