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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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 6cdd33f26bffb850db65a60837dbc2b8ed12abc0 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 20 Apr 2022 16:41:25 +0530 Subject: [PATCH 22/23] 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 e6e01627f89ab421963591ac0099376a24ff3aa3 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 22 Apr 2022 12:09:26 +0530 Subject: [PATCH 23/23] 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) {