refactor: split set_latest_location_and_custodian_in_asset into smaller functions

erpnext/assets/doctype/asset_movement/asset_movement.py

@@ -107,25 +107,29 @@ 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, `tabAsset Movement` asm
+			FROM `tabAsset Movement Item` asm_item
+			JOIN `tabAsset Movement` asm ON asm_item.parent = asm.name
 			WHERE
-					asm_item.parent=asm.name and
+				asm_item.asset = %(asset)s AND
-					asm_item.asset=%(asset)s and
+				asm.company = %(company)s AND
-					asm.company=%(company)s and
+				asm.docstatus = 1 AND {cond}
-					asm.docstatus=1 and {cond}
+			ORDER BY asm.transaction_date DESC
-				ORDER BY
+			LIMIT 1
-					asm.transaction_date desc limit 1
 			""",
 			args,
 		)
@@ -134,26 +138,32 @@ class AssetMovement(Document):
 			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)
+		return current_location, current_employee
-			frappe.db.set_value("Asset", d.asset, "custodian", current_employee, update_modified=False)
 
-			if current_location and 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(
-					d.asset,
+				asset_id,
 				_("Asset received at Location {0} and issued to Employee {1}").format(
-						get_link_to_form("Location", current_location),
+					get_link_to_form("Location", location),
-						get_link_to_form("Employee", current_employee),
+					get_link_to_form("Employee", employee),
 				),
 			)
-			elif current_location:
+		elif location:
 			add_asset_activity(
-					d.asset,
+				asset_id,
-					_("Asset transferred to Location {0}").format(
+				_("Asset transferred to Location {0}").format(get_link_to_form("Location", location)),
-						get_link_to_form("Location", current_location)
-					),
 			)
-			elif current_employee:
+		elif employee:
 			add_asset_activity(
-					d.asset,
+				asset_id,
-					_("Asset issued to Employee {0}").format(get_link_to_form("Employee", current_employee)),
+				_("Asset issued to Employee {0}").format(get_link_to_form("Employee", employee)),
 			)