Merge pull request #57244 from mihir-kandoi/fix-clear-old-logs-orphan-references

fix: clear linked comments, versions and attachments with old logs

erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py

@@ -6,9 +6,7 @@ from typing import Any
 import frappe
 from frappe import _
 from frappe.model.document import Document
-from frappe.query_builder import DocType, Interval
-from frappe.query_builder.functions import Now
-from frappe.utils import cint, cstr, date_diff, today
+from frappe.utils import add_days, cint, cstr, date_diff, now, today
 
 from erpnext.manufacturing.doctype.bom_update_log.bom_updation_utils import (
 	get_leaf_boms,
@@ -17,6 +15,7 @@ from erpnext.manufacturing.doctype.bom_update_log.bom_updation_utils import (
 	replace_bom,
 	set_values_in_log,
 )
+from erpnext.utilities import clear_logs_with_references
 
 
 class BOMMissingError(frappe.ValidationError):
@@ -48,10 +47,12 @@ class BOMUpdateLog(Document):
 	@staticmethod
 	def clear_old_logs(days=None):
 		days = days or 90
-		table = DocType("BOM Update Log")
-		frappe.db.delete(
-			table,
-			filters=((table.creation < (Now() - Interval(days=days))) & (table.update_type == "Update Cost")),
+		clear_logs_with_references(
+			"BOM Update Log",
+			{
+				"creation": ("<", add_days(now(), -days)),
+				"update_type": "Update Cost",
+			},
 		)
 
 	def validate(self):

erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py

@@ -6,9 +6,18 @@ from frappe import _
 from frappe.desk.form.load import get_attachments
 from frappe.exceptions import QueryDeadlockError, QueryTimeoutError
 from frappe.model.document import Document
-from frappe.query_builder import DocType, Interval
-from frappe.query_builder.functions import CombineDatetime, Max, Now
-from frappe.utils import cint, get_datetime, get_link_to_form, get_weekday, getdate, now, nowtime
+from frappe.query_builder import DocType
+from frappe.query_builder.functions import CombineDatetime, Max
+from frappe.utils import (
+	add_days,
+	cint,
+	get_datetime,
+	get_link_to_form,
+	get_weekday,
+	getdate,
+	now,
+	nowtime,
+)
 from frappe.utils.user import get_users_with_role
 from rq.timeouts import JobTimeoutException
 
@@ -22,6 +31,7 @@ from erpnext.stock.stock_ledger import (
 	repost_future_sle,
 )
 from erpnext.stock.utils import get_combine_datetime
+from erpnext.utilities import clear_logs_with_references
 
 RecoverableErrors = (JobTimeoutException, QueryDeadlockError, QueryTimeoutError)
 
@@ -65,13 +75,12 @@ class RepostItemValuation(Document):
 	@staticmethod
 	def clear_old_logs(days=None):
 		days = days or 90
-		table = DocType("Repost Item Valuation")
-		frappe.db.delete(
-			table,
-			filters=(
-				(table.creation < (Now() - Interval(days=days)))
-				& (table.status.isin(["Completed", "Skipped"]))
-			),
+		clear_logs_with_references(
+			"Repost Item Valuation",
+			{
+				"creation": ("<", add_days(now(), -days)),
+				"status": ("in", ["Completed", "Skipped"]),
+			},
 		)
 
 	def on_discard(self):

erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py

@@ -104,6 +104,15 @@ class TestRepostItemValuation(ERPNextTestSuite, StockTestMixin):
 			repost_doc.creation = add_days(now(), days=-i * 10)
 			repost_doc.db_update_all()
 
+		repost_doc.add_comment("Comment", "test comment")
+		frappe.new_doc(
+			"File",
+			file_name="test_clear_old_logs.txt",
+			content="test",
+			attached_to_doctype=repost_doc.doctype,
+			attached_to_name=repost_doc.name,
+		).insert(ignore_permissions=True)
+
 		logs = frappe.get_all("Repost Item Valuation", filters={"status": "Skipped"})
 		self.assertGreater(len(logs), 10)
 
@@ -114,6 +123,15 @@ class TestRepostItemValuation(ERPNextTestSuite, StockTestMixin):
 		logs = frappe.get_all("Repost Item Valuation", filters={"status": "Skipped"})
 		self.assertEqual(len(logs), 0)
 
+		orphan_reference = {"reference_doctype": repost_doc.doctype, "reference_name": repost_doc.name}
+		self.assertFalse(frappe.get_all("Comment", filters=orphan_reference))
+		self.assertFalse(
+			frappe.get_all(
+				"File",
+				filters={"attached_to_doctype": repost_doc.doctype, "attached_to_name": repost_doc.name},
+			)
+		)
+
 	def test_create_item_wise_repost_item_valuation_entries(self):
 		pr = make_purchase_receipt(
 			company="_Test Company with perpetual inventory",

erpnext/utilities/__init__.py

@@ -4,10 +4,37 @@ from contextlib import contextmanager
 
 import frappe
 from frappe import _
-from frappe.utils import cstr
+from frappe.utils import create_batch, cstr
 
 from erpnext.utilities.activation import get_level
 
+LOG_REFERENCE_FIELDS = {
+	"Comment": ("reference_doctype", "reference_name"),
+	"Version": ("ref_doctype", "docname"),
+	"ToDo": ("reference_type", "reference_name"),
+	"DocShare": ("share_doctype", "share_name"),
+	"View Log": ("reference_doctype", "reference_name"),
+	"Document Follow": ("ref_doctype", "ref_docname"),
+	"Notification Log": ("document_type", "document_name"),
+}
+
+
+def clear_logs_with_references(doctype, filters):
+	names = frappe.get_all(doctype, filters=filters, pluck="name")
+	for batch in create_batch(names, 1000):
+		attached_files = frappe.get_all(
+			"File",
+			filters={"attached_to_doctype": doctype, "attached_to_name": ("in", batch)},
+			pluck="name",
+		)
+		if attached_files:
+			frappe.delete_doc("File", attached_files, ignore_permissions=True, delete_permanently=True)
+
+		for reference_doctype, (doctype_field, name_field) in LOG_REFERENCE_FIELDS.items():
+			frappe.db.delete(reference_doctype, {doctype_field: doctype, name_field: ("in", batch)})
+
+		frappe.db.delete(doctype, {"name": ("in", batch)})
+
 
 def update_doctypes():
 	df = frappe.qb.DocType("DocField")