From 28f9802fd9c4ad4f2025b2e75e9e95fa158dd4e2 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 28 Mar 2022 18:46:41 +0530 Subject: [PATCH] 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: