fix: refactor Asset Repair and Stock Entry linkage to resolve amendme… (#41919)

* fix: refactor Asset Repair and Stock Entry linkage to resolve amendment issues

* chore: added missing patch to patches.txt

* chore: fixing previous changes

* chore: fixing minor issues

* fix: code changes to enhance efficiency

* chore: replaced frappe.qb with db.sql because of conflict

* fix: minor changes

erpnext/assets/doctype/asset_repair/asset_repair.js

@@ -20,14 +20,14 @@ frappe.ui.form.on("Asset Repair", {
 			};
 		};
 
-		frm.fields_dict.warehouse.get_query = function (doc) {
+		frm.set_query("warehouse", "stock_items", function () {
 			return {
 				filters: {
 					is_group: 0,
-					company: doc.company,
+					company: frm.doc.company,
 				},
 			};
-		};
+		});
 
 		frm.set_query("serial_and_batch_bundle", "stock_items", (doc, cdt, cdn) => {
 			let row = locals[cdt][cdn];
@@ -79,7 +79,7 @@ frappe.ui.form.on("Asset Repair", {
 			});
 		}
 
-		if (frm.doc.repair_status == "Completed") {
+		if (frm.doc.repair_status == "Completed" && !frm.doc.completion_date) {
 			frm.set_value("completion_date", frappe.datetime.now_datetime());
 		}
 	},
@@ -87,15 +87,48 @@ frappe.ui.form.on("Asset Repair", {
 	stock_items_on_form_rendered() {
 		erpnext.setup_serial_or_batch_no();
 	},
+
+	stock_consumption: function (frm) {
+		if (!frm.doc.stock_consumption) {
+			frm.clear_table("stock_items");
+			frm.refresh_field("stock_items");
+		}
+	},
+
+	purchase_invoice: function (frm) {
+		if (frm.doc.purchase_invoice) {
+			frappe.call({
+				method: "frappe.client.get_value",
+				args: {
+					doctype: "Purchase Invoice",
+					fieldname: "base_net_total",
+					filters: { name: frm.doc.purchase_invoice },
+				},
+				callback: function (r) {
+					if (r.message) {
+						frm.set_value("repair_cost", r.message.base_net_total);
+					}
+				},
+			});
+		} else {
+			frm.set_value("repair_cost", 0);
+		}
+	},
 });
 
 frappe.ui.form.on("Asset Repair Consumed Item", {
-	item_code: function (frm, cdt, cdn) {
+	warehouse: function (frm, cdt, cdn) {
 		var item = locals[cdt][cdn];
 
+		if (!item.item_code) {
+			frappe.msgprint(__("Please select an item code before setting the warehouse."));
+			frappe.model.set_value(cdt, cdn, "warehouse", "");
+			return;
+		}
+
 		let item_args = {
 			item_code: item.item_code,
-			warehouse: frm.doc.warehouse,
+			warehouse: item.warehouse,
 			qty: item.consumed_quantity,
 			serial_no: item.serial_no,
 			company: frm.doc.company,

erpnext/assets/doctype/asset_repair/asset_repair.json

@@ -22,16 +22,14 @@
   "column_break_14",
   "project",
   "accounting_details",
-  "repair_cost",
+  "purchase_invoice",
   "capitalize_repair_cost",
   "stock_consumption",
   "column_break_8",
-  "purchase_invoice",
+  "repair_cost",
   "stock_consumption_details_section",
-  "warehouse",
   "stock_items",
   "total_repair_cost",
-  "stock_entry",
   "asset_depreciation_details_section",
   "increase_in_asset_life",
   "section_break_9",
@@ -122,7 +120,8 @@
    "default": "0",
    "fieldname": "repair_cost",
    "fieldtype": "Currency",
-   "label": "Repair Cost"
+   "label": "Repair Cost",
+   "read_only": 1
   },
   {
    "fieldname": "amended_from",
@@ -218,13 +217,6 @@
    "label": "Total Repair Cost",
    "read_only": 1
   },
-  {
-   "depends_on": "stock_consumption",
-   "fieldname": "warehouse",
-   "fieldtype": "Link",
-   "label": "Warehouse",
-   "options": "Warehouse"
-  },
   {
    "depends_on": "capitalize_repair_cost",
    "fieldname": "asset_depreciation_details_section",
@@ -251,20 +243,12 @@
    "fieldtype": "Link",
    "label": "Company",
    "options": "Company"
-  },
-  {
-   "fieldname": "stock_entry",
-   "fieldtype": "Link",
-   "label": "Stock Entry",
-   "no_copy": 1,
-   "options": "Stock Entry",
-   "read_only": 1
   }
  ],
  "index_web_pages_for_search": 1,
  "is_submittable": 1,
  "links": [],
- "modified": "2024-03-27 13:06:35.397626",
+ "modified": "2024-06-13 16:14:14.398356",
  "modified_by": "Administrator",
  "module": "Assets",
  "name": "Asset Repair",

erpnext/assets/doctype/asset_repair/asset_repair.py

@@ -47,20 +47,25 @@ class AssetRepair(AccountsController):
 		repair_cost: DF.Currency
 		repair_status: DF.Literal["Pending", "Completed", "Cancelled"]
 		stock_consumption: DF.Check
-		stock_entry: DF.Link | None
 		stock_items: DF.Table[AssetRepairConsumedItem]
 		total_repair_cost: DF.Currency
-		warehouse: DF.Link | None
 	# end: auto-generated types
 
 	def validate(self):
 		self.asset_doc = frappe.get_doc("Asset", self.asset)
+		self.validate_dates()
 		self.update_status()
 
 		if self.get("stock_items"):
 			self.set_stock_items_cost()
 		self.calculate_total_repair_cost()
 
+	def validate_dates(self):
+		if self.completion_date and (self.failure_date > self.completion_date):
+			frappe.throw(
+				_("Completion Date can not be before Failure Date. Please adjust the dates accordingly.")
+			)
+
 	def update_status(self):
 		if self.repair_status == "Pending" and self.asset_doc.status != "Out of Order":
 			frappe.db.set_value("Asset", self.asset, "status", "Out of Order")
@@ -105,22 +110,22 @@ class AssetRepair(AccountsController):
 				if self.asset_doc.calculate_depreciation and self.increase_in_asset_life:
 					self.modify_depreciation_schedule()
 
-			notes = _(
-				"This schedule was created when Asset {0} was repaired through Asset Repair {1}."
-			).format(
-				get_link_to_form(self.asset_doc.doctype, self.asset_doc.name),
-				get_link_to_form(self.doctype, self.name),
-			)
-			self.asset_doc.flags.ignore_validate_update_after_submit = True
-			make_new_active_asset_depr_schedules_and_cancel_current_ones(self.asset_doc, notes)
-			self.asset_doc.save()
+				notes = _(
+					"This schedule was created when Asset {0} was repaired through Asset Repair {1}."
+				).format(
+					get_link_to_form(self.asset_doc.doctype, self.asset_doc.name),
+					get_link_to_form(self.doctype, self.name),
+				)
+				self.asset_doc.flags.ignore_validate_update_after_submit = True
+				make_new_active_asset_depr_schedules_and_cancel_current_ones(self.asset_doc, notes)
+				self.asset_doc.save()
 
-			add_asset_activity(
-				self.asset,
-				_("Asset updated after completion of Asset Repair {0}").format(
-					get_link_to_form("Asset Repair", self.name)
-				),
-			)
+				add_asset_activity(
+					self.asset,
+					_("Asset updated after completion of Asset Repair {0}").format(
+						get_link_to_form("Asset Repair", self.name)
+					),
+				)
 
 	def before_cancel(self):
 		self.asset_doc = frappe.get_doc("Asset", self.asset)
@@ -136,29 +141,28 @@ class AssetRepair(AccountsController):
 				self.asset_doc.total_asset_cost -= self.repair_cost
 				self.asset_doc.additional_asset_cost -= self.repair_cost
 
-			if self.get("stock_consumption"):
-				self.increase_stock_quantity()
 			if self.get("capitalize_repair_cost"):
 				self.ignore_linked_doctypes = ("GL Entry", "Stock Ledger Entry")
 				self.make_gl_entries(cancel=True)
-				self.db_set("stock_entry", None)
 				if self.asset_doc.calculate_depreciation and self.increase_in_asset_life:
 					self.revert_depreciation_schedule_on_cancellation()
 
-			notes = _("This schedule was created when Asset {0}'s Asset Repair {1} was cancelled.").format(
-				get_link_to_form(self.asset_doc.doctype, self.asset_doc.name),
-				get_link_to_form(self.doctype, self.name),
-			)
-			self.asset_doc.flags.ignore_validate_update_after_submit = True
-			make_new_active_asset_depr_schedules_and_cancel_current_ones(self.asset_doc, notes)
-			self.asset_doc.save()
+				notes = _(
+					"This schedule was created when Asset {0}'s Asset Repair {1} was cancelled."
+				).format(
+					get_link_to_form(self.asset_doc.doctype, self.asset_doc.name),
+					get_link_to_form(self.doctype, self.name),
+				)
+				self.asset_doc.flags.ignore_validate_update_after_submit = True
+				make_new_active_asset_depr_schedules_and_cancel_current_ones(self.asset_doc, notes)
+				self.asset_doc.save()
 
-			add_asset_activity(
-				self.asset,
-				_("Asset updated after cancellation of Asset Repair {0}").format(
-					get_link_to_form("Asset Repair", self.name)
-				),
-			)
+				add_asset_activity(
+					self.asset,
+					_("Asset updated after cancellation of Asset Repair {0}").format(
+						get_link_to_form("Asset Repair", self.name)
+					),
+				)
 
 	def after_delete(self):
 		frappe.get_doc("Asset", self.asset).set_status()
@@ -170,11 +174,6 @@ class AssetRepair(AccountsController):
 	def check_for_stock_items_and_warehouse(self):
 		if not self.get("stock_items"):
 			frappe.throw(_("Please enter Stock Items consumed during the Repair."), title=_("Missing Items"))
-		if not self.warehouse:
-			frappe.throw(
-				_("Please enter Warehouse from which Stock Items consumed during the Repair were taken."),
-				title=_("Missing Warehouse"),
-			)
 
 	def increase_asset_value(self):
 		total_value_of_stock_consumed = self.get_total_value_of_stock_consumed()
@@ -208,6 +207,7 @@ class AssetRepair(AccountsController):
 		stock_entry = frappe.get_doc(
 			{"doctype": "Stock Entry", "stock_entry_type": "Material Issue", "company": self.company}
 		)
+		stock_entry.asset_repair = self.name
 
 		for stock_item in self.get("stock_items"):
 			self.validate_serial_no(stock_item)
@@ -215,7 +215,7 @@ class AssetRepair(AccountsController):
 			stock_entry.append(
 				"items",
 				{
-					"s_warehouse": self.warehouse,
+					"s_warehouse": stock_item.warehouse,
 					"item_code": stock_item.item_code,
 					"qty": stock_item.consumed_quantity,
 					"basic_rate": stock_item.valuation_rate,
@@ -228,8 +228,6 @@ class AssetRepair(AccountsController):
 		stock_entry.insert()
 		stock_entry.submit()
 
-		self.db_set("stock_entry", stock_entry.name)
-
 	def validate_serial_no(self, stock_item):
 		if not stock_item.serial_and_batch_bundle and frappe.get_cached_value(
 			"Item", stock_item.item_code, "has_serial_no"
@@ -247,12 +245,6 @@ class AssetRepair(AccountsController):
 				"Serial and Batch Bundle", stock_item.serial_and_batch_bundle, values_to_update
 			)
 
-	def increase_stock_quantity(self):
-		if self.stock_entry:
-			stock_entry = frappe.get_doc("Stock Entry", self.stock_entry)
-			stock_entry.flags.ignore_links = True
-			stock_entry.cancel()
-
 	def make_gl_entries(self, cancel=False):
 		if flt(self.total_repair_cost) > 0:
 			gl_entries = self.get_gl_entries()
@@ -316,7 +308,7 @@ class AssetRepair(AccountsController):
 			return
 
 		# creating GL Entries for each row in Stock Items based on the Stock Entry created for it
-		stock_entry = frappe.get_doc("Stock Entry", self.stock_entry)
+		stock_entry = frappe.get_doc("Stock Entry", {"asset_repair": self.name})
 
 		default_expense_account = None
 		if not erpnext.is_perpetual_inventory_enabled(self.company):
@@ -357,7 +349,7 @@ class AssetRepair(AccountsController):
 							"cost_center": self.cost_center,
 							"posting_date": getdate(),
 							"against_voucher_type": "Stock Entry",
-							"against_voucher": self.stock_entry,
+							"against_voucher": stock_entry.name,
 							"company": self.company,
 						},
 						item=self,

erpnext/assets/doctype/asset_repair/test_asset_repair.py

@@ -76,14 +76,14 @@ class TestAssetRepair(unittest.TestCase):
 	def test_warehouse(self):
 		asset_repair = create_asset_repair(stock_consumption=1)
 		self.assertTrue(asset_repair.stock_consumption)
-		self.assertTrue(asset_repair.warehouse)
+		self.assertTrue(asset_repair.stock_items[0].warehouse)
 
 	def test_decrease_stock_quantity(self):
 		asset_repair = create_asset_repair(stock_consumption=1, submit=1)
 		stock_entry = frappe.get_last_doc("Stock Entry")
 
 		self.assertEqual(stock_entry.stock_entry_type, "Material Issue")
-		self.assertEqual(stock_entry.items[0].s_warehouse, asset_repair.warehouse)
+		self.assertEqual(stock_entry.items[0].s_warehouse, asset_repair.stock_items[0].warehouse)
 		self.assertEqual(stock_entry.items[0].item_code, asset_repair.stock_items[0].item_code)
 		self.assertEqual(stock_entry.items[0].qty, asset_repair.stock_items[0].consumed_quantity)
 
@@ -114,14 +114,14 @@ class TestAssetRepair(unittest.TestCase):
 		asset_repair.repair_status = "Completed"
 		self.assertRaises(frappe.ValidationError, asset_repair.submit)
 
-	def test_increase_in_asset_value_due_to_stock_consumption(self):
+	def test_no_increase_in_asset_value_when_not_capitalized(self):
 		asset = create_asset(calculate_depreciation=1, submit=1)
 		initial_asset_value = get_asset_value_after_depreciation(asset.name)
-		asset_repair = create_asset_repair(asset=asset, stock_consumption=1, submit=1)
+		create_asset_repair(asset=asset, stock_consumption=1, submit=1)
 		asset.reload()
 
 		increase_in_asset_value = get_asset_value_after_depreciation(asset.name) - initial_asset_value
-		self.assertEqual(asset_repair.stock_items[0].total_value, increase_in_asset_value)
+		self.assertEqual(increase_in_asset_value, 0)
 
 	def test_increase_in_asset_value_due_to_repair_cost_capitalisation(self):
 		asset = create_asset(calculate_depreciation=1, submit=1)
@@ -185,7 +185,7 @@ class TestAssetRepair(unittest.TestCase):
 			frappe.get_doc("Purchase Invoice", asset_repair.purchase_invoice).items[0].expense_account
 		)
 		stock_entry_expense_account = (
-			frappe.get_doc("Stock Entry", asset_repair.stock_entry).get("items")[0].expense_account
+			frappe.get_doc("Stock Entry", {"asset_repair": asset_repair.name}).get("items")[0].expense_account
 		)
 
 		expected_values = {
@@ -260,6 +260,12 @@ class TestAssetRepair(unittest.TestCase):
 			asset.finance_books[0].value_after_depreciation,
 		)
 
+	def test_asset_repiar_link_in_stock_entry(self):
+		asset = create_asset(calculate_depreciation=1, submit=1)
+		asset_repair = create_asset_repair(asset=asset, stock_consumption=1, submit=1)
+		stock_entry = frappe.get_last_doc("Stock Entry")
+		self.assertEqual(stock_entry.asset_repair, asset_repair.name)
+
 
 def num_of_depreciations(asset):
 	return asset.finance_books[0].total_number_of_depreciations
@@ -289,7 +295,7 @@ def create_asset_repair(**args):
 
 	if args.stock_consumption:
 		asset_repair.stock_consumption = 1
-		asset_repair.warehouse = args.warehouse or create_warehouse("Test Warehouse", company=asset.company)
+		warehouse = args.warehouse or create_warehouse("Test Warehouse", company=asset.company)
 
 		bundle = None
 		if args.serial_no:
@@ -297,8 +303,8 @@ def create_asset_repair(**args):
 				frappe._dict(
 					{
 						"item_code": args.item_code,
-						"warehouse": asset_repair.warehouse,
-						"company": frappe.get_cached_value("Warehouse", asset_repair.warehouse, "company"),
+						"warehouse": warehouse,
+						"company": frappe.get_cached_value("Warehouse", warehouse, "company"),
 						"qty": (flt(args.stock_qty) or 1) * -1,
 						"voucher_type": "Asset Repair",
 						"type_of_transaction": "Asset Repair",
@@ -314,6 +320,7 @@ def create_asset_repair(**args):
 			"stock_items",
 			{
 				"item_code": args.item_code or "_Test Stock Item",
+				"warehouse": warehouse,
 				"valuation_rate": args.rate if args.get("rate") is not None else 100,
 				"consumed_quantity": args.qty or 1,
 				"serial_and_batch_bundle": bundle,
@@ -333,7 +340,7 @@ def create_asset_repair(**args):
 			stock_entry.append(
 				"items",
 				{
-					"t_warehouse": asset_repair.warehouse,
+					"t_warehouse": asset_repair.stock_items[0].warehouse,
 					"item_code": asset_repair.stock_items[0].item_code,
 					"qty": asset_repair.stock_items[0].consumed_quantity,
 					"basic_rate": args.rate if args.get("rate") is not None else 100,
@@ -351,7 +358,7 @@ def create_asset_repair(**args):
 				company=asset.company,
 				expense_account=frappe.db.get_value("Company", asset.company, "default_expense_account"),
 				cost_center=asset_repair.cost_center,
-				warehouse=asset_repair.warehouse,
+				warehouse=args.warehouse or create_warehouse("Test Warehouse", company=asset.company),
 			)
 			asset_repair.purchase_invoice = pi.name
 

erpnext/assets/doctype/asset_repair_consumed_item/asset_repair_consumed_item.json

@@ -6,6 +6,7 @@
  "engine": "InnoDB",
  "field_order": [
   "item_code",
+  "warehouse",
   "valuation_rate",
   "consumed_quantity",
   "total_value",
@@ -44,19 +45,28 @@
    "fieldtype": "Link",
    "in_list_view": 1,
    "label": "Item",
-   "options": "Item"
+   "options": "Item",
+   "reqd": 1
   },
   {
    "fieldname": "serial_and_batch_bundle",
    "fieldtype": "Link",
    "label": "Serial and Batch Bundle",
    "options": "Serial and Batch Bundle"
+  },
+  {
+   "fieldname": "warehouse",
+   "fieldtype": "Link",
+   "in_list_view": 1,
+   "label": "Warehouse",
+   "options": "Warehouse",
+   "reqd": 1
   }
  ],
  "index_web_pages_for_search": 1,
  "istable": 1,
  "links": [],
- "modified": "2024-03-27 13:06:35.608355",
+ "modified": "2024-06-13 12:01:47.147333",
  "modified_by": "Administrator",
  "module": "Assets",
  "name": "Asset Repair Consumed Item",

erpnext/assets/doctype/asset_repair_consumed_item/asset_repair_consumed_item.py

@@ -15,7 +15,7 @@ class AssetRepairConsumedItem(Document):
 		from frappe.types import DF
 
 		consumed_quantity: DF.Data | None
-		item_code: DF.Link | None
+		item_code: DF.Link
 		parent: DF.Data
 		parentfield: DF.Data
 		parenttype: DF.Data
@@ -23,6 +23,7 @@ class AssetRepairConsumedItem(Document):
 		serial_no: DF.SmallText | None
 		total_value: DF.Currency
 		valuation_rate: DF.Currency
+		warehouse: DF.Link
 	# end: auto-generated types
 
 	pass

erpnext/patches.txt

@@ -370,4 +370,6 @@ erpnext.patches.v14_0.enable_set_priority_for_pricing_rules #1
 erpnext.patches.v15_0.rename_number_of_depreciations_booked_to_opening_booked_depreciations
 erpnext.patches.v15_0.add_default_operations
 erpnext.patches.v15_0.enable_old_serial_batch_fields
-erpnext.patches.v15_0.update_total_number_of_booked_depreciations
+erpnext.patches.v15_0.update_warehouse_field_in_asset_repair_consumed_item_doctype
+erpnext.patches.v15_0.update_asset_repair_field_in_stock_entry
+erpnext.patches.v15_0.update_total_number_of_booked_depreciations

erpnext/patches/v15_0/update_asset_repair_field_in_stock_entry.py

@@ -0,0 +1,15 @@
+import frappe
+from frappe.query_builder import DocType
+
+
+def execute():
+	if frappe.db.has_column("Asset Repair", "stock_entry"):
+		AssetRepair = DocType("Asset Repair")
+		StockEntry = DocType("Stock Entry")
+
+		(
+			frappe.qb.update(StockEntry)
+			.join(AssetRepair)
+			.on(StockEntry.name == AssetRepair.stock_entry)
+			.set(StockEntry.asset_repair, AssetRepair.name)
+		).run()

erpnext/patches/v15_0/update_warehouse_field_in_asset_repair_consumed_item_doctype.py

@@ -0,0 +1,14 @@
+import frappe
+
+
+# not able to use frappe.qb because of this bug https://github.com/frappe/frappe/issues/20292
+def execute():
+	if frappe.db.has_column("Asset Repair", "warehouse"):
+		# nosemgrep
+		frappe.db.sql(
+			"""UPDATE `tabAsset Repair Consumed Item` ar_item
+			JOIN `tabAsset Repair` ar
+			ON ar.name = ar_item.parent
+			SET ar_item.warehouse = ar.warehouse
+			WHERE ifnull(ar.warehouse, '') != ''"""
+		)

erpnext/stock/doctype/stock_entry/stock_entry.json

@@ -21,6 +21,7 @@
   "sales_invoice_no",
   "pick_list",
   "purchase_receipt_no",
+  "asset_repair",
   "col2",
   "company",
   "posting_date",
@@ -674,6 +675,14 @@
   {
    "fieldname": "column_break_eaoa",
    "fieldtype": "Column Break"
+  },
+  {
+   "depends_on": "eval:doc.asset_repair",
+   "fieldname": "asset_repair",
+   "fieldtype": "Link",
+   "label": "Asset Repair",
+   "options": "Asset Repair",
+   "read_only": 1
   }
  ],
  "icon": "fa fa-file-text",
@@ -681,7 +690,7 @@
  "index_web_pages_for_search": 1,
  "is_submittable": 1,
  "links": [],
- "modified": "2024-03-27 13:10:43.701839",
+ "modified": "2024-06-26 19:12:17.937088",
  "modified_by": "Administrator",
  "module": "Stock",
  "name": "Stock Entry",

erpnext/stock/doctype/stock_entry/stock_entry.py

@@ -98,6 +98,7 @@ class StockEntry(StockController):
 		address_display: DF.TextEditor | None
 		amended_from: DF.Link | None
 		apply_putaway_rule: DF.Check
+		asset_repair: DF.Link | None
 		bom_no: DF.Link | None
 		company: DF.Link
 		credit_note: DF.Link | None