From 0b418d27281fecded77a5c545bbd606689119fa5 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 25 Feb 2022 01:08:31 +0530 Subject: [PATCH 01/53] perf: Pass document through rename_doc API to bypass extra get_doc --- frappe/model/rename_doc.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/frappe/model/rename_doc.py b/frappe/model/rename_doc.py index 787f276b17..8891bd20ea 100644 --- a/frappe/model/rename_doc.py +++ b/frappe/model/rename_doc.py @@ -4,6 +4,7 @@ from typing import TYPE_CHECKING, Dict, List, Optional import frappe from frappe import _, bold +from frappe.model.document import Document from frappe.model.dynamic_links import get_dynamic_link_map from frappe.model.naming import validate_name from frappe.model.utils.user_settings import sync_user_settings, update_user_settings_data @@ -47,7 +48,7 @@ def update_document_title( name_updated = updated_name != doc.name if name_updated: - docname = rename_doc(doctype=doctype, old=docname, new=updated_name, merge=merge) + docname = rename_doc(doc=doc, new=updated_name, merge=merge) if title_updated: try: @@ -65,17 +66,27 @@ def update_document_title( return docname def rename_doc( - doctype: str, - old: str, - new: str, + doctype: Optional[str] = None, + old: Optional[str] = None, + new: str = None, force: bool = False, merge: bool = False, ignore_permissions: bool = False, ignore_if_exists: bool = False, show_alert: bool = True, rebuild_search: bool = True, + doc: Optional[Document] = None, ) -> str: """Rename a doc(dt, old) to doc(dt, new) and update all linked fields of type "Link".""" + old_usage_style = doctype and old and new + new_usage_style = doc and new + + if not (new_usage_style or old_usage_style): + raise TypeError("{doctype, old, new} or {doc, new} are required arguments for frappe.model.rename_doc") + + old = old or doc.name + doctype = doctype or doc.doctype + if not frappe.db.exists(doctype, old): return @@ -91,7 +102,7 @@ def rename_doc( meta = frappe.get_meta(doctype) # call before_rename - old_doc = frappe.get_doc(doctype, old) + old_doc = doc or frappe.get_doc(doctype, old) out = old_doc.run_method("before_rename", old, new, merge) or {} new = (out.get("new") or new) if isinstance(out, dict) else (out or new) new = validate_rename(doctype, new, meta, merge, force, ignore_permissions) From 5a3d8f925ee93a393ff5143d372b4e97bdfa4bb2 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 25 Feb 2022 01:27:21 +0530 Subject: [PATCH 02/53] feat: Document.rename API Transform current document object using the rename_doc API. The design of the API should allow for easy action queueing. Defined as `rename(self, name: str, merge: bool = False, force: bool = False)` Usage: In [1]: doc = frappe.get_doc("Person", "5a188f66c1") In [2]: doc.rename("5a188f66c2") In [3]: doc.name Out[3]: '5a188f66c2' --- frappe/model/document.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/frappe/model/document.py b/frappe/model/document.py index cb36c18b47..5017d342c1 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -938,6 +938,14 @@ class Document(BaseDocument): self.docstatus = DocStatus.cancelled() return self.save() + @whitelist.__func__ + def _rename(self, name: str, merge: bool = False, force: bool = False): + """Cancel the document. Sets `docstatus` = 2, then saves. + """ + from frappe.model.rename_doc import rename_doc + self.name = rename_doc(doc=self, new=name, merge=merge, force=force) + self.reload() + @whitelist.__func__ def submit(self): """Submit the document. Sets `docstatus` = 1, then saves.""" @@ -948,6 +956,12 @@ class Document(BaseDocument): """Cancel the document. Sets `docstatus` = 2, then saves.""" return self._cancel() + @whitelist.__func__ + def rename(self, name: str, merge: bool = False, force: bool = False): + """Rename the document to `name`. This transforms the current object. + """ + return self._rename(name=name, merge=merge, force=force) + def delete(self, ignore_permissions=False): """Delete document.""" frappe.delete_doc(self.doctype, self.name, ignore_permissions = ignore_permissions, flags=self.flags) From 040e01eb59b90b7c2b0ebf103324a83065306c08 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 25 Feb 2022 01:43:11 +0530 Subject: [PATCH 03/53] refactor: doc.enqueue_action & execute_action * Make args "internal" to avoid name collision. Added __ since these args would be utilized by the internal API alone * perf(EAFP): Try taking a lock and then figure out if there's an error instead of the double computation * Notify document update on completion of Document Action --- frappe/model/document.py | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/frappe/model/document.py b/frappe/model/document.py index 5017d342c1..0bfba3d78b 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -1318,16 +1318,17 @@ class Document(BaseDocument): # See: Stock Reconciliation from frappe.utils.background_jobs import enqueue - if hasattr(self, '_' + action): - action = '_' + action + if hasattr(self, f"_{action}"): + action = f"_{action}" - if file_lock.lock_exists(self.get_signature()): + try: + self.lock() + except frappe.DocumentLockedError: frappe.throw(_('This document is currently queued for execution. Please try again'), title=_('Document Queued')) - self.lock() - enqueue('frappe.model.document.execute_action', doctype=self.doctype, name=self.name, - action=action, **kwargs) + return enqueue('frappe.model.document.execute_action', __doctype=self.doctype, __name=self.name, + __action=action, **kwargs) def lock(self, timeout=None): """Creates a lock file for the given document. If timeout is set, @@ -1402,12 +1403,12 @@ class Document(BaseDocument): return f"{doctype}({name})" -def execute_action(doctype, name, action, **kwargs): +def execute_action(__doctype, __name, __action, **kwargs): """Execute an action on a document (called by background worker)""" - doc = frappe.get_doc(doctype, name) + doc = frappe.get_doc(__doctype, __name) doc.unlock() try: - getattr(doc, action)(**kwargs) + getattr(doc, __action)(**kwargs) except Exception: frappe.db.rollback() @@ -1418,7 +1419,4 @@ def execute_action(doctype, name, action, **kwargs): msg = '
' + frappe.get_traceback() + '
' doc.add_comment('Comment', _('Action Failed') + '

' + msg) - doc.notify_update() - - - + doc.notify_update() From 6c5a975cec8b2cd625651cf768e07a08fa0da379 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 28 Feb 2022 18:02:38 +0530 Subject: [PATCH 04/53] test: Added test_doc_rename --- frappe/tests/test_rename_doc.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/frappe/tests/test_rename_doc.py b/frappe/tests/test_rename_doc.py index 0c1cba31fb..7630356c6d 100644 --- a/frappe/tests/test_rename_doc.py +++ b/frappe/tests/test_rename_doc.py @@ -237,3 +237,10 @@ class TestRenameDoc(unittest.TestCase): update_linked_doctypes("User", "ToDo", "str", "str") self.assertTrue("Function frappe.model.rename_doc.update_linked_doctypes" in stdout.getvalue()) + + def test_doc_rename(self): + name = choice(self.available_documents) + new_name = f"{name}-{frappe.generate_hash(length=4)}" + doc = frappe.get_doc(self.test_doctype, name) + doc.rename(new_name, merge=frappe.db.exists(self.test_doctype, new_name)) + self.assertEqual(doc.name, new_name) From 429162ba313f117ade5cd4857e65407d1e93ef74 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 28 Feb 2022 18:05:36 +0530 Subject: [PATCH 05/53] fix: Use rename doc method wrapper --- frappe/model/rename_doc.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/model/rename_doc.py b/frappe/model/rename_doc.py index 8891bd20ea..196cbee27d 100644 --- a/frappe/model/rename_doc.py +++ b/frappe/model/rename_doc.py @@ -48,7 +48,7 @@ def update_document_title( name_updated = updated_name != doc.name if name_updated: - docname = rename_doc(doc=doc, new=updated_name, merge=merge) + doc.rename(updated_name, merge=merge) if title_updated: try: @@ -63,7 +63,7 @@ def update_document_title( ) raise - return docname + return doc.name def rename_doc( doctype: Optional[str] = None, From cd824f1c0815dcca2b784bcf185a90de59d27cd0 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 28 Feb 2022 18:58:52 +0530 Subject: [PATCH 06/53] test(fix): Add property setter for ToDo to be made rename-able --- frappe/tests/test_rename_doc.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/frappe/tests/test_rename_doc.py b/frappe/tests/test_rename_doc.py index 7630356c6d..f02a43a83d 100644 --- a/frappe/tests/test_rename_doc.py +++ b/frappe/tests/test_rename_doc.py @@ -48,6 +48,14 @@ class TestRenameDoc(unittest.TestCase): # data generation: for base and merge tests self.available_documents = [] self.test_doctype = "ToDo" + self.property_setter = frappe.get_doc({ + "doctype":"Property Setter", + "doctype_or_field": "DocType", + "doc_type": self.test_doctype, + "property": "allow_rename", + "property_type": "Check", + "value": "1", + }).insert() for num in range(1, 5): doc = frappe.get_doc({ @@ -85,6 +93,7 @@ class TestRenameDoc(unittest.TestCase): # delete the documents created for docname in self.available_documents: frappe.delete_doc(self.test_doctype, docname) + self.property_setter.delete() for dt in self.doctype.values(): if frappe.db.exists("DocType", dt): From 311a42ae5279b910f3e195f6b797e2e6eb14c993 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 28 Feb 2022 19:00:59 +0530 Subject: [PATCH 07/53] fix: Print to STDOUT to help debug via is_scheduler_inactive --- frappe/utils/scheduler.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/frappe/utils/scheduler.py b/frappe/utils/scheduler.py index 8ebb4b2937..346049141a 100755 --- a/frappe/utils/scheduler.py +++ b/frappe/utils/scheduler.py @@ -24,6 +24,15 @@ from frappe.utils.background_jobs import get_jobs DATETIME_FORMAT = '%Y-%m-%d %H:%M:%S' + +def cprint(*args, **kwargs): + """Prints only if called from STDOUT""" + try: + os.get_terminal_size() + print(*args, **kwargs) + except Exception: + pass + def start_scheduler(): '''Run enqueue_events_for_all_sites every 2 minutes (default). Specify scheduler_interval in seconds in common_site_config.json''' @@ -86,9 +95,11 @@ def enqueue_events(site): def is_scheduler_inactive(): if frappe.local.conf.maintenance_mode: + cprint("Maintenance mode is ON") return True if frappe.local.conf.pause_scheduler: + cprint("frappe.conf.pause_scheduler is SET") return True if is_scheduler_disabled(): @@ -98,9 +109,13 @@ def is_scheduler_inactive(): def is_scheduler_disabled(): if frappe.conf.disable_scheduler: + cprint("frappe.conf.disable_scheduler is SET") return True - return not frappe.utils.cint(frappe.db.get_single_value("System Settings", "enable_scheduler")) + scheduler_disabled = not frappe.utils.cint(frappe.db.get_single_value("System Settings", "enable_scheduler")) + if scheduler_disabled: + cprint("SystemSettings.enable_scheduler is UNSET") + return scheduler_disabled def toggle_scheduler(enable): frappe.db.set_value("System Settings", None, "enable_scheduler", 1 if enable else 0) From 2b3d9cbcd8566622a19457643f56f19af611d161 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 28 Feb 2022 19:46:58 +0530 Subject: [PATCH 08/53] feat: Rename Document via Background Job --- frappe/model/rename_doc.py | 6 +++++- frappe/public/js/frappe/form/toolbar.js | 11 +++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/frappe/model/rename_doc.py b/frappe/model/rename_doc.py index 196cbee27d..3231c82230 100644 --- a/frappe/model/rename_doc.py +++ b/frappe/model/rename_doc.py @@ -11,6 +11,7 @@ from frappe.model.utils.user_settings import sync_user_settings, update_user_set from frappe.query_builder import Field from frappe.utils import cint from frappe.utils.password import rename_password +from frappe.utils.scheduler import is_scheduler_inactive if TYPE_CHECKING: from frappe.model.meta import Meta @@ -48,7 +49,10 @@ def update_document_title( name_updated = updated_name != doc.name if name_updated: - doc.rename(updated_name, merge=merge) + if is_scheduler_inactive(): + doc.rename(updated_name, merge=merge) + else: + doc.queue_action("rename", name=updated_name, merge=merge) if title_updated: try: diff --git a/frappe/public/js/frappe/form/toolbar.js b/frappe/public/js/frappe/form/toolbar.js index 500ec88068..a5938391c2 100644 --- a/frappe/public/js/frappe/form/toolbar.js +++ b/frappe/public/js/frappe/form/toolbar.js @@ -98,6 +98,17 @@ frappe.ui.form.Toolbar = class Toolbar { } let rename_document = () => { + frappe.socketio.doc_subscribe(doctype, new_name); + frappe.realtime.on("doc_update", data => { + if (data.doctype == doctype && data.name == new_name) { + $(document).trigger("rename", [doctype, docname, new_name]); + if (locals[doctype] && locals[doctype][docname]) delete locals[doctype][docname]; + this.frm.reload_doc(); + frappe.show_alert( + __('Document renamed from {0} to {1}', [docname.bold(), new_name.bold()]) + ) + } + }); return frappe.xcall("frappe.model.rename_doc.update_document_title", { doctype, docname, From 1ff0bbf6c64e0b940e9d44faebddc1b25178a89b Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 1 Mar 2022 11:49:44 +0530 Subject: [PATCH 09/53] test(fix): Add cleanup for test_doc_rename_method --- frappe/tests/test_rename_doc.py | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/frappe/tests/test_rename_doc.py b/frappe/tests/test_rename_doc.py index f02a43a83d..2a4a3805f7 100644 --- a/frappe/tests/test_rename_doc.py +++ b/frappe/tests/test_rename_doc.py @@ -48,14 +48,6 @@ class TestRenameDoc(unittest.TestCase): # data generation: for base and merge tests self.available_documents = [] self.test_doctype = "ToDo" - self.property_setter = frappe.get_doc({ - "doctype":"Property Setter", - "doctype_or_field": "DocType", - "doc_type": self.test_doctype, - "property": "allow_rename", - "property_type": "Check", - "value": "1", - }).insert() for num in range(1, 5): doc = frappe.get_doc({ @@ -93,7 +85,6 @@ class TestRenameDoc(unittest.TestCase): # delete the documents created for docname in self.available_documents: frappe.delete_doc(self.test_doctype, docname) - self.property_setter.delete() for dt in self.doctype.values(): if frappe.db.exists("DocType", dt): @@ -107,8 +98,23 @@ class TestRenameDoc(unittest.TestCase): def setUp(self): frappe.flags.link_fields = {} + if self._testMethodName == "test_doc_rename_method": + self.property_setter = frappe.get_doc({ + "doctype":"Property Setter", + "doctype_or_field": "DocType", + "doc_type": self.test_doctype, + "property": "allow_rename", + "property_type": "Check", + "value": "1", + }).insert() + super().setUp() + def tearDown(self) -> None: + if self._testMethodName == "test_doc_rename_method": + self.property_setter.delete() + return super().tearDown() + def test_rename_doc(self): """Rename an existing document via frappe.rename_doc""" old_name = choice(self.available_documents) @@ -247,9 +253,11 @@ class TestRenameDoc(unittest.TestCase): update_linked_doctypes("User", "ToDo", "str", "str") self.assertTrue("Function frappe.model.rename_doc.update_linked_doctypes" in stdout.getvalue()) - def test_doc_rename(self): + def test_doc_rename_method(self): name = choice(self.available_documents) new_name = f"{name}-{frappe.generate_hash(length=4)}" doc = frappe.get_doc(self.test_doctype, name) doc.rename(new_name, merge=frappe.db.exists(self.test_doctype, new_name)) self.assertEqual(doc.name, new_name) + self.available_documents.append(new_name) + self.available_documents.remove(name) From 81a3058808c9cac2c0543f9606408fe086341fdf Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 1 Mar 2022 13:22:46 +0530 Subject: [PATCH 10/53] fix: enqueue option for update_document_title API Pass enqueue=True to enqueue rename document operation. Desk will be using this from now on ;) --- frappe/model/rename_doc.py | 17 +++++++++++++---- frappe/public/js/frappe/form/toolbar.js | 3 ++- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/frappe/model/rename_doc.py b/frappe/model/rename_doc.py index 3231c82230..8c66f408b3 100644 --- a/frappe/model/rename_doc.py +++ b/frappe/model/rename_doc.py @@ -25,10 +25,19 @@ def update_document_title( title: Optional[str] = None, name: Optional[str] = None, merge: bool = False, + enqueue: bool = False, **kwargs ) -> str: """ - Update title from header in form view + Update the name or title of a document. Returns `name` if document was renamed, + `docname` if renaming operation was queued. + + :param doctype: DocType of the document + :param docname: Name of the document + :param title: New Title of the document + :param name: New Name of the document + :param merge: Merge the current Document with the existing one if exists + :param enqueue: Enqueue the rename operation, title is updated in current process """ # to maintain backwards API compatibility @@ -49,10 +58,10 @@ def update_document_title( name_updated = updated_name != doc.name if name_updated: - if is_scheduler_inactive(): - doc.rename(updated_name, merge=merge) - else: + if enqueue and not is_scheduler_inactive(): doc.queue_action("rename", name=updated_name, merge=merge) + else: + doc.rename(updated_name, merge=merge) if title_updated: try: diff --git a/frappe/public/js/frappe/form/toolbar.js b/frappe/public/js/frappe/form/toolbar.js index a5938391c2..c0c24d86bf 100644 --- a/frappe/public/js/frappe/form/toolbar.js +++ b/frappe/public/js/frappe/form/toolbar.js @@ -106,7 +106,7 @@ frappe.ui.form.Toolbar = class Toolbar { this.frm.reload_doc(); frappe.show_alert( __('Document renamed from {0} to {1}', [docname.bold(), new_name.bold()]) - ) + ); } }); return frappe.xcall("frappe.model.rename_doc.update_document_title", { @@ -114,6 +114,7 @@ frappe.ui.form.Toolbar = class Toolbar { docname, name: new_name, title: new_title, + enqueue: true, merge }).then(new_docname => { if (new_name != docname) { From 9e36005c70a815089f3dacdd75c0262d5169eb6e Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 3 Mar 2022 19:59:56 +0530 Subject: [PATCH 11/53] fix: Remove taken file locks at end of background job --- frappe/__init__.py | 1 + frappe/model/document.py | 2 ++ frappe/utils/background_jobs.py | 5 +++++ 3 files changed, 8 insertions(+) diff --git a/frappe/__init__.py b/frappe/__init__.py index 8a8b70afe3..c985270b70 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -181,6 +181,7 @@ def init(site, sites_path=None, new_site=False): "new_site": new_site }) local.rollback_observers = [] + local.locked_documents = [] local.before_commit = [] local.test_objects = {} diff --git a/frappe/model/document.py b/frappe/model/document.py index 0bfba3d78b..68a02c883d 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -1347,10 +1347,12 @@ class Document(BaseDocument): if lock_exists: raise frappe.DocumentLockedError file_lock.create_lock(signature) + frappe.local.locked_documents.append(self) def unlock(self): """Delete the lock file for this document""" file_lock.delete_lock(self.get_signature()) + frappe.local.locked_documents.remove(self) # validation helpers def validate_from_to_dates(self, from_date_field, to_date_field): diff --git a/frappe/utils/background_jobs.py b/frappe/utils/background_jobs.py index 58029dbc5f..b93fcc5605 100755 --- a/frappe/utils/background_jobs.py +++ b/frappe/utils/background_jobs.py @@ -147,6 +147,11 @@ def execute_job(site, method, event, job_name, kwargs, user=None, is_async=True, frappe.db.commit() finally: + # background job hygiene: release file locks if unreleased + # if this breaks something, move it to failed jobs alone - gavin@frappe.io + for doc in frappe.local.locked_documents: + doc.unlock() + frappe.monitor.stop() if is_async: frappe.destroy() From c6a0469f6a4d946d6a348de7bbf180e3fbc1669e Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 8 Mar 2022 17:57:18 +0530 Subject: [PATCH 12/53] fix: Remove lock record if exists --- frappe/model/document.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frappe/model/document.py b/frappe/model/document.py index 5443dcff2f..883b90b464 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -1350,7 +1350,8 @@ class Document(BaseDocument): def unlock(self): """Delete the lock file for this document""" file_lock.delete_lock(self.get_signature()) - frappe.local.locked_documents.remove(self) + if self in frappe.local.locked_documents: + frappe.local.locked_documents.remove(self) # validation helpers def validate_from_to_dates(self, from_date_field, to_date_field): From ea0e0a5a7f66e4b37e48e51fd5941b993b32439f Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 8 Mar 2022 18:01:17 +0530 Subject: [PATCH 13/53] fix: Setup queued renaming listener based on API response --- frappe/public/js/frappe/form/toolbar.js | 32 +++++++++++++++---------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/frappe/public/js/frappe/form/toolbar.js b/frappe/public/js/frappe/form/toolbar.js index 44625c66ea..5cd429a4c3 100644 --- a/frappe/public/js/frappe/form/toolbar.js +++ b/frappe/public/js/frappe/form/toolbar.js @@ -85,12 +85,11 @@ frappe.ui.form.Toolbar = class Toolbar { }); } rename_document_title(new_name, new_title, merge=false) { + let confirm_message = null; const docname = this.frm.doc.name; const title_field = this.frm.meta.title_field || ''; const doctype = this.frm.doctype; - let confirm_message=null; - if (new_name) { const warning = __("This cannot be undone"); const message = __("Are you sure you want to merge {0} with {1}?", [docname.bold(), new_name.bold()]); @@ -98,17 +97,6 @@ frappe.ui.form.Toolbar = class Toolbar { } let rename_document = () => { - frappe.socketio.doc_subscribe(doctype, new_name); - frappe.realtime.on("doc_update", data => { - if (data.doctype == doctype && data.name == new_name) { - $(document).trigger("rename", [doctype, docname, new_name]); - if (locals[doctype] && locals[doctype][docname]) delete locals[doctype][docname]; - this.frm.reload_doc(); - frappe.show_alert( - __('Document renamed from {0} to {1}', [docname.bold(), new_name.bold()]) - ); - } - }); return frappe.xcall("frappe.model.rename_doc.update_document_title", { doctype, docname, @@ -119,6 +107,24 @@ frappe.ui.form.Toolbar = class Toolbar { freeze: true, freeze_message: __("Updating related fields...") }).then(new_docname => { + // handle document renaming queued action + if (new_docname == docname) { + frappe.socketio.doc_subscribe(doctype, new_name); + frappe.realtime.on("doc_update", data => { + if (data.doctype == doctype && data.name == new_name) { + $(document).trigger("rename", [doctype, docname, new_name]); + if (locals[doctype] && locals[doctype][docname]) delete locals[doctype][docname]; + this.frm.reload_doc(); + frappe.show_alert({ + message: __('Document renamed from {0} to {1}', [docname.bold(), new_name.bold()]), + indicator: 'success', + }); + } + }); + frappe.show_alert( + __('Document renaming from {0} to {1} has been queued', [docname.bold(), new_name.bold()]) + ); + } if (new_name != docname) { $(document).trigger("rename", [doctype, docname, new_docname || new_name]); if (locals[doctype] && locals[doctype][docname]) delete locals[doctype][docname]; From d92a64e76795209fb422f5bd9c363c8cf905cde4 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 22 Mar 2022 14:53:22 +0530 Subject: [PATCH 14/53] fix: validate before enqueuing rename_doc * refactor validate_rename * don't run before_rename hooks twice * validate_rename kwarg in doc.rename --- frappe/model/document.py | 8 ++--- frappe/model/rename_doc.py | 67 ++++++++++++++++++++++++++------------ 2 files changed, 50 insertions(+), 25 deletions(-) diff --git a/frappe/model/document.py b/frappe/model/document.py index 883b90b464..52bd1f3196 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -939,11 +939,11 @@ class Document(BaseDocument): return self.save() @whitelist.__func__ - def _rename(self, name: str, merge: bool = False, force: bool = False): + def _rename(self, name: str, merge: bool = False, force: bool = False, validate_rename: bool = True): """Cancel the document. Sets `docstatus` = 2, then saves. """ from frappe.model.rename_doc import rename_doc - self.name = rename_doc(doc=self, new=name, merge=merge, force=force) + self.name = rename_doc(doc=self, new=name, merge=merge, force=force, validate=validate_rename) self.reload() @whitelist.__func__ @@ -957,10 +957,10 @@ class Document(BaseDocument): return self._cancel() @whitelist.__func__ - def rename(self, name: str, merge: bool = False, force: bool = False): + def rename(self, name: str, merge: bool = False, force: bool = False, validate_rename: bool = True): """Rename the document to `name`. This transforms the current object. """ - return self._rename(name=name, merge=merge, force=force) + return self._rename(name=name, merge=merge, force=force, validate_rename=validate_rename) def delete(self, ignore_permissions=False): """Delete document.""" diff --git a/frappe/model/rename_doc.py b/frappe/model/rename_doc.py index aad16664c8..e1a823ddf5 100644 --- a/frappe/model/rename_doc.py +++ b/frappe/model/rename_doc.py @@ -49,6 +49,10 @@ def update_document_title( if not isinstance(obj, (str, type(None))): frappe.throw(f"{obj=} must be of type str or None") + # handle bad API usages + merge = cint(merge) + enqueue = cint(enqueue) + doc = frappe.get_doc(doctype, docname) doc.check_permission(permtype="write") @@ -59,7 +63,25 @@ def update_document_title( if name_updated: if enqueue and not is_scheduler_inactive(): - doc.queue_action("rename", name=updated_name, merge=merge) + current_name = doc.name + + # before_name hook may have DocType specific validations or transformations + transformed_name = doc.run_method("before_rename", current_name, updated_name, merge) + if isinstance(transformed_name, dict): + transformed_name = transformed_name.get("new") + transformed_name = transformed_name or updated_name + + # run rename validations before queueing + new_name = validate_rename( + doctype=doctype, + old=current_name, + new=transformed_name, + meta=doc.meta, + merge=merge, + ) + + # we don't want to re-validate since the before_rename hooks would be run again ;) + doc.queue_action("rename", name=new_name, merge=merge, validate_rename=False) else: doc.rename(updated_name, merge=merge) @@ -89,6 +111,7 @@ def rename_doc( show_alert: bool = True, rebuild_search: bool = True, doc: Optional[Document] = None, + validate: bool = True, ) -> str: """Rename a doc(dt, old) to doc(dt, new) and update all linked fields of type "Link".""" old_usage_style = doctype and old and new @@ -99,28 +122,24 @@ def rename_doc( old = old or doc.name doctype = doctype or doc.doctype - - if not frappe.db.exists(doctype, old): - frappe.errprint(_("Failed: {0} to {1} because {0} doesn't exist.").format(old, new)) - return - - if ignore_if_exists and frappe.db.exists(doctype, new): - frappe.errprint(_("Failed: {0} to {1} because {1} already exists.").format(old, new)) - return - - if old==new: - frappe.errprint(_("Ignored: {0} to {1} no changes made because old and new name are the same.").format(old, new)) - return - force = cint(force) merge = cint(merge) meta = frappe.get_meta(doctype) - # call before_rename - old_doc = doc or frappe.get_doc(doctype, old) - out = old_doc.run_method("before_rename", old, new, merge) or {} - new = (out.get("new") or new) if isinstance(out, dict) else (out or new) - new = validate_rename(doctype, new, meta, merge, force, ignore_permissions) + if validate: + old_doc = doc or frappe.get_doc(doctype, old) + out = old_doc.run_method("before_rename", old, new, merge) or {} + new = (out.get("new") or new) if isinstance(out, dict) else (out or new) + new = validate_rename( + doctype=doctype, + old=old, + new=new, + meta=meta, + merge=merge, + force=force, + ignore_permissions=ignore_permissions, + ignore_if_exists=ignore_if_exists, + ) if not merge: rename_parent_and_child(doctype, old, new, meta) @@ -272,7 +291,7 @@ def update_autoname_field(doctype: str, new: str, meta: "Meta") -> None: if field and field[0] == "field": frappe.db.sql("UPDATE `tab{0}` SET `{1}`={2} WHERE `name`={2}".format(doctype, field[1], '%s'), (new, new)) -def validate_rename(doctype: str, new: str, meta: "Meta", merge: bool, force: bool, ignore_permissions: bool) -> str: +def validate_rename(doctype: str, old: str, new: str, meta: "Meta", merge: bool, force: bool = False, ignore_permissions: bool = False, ignore_if_exists: bool = False) -> str: # using for update so that it gets locked and someone else cannot edit it while this rename is going on! exists = ( frappe.qb.from_(doctype) @@ -283,6 +302,12 @@ def validate_rename(doctype: str, new: str, meta: "Meta", merge: bool, force: bo ) exists = exists[0] if exists else None + if not frappe.db.exists(doctype, old): + frappe.throw(_("Can't rename {0} to {1} because {0} doesn't exist.").format(old, new)) + + if old == new: + frappe.throw(_("No changes made because old and new name are the same.").format(old, new)) + if merge and not exists: frappe.throw(_("{0} {1} does not exist, select a new target to merge").format(doctype, new)) @@ -290,7 +315,7 @@ def validate_rename(doctype: str, new: str, meta: "Meta", merge: bool, force: bo # for fixing case, accents exists = None - if (not merge) and exists: + if not merge and exists and not ignore_if_exists: frappe.throw(_("Another {0} with name {1} exists, select another name").format(doctype, new)) if not (ignore_permissions or frappe.permissions.has_permission(doctype, "write", raise_exception=False)): From e2489f8377d9a141a8da50955014e984b6a5a54a Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 22 Mar 2022 14:55:10 +0530 Subject: [PATCH 15/53] fix: Validate title updates via update_document_title API --- frappe/model/rename_doc.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frappe/model/rename_doc.py b/frappe/model/rename_doc.py index e1a823ddf5..eea47c3b38 100644 --- a/frappe/model/rename_doc.py +++ b/frappe/model/rename_doc.py @@ -87,7 +87,8 @@ def update_document_title( if title_updated: try: - frappe.db.set_value(doctype, docname, title_field, updated_title) + setattr(doc, title_field, updated_title) + doc.save() frappe.msgprint(_("Saved"), alert=True, indicator="green") except Exception as e: if frappe.db.is_duplicate_entry(e): From 782a0b0c8482ab7c7a9adca4b442d174a4d9941c Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 22 Mar 2022 15:06:58 +0530 Subject: [PATCH 16/53] fix: Check if name was changed in client --- frappe/public/js/frappe/form/toolbar.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/form/toolbar.js b/frappe/public/js/frappe/form/toolbar.js index 9606c7ccf7..01bfae2cdd 100644 --- a/frappe/public/js/frappe/form/toolbar.js +++ b/frappe/public/js/frappe/form/toolbar.js @@ -108,7 +108,7 @@ frappe.ui.form.Toolbar = class Toolbar { freeze_message: __("Updating related fields...") }).then(new_docname => { // handle document renaming queued action - if (new_docname == docname) { + if (new_name && (new_docname == docname)) { frappe.socketio.doc_subscribe(doctype, new_name); frappe.realtime.on("doc_update", data => { if (data.doctype == doctype && data.name == new_name) { @@ -125,7 +125,8 @@ frappe.ui.form.Toolbar = class Toolbar { __('Document renaming from {0} to {1} has been queued', [docname.bold(), new_name.bold()]) ); } - if (new_name != docname) { + // handle document sync rename action + if (new_name && (new_name != docname)) { $(document).trigger("rename", [doctype, docname, new_docname || new_name]); if (locals[doctype] && locals[doctype][docname]) delete locals[doctype][docname]; } From 0e87013421a0e5d2022026be0f3ec0cb0173146c Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 22 Mar 2022 16:12:36 +0530 Subject: [PATCH 17/53] chore: Add docstring for rename_doc --- frappe/model/rename_doc.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/frappe/model/rename_doc.py b/frappe/model/rename_doc.py index eea47c3b38..a321963527 100644 --- a/frappe/model/rename_doc.py +++ b/frappe/model/rename_doc.py @@ -114,7 +114,20 @@ def rename_doc( doc: Optional[Document] = None, validate: bool = True, ) -> str: - """Rename a doc(dt, old) to doc(dt, new) and update all linked fields of type "Link".""" + """Rename a doc(dt, old) to doc(dt, new) and update all linked fields of type "Link". + + doc: Document object to be renamed. + new: New name for the record. If None, and doctype is specified, new name may be automatically generated via before_rename hooks. + doctype: DocType of the document. Not required if doc is passed. + old: Current name of the document. Not required if doc is passed. + force: Allow even if document is not allowed to be renamed. + merge: Merge with existing document of new name. + ignore_permissions: Ignore user permissions while renaming. + ignore_if_exists: Don't raise exception if document with new name already exists. This will quietely overwrite the existing document. + show_alert: Display alert if document is renamed successfully. + rebuild_search: Rebuild linked doctype search after renaming. + validate: Validate before renaming. If False, it is assumed that the caller has already validated. + """ old_usage_style = doctype and old and new new_usage_style = doc and new From 1cb956d83540a58079c0d1dee98687132388e3de Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 22 Mar 2022 17:33:51 +0530 Subject: [PATCH 18/53] fix(rename_doc): Use sbool instead of cint cint("false") returns True which is what is sent by frappe dialog. This may be required to be fixed in the client alone but making this change to make the API more "robust" as this has been working in this particular way for far too long now :') --- frappe/model/rename_doc.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/frappe/model/rename_doc.py b/frappe/model/rename_doc.py index a321963527..5b609d35fb 100644 --- a/frappe/model/rename_doc.py +++ b/frappe/model/rename_doc.py @@ -9,7 +9,7 @@ from frappe.model.dynamic_links import get_dynamic_link_map from frappe.model.naming import validate_name from frappe.model.utils.user_settings import sync_user_settings, update_user_settings_data from frappe.query_builder import Field -from frappe.utils import cint +from frappe.utils.data import sbool from frappe.utils.password import rename_password from frappe.utils.scheduler import is_scheduler_inactive @@ -50,8 +50,8 @@ def update_document_title( frappe.throw(f"{obj=} must be of type str or None") # handle bad API usages - merge = cint(merge) - enqueue = cint(enqueue) + merge = sbool(merge) + enqueue = sbool(enqueue) doc = frappe.get_doc(doctype, docname) doc.check_permission(permtype="write") @@ -136,8 +136,8 @@ def rename_doc( old = old or doc.name doctype = doctype or doc.doctype - force = cint(force) - merge = cint(merge) + force = sbool(force) + merge = sbool(merge) meta = frappe.get_meta(doctype) if validate: From 3351cc9c805e5591c095be273d89a07f7f8e738a Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 22 Mar 2022 19:26:58 +0530 Subject: [PATCH 19/53] refactor(rename_doc): Use QB notation inplace of raw SQLs * Converted ~22 queries from raw SQL to use frappe.qb notation * Made queries DRY-er --- frappe/model/rename_doc.py | 288 +++++++++++++++++++------------------ 1 file changed, 147 insertions(+), 141 deletions(-) diff --git a/frappe/model/rename_doc.py b/frappe/model/rename_doc.py index 5b609d35fb..b68af0f1ce 100644 --- a/frappe/model/rename_doc.py +++ b/frappe/model/rename_doc.py @@ -191,8 +191,12 @@ def rename_doc( rename_password(doctype, old, new) # update user_permissions - frappe.db.sql("""UPDATE `tabDefaultValue` SET `defvalue`=%s WHERE `parenttype`='User Permission' - AND `defkey`=%s AND `defvalue`=%s""", (new, doctype, old)) + DefaultValue = DocType("DefaultValue") + frappe.qb.update(DefaultValue).set(DefaultValue.defvalue, new).where( + (DefaultValue.parenttype == "User Permission") + & (DefaultValue.defkey == doctype) + & (DefaultValue.defvalue == old) + ).run() if merge: new_doc.add_comment('Edit', _("merged {0} into {1}").format(frappe.bold(old), frappe.bold(new))) @@ -246,10 +250,11 @@ def update_user_settings(old: str, new: str, link_fields: List[Dict]) -> None: # find the user settings for the linked doctypes linked_doctypes = {d.parent for d in link_fields if not d.issingle} - user_settings_details = frappe.db.sql('''SELECT `user`, `doctype`, `data` - FROM `__UserSettings` - WHERE `data` like %s - AND `doctype` IN ('{doctypes}')'''.format(doctypes="', '".join(linked_doctypes)), (old), as_dict=1) + UserSettings = Table("__UserSettings") + + user_settings_details = frappe.qb.from_(UserSettings).select("user", "doctype", "data").where( + UserSettings.data.like(old) & UserSettings.doctype.isin(linked_doctypes) + ).run(as_dict=True) # create the dict using the doctype name as key and values as list of the user settings from collections import defaultdict @@ -270,31 +275,32 @@ def update_customizations(old: str, new: str) -> None: frappe.db.set_value("Custom DocPerm", {"parent": old}, "parent", new, update_modified=False) def update_attachments(doctype: str, old: str, new: str) -> None: - try: - if old != "File Data" and doctype != "DocType": - frappe.db.sql("""update `tabFile` set attached_to_name=%s - where attached_to_name=%s and attached_to_doctype=%s""", (new, old, doctype)) - except frappe.db.ProgrammingError as e: - if not frappe.db.is_column_missing(e): - raise + if doctype != "DocType": + File = DocType("File") + + frappe.qb.update(File).set(File.attached_to_name, new).where( + (File.attached_to_name == old) & (File.attached_to_doctype == doctype) + ).run() + def rename_versions(doctype: str, old: str, new: str) -> None: - frappe.db.sql("""UPDATE `tabVersion` SET `docname`=%s WHERE `ref_doctype`=%s AND `docname`=%s""", - (new, doctype, old)) + Version = DocType("Version") + + frappe.qb.update(Version).set(Version.docname, new).where( + (Version.docname == old) & (Version.ref_doctype == doctype) + ).run() + def rename_eps_records(doctype: str, old: str, new: str) -> None: - epl = frappe.qb.DocType("Energy Point Log") - (frappe.qb.update(epl) - .set(epl.reference_name, new) - .where( - (epl.reference_doctype == doctype) - & (epl.reference_name == old) - ) + EPL = DocType("Energy Point Log") + + frappe.qb.update(EPL).set(EPL.reference_name, new).where( + (EPL.reference_doctype == doctype) & (EPL.reference_name == old) ).run() def rename_parent_and_child(doctype: str, old: str, new: str, meta: "Meta") -> None: - # rename the doc - frappe.db.sql("UPDATE `tab{0}` SET `name`={1} WHERE `name`={1}".format(doctype, '%s'), (new, old)) + frappe.qb.update(doctype).set("name", new).where(Field("name") == old).run() + update_autoname_field(doctype, new, meta) update_child_docs(old, new, meta) @@ -303,7 +309,9 @@ def update_autoname_field(doctype: str, new: str, meta: "Meta") -> None: if meta.get('autoname'): field = meta.get('autoname').split(':') if field and field[0] == "field": - frappe.db.sql("UPDATE `tab{0}` SET `{1}`={2} WHERE `name`={2}".format(doctype, field[1], '%s'), (new, new)) + frappe.qb.update(doctype).set(field[1], new).where( + Field("name") == new + ).run() def validate_rename(doctype: str, old: str, new: str, meta: "Meta", merge: bool, force: bool = False, ignore_permissions: bool = False, ignore_if_exists: bool = False) -> str: # using for update so that it gets locked and someone else cannot edit it while this rename is going on! @@ -361,8 +369,7 @@ def rename_doctype(doctype: str, old: str, new: str) -> None: def update_child_docs(old: str, new: str, meta: "Meta") -> None: # update "parent" for df in meta.get_table_fields(): - frappe.db.sql("update `tab%s` set parent=%s where parent=%s" \ - % (df.options, '%s', '%s'), (new, old)) + frappe.qb.update(df.options).set("parent", new).where(Field("parent") == old).run() def update_link_field_values(link_fields: List[Dict], old: str, new: str, doctype: str) -> None: for field in link_fields: @@ -405,44 +412,35 @@ def get_link_fields(doctype: str) -> List[Dict]: frappe.flags.link_fields = {} if doctype not in frappe.flags.link_fields: - link_fields = frappe.db.sql("""\ - select parent, fieldname, - (select issingle from tabDocType dt - where dt.name = df.parent) as issingle - from tabDocField df - where - df.options=%s and df.fieldtype='Link'""", (doctype,), as_dict=1) + dt = DocType("DocType") + df = DocType("DocField") + cf = DocType("Custom Field") + ps = DocType("Property Setter") - # get link fields from tabCustom Field - custom_link_fields = frappe.db.sql("""\ - select dt as parent, fieldname, - (select issingle from tabDocType dt - where dt.name = df.dt) as issingle - from `tabCustom Field` df - where - df.options=%s and df.fieldtype='Link'""", (doctype,), as_dict=1) + st_issingle = frappe.qb.from_(dt).select(dt.issingle).where(dt.name == df.parent).as_("issingle") + standard_fields = frappe.qb.from_(df).select(df.parent, df.fieldname, st_issingle).where( + (df.options == doctype) & (df.fieldtype == "Link") + ).run(as_dict=True) - # add custom link fields list to link fields list - link_fields += custom_link_fields - # remove fields whose options have been changed using property setter - property_setter_link_fields = frappe.db.sql("""\ - select ps.doc_type as parent, ps.field_name as fieldname, - (select issingle from tabDocType dt - where dt.name = ps.doc_type) as issingle - from `tabProperty Setter` ps - where - ps.property_type='options' and - ps.field_name is not null and - ps.value=%s""", (doctype,), as_dict=1) + cf_issingle = frappe.qb.from_(dt).select(dt.issingle).where(dt.name == cf.dt).as_("issingle") + custom_fields = frappe.qb.from_(cf).select(cf.dt.as_("parent"), cf.fieldname, cf_issingle).where( + (cf.options == doctype) & (cf.fieldtype == "Link") + ).run(as_dict=True) - link_fields += property_setter_link_fields + ps_issingle = frappe.qb.from_(dt).select(dt.issingle).where(dt.name == ps.doc_type).as_("issingle") + property_setter_fields = frappe.qb.from_(ps).select(ps.doc_type.as_("parent"), ps.field_name.as_("fieldname"), ps_issingle).where( + (ps.property == "options") & (ps.value == doctype) & (ps.field_name.notnull()) + ).run(as_dict=True) - frappe.flags.link_fields[doctype] = link_fields + frappe.flags.link_fields[doctype] = standard_fields + custom_fields + property_setter_fields return frappe.flags.link_fields[doctype] def update_options_for_fieldtype(fieldtype: str, old: str, new: str) -> None: + CustomField = DocType("Custom Field") + PropertySetter = DocType("Property Setter") + if frappe.conf.developer_mode: for name in frappe.get_all("DocField", filters={"options": old}, pluck="parent"): doctype = frappe.get_doc("DocType", name) @@ -454,133 +452,141 @@ def update_options_for_fieldtype(fieldtype: str, old: str, new: str) -> None: if save: doctype.save() else: - frappe.db.sql("""update `tabDocField` set options=%s - where fieldtype=%s and options=%s""", (new, fieldtype, old)) + DocField = DocType("DocField") + frappe.qb.update(DocField).set(DocField.options, new).where( + (DocField.fieldtype == fieldtype) & (DocField.options == old) + ).run() - frappe.db.sql("""update `tabCustom Field` set options=%s - where fieldtype=%s and options=%s""", (new, fieldtype, old)) + frappe.qb.update(CustomField).set(CustomField.options, new).where( + (CustomField.fieldtype == fieldtype) & (CustomField.options == old) + ).run() + + frappe.qb.update(PropertySetter).set(PropertySetter.value, new).where( + (PropertySetter.property == "options") & (PropertySetter.value == old) + ).run() - frappe.db.sql("""update `tabProperty Setter` set value=%s - where property='options' and value=%s""", (new, old)) def get_select_fields(old: str, new: str) -> List[Dict]: """ get select type fields where doctype's name is hardcoded as new line separated list """ + df = DocType("DocField") + dt = DocType("DocType") + cf = DocType("Custom Field") + ps = DocType("Property Setter") + # get link fields from tabDocField - select_fields = frappe.db.sql(""" - select parent, fieldname, - (select issingle from tabDocType dt - where dt.name = df.parent) as issingle - from tabDocField df - where - df.parent != %s and df.fieldtype = 'Select' and - df.options like {0} """.format(frappe.db.escape('%' + old + '%')), (new,), as_dict=1) + st_issingle = frappe.qb.from_(dt).select(dt.issingle).where(dt.name == df.parent).as_("issingle") + standard_fields = frappe.qb.from_(df).select( + df.parent, df.fieldname, st_issingle + ).where( + (df.parent != new) + & (df.fieldtype == "Select") + & (df.options.like(f"%{old}%")) + ).run(as_dict=True) # get link fields from tabCustom Field - custom_select_fields = frappe.db.sql(""" - select dt as parent, fieldname, - (select issingle from tabDocType dt - where dt.name = df.dt) as issingle - from `tabCustom Field` df - where - df.dt != %s and df.fieldtype = 'Select' and - df.options like {0} """ .format(frappe.db.escape('%' + old + '%')), (new,), as_dict=1) - - # add custom link fields list to link fields list - select_fields += custom_select_fields + cf_issingle = frappe.qb.from_(dt).select(dt.issingle).where(dt.name == cf.dt).as_("issingle") + custom_select_fields = frappe.qb.from_(cf).select( + cf.dt.as_("parent"), cf.fieldname, cf_issingle + ).where( + (cf.dt != new) + & (cf.fieldtype == "Select") + & (cf.options.like(f"%{old}%")) + ).run(as_dict=True) # remove fields whose options have been changed using property setter - property_setter_select_fields = frappe.db.sql(""" - select ps.doc_type as parent, ps.field_name as fieldname, - (select issingle from tabDocType dt - where dt.name = ps.doc_type) as issingle - from `tabProperty Setter` ps - where - ps.doc_type != %s and - ps.property_type='options' and - ps.field_name is not null and - ps.value like {0} """.format(frappe.db.escape('%' + old + '%')), (new,), as_dict=1) + ps_issingle = frappe.qb.from_(dt).select(dt.issingle).where(dt.name == ps.doc_type).as_("issingle") + property_setter_select_fields = frappe.qb.from_(ps).select( + ps.doc_type.as_("parent"), ps.field_name.as_("fieldname"), ps_issingle + ).where( + (ps.doc_type != new) + & (ps.property == "options") + & (ps.field_name.notnull()) + & (ps.value.like(f"%{old}%")) + ).run(as_dict=True) - select_fields += property_setter_select_fields + return standard_fields + custom_select_fields + property_setter_select_fields - return select_fields def update_select_field_values(old: str, new: str): - frappe.db.sql(""" - update `tabDocField` set options=replace(options, %s, %s) - where - parent != %s and fieldtype = 'Select' and - (options like {0} or options like {1})""" - .format(frappe.db.escape('%' + '\n' + old + '%'), frappe.db.escape('%' + old + '\n' + '%')), (old, new, new)) + from frappe.query_builder.functions import Replace - frappe.db.sql(""" - update `tabCustom Field` set options=replace(options, %s, %s) - where - dt != %s and fieldtype = 'Select' and - (options like {0} or options like {1})""" - .format(frappe.db.escape('%' + '\n' + old + '%'), frappe.db.escape('%' + old + '\n' + '%')), (old, new, new)) + DocField = DocType("DocField") + CustomField = DocType("Custom Field") + PropertySetter = DocType("Property Setter") + + frappe.qb.update(DocField).set(DocField.options, Replace(DocField.options, old, new)).where( + (DocField.fieldtype == "Select") + & (DocField.parent != new) + & (DocField.options.like(f"%\n{old}%") | DocField.options.like(f"%{old}\n%")) + ).run() + + frappe.qb.update(CustomField).set(CustomField.options, Replace(CustomField.options, old, new)).where( + (CustomField.fieldtype == "Select") + & (CustomField.dt != new) + & (CustomField.options.like(f"%\n{old}%") | CustomField.options.like(f"%{old}\n%")) + ).run() + + frappe.qb.update(PropertySetter).set(PropertySetter.value, Replace(PropertySetter.value, old, new)).where( + (PropertySetter.property == "options") + & (PropertySetter.field_name.notnull()) + & (PropertySetter.doc_type != new) + & (PropertySetter.value.like(f"%\n{old}%") | PropertySetter.value.like(f"%{old}\n%")) + ).run() - frappe.db.sql(""" - update `tabProperty Setter` set value=replace(value, %s, %s) - where - doc_type != %s and field_name is not null and - property='options' and - (value like {0} or value like {1})""" - .format(frappe.db.escape('%' + '\n' + old + '%'), frappe.db.escape('%' + old + '\n' + '%')), (old, new, new)) def update_parenttype_values(old: str, new: str): - child_doctypes = frappe.db.get_all('DocField', - fields=['options', 'fieldname'], - filters={ - 'parent': new, - 'fieldtype': ['in', frappe.model.table_fields] - } + child_doctypes = frappe.get_all( + "DocField", + fields=["options", "fieldname"], + filters={"parent": new, "fieldtype": ["in", frappe.model.table_fields]}, ) - custom_child_doctypes = frappe.db.get_all('Custom Field', - fields=['options', 'fieldname'], - filters={ - 'dt': new, - 'fieldtype': ['in', frappe.model.table_fields] - } + custom_child_doctypes = frappe.get_all( + "Custom Field", + fields=["options", "fieldname"], + filters={"dt": new, "fieldtype": ["in", frappe.model.table_fields]}, ) child_doctypes += custom_child_doctypes - fields = [d['fieldname'] for d in child_doctypes] + fields = [d["fieldname"] for d in child_doctypes] property_setter_child_doctypes = frappe.get_all( "Property Setter", - filters={ - "doc_type": new, - "property": "options", - "field_name": ("in", fields) - }, - pluck="value" + filters={"doc_type": new, "property": "options", "field_name": ("in", fields)}, + pluck="value", ) - child_doctypes = list(d['options'] for d in child_doctypes) - child_doctypes += property_setter_child_doctypes + child_doctypes = set(list(d["options"] for d in child_doctypes) + property_setter_child_doctypes) for doctype in child_doctypes: - frappe.db.sql(f"update `tab{doctype}` set parenttype=%s where parenttype=%s", (new, old)) + Table = DocType(doctype) + frappe.qb.update(Table).set(Table.parenttype, new).where(Table.parenttype == old).run() + def rename_dynamic_links(doctype: str, old: str, new: str): + Singles = DocType("Singles") for df in get_dynamic_link_map().get(doctype, []): # dynamic link in single, just one value to check if frappe.get_meta(df.parent).issingle: refdoc = frappe.db.get_singles_dict(df.parent) - if refdoc.get(df.options)==doctype and refdoc.get(df.fieldname)==old: - - frappe.db.sql("""update tabSingles set value=%s where - field=%s and value=%s and doctype=%s""", (new, df.fieldname, old, df.parent)) + if refdoc.get(df.options) == doctype and refdoc.get(df.fieldname) == old: + frappe.qb.update(Singles).set(Singles.value, new).where( + (Singles.field == df.fieldname) + & (Singles.doctype == df.parent) + & (Singles.value == old) + ).run() else: # because the table hasn't been renamed yet! parent = df.parent if df.parent != new else old - frappe.db.sql("""update `tab{parent}` set {fieldname}=%s - where {options}=%s and {fieldname}=%s""".format(parent = parent, - fieldname=df.fieldname, options=df.options), (new, doctype, old)) + + frappe.qb.update(parent).set(df.fieldname, new).where( + (Field(df.options) == doctype) + & (Field(df.fieldname) == old) + ).run() + def bulk_rename(doctype: str, rows: Optional[List[List]] = None, via_console: bool = False) -> Optional[List[str]]: """Bulk rename documents From f92f77dab798c5ac0ba02bacd5f46b576c7f6ce1 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 22 Mar 2022 19:38:53 +0530 Subject: [PATCH 20/53] fix(qb): Make Table importable --- frappe/model/rename_doc.py | 1 + frappe/query_builder/utils.py | 3 +++ 2 files changed, 4 insertions(+) diff --git a/frappe/model/rename_doc.py b/frappe/model/rename_doc.py index b68af0f1ce..f25613f71e 100644 --- a/frappe/model/rename_doc.py +++ b/frappe/model/rename_doc.py @@ -9,6 +9,7 @@ from frappe.model.dynamic_links import get_dynamic_link_map from frappe.model.naming import validate_name from frappe.model.utils.user_settings import sync_user_settings, update_user_settings_data from frappe.query_builder import Field +from frappe.query_builder.utils import DocType, Table from frappe.utils.data import sbool from frappe.utils.password import rename_password from frappe.utils.scheduler import is_scheduler_inactive diff --git a/frappe/query_builder/utils.py b/frappe/query_builder/utils.py index 1ddf4fc034..8c95eb8a70 100644 --- a/frappe/query_builder/utils.py +++ b/frappe/query_builder/utils.py @@ -49,6 +49,9 @@ def get_attr(method_string): def DocType(*args, **kwargs): return frappe.qb.DocType(*args, **kwargs) +def Table(*args, **kwargs): + return frappe.qb.Table(*args, **kwargs) + def patch_query_execute(): """Patch the Query Builder with helper execute method This excludes the use of `frappe.db.sql` method while From 28f9802fd9c4ad4f2025b2e75e9e95fa158dd4e2 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 28 Mar 2022 18:46:41 +0530 Subject: [PATCH 21/53] fix: rollback to savepoint to avoid partial commits It's better to keep the validations in and out AND separate...this is a humble attempt for the same :) --- frappe/model/document.py | 2 +- frappe/model/rename_doc.py | 16 ++++++++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/frappe/model/document.py b/frappe/model/document.py index 52bd1f3196..ab16b68e6a 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -940,7 +940,7 @@ class Document(BaseDocument): @whitelist.__func__ def _rename(self, name: str, merge: bool = False, force: bool = False, validate_rename: bool = True): - """Cancel the document. Sets `docstatus` = 2, then saves. + """Rename the document. Triggers frappe.rename_doc, then reloads. """ from frappe.model.rename_doc import rename_doc self.name = rename_doc(doc=self, new=name, merge=merge, force=force, validate=validate_rename) diff --git a/frappe/model/rename_doc.py b/frappe/model/rename_doc.py index f25613f71e..f27484697f 100644 --- a/frappe/model/rename_doc.py +++ b/frappe/model/rename_doc.py @@ -73,16 +73,17 @@ def update_document_title( transformed_name = transformed_name or updated_name # run rename validations before queueing - new_name = validate_rename( + # use savepoints to avoid partial renames / commits + validate_rename( doctype=doctype, old=current_name, new=transformed_name, meta=doc.meta, merge=merge, + save_point=True, ) - # we don't want to re-validate since the before_rename hooks would be run again ;) - doc.queue_action("rename", name=new_name, merge=merge, validate_rename=False) + doc.queue_action("rename", name=transformed_name, merge=merge) else: doc.rename(updated_name, merge=merge) @@ -314,8 +315,12 @@ def update_autoname_field(doctype: str, new: str, meta: "Meta") -> None: Field("name") == new ).run() -def validate_rename(doctype: str, old: str, new: str, meta: "Meta", merge: bool, force: bool = False, ignore_permissions: bool = False, ignore_if_exists: bool = False) -> str: +def validate_rename(doctype: str, old: str, new: str, meta: "Meta", merge: bool, force: bool = False, ignore_permissions: bool = False, ignore_if_exists: bool = False, save_point=False) -> str: # using for update so that it gets locked and someone else cannot edit it while this rename is going on! + if save_point: + _SAVE_POINT = f"validate_rename_{frappe.generate_hash(8)}" + frappe.db.savepoint(_SAVE_POINT) + exists = ( frappe.qb.from_(doctype) .where(Field("name") == new) @@ -350,6 +355,9 @@ def validate_rename(doctype: str, old: str, new: str, meta: "Meta", merge: bool, # validate naming like it's done in doc.py new = validate_name(doctype, new) + if save_point: + frappe.db.rollback(save_point=_SAVE_POINT) + return new def rename_doctype(doctype: str, old: str, new: str) -> None: From 666a33a5750d951f5b48529f288fb348f86f1016 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 30 Mar 2022 11:32:16 +0530 Subject: [PATCH 22/53] fix: dont animate "Jump to field" modal It's supposed to be keyboard driven and currrently wasting 500 ms of my precious life. sd --- frappe/public/js/frappe/form/toolbar.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/form/toolbar.js b/frappe/public/js/frappe/form/toolbar.js index e55eb9fdeb..e4b1d09c68 100644 --- a/frappe/public/js/frappe/form/toolbar.js +++ b/frappe/public/js/frappe/form/toolbar.js @@ -569,7 +569,8 @@ frappe.ui.form.Toolbar = class Toolbar { primary_action: ({ fieldname }) => { dialog.hide(); this.frm.scroll_to_field(fieldname); - } + }, + animate: false, }); dialog.show(); From 2d3c75a4a759d1bdb3315e88b72d3b0fea73ef76 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 14 Apr 2022 11:20:46 +0530 Subject: [PATCH 23/53] fix(ux): lose focus from all fields on ESC --- frappe/public/js/frappe/ui/keyboard.js | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/frappe/public/js/frappe/ui/keyboard.js b/frappe/public/js/frappe/ui/keyboard.js index e28a8f680d..40d1a93b8c 100644 --- a/frappe/public/js/frappe/ui/keyboard.js +++ b/frappe/public/js/frappe/ui/keyboard.js @@ -221,11 +221,11 @@ frappe.ui.keys.add_shortcut({ }); frappe.ui.keys.on('escape', function(e) { - close_grid_and_dialog(); + handle_escape_key(); }); frappe.ui.keys.on('esc', function(e) { - close_grid_and_dialog(); + handle_escape_key(); }); frappe.ui.keys.on('enter', function(e) { @@ -293,6 +293,11 @@ frappe.ui.keyCode = { BACKSPACE: 8 } +function handle_escape_key() { + close_grid_and_dialog(); + document.activeElement?.blur(); +} + function close_grid_and_dialog() { // close open grid row var open_row = $(".grid-row-open"); @@ -308,10 +313,3 @@ function close_grid_and_dialog() { return false; } } - -// blur when escape is pressed on dropdowns -$(document).on('keydown', '.dropdown-toggle', (e) => { - if (e.which === frappe.ui.keyCode.ESCAPE) { - $(e.currentTarget).blur(); - } -}); From af0b98334f7c96b78d51c49f43e0ab51f4eeae8e Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 14 Apr 2022 11:36:02 +0530 Subject: [PATCH 24/53] chore: eslint - use es2020 for parsing --- .eslintrc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.eslintrc b/.eslintrc index 937f11586c..adc4aebb28 100644 --- a/.eslintrc +++ b/.eslintrc @@ -5,7 +5,7 @@ "es6": true }, "parserOptions": { - "ecmaVersion": 9, + "ecmaVersion": 11, "sourceType": "module" }, "extends": "eslint:recommended", From c68da5e6c725053794ab9f5995c29f9b83bdaadd Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 16 Apr 2022 14:55:02 +0530 Subject: [PATCH 25/53] fix(UX): focus field immediately after jump Not sure why we need to wait 1sec :) --- frappe/public/js/frappe/form/form.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index 0c8939cf5d..13fa712c05 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -1765,11 +1765,13 @@ frappe.ui.form.Form = class FrappeForm { // scroll to input frappe.utils.scroll_to($el, true, 15); + // focus if text field + $el.find('input, select, textarea').focus(); + // highlight input $el.addClass('has-error'); setTimeout(() => { $el.removeClass('has-error'); - $el.find('input, select, textarea').focus(); }, 1000); } From 8df645c9040317a3bdf382a9797d592ef8af7f4a Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 16 Apr 2022 15:28:05 +0530 Subject: [PATCH 26/53] fix(UX): better highlighting for jumped field --- frappe/public/js/frappe/form/form.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index 13fa712c05..3c07907c50 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -1768,11 +1768,12 @@ frappe.ui.form.Form = class FrappeForm { // focus if text field $el.find('input, select, textarea').focus(); - // highlight input - $el.addClass('has-error'); + // highlight control inside field + let control_element = $el.find('.form-control') + control_element.addClass('highlight'); setTimeout(() => { - $el.removeClass('has-error'); - }, 1000); + control_element.removeClass('highlight'); + }, 2000); } setup_docinfo_change_listener() { From 6cdd33f26bffb850db65a60837dbc2b8ed12abc0 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 20 Apr 2022 16:41:25 +0530 Subject: [PATCH 27/53] fix: Generate hash of length 8 for save point --- frappe/model/rename_doc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/model/rename_doc.py b/frappe/model/rename_doc.py index fede42a333..a0cd10f967 100644 --- a/frappe/model/rename_doc.py +++ b/frappe/model/rename_doc.py @@ -350,7 +350,7 @@ def validate_rename( ) -> str: # using for update so that it gets locked and someone else cannot edit it while this rename is going on! if save_point: - _SAVE_POINT = f"validate_rename_{frappe.generate_hash(8)}" + _SAVE_POINT = f"validate_rename_{frappe.generate_hash(length=8)}" frappe.db.savepoint(_SAVE_POINT) exists = ( From da191390a5eb4da803c8ac732fb46aad60920519 Mon Sep 17 00:00:00 2001 From: phot0n Date: Wed, 20 Apr 2022 14:40:24 +0530 Subject: [PATCH 28/53] fix: support for multiple order by in add_conditions --- frappe/database/query.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 8d8a767370..ba19ab6356 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -108,11 +108,14 @@ def change_orderby(order: str): tuple: field, order """ order = order.split() - if order[1].lower() == "asc": - orderby, order = order[0], Order.asc - return orderby, order - orderby, order = order[0], Order.desc - return orderby, order + + try: + if order[1].lower() == "asc": + return order[0], Order.asc + except IndexError: + pass + + return order[0], Order.desc OPERATOR_MAP = { @@ -175,10 +178,13 @@ class Query: """ if kwargs.get("orderby"): orderby = kwargs.get("orderby") - order = kwargs.get("order") if kwargs.get("order") else Order.desc if isinstance(orderby, str) and len(orderby.split()) > 1: - orderby, order = change_orderby(orderby) - conditions = conditions.orderby(orderby, order=order) + for ordby in orderby.split(","): + if ordby := ordby.strip(): + orderby, order = change_orderby(ordby) + conditions = conditions.orderby(orderby, order=order) + else: + conditions = conditions.orderby(orderby, order=kwargs.get("order") or Order.desc) if kwargs.get("limit"): conditions = conditions.limit(kwargs.get("limit")) From c51635702716c64e9c3947176520828adaab27d9 Mon Sep 17 00:00:00 2001 From: phot0n Date: Wed, 20 Apr 2022 17:18:13 +0530 Subject: [PATCH 29/53] test(get_value): test for multiple order bys --- frappe/tests/test_db.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index 5b469cd5db..b83c290268 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -87,6 +87,13 @@ class TestDB(unittest.TestCase): frappe.db.get_values("User", filters=[["name", "=", "Administrator"]], fieldname="email"), ) + # test multiple orderby's + delimiter = '"' if frappe.db.db_type == "postgres" else "`" + self.assertIn( + "ORDER BY {deli}creation{deli} DESC,{deli}modified{deli} ASC,{deli}name{deli} DESC".format(deli=delimiter), + frappe.db.get_value("DocType", "DocField", order_by="creation desc, modified asc, name", run=0) + ) + def test_get_value_limits(self): # check both dict and list style filters From 6405b0510a39488d7d51b3e23a575eddf790e365 Mon Sep 17 00:00:00 2001 From: phot0n Date: Wed, 20 Apr 2022 17:32:09 +0530 Subject: [PATCH 30/53] chore: fix linter --- frappe/database/query.py | 2 +- frappe/tests/test_db.py | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index ba19ab6356..136f5c86b6 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -294,7 +294,7 @@ class Query: table: str, fields: Union[List, Tuple], filters: Union[Dict[str, Union[str, int]], str, int] = None, - **kwargs + **kwargs, ): criterion = self.build_conditions(table, filters, **kwargs) if isinstance(fields, (list, tuple)): diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index b83c290268..6cba55c425 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -90,8 +90,10 @@ class TestDB(unittest.TestCase): # test multiple orderby's delimiter = '"' if frappe.db.db_type == "postgres" else "`" self.assertIn( - "ORDER BY {deli}creation{deli} DESC,{deli}modified{deli} ASC,{deli}name{deli} DESC".format(deli=delimiter), - frappe.db.get_value("DocType", "DocField", order_by="creation desc, modified asc, name", run=0) + "ORDER BY {deli}creation{deli} DESC,{deli}modified{deli} ASC,{deli}name{deli} DESC".format( + deli=delimiter + ), + frappe.db.get_value("DocType", "DocField", order_by="creation desc, modified asc, name", run=0), ) def test_get_value_limits(self): From e6e01627f89ab421963591ac0099376a24ff3aa3 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 22 Apr 2022 12:09:26 +0530 Subject: [PATCH 31/53] refactor: rename_document_title * Remove extra reload_doc that caused extra onload * Refactor variable naming, DRY-er * Simplify flow --- frappe/public/js/frappe/form/toolbar.js | 39 +++++++++++++------------ 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/frappe/public/js/frappe/form/toolbar.js b/frappe/public/js/frappe/form/toolbar.js index 01bfae2cdd..00b4efbf30 100644 --- a/frappe/public/js/frappe/form/toolbar.js +++ b/frappe/public/js/frappe/form/toolbar.js @@ -84,15 +84,15 @@ frappe.ui.form.Toolbar = class Toolbar { message: __("Unchanged") }); } - rename_document_title(new_name, new_title, merge=false) { + rename_document_title(input_name, input_title, merge=false) { let confirm_message = null; const docname = this.frm.doc.name; const title_field = this.frm.meta.title_field || ''; const doctype = this.frm.doctype; - if (new_name) { + if (input_name) { const warning = __("This cannot be undone"); - const message = __("Are you sure you want to merge {0} with {1}?", [docname.bold(), new_name.bold()]); + const message = __("Are you sure you want to merge {0} with {1}?", [docname.bold(), input_name.bold()]); confirm_message = `${message}
${warning}`; } @@ -100,42 +100,45 @@ frappe.ui.form.Toolbar = class Toolbar { return frappe.xcall("frappe.model.rename_doc.update_document_title", { doctype, docname, - name: new_name, - title: new_title, + name: input_name, + title: input_title, enqueue: true, merge, freeze: true, freeze_message: __("Updating related fields...") }).then(new_docname => { + const reload_form = (input_name) => { + $(document).trigger("rename", [doctype, docname, input_name]); + if (locals[doctype] && locals[doctype][docname]) delete locals[doctype][docname]; + this.frm.reload_doc(); + } + // handle document renaming queued action - if (new_name && (new_docname == docname)) { - frappe.socketio.doc_subscribe(doctype, new_name); + if (input_name && (new_docname == docname)) { + frappe.socketio.doc_subscribe(doctype, input_name); frappe.realtime.on("doc_update", data => { - if (data.doctype == doctype && data.name == new_name) { - $(document).trigger("rename", [doctype, docname, new_name]); - if (locals[doctype] && locals[doctype][docname]) delete locals[doctype][docname]; - this.frm.reload_doc(); + if (data.doctype == doctype && data.name == input_name) { + reload_form(input_name); frappe.show_alert({ - message: __('Document renamed from {0} to {1}', [docname.bold(), new_name.bold()]), + message: __('Document renamed from {0} to {1}', [docname.bold(), input_name.bold()]), indicator: 'success', }); } }); frappe.show_alert( - __('Document renaming from {0} to {1} has been queued', [docname.bold(), new_name.bold()]) + __('Document renaming from {0} to {1} has been queued', [docname.bold(), input_name.bold()]) ); } + // handle document sync rename action - if (new_name && (new_name != docname)) { - $(document).trigger("rename", [doctype, docname, new_docname || new_name]); - if (locals[doctype] && locals[doctype][docname]) delete locals[doctype][docname]; + if (input_name && ((new_docname || input_name) != docname)) { + reload_form(new_docname || input_name); } - this.frm.reload_doc(); }); }; return new Promise((resolve, reject) => { - if (new_title === this.frm.doc[title_field] && new_name === docname) { + if (input_title === this.frm.doc[title_field] && input_name === docname) { this.show_unchanged_document_alert(); resolve(); } else if (merge) { From 6ac1d955843adbde8169824d06fc91c10ca82110 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 15 Apr 2022 11:37:50 +0530 Subject: [PATCH 32/53] test: fix badly written tests --- frappe/tests/test_permissions.py | 33 ++++++++++--------- .../doctype/blog_post/test_blog_post.py | 6 ++-- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/frappe/tests/test_permissions.py b/frappe/tests/test_permissions.py index 70297a4f54..4164b0be36 100644 --- a/frappe/tests/test_permissions.py +++ b/frappe/tests/test_permissions.py @@ -24,25 +24,26 @@ test_dependencies = ["Blogger", "Blog Post", "User", "Contact", "Salutation"] class TestPermissions(FrappeTestCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + frappe.clear_cache(doctype="Blog Post") + user = frappe.get_doc("User", "test1@example.com") + user.add_roles("Website Manager") + user.add_roles("System Manager") + + user = frappe.get_doc("User", "test2@example.com") + user.add_roles("Blogger") + + user = frappe.get_doc("User", "test3@example.com") + user.add_roles("Sales User") + + user = frappe.get_doc("User", "testperm@example.com") + user.add_roles("Website Manager") + def setUp(self): frappe.clear_cache(doctype="Blog Post") - if not frappe.flags.permission_user_setup_done: - user = frappe.get_doc("User", "test1@example.com") - user.add_roles("Website Manager") - user.add_roles("System Manager") - - user = frappe.get_doc("User", "test2@example.com") - user.add_roles("Blogger") - - user = frappe.get_doc("User", "test3@example.com") - user.add_roles("Sales User") - - user = frappe.get_doc("User", "testperm@example.com") - user.add_roles("Website Manager") - - frappe.flags.permission_user_setup_done = True - reset("Blogger") reset("Blog Post") diff --git a/frappe/website/doctype/blog_post/test_blog_post.py b/frappe/website/doctype/blog_post/test_blog_post.py index 0eddad4bfe..558795458b 100644 --- a/frappe/website/doctype/blog_post/test_blog_post.py +++ b/frappe/website/doctype/blog_post/test_blog_post.py @@ -1,12 +1,12 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE import re -import unittest from bs4 import BeautifulSoup import frappe from frappe.custom.doctype.customize_form.customize_form import reset_customization +from frappe.tests.utils import FrappeTestCase from frappe.utils import random_string, set_request from frappe.website.doctype.blog_post.blog_post import get_blog_list from frappe.website.serve import get_response @@ -16,7 +16,7 @@ from frappe.website.website_generator import WebsiteGenerator test_dependencies = ["Blog Post"] -class TestBlogPost(unittest.TestCase): +class TestBlogPost(FrappeTestCase): def setUp(self): reset_customization("Blog Post") @@ -61,7 +61,7 @@ class TestBlogPost(unittest.TestCase): category_page_link = list(soup.find_all("a", href=re.compile(blog.blog_category)))[0] category_page_url = category_page_link["href"] - cached_value = frappe.db.value_cache[("DocType", "Blog Post", "name")] + cached_value = frappe.db.value_cache.get(("DocType", "Blog Post", "name")) frappe.db.value_cache[("DocType", "Blog Post", "name")] = (("Blog Post",),) # Visit the category page (by following the link found in above stage) From f748ae85fc8f51b4c4dcf9029f452703aa238b7a Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 15 Apr 2022 11:48:13 +0530 Subject: [PATCH 33/53] fix: set docstatus to 0 if None present --- frappe/model/base_document.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index f8d60d0763..fd8bb8e71d 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -348,7 +348,7 @@ class BaseDocument(object): @property def docstatus(self): - return DocStatus(self.get("docstatus")) + return DocStatus(cint(self.get("docstatus"))) @docstatus.setter def docstatus(self, value): From 8a0a5c54da9caa0fc3323a90415d9e8e045360cc Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 15 Apr 2022 11:56:34 +0530 Subject: [PATCH 34/53] test!: dont autocommit on test object recreation --- frappe/parallel_test_runner.py | 6 +++--- frappe/test_runner.py | 29 ++++++++++++++++------------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/frappe/parallel_test_runner.py b/frappe/parallel_test_runner.py index 65c8eb470b..4fd03773ef 100644 --- a/frappe/parallel_test_runner.py +++ b/frappe/parallel_test_runner.py @@ -46,7 +46,7 @@ class ParallelTestRunner: if hasattr(test_module, "global_test_dependencies"): for doctype in test_module.global_test_dependencies: - make_test_records(doctype) + make_test_records(doctype, commit=True) elapsed = time.time() - start_time elapsed = click.style(f" ({elapsed:.03}s)", fg="red") @@ -76,7 +76,7 @@ class ParallelTestRunner: def create_test_dependency_records(self, module, path, filename): if hasattr(module, "test_dependencies"): for doctype in module.test_dependencies: - make_test_records(doctype) + make_test_records(doctype, commit=True) if os.path.basename(os.path.dirname(path)) == "doctype": # test_data_migration_connector.py > data_migration_connector.json @@ -86,7 +86,7 @@ class ParallelTestRunner: with open(test_record_file_path, "r") as f: doc = json.loads(f.read()) doctype = doc["name"] - make_test_records(doctype) + make_test_records(doctype, commit=True) def get_module(self, path, filename): app_path = frappe.get_pymodule_path(self.app) diff --git a/frappe/test_runner.py b/frappe/test_runner.py index 509be36f86..96feac532f 100644 --- a/frappe/test_runner.py +++ b/frappe/test_runner.py @@ -226,7 +226,7 @@ def run_tests_for_doctype( if force: for name in frappe.db.sql_list("select name from `tab%s`" % doctype): frappe.delete_doc(doctype, name, force=True) - make_test_records(doctype, verbose=verbose, force=force) + make_test_records(doctype, verbose=verbose, force=force, commit=True) modules.append(importlib.import_module(test_module)) return _run_unittest( @@ -245,7 +245,7 @@ def run_tests_for_module( module = importlib.import_module(module) if hasattr(module, "test_dependencies"): for doctype in module.test_dependencies: - make_test_records(doctype, verbose=verbose) + make_test_records(doctype, verbose=verbose, commit=True) frappe.db.commit() return _run_unittest( @@ -330,7 +330,7 @@ def _add_test(app, path, filename, verbose, test_suite=None, ui_tests=False): if hasattr(module, "test_dependencies"): for doctype in module.test_dependencies: - make_test_records(doctype, verbose=verbose) + make_test_records(doctype, verbose=verbose, commit=True) is_ui_test = True if hasattr(module, "TestDriver") else False @@ -346,12 +346,12 @@ def _add_test(app, path, filename, verbose, test_suite=None, ui_tests=False): with open(txt_file, "r") as f: doc = json.loads(f.read()) doctype = doc["name"] - make_test_records(doctype, verbose) + make_test_records(doctype, verbose, commit=True) test_suite.addTest(unittest.TestLoader().loadTestsFromModule(module)) -def make_test_records(doctype, verbose=0, force=False): +def make_test_records(doctype, verbose=0, force=False, commit=False): if not frappe.db: frappe.connect() @@ -364,8 +364,8 @@ def make_test_records(doctype, verbose=0, force=False): if options not in frappe.local.test_objects: frappe.local.test_objects[options] = [] - make_test_records(options, verbose, force) - make_test_records_for_doctype(options, verbose, force) + make_test_records(options, verbose, force, commit=commit) + make_test_records_for_doctype(options, verbose, force, commit=commit) def get_modules(doctype): @@ -405,7 +405,7 @@ def get_dependencies(doctype): return options_list -def make_test_records_for_doctype(doctype, verbose=0, force=False): +def make_test_records_for_doctype(doctype, verbose=0, force=False, commit=False): if not force and doctype in get_test_record_log(): return @@ -420,17 +420,19 @@ def make_test_records_for_doctype(doctype, verbose=0, force=False): elif hasattr(test_module, "test_records"): if doctype in frappe.local.test_objects: frappe.local.test_objects[doctype] += make_test_objects( - doctype, test_module.test_records, verbose, force + doctype, test_module.test_records, verbose, force, commit=commit ) else: frappe.local.test_objects[doctype] = make_test_objects( - doctype, test_module.test_records, verbose, force + doctype, test_module.test_records, verbose, force, commit=commit ) else: test_records = frappe.get_test_records(doctype) if test_records: - frappe.local.test_objects[doctype] += make_test_objects(doctype, test_records, verbose, force) + frappe.local.test_objects[doctype] += make_test_objects( + doctype, test_records, verbose, force, commit=commit + ) elif verbose: print_mandatory_fields(doctype) @@ -438,7 +440,7 @@ def make_test_records_for_doctype(doctype, verbose=0, force=False): add_to_test_record_log(doctype) -def make_test_objects(doctype, test_records=None, verbose=None, reset=False): +def make_test_objects(doctype, test_records=None, verbose=None, reset=False, commit=False): """Make test objects from given list of `test_records` or from `test_records.json`""" records = [] @@ -495,7 +497,8 @@ def make_test_objects(doctype, test_records=None, verbose=None, reset=False): records.append(d.name) - frappe.db.commit() + if commit: + frappe.db.commit() return records From 7274014b3b453a537d7bafc6c6fc37725fd143d0 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 22 Mar 2022 17:22:27 +0530 Subject: [PATCH 35/53] test: improve FrappeTestCase - discard state after test finishes - add assertDocumentEqual for quick document check - add commit "watcher" to find commits during tests - add tests for tests. who watches the watchmen? --- frappe/tests/test_test_utils.py | 34 +++++++++++++++ frappe/tests/utils.py | 77 ++++++++++++++++++++++++++++++--- 2 files changed, 104 insertions(+), 7 deletions(-) create mode 100644 frappe/tests/test_test_utils.py diff --git a/frappe/tests/test_test_utils.py b/frappe/tests/test_test_utils.py new file mode 100644 index 0000000000..4e5c424ca6 --- /dev/null +++ b/frappe/tests/test_test_utils.py @@ -0,0 +1,34 @@ +import frappe +from frappe.tests.utils import FrappeTestCase, change_settings + + +class TestTestUtils(FrappeTestCase): + SHOW_TRANSACTION_COMMIT_WARNINGS = True + + def test_document_assertions(self): + + currency = frappe.new_doc("Currency") + currency.currency_name = "STONKS" + currency.smallest_currency_fraction_value = 0.420_001 + currency.save() + + self.assertDocumentEqual(currency.as_dict(), currency) + + def test_thread_locals(self): + frappe.flags.temp_flag_to_be_discarded = True + + def test_temp_setting_changes(self): + current_setting = frappe.get_system_settings("logout_on_password_reset") + + with change_settings("System Settings", {"logout_on_password_reset": int(not current_setting)}): + updated_settings = frappe.get_system_settings("logout_on_password_reset") + self.assertNotEqual(current_setting, updated_settings) + + restored_settings = frappe.get_system_settings("logout_on_password_reset") + self.assertEqual(current_setting, restored_settings) + + +def tearDownModule(): + """assertions for ensuring tests didn't leave state behind""" + assert "temp_flag_to_be_discarded" not in frappe.flags + assert not frappe.db.exists("Currency", "STONKS") diff --git a/frappe/tests/utils.py b/frappe/tests/utils.py index bad368afd0..d5c25db26a 100644 --- a/frappe/tests/utils.py +++ b/frappe/tests/utils.py @@ -1,23 +1,86 @@ import copy +import datetime import signal import unittest from contextlib import contextmanager import frappe +from frappe.model.base_document import BaseDocument +from frappe.utils import cint + +datetime_like_types = (datetime.datetime, datetime.date, datetime.time, datetime.timedelta) class FrappeTestCase(unittest.TestCase): """Base test class for Frappe tests.""" - @classmethod - def setUpClass(cls) -> None: - frappe.db.commit() - return super().setUpClass() + SHOW_TRANSACTION_COMMIT_WARNINGS = False @classmethod - def tearDownClass(cls) -> None: - frappe.db.rollback() - return super().tearDownClass() + def setUpClass(cls) -> None: + # flush changes done so far to avoid flake + frappe.db.commit() + if cls.SHOW_TRANSACTION_COMMIT_WARNINGS: + frappe.db.add_before_commit(_commit_watcher) + + # enqueue teardown actions (executed in LIFO order) + cls.addClassCleanup(_restore_thread_locals, copy.deepcopy(frappe.local.flags)) + cls.addClassCleanup(_rollback_db) + + return super().setUpClass() + + # --- Frappe Framework specific assertions + def assertDocumentEqual(self, expected, actual): + """Compare a (partial) expected document with actual Document.""" + + if isinstance(expected, BaseDocument): + expected = expected.as_dict() + + for field, value in expected.items(): + if isinstance(value, list): + actual_child_docs = actual.get(field) + self.assertEqual(len(value), len(actual_child_docs), msg=f"{field} length should be same") + for exp_child, actual_child in zip(value, actual_child_docs): + self.assertDocumentEqual(exp_child, actual_child) + else: + self._compare_field(value, actual.get(field), actual, field) + + def _compare_field(self, expected, actual, doc, field): + msg = f"{field} should be same." + + if isinstance(expected, float): + precision = doc.precision(field) + self.assertAlmostEqual(expected, actual, f"{field} should be same to {precision} digits") + elif isinstance(expected, (bool, int)): + self.assertEqual(expected, cint(actual), msg=msg) + elif isinstance(expected, datetime_like_types): + self.assertEqual(str(expected), str(actual), msg=msg) + else: + self.assertEqual(expected, actual, msg=msg) + + +def _commit_watcher(): + import traceback + + print("Warning:, transaction committed during tests.") + traceback.print_stack(limit=5) + + +def _rollback_db(): + frappe.local.before_commit = [] + frappe.local.rollback_observers = [] + frappe.db.value_cache = {} + frappe.db.rollback() + + +def _restore_thread_locals(flags): + frappe.local.flags = flags + frappe.local.error_log = [] + frappe.local.message_log = [] + frappe.local.debug_log = [] + frappe.local.realtime_log = [] + frappe.local.conf = frappe._dict(frappe.get_site_config()) + frappe.local.cache = {} @contextmanager From 1fa60ba3e729b5e006b592c53a94c3466eca8b05 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 23 Apr 2022 13:08:02 +0530 Subject: [PATCH 36/53] test: explicitly start transaction Postgres for some reason is going in autocommit mode if transaction isn't started with BEGIN... will fix it separately. --- frappe/tests/utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/tests/utils.py b/frappe/tests/utils.py index d5c25db26a..7d00a0c1f9 100644 --- a/frappe/tests/utils.py +++ b/frappe/tests/utils.py @@ -20,6 +20,7 @@ class FrappeTestCase(unittest.TestCase): def setUpClass(cls) -> None: # flush changes done so far to avoid flake frappe.db.commit() + frappe.db.begin() if cls.SHOW_TRANSACTION_COMMIT_WARNINGS: frappe.db.add_before_commit(_commit_watcher) From 4418c032371116ed9488f6e770aeda55bdc509d6 Mon Sep 17 00:00:00 2001 From: phot0n Date: Mon, 25 Apr 2022 20:13:10 +0530 Subject: [PATCH 37/53] fix: remove default null definition for name column in tabSeries for postgres --- frappe/database/postgres/framework_postgres.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/database/postgres/framework_postgres.sql b/frappe/database/postgres/framework_postgres.sql index 1e79bf67d8..2cae3ab82f 100644 --- a/frappe/database/postgres/framework_postgres.sql +++ b/frappe/database/postgres/framework_postgres.sql @@ -240,7 +240,7 @@ CREATE TABLE "tabDocType" ( DROP TABLE IF EXISTS "tabSeries"; CREATE TABLE "tabSeries" ( - "name" varchar(100) DEFAULT NULL, + "name" varchar(100), "current" bigint NOT NULL DEFAULT 0, PRIMARY KEY ("name") ) ; From 59ee952d82548b1616ce79349487bd4210dd5863 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Tue, 26 Apr 2022 12:53:44 +0530 Subject: [PATCH 38/53] chore: failing semantic release --- .github/workflows/release.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a6c1243f64..9c7ecf989e 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -22,7 +22,6 @@ jobs: npm install @semantic-release/git @semantic-release/exec --no-save - name: Create Release env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} GH_TOKEN: ${{ secrets.RELEASE_TOKEN }} GITHUB_TOKEN: ${{ secrets.RELEASE_TOKEN }} GIT_AUTHOR_NAME: "Frappe PR Bot" From e0c89cdc7375092e150bfbbbd61288cc7099ae7c Mon Sep 17 00:00:00 2001 From: Shadrak Gurupnor <30501401+shadrak98@users.noreply.github.com> Date: Tue, 26 Apr 2022 15:17:19 +0530 Subject: [PATCH 39/53] fix: properly validate google sheets url (#16683) * fix: properly validate google sheets url * fix: check for spreadsheets in path * fix(minor): error should throw if any of the cond fails --- frappe/utils/csvutils.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frappe/utils/csvutils.py b/frappe/utils/csvutils.py index 547372778b..b61e209b72 100644 --- a/frappe/utils/csvutils.py +++ b/frappe/utils/csvutils.py @@ -215,7 +215,10 @@ def get_csv_content_from_google_sheets(url): def validate_google_sheets_url(url): - if "docs.google.com/spreadsheets" not in url: + from urllib.parse import urlparse + + u = urlparse(url) + if u.scheme != "https" or u.netloc != "docs.google.com" or "/spreadsheets/" not in u.path: frappe.throw( _('"{0}" is not a valid Google Sheets URL').format(url), title=_("Invalid URL"), From e2d3d1d0be53fd09409febf038159805c27ec5b3 Mon Sep 17 00:00:00 2001 From: Saurabh Date: Tue, 26 Apr 2022 15:32:30 +0530 Subject: [PATCH 40/53] feat: provision to handle payment authorization event in server script for custom documents (#16712) Currently, there is no provision to handle payment authorization events via server script. So it's not possible if a user wants to link payments against custom documents. Thus adding a provision in server script - Setup checkout for custom doc Screenshot 2022-03-11 at 2 44 19 PM - Handle payment callback Screenshot 2022-04-22 at 11 28 19 AM ## Documentation https://frappeframework.com/docs/v13/user/en/desk/scripting/server-script/edit?wiki_page_patch=bbed0fcd9a --- frappe/core/doctype/server_script/server_script.json | 4 ++-- frappe/core/doctype/server_script/server_script_utils.py | 1 + frappe/utils/safe_exec.py | 1 + 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/server_script/server_script.json b/frappe/core/doctype/server_script/server_script.json index 548d21bb60..700a97cfeb 100644 --- a/frappe/core/doctype/server_script/server_script.json +++ b/frappe/core/doctype/server_script/server_script.json @@ -49,7 +49,7 @@ "fieldname": "doctype_event", "fieldtype": "Select", "label": "DocType Event", - "options": "Before Insert\nBefore Validate\nBefore Save\nAfter Insert\nAfter Save\nBefore Submit\nAfter Submit\nBefore Cancel\nAfter Cancel\nBefore Delete\nAfter Delete\nBefore Save (Submitted Document)\nAfter Save (Submitted Document)" + "options": "Before Insert\nBefore Validate\nBefore Save\nAfter Save\nBefore Submit\nAfter Submit\nBefore Cancel\nAfter Cancel\nBefore Delete\nAfter Delete\nBefore Save (Submitted Document)\nAfter Save (Submitted Document)\nOn Payment Authorization" }, { "depends_on": "eval:doc.script_type==='API'", @@ -109,7 +109,7 @@ "link_fieldname": "server_script" } ], - "modified": "2022-04-07 19:41:23.178772", + "modified": "2022-04-22 11:24:01.151662", "modified_by": "Administrator", "module": "Core", "name": "Server Script", diff --git a/frappe/core/doctype/server_script/server_script_utils.py b/frappe/core/doctype/server_script/server_script_utils.py index 5300baa199..b807b43d10 100644 --- a/frappe/core/doctype/server_script/server_script_utils.py +++ b/frappe/core/doctype/server_script/server_script_utils.py @@ -17,6 +17,7 @@ EVENT_MAP = { "after_delete": "After Delete", "before_update_after_submit": "Before Save (Submitted Document)", "on_update_after_submit": "After Save (Submitted Document)", + "on_payment_authorized": "On Payment Authorization", } diff --git a/frappe/utils/safe_exec.py b/frappe/utils/safe_exec.py index fc53243021..30cf38bcf9 100644 --- a/frappe/utils/safe_exec.py +++ b/frappe/utils/safe_exec.py @@ -146,6 +146,7 @@ def get_safe_globals(): ), make_get_request=frappe.integrations.utils.make_get_request, make_post_request=frappe.integrations.utils.make_post_request, + get_payment_gateway_controller=frappe.integrations.utils.get_payment_gateway_controller, socketio_port=frappe.conf.socketio_port, get_hooks=get_hooks, enqueue=safe_enqueue, From 57e379ac6815af872449585ba96e08333fcc8bfa Mon Sep 17 00:00:00 2001 From: Shariq Ansari <30859809+shariquerik@users.noreply.github.com> Date: Tue, 26 Apr 2022 15:48:06 +0530 Subject: [PATCH 41/53] fix: Newsletter unsubscribe button not working (#16756) --- frappe/www/unsubscribe.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/www/unsubscribe.py b/frappe/www/unsubscribe.py index bae54f740d..d679bb3319 100644 --- a/frappe/www/unsubscribe.py +++ b/frappe/www/unsubscribe.py @@ -8,7 +8,7 @@ no_cache = True def get_context(context): frappe.flags.ignore_permissions = True # Called for confirmation. - if "email" in frappe.form_dict: + if "email" in frappe.form_dict and frappe.request.method == "GET": if verify_request(): user_email = frappe.form_dict["email"] context.email = user_email @@ -18,7 +18,7 @@ def get_context(context): context.status = "waiting_for_confirmation" # Called when form is submitted. - elif "user_email" in frappe.form_dict: + elif "user_email" in frappe.form_dict and frappe.request.method == "POST": context.status = "unsubscribed" email = frappe.form_dict["user_email"] email_group = get_email_groups(email) From 224dd319e890c25a7531ef8e66616254bc1b762b Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Tue, 26 Apr 2022 15:52:03 +0530 Subject: [PATCH 42/53] perf(BaseDocument): remove duplicate code (#16733) --- frappe/model/base_document.py | 1 - 1 file changed, 1 deletion(-) diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index b236bce1f3..186ef52c12 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -241,7 +241,6 @@ class BaseDocument(object): raise AttributeError(key) value = get_controller(value["doctype"])(value) - value.init_valid_columns() value.parent = self.name value.parenttype = self.doctype From 5d61482fb35cba4dba8032aa6c14092e28bcff88 Mon Sep 17 00:00:00 2001 From: Raffael Meyer <14891507+barredterra@users.noreply.github.com> Date: Tue, 26 Apr 2022 12:30:46 +0200 Subject: [PATCH 43/53] fix: Replace password with asterisks before logging (#16743) --- frappe/utils/backups.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index e2579444bd..927ae9c2db 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -461,7 +461,7 @@ class BackupGenerator: ) if self.verbose: - print(command + "\n") + print(command.replace(args.password, "*" * 10) + "\n") frappe.utils.execute_in_shell(command, low_priority=True) From 0551a2f1e7b80c2f5e3ae16ba579aa7d2d063674 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 26 Apr 2022 16:34:34 +0530 Subject: [PATCH 44/53] fix(mobile): Show checkbox in file list view --- frappe/public/js/frappe/views/file/file_view.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/views/file/file_view.js b/frappe/public/js/frappe/views/file/file_view.js index b351ce6109..de06e6013e 100644 --- a/frappe/public/js/frappe/views/file/file_view.js +++ b/frappe/public/js/frappe/views/file/file_view.js @@ -390,7 +390,7 @@ frappe.views.FileView = class FileView extends frappe.views.ListView { return `
- From 0334936449e5e2fdb80782316c0e543ca666542f Mon Sep 17 00:00:00 2001 From: Tom-Finke Date: Tue, 26 Apr 2022 14:38:18 +0200 Subject: [PATCH 45/53] fix: filters not working on "select items" dialog boxes (#16765) * Fix: is_child_selection_enabled for multiselect dialog filter * fix: check if field exists before fetching value Co-authored-by: Ankush Menat --- frappe/public/js/frappe/form/multi_select_dialog.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/form/multi_select_dialog.js b/frappe/public/js/frappe/form/multi_select_dialog.js index 61922a2422..92d2759f7f 100644 --- a/frappe/public/js/frappe/form/multi_select_dialog.js +++ b/frappe/public/js/frappe/form/multi_select_dialog.js @@ -151,7 +151,7 @@ frappe.ui.form.MultiSelectDialog = class MultiSelectDialog { } is_child_selection_enabled() { - return this.dialog.fields_dict['allow_child_item_selection'].get_value(); + return this.dialog.fields_dict['allow_child_item_selection']?.get_value(); } toggle_child_selection() { From 8f53a039a6665d3e4cda28f1075ab41cb7761bda Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Tue, 26 Apr 2022 23:43:07 +0200 Subject: [PATCH 46/53] refactor: Sync fixtures --- frappe/utils/fixtures.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/frappe/utils/fixtures.py b/frappe/utils/fixtures.py index f00d310c9d..42e86e9f11 100644 --- a/frappe/utils/fixtures.py +++ b/frappe/utils/fixtures.py @@ -17,11 +17,9 @@ def sync_fixtures(app=None): frappe.flags.in_fixtures = True for app in apps: - if os.path.exists(frappe.get_app_path(app, "fixtures")): - fixture_files = sorted(os.listdir(frappe.get_app_path(app, "fixtures"))) - for fname in fixture_files: - if fname.endswith(".json"): - import_doc(frappe.get_app_path(app, "fixtures", fname)) + fixtures_path = frappe.get_app_path(app, "fixtures") + if os.path.exists(fixtures_path): + import_doc(fixtures_path) import_custom_scripts(app) From 2c4773ff4482ed3a2902846c886a7a16905f11a8 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Wed, 27 Apr 2022 09:48:17 +0530 Subject: [PATCH 47/53] fix(newsletter): Pass parent instead of parentfield while getting successful recipients of a newsletter --- frappe/email/doctype/newsletter/newsletter.py | 4 ++-- .../doctype/newsletter/test_newsletter.py | 21 +++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/frappe/email/doctype/newsletter/newsletter.py b/frappe/email/doctype/newsletter/newsletter.py index 6aa881ed5c..b04ad4db40 100644 --- a/frappe/email/doctype/newsletter/newsletter.py +++ b/frappe/email/doctype/newsletter/newsletter.py @@ -124,7 +124,7 @@ class Newsletter(WebsiteGenerator): ) def get_success_recipients(self) -> List[str]: - """Recipients who have already recieved the newsletter. + """Recipients who have already received the newsletter. Couldn't think of a better name ;) """ @@ -132,7 +132,7 @@ class Newsletter(WebsiteGenerator): "Email Queue Recipient", filters={ "status": ("in", ["Not Sent", "Sending", "Sent"]), - "parentfield": ("in", self.get_linked_email_queue()), + "parent": ("in", self.get_linked_email_queue()), }, pluck="recipient", ) diff --git a/frappe/email/doctype/newsletter/test_newsletter.py b/frappe/email/doctype/newsletter/test_newsletter.py index c62b7e84aa..81702f3a09 100644 --- a/frappe/email/doctype/newsletter/test_newsletter.py +++ b/frappe/email/doctype/newsletter/test_newsletter.py @@ -221,3 +221,24 @@ class TestNewsletter(TestNewsletterMixin, unittest.TestCase): newsletter.reload() self.assertEqual(newsletter.email_sent, 0) + + def test_retry_partially_sent_newsletter(self): + frappe.db.delete("Email Queue") + frappe.db.delete("Email Queue Recipient") + frappe.db.delete("Newsletter") + + newsletter = self.get_newsletter() + newsletter.send_emails() + email_queue_list = [frappe.get_doc("Email Queue", e.name) for e in frappe.get_all("Email Queue")] + self.assertEqual(len(email_queue_list), 4) + + # emulate partial send + email_queue_list[0].status = "Error" + email_queue_list[0].recipients[0].status = "Error" + email_queue_list[0].save() + newsletter.email_sent = False + + # retry + newsletter.send_emails() + email_queue_list = [frappe.get_doc("Email Queue", e.name) for e in frappe.get_all("Email Queue")] + self.assertEqual(len(email_queue_list), 5) From b714dbb190311b43f22c898d5925d045b4150a62 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 27 Apr 2022 10:49:56 +0530 Subject: [PATCH 48/53] fix: dont throw error for a bad translation (#16769) --- frappe/translate.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/frappe/translate.py b/frappe/translate.py index d95c8eb3e8..0ebf4eaf1b 100644 --- a/frappe/translate.py +++ b/frappe/translate.py @@ -22,7 +22,7 @@ from pypika.terms import PseudoColumn import frappe from frappe.model.utils import InvalidIncludePath, render_include from frappe.query_builder import DocType, Field -from frappe.utils import get_bench_path, is_html, strip, strip_html_tags +from frappe.utils import cstr, get_bench_path, is_html, strip, strip_html_tags TRANSLATE_PATTERN = re.compile( r"_\([\s\n]*" # starts with literal `_(`, ignore following whitespace/newlines @@ -319,11 +319,11 @@ def get_translation_dict_from_file(path, lang, app): elif len(item) in [2, 3]: translation_map[item[0]] = strip(item[1]) elif item: - raise Exception( - "Bad translation in '{app}' for language '{lang}': {values}".format( - app=app, lang=lang, values=repr(item).encode("utf-8") - ) + msg = "Bad translation in '{app}' for language '{lang}': {values}".format( + app=app, lang=lang, values=cstr(item) ) + frappe.log_error(message=msg, title="Error in translation file") + frappe.msgprint(msg) return translation_map From 274d5e69ee4c0f2259315ee6286cf6ceb77f849b Mon Sep 17 00:00:00 2001 From: Raffael Meyer <14891507+barredterra@users.noreply.github.com> Date: Wed, 27 Apr 2022 07:26:58 +0200 Subject: [PATCH 49/53] feat: add default date and time format for Germany (#16772) --- frappe/geo/country_info.json | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frappe/geo/country_info.json b/frappe/geo/country_info.json index 23cadc2156..c1031fe211 100644 --- a/frappe/geo/country_info.json +++ b/frappe/geo/country_info.json @@ -954,6 +954,8 @@ "smallest_currency_fraction_value": 0.01, "currency_symbol": "\u20ac", "number_format": "#.###,##", + "date_format": "dd.mm.yyyy", + "time_format": "HH:mm", "timezones": [ "Europe/Berlin" ] From a9492ba5fa9fa7c0d6186714c290c9930fde97cd Mon Sep 17 00:00:00 2001 From: chillaranand Date: Fri, 22 Apr 2022 11:28:00 +0530 Subject: [PATCH 50/53] refactor: Remove wrapt package Also replaced inspect.getfullargspec with inspect.signature to preserve signature of a decorated functions. --- frappe/__init__.py | 14 ++++++-------- frappe/desk/search.py | 25 +++++++++++++------------ requirements.txt | 1 - 3 files changed, 19 insertions(+), 21 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 97e605394b..bcd6ef4f45 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -1490,10 +1490,11 @@ def get_newargs(fn, kwargs): if hasattr(fn, "fnargs"): fnargs = fn.fnargs else: - fullargspec = inspect.getfullargspec(fn) - fnargs = fullargspec.args - fnargs.extend(fullargspec.kwonlyargs) - varkw = fullargspec.varkw + signature = inspect.signature(fn) + fnargs = list(signature.parameters) + varkw = "kwargs" in fnargs + if varkw: + fnargs.pop(-1) newargs = {} for a in kwargs: @@ -2252,7 +2253,4 @@ def mock(type, size=1, locale="en"): return squashify(results) -def validate_and_sanitize_search_inputs(fn): - from frappe.desk.search import validate_and_sanitize_search_inputs as func - - return func(fn) +from frappe.desk.search import validate_and_sanitize_search_inputs # noqa diff --git a/frappe/desk/search.py b/frappe/desk/search.py index ba4c5fb4fb..eb1a2e82ba 100644 --- a/frappe/desk/search.py +++ b/frappe/desk/search.py @@ -1,12 +1,10 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE +import functools import json import re -import wrapt - -# Search import frappe from frappe import _, is_whitelisted from frappe.permissions import has_permission @@ -314,17 +312,20 @@ def relevance_sorter(key, query, as_dict): return (cstr(value).lower().startswith(query.lower()) is not True, value) -@wrapt.decorator -def validate_and_sanitize_search_inputs(fn, instance, args, kwargs): - kwargs.update(dict(zip(fn.__code__.co_varnames, args))) - sanitize_searchfield(kwargs["searchfield"]) - kwargs["start"] = cint(kwargs["start"]) - kwargs["page_len"] = cint(kwargs["page_len"]) +def validate_and_sanitize_search_inputs(fn): + @functools.wraps(fn) + def wrapper(*args, **kwargs): + kwargs.update(dict(zip(fn.__code__.co_varnames, args))) + sanitize_searchfield(kwargs["searchfield"]) + kwargs["start"] = cint(kwargs["start"]) + kwargs["page_len"] = cint(kwargs["page_len"]) - if kwargs["doctype"] and not frappe.db.exists("DocType", kwargs["doctype"]): - return [] + if kwargs["doctype"] and not frappe.db.exists("DocType", kwargs["doctype"]): + return [] - return fn(**kwargs) + return fn(**kwargs) + + return wrapper @frappe.whitelist() diff --git a/requirements.txt b/requirements.txt index 099e49a0b0..ec8d76367e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -66,7 +66,6 @@ terminaltables~=3.1.0 urllib3~=1.26.4 Werkzeug~=2.0.3 Whoosh~=2.7.4 -wrapt~=1.14.0 xlrd~=2.0.1 zxcvbn-python~=4.4.24 tenacity~=8.0.1 From 834706e33be590cc9170dd7a7c64909b72c6c77a Mon Sep 17 00:00:00 2001 From: Saurabh Date: Wed, 27 Apr 2022 11:44:08 +0530 Subject: [PATCH 51/53] chore: add missing doc event type --- frappe/core/doctype/server_script/server_script.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/server_script/server_script.json b/frappe/core/doctype/server_script/server_script.json index 700a97cfeb..9312ae178b 100644 --- a/frappe/core/doctype/server_script/server_script.json +++ b/frappe/core/doctype/server_script/server_script.json @@ -49,7 +49,7 @@ "fieldname": "doctype_event", "fieldtype": "Select", "label": "DocType Event", - "options": "Before Insert\nBefore Validate\nBefore Save\nAfter Save\nBefore Submit\nAfter Submit\nBefore Cancel\nAfter Cancel\nBefore Delete\nAfter Delete\nBefore Save (Submitted Document)\nAfter Save (Submitted Document)\nOn Payment Authorization" + "options": "Before Insert\nBefore Validate\nBefore Save\nAfter Insert\nAfter Save\nBefore Submit\nAfter Submit\nBefore Cancel\nAfter Cancel\nBefore Delete\nAfter Delete\nBefore Save (Submitted Document)\nAfter Save (Submitted Document)\nOn Payment Authorization" }, { "depends_on": "eval:doc.script_type==='API'", @@ -109,7 +109,7 @@ "link_fieldname": "server_script" } ], - "modified": "2022-04-22 11:24:01.151662", + "modified": "2022-04-27 11:42:52.032963", "modified_by": "Administrator", "module": "Core", "name": "Server Script", From 9bec3480ed90376f12052b2933123aeab5a456ce Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 27 Apr 2022 14:25:45 +0530 Subject: [PATCH 52/53] ci: failfast in case of conflicts (#16777) --- .github/helper/install_dependencies.sh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/helper/install_dependencies.sh b/.github/helper/install_dependencies.sh index d16f5b62ad..f0e8016860 100644 --- a/.github/helper/install_dependencies.sh +++ b/.github/helper/install_dependencies.sh @@ -2,6 +2,13 @@ set -e +# Check for merge conflicts before proceeding +python -m compileall -f "${GITHUB_WORKSPACE}" +if grep -lr --exclude-dir=node_modules "^<<<<<<< " "${GITHUB_WORKSPACE}" + then echo "Found merge conflicts" + exit 1 +fi + # install wkhtmltopdf wget -O /tmp/wkhtmltox.tar.xz https://github.com/frappe/wkhtmltopdf/raw/master/wkhtmltox-0.12.3_linux-generic-amd64.tar.xz tar -xf /tmp/wkhtmltox.tar.xz -C /tmp From 88992c66862b3f958db5c29b3bdb2e8d8f69da38 Mon Sep 17 00:00:00 2001 From: Max Solanki <84378369+lapardnemihk1099@users.noreply.github.com> Date: Wed, 27 Apr 2022 17:08:26 +0530 Subject: [PATCH 53/53] fix: console error while using TAB shortcut in grid (#16701) --- frappe/public/js/frappe/form/form.js | 2 +- frappe/public/js/frappe/ui/keyboard.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index 3c07907c50..c56ffc592d 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -179,7 +179,7 @@ frappe.ui.form.Form = class FrappeForm { grid_shortcut_keys.forEach(row => { frappe.ui.keys.add_shortcut({ shortcut: row.shortcut, - page: this, + page: this.page, description: __(row.description), ignore_inputs: true, condition: () => !this.is_new() diff --git a/frappe/public/js/frappe/ui/keyboard.js b/frappe/public/js/frappe/ui/keyboard.js index 40d1a93b8c..85ce248175 100644 --- a/frappe/public/js/frappe/ui/keyboard.js +++ b/frappe/public/js/frappe/ui/keyboard.js @@ -37,7 +37,7 @@ frappe.ui.keys.add_shortcut = ({shortcut, action, description, page, target, con if (is_input_focused && !ignore_inputs) return; if (!condition()) return; - if (!page || page.wrapper.is(':visible')) { + if (action && (!page || page.wrapper.is(':visible'))) { let prevent_default = action(e); // prevent default if true is explicitly returned // or nothing returned (undefined)