From 21442f5cbaa5c7b1a5a0aa1c8a376ddbc6d04ad6 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 12 Jun 2022 17:19:13 +0530 Subject: [PATCH] perf: disable creating version for new docs Each new doc inserts a version, this contains nothing but creator and creation time.. which is already immutable information on the original document. This was added for cases like data import to track from where document got created, ref: https://github.com/frappe/frappe/commit/b7dfe7969dd6b2851844b1e9c090cbb6447748de Fix: only add a version on creation IF creation info is present on flags --- frappe/core/doctype/version/test_version.py | 13 +++++++++++++ frappe/core/doctype/version/version.py | 19 ++++++++++++++++--- frappe/model/document.py | 7 +++---- frappe/tests/test_form_load.py | 2 +- 4 files changed, 33 insertions(+), 8 deletions(-) diff --git a/frappe/core/doctype/version/test_version.py b/frappe/core/doctype/version/test_version.py index c35430b17b..3e82f30f06 100644 --- a/frappe/core/doctype/version/test_version.py +++ b/frappe/core/doctype/version/test_version.py @@ -32,6 +32,19 @@ class TestVersion(unittest.TestCase): self.assertEqual(get_old_values(diff)[1], "01-01-2014 00:00:00") self.assertEqual(get_new_values(diff)[1], "07-20-2017 00:00:00") + def test_no_version_on_new_doc(self): + from frappe.desk.form.load import get_versions + + t = frappe.get_doc(doctype="ToDo", description="something") + t.save(ignore_version=False) + + self.assertFalse(get_versions(t)) + + t = frappe.get_doc(t.doctype, t.name) + t.description = "changed" + t.save(ignore_version=False) + self.assertTrue(get_versions(t)) + def get_fieldnames(change_array): return [d[0] for d in change_array] diff --git a/frappe/core/doctype/version/version.py b/frappe/core/doctype/version/version.py index 863885e85c..fa6ba0a9cf 100644 --- a/frappe/core/doctype/version/version.py +++ b/frappe/core/doctype/version/version.py @@ -2,6 +2,7 @@ # License: MIT. See LICENSE import json +from typing import Optional import frappe from frappe.model import no_value_fields, table_fields @@ -9,7 +10,15 @@ from frappe.model.document import Document class Version(Document): - def set_diff(self, old, new): + def update_version_info(self, old: Optional[Document], new: Document) -> bool: + """Update changed info and return true if change contains useful data.""" + if not old: + # Check if doc has some information about creation source like data import + return self.for_insert(new) + else: + return self.set_diff(old, new) + + def set_diff(self, old: Document, new: Document) -> bool: """Set the data property with the diff of the docs if present""" diff = get_diff(old, new) if diff: @@ -20,8 +29,11 @@ class Version(Document): else: return False - def for_insert(self, doc): + def for_insert(self, doc: Document) -> bool: updater_reference = doc.flags.updater_reference + if not updater_reference: + return False + data = { "creation": doc.creation, "updater_reference": updater_reference, @@ -29,7 +41,8 @@ class Version(Document): } self.ref_doctype = doc.doctype self.docname = doc.name - self.data = frappe.as_json(data) + self.data = frappe.as_json(data, indent=None, separators=(",", ":")) + return True def get_data(self): return json.loads(self.data) diff --git a/frappe/model/document.py b/frappe/model/document.py index fa1f423d11..a98d49e58c 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -1198,11 +1198,10 @@ class Document(BaseDocument): return version = frappe.new_doc("Version") - if not self._doc_before_save: - version.for_insert(self) - version.insert(ignore_permissions=True) - elif version.set_diff(self._doc_before_save, self): + + if is_useful_diff := version.update_version_info(self._doc_before_save, self): version.insert(ignore_permissions=True) + if not frappe.flags.in_migrate: # follow since you made a change? if frappe.get_cached_value("User", frappe.session.user, "follow_created_documents"): diff --git a/frappe/tests/test_form_load.py b/frappe/tests/test_form_load.py index e92b8c3ff2..22db56eeef 100644 --- a/frappe/tests/test_form_load.py +++ b/frappe/tests/test_form_load.py @@ -181,7 +181,7 @@ class TestFormLoad(unittest.TestCase): self.assertEqual(len(docinfo.comments), 1) self.assertIn("test", docinfo.comments[0].content) - self.assertGreaterEqual(len(docinfo.versions), 2) + self.assertGreaterEqual(len(docinfo.versions), 1) self.assertEqual(set(docinfo.tags.split(",")), {"more_tag", "test_tag"})