Merge pull request #43017 from blaggacao/fix/status-updater-notifications

fix: updater tigger notifications on status updates & perf

erpnext/controllers/status_updater.py

@@ -181,45 +181,65 @@ class StatusUpdater(Document):
 				self.status = "Draft"
 			return
 
-		if self.doctype in status_map:
+		if self.doctype not in status_map:
-			_status = self.status
+			return
-			if status and update:
+		_status = self.status
-				self.db_set("status", status)
+		# TODO: revise accidential complexity where update has a double meaning, see below
+		self.status = status if status else self.status
+		new_status = self.get_status()["status"]
 
-			sl = status_map[self.doctype][:]
+		if new_status != _status:
-			sl.reverse()
+			if new_status not in ("Cancelled", "Partially Ordered", "Ordered", "Issued", "Transferred"):
-			for s in sl:
+				self.add_comment("Label", _(new_status))
-				if not s[1]:
-					self.status = s[0]
-					break
-				elif s[1].startswith("eval:"):
-					if frappe.safe_eval(
-						s[1][5:],
-						None,
-						{
-							"self": self.as_dict(),
-							"getdate": getdate,
-							"nowdate": nowdate,
-							"get_value": frappe.db.get_value,
-						},
-					):
-						self.status = s[0]
-						break
-				elif getattr(self, s[1])():
-					self.status = s[0]
-					break
-
-			if self.status != _status and self.status not in (
-				"Cancelled",
-				"Partially Ordered",
-				"Ordered",
-				"Issued",
-				"Transferred",
-			):
-				self.add_comment("Label", _(self.status))
 
+			self.status = new_status
 			if update:
-				self.db_set("status", self.status, update_modified=update_modified)
+				self.db_set("status", new_status, update_modified=update_modified)
+
+	def get_status(self):
+		"""
+		Get the status of the document.
+
+		Returns:
+		        dict: A dictionary containing the status. This allows callers to receive
+		        a dictionary for efficient bulk updates, for example when `per_billed`
+		        and other status fields also need to be updated.
+
+		Note:
+		        Can be overriden on a doctype to implement more localized status updater logic.
+
+		Example:
+		        {
+		                "status": "Draft",
+		                "per_billed": 50,
+		                "billing_status": "Partly Billed"
+		        }
+		"""
+		if self.doctype not in status_map:
+			return {"status": self.status}
+
+		sl = status_map[self.doctype][:]
+		sl.reverse()
+
+		for s in sl:
+			if not s[1]:
+				return {"status": s[0]}
+			elif s[1].startswith("eval:"):
+				if frappe.safe_eval(
+					s[1][5:],
+					None,
+					{
+						"self": self.as_dict(),
+						"getdate": getdate,
+						"nowdate": nowdate,
+						"get_value": frappe.db.get_value,
+					},
+				):
+					return {"status": s[0]}
+			elif getattr(self, s[1])():
+				return {"status": s[0]}
+
+		return {"status": self.status}
 
 	def validate_qty(self):
 		"""Validates qty at row level"""
@@ -447,6 +467,39 @@ class StatusUpdater(Document):
 					where name='{detail_id}'""".format(**args)
 				)
 
+	@staticmethod
+	def _calculate_target_parent_percentage(
+		name, target_parent_dt, target_dt, target_ref_field, target_field
+	):
+		child_records = frappe.get_all(
+			target_dt,
+			filters={"parent": name, "parenttype": target_parent_dt},
+			fields=[target_ref_field, target_field],
+		)
+
+		sum_ref = sum(abs(record[target_ref_field]) for record in child_records)
+
+		if sum_ref > 0:
+			percentage = round(
+				sum(min(abs(record[target_field]), abs(record[target_ref_field])) for record in child_records)
+				/ sum_ref
+				* 100,
+				6,
+			)
+		else:
+			percentage = 0
+
+		return percentage
+
+	@staticmethod
+	def _determine_status(percentage, keyword):
+		if percentage < 0.001:
+			return f"Not {keyword}"
+		elif percentage >= 99.999999:
+			return f"Fully {keyword}"
+		else:
+			return f"Partly {keyword}"
+
 	def _update_percent_field_in_targets(self, args, update_modified=True):
 		"""Update percent field in parent transaction"""
 		if args.get("percent_join_field_parent"):
@@ -467,34 +520,28 @@ class StatusUpdater(Document):
 	def _update_percent_field(self, args, update_modified=True):
 		"""Update percent field in parent transaction"""
 
-		self._update_modified(args, update_modified)
+		update_data = {}
 
 		if args.get("target_parent_field"):
-			frappe.db.sql(
+			update_data[args.get("target_parent_field")] = self._calculate_target_parent_percentage(
-				"""update `tab{target_parent_dt}`
+				args["name"],
-				set {target_parent_field} = round(
+				args["target_parent_dt"],
-					ifnull((select
+				args["target_dt"],
-						ifnull(sum(case when abs({target_ref_field}) > abs({target_field}) then abs({target_field}) else abs({target_ref_field}) end), 0)
+				args["target_ref_field"],
-						/ sum(abs({target_ref_field})) * 100
+				args["target_field"],
-					from `tab{target_dt}` where parent='{name}' and parenttype='{target_parent_dt}' having sum(abs({target_ref_field})) > 0), 0), 6)
-					{update_modified}
-				where name='{name}'""".format(**args)
 			)
-
 			# update field
 			if args.get("status_field"):
-				frappe.db.sql(
+				update_data[args.get("status_field")] = self._determine_status(
-					"""update `tab{target_parent_dt}`
+					update_data[args.get("target_parent_field")], args["keyword"]
-					set {status_field} = (case when {target_parent_field}<0.001 then 'Not {keyword}'
-					else case when {target_parent_field}>=99.999999 then 'Fully {keyword}'
-					else 'Partly {keyword}' end end)
-					where name='{name}'""".format(**args)
 				)
 
-			if update_modified:
+		if update_data:
-				target = frappe.get_doc(args["target_parent_dt"], args["name"])
+			target = frappe.get_doc(args["target_parent_dt"], args["name"])
-				target.set_status(update=True)
+			target.update(update_data)  # status calculus might depend on it
-				target.notify_update()
+			status = target.get_status()
+			update_data.update(status)
+			target.db_set(update_data, update_modified=update_modified, notify=True)
 
 	def _update_modified(self, args, update_modified):
 		if not update_modified: