From fcf657ba80d19520a2b51d8919473ac9d8fe2950 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Mon, 24 Feb 2025 12:27:48 +0530 Subject: [PATCH] perf: improved `DocStatus` API and other minor improvements --- frappe/desk/form/save.py | 8 ++++---- frappe/model/base_document.py | 33 +++++++++++++++++++++------------ frappe/model/docstatus.py | 31 +++++++++++++++++++------------ frappe/model/document.py | 23 ++++++++++------------- frappe/model/workflow.py | 10 +++++----- frappe/tests/test_docstatus.py | 24 ++++++++++++------------ 6 files changed, 71 insertions(+), 58 deletions(-) diff --git a/frappe/desk/form/save.py b/frappe/desk/form/save.py index d56ff989f4..89315eef4e 100644 --- a/frappe/desk/form/save.py +++ b/frappe/desk/form/save.py @@ -28,10 +28,10 @@ def savedocs(doc, action): # action doc.docstatus = { - "Save": DocStatus.draft(), - "Submit": DocStatus.submitted(), - "Update": DocStatus.submitted(), - "Cancel": DocStatus.cancelled(), + "Save": DocStatus.DRAFT, + "Submit": DocStatus.SUMBITTED, + "Update": DocStatus.SUMBITTED, + "Cancel": DocStatus.CANCELLED, }[action] if doc.docstatus.is_submitted(): diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index d720feff83..ca8400a11c 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -189,9 +189,9 @@ class BaseDocument: if "name" in d: self.name = d["name"] - ignore_children = hasattr(self, "flags") and self.flags.ignore_children + as_value = not self._table_fieldnames or self.flags.get("ignore_children", False) for key, value in d.items(): - self.set(key, value, as_value=ignore_children) + self.set(key, value, as_value=as_value) return self @@ -334,8 +334,8 @@ class BaseDocument: value.parenttype = self.doctype value.parentfield = key - if value.docstatus is None: - value.docstatus = DocStatus.draft() + if value.__dict__.get("docstatus") is None: + value.__dict__["docstatus"] = DocStatus.DRAFT if not getattr(value, "idx", None): if table := getattr(self, key, None): @@ -457,7 +457,7 @@ class BaseDocument: if self.__dict__[key] is None: if key == "docstatus": - self.docstatus = DocStatus.draft() + self.__dict__[key] = DocStatus.DRAFT elif key == "idx": self.__dict__[key] = 0 @@ -482,12 +482,21 @@ class BaseDocument: return self.get("__islocal") @property - def docstatus(self): - return DocStatus(cint(self.get("docstatus"))) + def docstatus(self) -> DocStatus: + value = self.__dict__.get("docstatus") + + if not isinstance(value, DocStatus): + value = DocStatus(value or 0) + self.__dict__["docstatus"] = value + + return value @docstatus.setter - def docstatus(self, value): - self.__dict__["docstatus"] = DocStatus(cint(value)) + def docstatus(self, value) -> None: + if not isinstance(value, DocStatus): + value = DocStatus(value or 0) + + self.__dict__["docstatus"] = value def as_dict( self, @@ -740,8 +749,8 @@ class BaseDocument: elif df.fieldtype in ("Float", "Currency", "Percent"): self.set(df.fieldname, flt(self.get(df.fieldname))) - if self.docstatus is not None: - self.docstatus = DocStatus(cint(self.docstatus)) + # calling the docstatus property does the job + self.docstatus def _get_missing_mandatory_fields(self): """Get mandatory fields that do not have any values""" @@ -869,7 +878,7 @@ class BaseDocument: df.fieldname != "amended_from" and (is_submittable or self.meta.is_submittable) and frappe.get_meta(doctype).is_submittable - and cint(frappe.db.get_value(doctype, docname, "docstatus")) == DocStatus.cancelled() + and DocStatus(frappe.db.get_value(doctype, docname, "docstatus") or 0).is_cancelled() ): cancelled_links.append((df.fieldname, docname, get_msg(df, docname))) diff --git a/frappe/model/docstatus.py b/frappe/model/docstatus.py index 01aab1e491..8b4f38ec18 100644 --- a/frappe/model/docstatus.py +++ b/frappe/model/docstatus.py @@ -4,22 +4,29 @@ class DocStatus(int): def is_draft(self): - return self == self.draft() + return self == DocStatus.DRAFT def is_submitted(self): - return self == self.submitted() + return self == DocStatus.SUMBITTED def is_cancelled(self): - return self == self.cancelled() + return self == DocStatus.CANCELLED - @classmethod - def draft(cls): - return cls(0) + # following methods have been kept for backwards compatibility - @classmethod - def submitted(cls): - return cls(1) + @staticmethod + def draft(): + return DocStatus.DRAFT - @classmethod - def cancelled(cls): - return cls(2) + @staticmethod + def submitted(): + return DocStatus.SUMBITTED + + @staticmethod + def cancelled(): + return DocStatus.CANCELLED + + +DocStatus.DRAFT = DocStatus(0) +DocStatus.SUMBITTED = DocStatus(1) +DocStatus.CANCELLED = DocStatus(2) diff --git a/frappe/model/document.py b/frappe/model/document.py index a7c820cea4..2268b4bb69 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -703,11 +703,11 @@ class Document(BaseDocument, DocRef): frappe.flags.currently_saving.append((self.doctype, self.name)) def set_docstatus(self): - if self.docstatus is None: - self.docstatus = DocStatus.draft() + # docstatus property automatically sets a docstatus if not set + docstatus = self.docstatus for d in self.get_all_children(): - d.docstatus = self.docstatus + d.set("docstatus", docstatus) def _validate(self): self._validate_mandatory() @@ -972,10 +972,7 @@ class Document(BaseDocument, DocRef): - Submit (1) > Cancel (2) """ - if not self.docstatus: - self.docstatus = DocStatus.draft() - - if to_docstatus == DocStatus.draft(): + if to_docstatus == DocStatus.DRAFT: if self.docstatus.is_draft(): self._action = "save" elif self.docstatus.is_submitted(): @@ -988,7 +985,7 @@ class Document(BaseDocument, DocRef): else: raise frappe.ValidationError(_("Invalid docstatus"), self.docstatus) - elif to_docstatus == DocStatus.submitted(): + elif to_docstatus == DocStatus.SUMBITTED: if self.docstatus.is_submitted(): self._action = "update_after_submit" self.check_permission("submit") @@ -1002,7 +999,7 @@ class Document(BaseDocument, DocRef): else: raise frappe.ValidationError(_("Invalid docstatus"), self.docstatus) - elif to_docstatus == DocStatus.cancelled(): + elif to_docstatus == DocStatus.CANCELLED: raise frappe.ValidationError(_("Cannot edit cancelled document")) def set_parent_in_children(self): @@ -1168,12 +1165,12 @@ class Document(BaseDocument, DocRef): def _submit(self): """Submit the document. Sets `docstatus` = 1, then saves.""" - self.docstatus = DocStatus.submitted() + self.docstatus = DocStatus.SUMBITTED return self.save() def _cancel(self): """Cancel the document. Sets `docstatus` = 2, then saves.""" - self.docstatus = DocStatus.cancelled() + self.docstatus = DocStatus.CANCELLED return self.save() def _rename(self, name: str, merge: bool = False, force: bool = False, validate_rename: bool = True): @@ -1204,13 +1201,13 @@ class Document(BaseDocument, DocRef): self.set_user_and_timestamp() self.check_if_latest() - if not self.docstatus == DocStatus.draft(): + if not self.docstatus.is_draft(): raise frappe.ValidationError(_("Only draft documents can be discarded"), self.docstatus) self.check_permission("write") self.run_method("before_discard") - self.db_set("docstatus", DocStatus.cancelled()) + self.db_set("docstatus", DocStatus.CANCELLED) delattr(self, "_action") self.run_method("on_discard") diff --git a/frappe/model/workflow.py b/frappe/model/workflow.py index 447cfbd019..1c490cfeb7 100644 --- a/frappe/model/workflow.py +++ b/frappe/model/workflow.py @@ -126,10 +126,10 @@ def apply_workflow(doc, action): if next_state.update_field: doc.set(next_state.update_field, next_state.update_value) - new_docstatus = cint(next_state.doc_status) - if doc.docstatus.is_draft() and new_docstatus == DocStatus.draft(): + new_docstatus = DocStatus(next_state.doc_status or 0) + if doc.docstatus.is_draft() and new_docstatus.is_draft(): doc.save() - elif doc.docstatus.is_draft() and new_docstatus == DocStatus.submitted(): + elif doc.docstatus.is_draft() and new_docstatus.is_submitted(): from frappe.core.doctype.submission_queue.submission_queue import queue_submission from frappe.utils.scheduler import is_scheduler_inactive @@ -138,9 +138,9 @@ def apply_workflow(doc, action): return doc.submit() - elif doc.docstatus.is_submitted() and new_docstatus == DocStatus.submitted(): + elif doc.docstatus.is_submitted() and new_docstatus.is_submitted(): doc.save() - elif doc.docstatus.is_submitted() and new_docstatus == DocStatus.cancelled(): + elif doc.docstatus.is_submitted() and new_docstatus.is_cancelled(): doc.cancel() else: frappe.throw(_("Illegal Document Status for {0}").format(next_state.state)) diff --git a/frappe/tests/test_docstatus.py b/frappe/tests/test_docstatus.py index 7d428720a6..674bbe470d 100644 --- a/frappe/tests/test_docstatus.py +++ b/frappe/tests/test_docstatus.py @@ -4,22 +4,22 @@ from frappe.tests import IntegrationTestCase class TestDocStatus(IntegrationTestCase): def test_draft(self): - self.assertEqual(DocStatus(0), DocStatus.draft()) + self.assertEqual(DocStatus(0), DocStatus.DRAFT) - self.assertTrue(DocStatus.draft().is_draft()) - self.assertFalse(DocStatus.draft().is_cancelled()) - self.assertFalse(DocStatus.draft().is_submitted()) + self.assertTrue(DocStatus.DRAFT.is_draft()) + self.assertFalse(DocStatus.DRAFT.is_cancelled()) + self.assertFalse(DocStatus.DRAFT.is_submitted()) def test_submitted(self): - self.assertEqual(DocStatus(1), DocStatus.submitted()) + self.assertEqual(DocStatus(1), DocStatus.SUMBITTED) - self.assertFalse(DocStatus.submitted().is_draft()) - self.assertTrue(DocStatus.submitted().is_submitted()) - self.assertFalse(DocStatus.submitted().is_cancelled()) + self.assertFalse(DocStatus.SUMBITTED.is_draft()) + self.assertTrue(DocStatus.SUMBITTED.is_submitted()) + self.assertFalse(DocStatus.SUMBITTED.is_cancelled()) def test_cancelled(self): - self.assertEqual(DocStatus(2), DocStatus.cancelled()) + self.assertEqual(DocStatus(2), DocStatus.CANCELLED) - self.assertFalse(DocStatus.cancelled().is_draft()) - self.assertFalse(DocStatus.cancelled().is_submitted()) - self.assertTrue(DocStatus.cancelled().is_cancelled()) + self.assertFalse(DocStatus.CANCELLED.is_draft()) + self.assertFalse(DocStatus.CANCELLED.is_submitted()) + self.assertTrue(DocStatus.CANCELLED.is_cancelled())