From 7e52cb2856cb316ed3daaeefa6258ee7f1cd8f4c Mon Sep 17 00:00:00 2001
From: Khushi Rawat <142375893+khushi8112@users.noreply.github.com>
Date: Wed, 4 Jun 2025 19:20:43 +0530
Subject: [PATCH] refactor: split set_latest_location_and_custodian_in_asset
 into smaller functions

---
 .../doctype/asset_movement/asset_movement.py  | 100 ++++++++++--------
 1 file changed, 55 insertions(+), 45 deletions(-)

diff --git a/erpnext/assets/doctype/asset_movement/asset_movement.py b/erpnext/assets/doctype/asset_movement/asset_movement.py
index bba5789f818..eb1006ab6ed 100644
--- a/erpnext/assets/doctype/asset_movement/asset_movement.py
+++ b/erpnext/assets/doctype/asset_movement/asset_movement.py
@@ -107,53 +107,63 @@ class AssetMovement(Document):
 		self.set_latest_location_and_custodian_in_asset()
 
 	def set_latest_location_and_custodian_in_asset(self):
+		for d in self.assets:
+			current_location, current_employee = self.get_latest_location_and_custodian(d.asset)
+			self.update_asset_location_and_custodian(d.asset, current_location, current_employee)
+			self.log_asset_activity(d.asset, current_location, current_employee)
+
+	def get_latest_location_and_custodian(self, asset):
 		current_location, current_employee = "", ""
 		cond = "1=1"
 
-		for d in self.assets:
-			args = {"asset": d.asset, "company": self.company}
+		# latest entry corresponds to current document's location, employee when transaction date > previous dates
+		# In case of cancellation it corresponds to previous latest document's location, employee
+		args = {"asset": asset, "company": self.company}
+		latest_movement_entry = frappe.db.sql(
+			f"""
+			SELECT asm_item.target_location, asm_item.to_employee
+			FROM `tabAsset Movement Item` asm_item
+			JOIN `tabAsset Movement` asm ON asm_item.parent = asm.name
+			WHERE
+				asm_item.asset = %(asset)s AND
+				asm.company = %(company)s AND
+				asm.docstatus = 1 AND {cond}
+			ORDER BY asm.transaction_date DESC
+			LIMIT 1
+			""",
+			args,
+		)
 
-			# latest entry corresponds to current document's location, employee when transaction date > previous dates
-			# In case of cancellation it corresponds to previous latest document's location, employee
-			latest_movement_entry = frappe.db.sql(
-				f"""
-				SELECT asm_item.target_location, asm_item.to_employee
-				FROM `tabAsset Movement Item` asm_item, `tabAsset Movement` asm
-				WHERE
-					asm_item.parent=asm.name and
-					asm_item.asset=%(asset)s and
-					asm.company=%(company)s and
-					asm.docstatus=1 and {cond}
-				ORDER BY
-					asm.transaction_date desc limit 1
-				""",
-				args,
+		if latest_movement_entry:
+			current_location = latest_movement_entry[0][0]
+			current_employee = latest_movement_entry[0][1]
+
+		return current_location, current_employee
+
+	def update_asset_location_and_custodian(self, asset_id, location, employee):
+		asset = frappe.get_doc("Asset", asset_id)
+
+		if employee and employee != asset.custodian:
+			frappe.db.set_value("Asset", asset_id, "custodian", employee)
+		if location and location != asset.location:
+			frappe.db.set_value("Asset", asset_id, "location", location)
+
+	def log_asset_activity(self, asset_id, location, employee):
+		if location and employee:
+			add_asset_activity(
+				asset_id,
+				_("Asset received at Location {0} and issued to Employee {1}").format(
+					get_link_to_form("Location", location),
+					get_link_to_form("Employee", employee),
+				),
+			)
+		elif location:
+			add_asset_activity(
+				asset_id,
+				_("Asset transferred to Location {0}").format(get_link_to_form("Location", location)),
+			)
+		elif employee:
+			add_asset_activity(
+				asset_id,
+				_("Asset issued to Employee {0}").format(get_link_to_form("Employee", employee)),
 			)
-
-			if latest_movement_entry:
-				current_location = latest_movement_entry[0][0]
-				current_employee = latest_movement_entry[0][1]
-
-			frappe.db.set_value("Asset", d.asset, "location", current_location, update_modified=False)
-			frappe.db.set_value("Asset", d.asset, "custodian", current_employee, update_modified=False)
-
-			if current_location and current_employee:
-				add_asset_activity(
-					d.asset,
-					_("Asset received at Location {0} and issued to Employee {1}").format(
-						get_link_to_form("Location", current_location),
-						get_link_to_form("Employee", current_employee),
-					),
-				)
-			elif current_location:
-				add_asset_activity(
-					d.asset,
-					_("Asset transferred to Location {0}").format(
-						get_link_to_form("Location", current_location)
-					),
-				)
-			elif current_employee:
-				add_asset_activity(
-					d.asset,
-					_("Asset issued to Employee {0}").format(get_link_to_form("Employee", current_employee)),
-				)