fix: prevent max recursion on supplier scorecard save (#58706)

erpnext/buying/doctype/supplier_scorecard/supplier_scorecard.py

@@ -55,8 +55,7 @@ class SupplierScorecard(Document):
 		self.update_standing()
 
 	def on_update(self):
-		score = make_all_scorecards(self.name)
-		if score > 0:
+		if make_all_scorecards(self.name) > 0:
 			self.save()
 
 	def validate_standings(self):
@@ -198,7 +197,7 @@ def refresh_scorecards():
 
 
 @frappe.whitelist()
-def make_all_scorecards(docname):
+def make_all_scorecards(docname: str):
 	sc = frappe.get_doc("Supplier Scorecard", docname)
 	supplier = frappe.get_doc("Supplier", sc.supplier)
 	supplier.check_permission("write")
@@ -213,25 +212,18 @@ def make_all_scorecards(docname):
 
 	while (start_date < todays) and (end_date <= todays):
 		# check to make sure there is no scorecard period already created
-		scorecards = frappe.db.sql(
-			"""
-			SELECT
-				scp.name
-			FROM
-				`tabSupplier Scorecard Period` scp
-			WHERE
-				scp.scorecard = %(sc)s
-				AND scp.docstatus = 1
-				AND (
-					(scp.start_date > %(end_date)s
-					AND scp.end_date < %(start_date)s)
-				OR
-					(scp.start_date < %(end_date)s
-					AND scp.end_date > %(start_date)s))
-			ORDER BY
-				scp.end_date DESC""",
-			{"sc": docname, "start_date": start_date, "end_date": end_date},
-			as_dict=1,
+		# (inclusive bounds: a single-day period — supplier created on a month's
+		# last day — must match its own window, else it is re-created every run)
+		scorecards = frappe.get_all(
+			"Supplier Scorecard Period",
+			fields=["name"],
+			filters={
+				"scorecard": docname,
+				"docstatus": 1,
+				"start_date": ["<=", end_date],
+				"end_date": [">=", start_date],
+			},
+			order_by="end_date desc",
 		)
 		if len(scorecards) == 0:
 			period_card = make_supplier_scorecard(docname, None)

erpnext/buying/doctype/supplier_scorecard/test_supplier_scorecard.py

@@ -4,6 +4,9 @@
 
 import frappe
 from frappe.tests.utils import FrappeTestCase
+from frappe.utils import add_months, get_last_day, getdate
+
+from erpnext.buying.doctype.supplier.test_supplier import create_supplier
 
 
 class TestSupplierScorecard(FrappeTestCase):
@@ -18,6 +21,32 @@ class TestSupplierScorecard(FrappeTestCase):
 			d.weight = 0
 		self.assertRaises(frappe.ValidationError, my_doc.insert)
 
+	def test_no_recursion_for_supplier_created_on_month_end(self):
+		make_supplier_scorecard()  # ensures the "Delivery" criteria master exists
+
+		supplier = create_supplier(supplier_name="_Test Month End Scorecard Supplier")
+		month_end = get_last_day(add_months(getdate(), -1))
+		frappe.db.set_value("Supplier", supplier.name, "creation", month_end, update_modified=False)
+
+		scorecard = frappe.get_doc(valid_scorecard[0])
+		scorecard.supplier = supplier.name
+		scorecard.name = supplier.name
+		scorecard.insert()
+
+		periods = frappe.get_all(
+			"Supplier Scorecard Period",
+			filters={"scorecard": scorecard.name},
+			fields=["start_date", "end_date"],
+		)
+		self.assertEqual(len(periods), 1)
+		self.assertEqual(periods[0].start_date, month_end)
+		self.assertEqual(periods[0].end_date, month_end)
+
+		# saving again must not re-create the single-day period or recurse
+		frappe.get_doc("Supplier Scorecard", scorecard.name).save()
+		periods = frappe.get_all("Supplier Scorecard Period", filters={"scorecard": scorecard.name})
+		self.assertEqual(len(periods), 1)
+
 
 def make_supplier_scorecard():
 	my_doc = frappe.get_doc(valid_scorecard[0])