From c71557f4321259e15877c97a3f9862f8dcba0793 Mon Sep 17 00:00:00 2001 From: ljain112 Date: Mon, 9 Mar 2026 18:03:59 +0530 Subject: [PATCH 1/5] fix: correct logic for repair cost in asset repair --- .../doctype/asset_repair/asset_repair.js | 8 +- .../doctype/asset_repair/asset_repair.py | 91 ++++++++++++++++++- .../doctype/asset_repair/test_asset_repair.py | 48 ++++++++++ 3 files changed, 139 insertions(+), 8 deletions(-) diff --git a/erpnext/assets/doctype/asset_repair/asset_repair.js b/erpnext/assets/doctype/asset_repair/asset_repair.js index 48bf4ff478d..a3ddd066ebd 100644 --- a/erpnext/assets/doctype/asset_repair/asset_repair.js +++ b/erpnext/assets/doctype/asset_repair/asset_repair.js @@ -111,15 +111,13 @@ frappe.ui.form.on("Asset Repair", { purchase_invoice: function (frm) { if (frm.doc.purchase_invoice) { frappe.call({ - method: "frappe.client.get_value", + method: "erpnext.assets.doctype.asset_repair.asset_repair.get_repair_cost_for_purchase_invoice", args: { - doctype: "Purchase Invoice", - fieldname: "base_net_total", - filters: { name: frm.doc.purchase_invoice }, + purchase_invoice: frm.doc.purchase_invoice, }, callback: function (r) { if (r.message) { - frm.set_value("repair_cost", r.message.base_net_total); + frm.set_value("repair_cost", r.message); } }, }); diff --git a/erpnext/assets/doctype/asset_repair/asset_repair.py b/erpnext/assets/doctype/asset_repair/asset_repair.py index e0d41919c4a..4d59ba5b96c 100644 --- a/erpnext/assets/doctype/asset_repair/asset_repair.py +++ b/erpnext/assets/doctype/asset_repair/asset_repair.py @@ -3,6 +3,7 @@ import frappe from frappe import _ +from frappe.query_builder.functions import Sum from frappe.utils import add_months, cint, flt, get_link_to_form, getdate, time_diff_in_hours import erpnext @@ -308,9 +309,14 @@ class AssetRepair(AccountsController): if flt(self.repair_cost) <= 0: return - pi_expense_account = ( - frappe.get_doc("Purchase Invoice", self.purchase_invoice).items[0].expense_account - ) + expense_accounts = _get_expense_accounts_for_purchase_invoice(self.purchase_invoice) + + if not expense_accounts: + frappe.throw( + _("No expense accounts found for Purchase Invoice {0}").format(self.purchase_invoice) + ) + + pi_expense_account = expense_accounts[0] gl_entries.append( self.get_gl_dict( @@ -473,3 +479,82 @@ class AssetRepair(AccountsController): def get_downtime(failure_date, completion_date): downtime = time_diff_in_hours(completion_date, failure_date) return round(downtime, 2) + + +@frappe.whitelist() +def get_repair_cost_for_purchase_invoice(purchase_invoice: str) -> float: + """ + Get the total repair cost from GL entries for a purchase invoice. + Only considers expense accounts for non-stock, non-fixed-asset items. + """ + if not purchase_invoice: + return 0.0 + + expense_accounts = _get_expense_accounts_for_purchase_invoice(purchase_invoice) + + if not expense_accounts: + return 0.0 + + return _get_total_expense_amount(purchase_invoice, expense_accounts) + + +def _get_expense_accounts_for_purchase_invoice(purchase_invoice: str) -> list[str]: + """ + Get expense accounts for non-stock items from the purchase invoice. + """ + pi_items = frappe.db.get_list( + "Purchase Invoice Item", + filters={"parent": purchase_invoice}, + fields=["item_code", "expense_account", "is_fixed_asset"], + ) + + if not pi_items: + return [] + + # Get list of stock item codes from the invoice + item_codes = {item.item_code for item in pi_items if item.item_code} + stock_items = set() + if item_codes: + stock_items = set( + frappe.db.get_all( + "Item", filters={"name": ["in", list(item_codes)], "is_stock_item": 1}, pluck="name" + ) + ) + + expense_accounts = set() + + for item in pi_items: + # Skip stock items - they use warehouse accounts + if item.item_code and item.item_code in stock_items: + continue + + # Skip fixed assets - they use asset accounts + if item.is_fixed_asset: + continue + + # Use expense account from Purchase Invoice Item + if item.expense_account: + expense_accounts.add(item.expense_account) + + return list(expense_accounts) + + +def _get_total_expense_amount(purchase_invoice: str, expense_accounts: list[str]) -> float: + """Get the total expense amount from GL entries for a purchase invoice and accounts.""" + if not expense_accounts: + return 0.0 + + gl_entry = frappe.qb.DocType("GL Entry") + + result = ( + frappe.qb.from_(gl_entry) + .select((Sum(gl_entry.debit) - Sum(gl_entry.credit)).as_("total")) + .where( + (gl_entry.voucher_type == "Purchase Invoice") + & (gl_entry.voucher_no == purchase_invoice) + & (gl_entry.account.isin(expense_accounts)) + & (gl_entry.is_cancelled == 0) + ) + ).run(as_dict=True) + + return flt(result[0].total) if result else 0.0 diff --git a/erpnext/assets/doctype/asset_repair/test_asset_repair.py b/erpnext/assets/doctype/asset_repair/test_asset_repair.py index 4cd304fbfd0..1b5290ca8ea 100644 --- a/erpnext/assets/doctype/asset_repair/test_asset_repair.py +++ b/erpnext/assets/doctype/asset_repair/test_asset_repair.py @@ -8,6 +8,7 @@ from frappe import qb from frappe.query_builder.functions import Sum from frappe.utils import add_days, add_months, flt, get_first_day, nowdate, nowtime, today +from erpnext.accounts.doctype.purchase_invoice.test_purchase_invoice import make_purchase_invoice from erpnext.assets.doctype.asset.asset import ( get_asset_account, get_asset_value_after_depreciation, @@ -21,6 +22,7 @@ from erpnext.assets.doctype.asset.test_asset import ( from erpnext.assets.doctype.asset_depreciation_schedule.asset_depreciation_schedule import ( get_asset_depr_schedule_doc, ) +from erpnext.assets.doctype.asset_repair.asset_repair import get_repair_cost_for_purchase_invoice from erpnext.stock.doctype.item.test_item import create_item from erpnext.stock.doctype.serial_and_batch_bundle.test_serial_and_batch_bundle import ( get_serial_nos_from_bundle, @@ -321,6 +323,52 @@ class TestAssetRepair(unittest.TestCase): self.assertEqual(asset.additional_asset_cost, asset_repair.repair_cost) self.assertEqual(booked_value, asset_repair.repair_cost) + def test_repair_cost_fetches_only_service_item_amount(self): + """Test that repair cost only includes service (non-stock) item amounts from purchase invoice.""" + + service_item = create_item( + "_Test Service Item for Repair", + is_stock_item=0, + company="_Test Company", + ) + + stock_item = create_item( + "_Test Stock Item for Repair", + is_stock_item=1, + company="_Test Company", + ) + + expense_account = frappe.db.get_value("Company", "_Test Company", "default_expense_account") + cost_center = frappe.db.get_value("Company", "_Test Company", "cost_center") + + pi = make_purchase_invoice( + item_code=service_item.name, + qty=1, + rate=500, + expense_account=expense_account, + cost_center=cost_center, + update_stock=0, + do_not_submit=1, + ) + + pi.update_stock = 1 + pi.append( + "items", + { + "item_code": stock_item.name, + "qty": 2, + "rate": 300, + "warehouse": "_Test Warehouse - _TC", + "cost_center": cost_center, + }, + ) + pi.save() + pi.submit() + + repair_cost = get_repair_cost_for_purchase_invoice(pi.name) + + self.assertEqual(repair_cost, 500) + def num_of_depreciations(asset): return asset.finance_books[0].total_number_of_depreciations From 0b1746a4c8e4d4109c06ada55a16d3b7cca95027 Mon Sep 17 00:00:00 2001 From: ljain112 Date: Mon, 9 Mar 2026 18:34:46 +0530 Subject: [PATCH 2/5] fix: set default repair cost to 0 if no value is returned --- erpnext/assets/doctype/asset_repair/asset_repair.js | 4 +--- .../assets/doctype/asset_repair/test_asset_repair.py | 12 +++++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/erpnext/assets/doctype/asset_repair/asset_repair.js b/erpnext/assets/doctype/asset_repair/asset_repair.js index a3ddd066ebd..646a7eee7ef 100644 --- a/erpnext/assets/doctype/asset_repair/asset_repair.js +++ b/erpnext/assets/doctype/asset_repair/asset_repair.js @@ -116,9 +116,7 @@ frappe.ui.form.on("Asset Repair", { purchase_invoice: frm.doc.purchase_invoice, }, callback: function (r) { - if (r.message) { - frm.set_value("repair_cost", r.message); - } + frm.set_value("repair_cost", r.message || 0); }, }); } else { diff --git a/erpnext/assets/doctype/asset_repair/test_asset_repair.py b/erpnext/assets/doctype/asset_repair/test_asset_repair.py index 1b5290ca8ea..10540e462a9 100644 --- a/erpnext/assets/doctype/asset_repair/test_asset_repair.py +++ b/erpnext/assets/doctype/asset_repair/test_asset_repair.py @@ -326,20 +326,21 @@ class TestAssetRepair(unittest.TestCase): def test_repair_cost_fetches_only_service_item_amount(self): """Test that repair cost only includes service (non-stock) item amounts from purchase invoice.""" + company = "_Test Company with perpetual inventory" service_item = create_item( "_Test Service Item for Repair", is_stock_item=0, - company="_Test Company", + company=company, ) stock_item = create_item( "_Test Stock Item for Repair", is_stock_item=1, - company="_Test Company", + company=company, ) - expense_account = frappe.db.get_value("Company", "_Test Company", "default_expense_account") - cost_center = frappe.db.get_value("Company", "_Test Company", "cost_center") + expense_account = frappe.db.get_value("Company", company, "default_expense_account") + cost_center = frappe.db.get_value("Company", company, "cost_center") pi = make_purchase_invoice( item_code=service_item.name, @@ -349,6 +350,7 @@ class TestAssetRepair(unittest.TestCase): cost_center=cost_center, update_stock=0, do_not_submit=1, + company=company, ) pi.update_stock = 1 @@ -358,7 +360,7 @@ class TestAssetRepair(unittest.TestCase): "item_code": stock_item.name, "qty": 2, "rate": 300, - "warehouse": "_Test Warehouse - _TC", + "warehouse": "Stores - TCP1", "cost_center": cost_center, }, ) From ed428ceb1c271da5f6018583377258e0b7c2ce7c Mon Sep 17 00:00:00 2001 From: ljain112 Date: Mon, 9 Mar 2026 18:54:43 +0530 Subject: [PATCH 3/5] fix(test): ensure warehouse is consistently referenced in asset repair tests --- erpnext/assets/doctype/asset_repair/test_asset_repair.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/erpnext/assets/doctype/asset_repair/test_asset_repair.py b/erpnext/assets/doctype/asset_repair/test_asset_repair.py index 10540e462a9..d39ce225952 100644 --- a/erpnext/assets/doctype/asset_repair/test_asset_repair.py +++ b/erpnext/assets/doctype/asset_repair/test_asset_repair.py @@ -327,15 +327,19 @@ class TestAssetRepair(unittest.TestCase): """Test that repair cost only includes service (non-stock) item amounts from purchase invoice.""" company = "_Test Company with perpetual inventory" + warehouse = "Stores - TCP1" + service_item = create_item( "_Test Service Item for Repair", is_stock_item=0, + warehouse=warehouse, company=company, ) stock_item = create_item( "_Test Stock Item for Repair", is_stock_item=1, + warehouse=warehouse, company=company, ) @@ -360,7 +364,7 @@ class TestAssetRepair(unittest.TestCase): "item_code": stock_item.name, "qty": 2, "rate": 300, - "warehouse": "Stores - TCP1", + "warehouse": warehouse, "cost_center": cost_center, }, ) From bcc542b1f941ed77220e8d0bc8ca0806d54dca92 Mon Sep 17 00:00:00 2001 From: ljain112 Date: Mon, 9 Mar 2026 19:11:02 +0530 Subject: [PATCH 4/5] fix(test): include warehouse parameter in asset repair test case --- erpnext/assets/doctype/asset_repair/test_asset_repair.py | 1 + 1 file changed, 1 insertion(+) diff --git a/erpnext/assets/doctype/asset_repair/test_asset_repair.py b/erpnext/assets/doctype/asset_repair/test_asset_repair.py index d39ce225952..f59dc49b1a7 100644 --- a/erpnext/assets/doctype/asset_repair/test_asset_repair.py +++ b/erpnext/assets/doctype/asset_repair/test_asset_repair.py @@ -352,6 +352,7 @@ class TestAssetRepair(unittest.TestCase): rate=500, expense_account=expense_account, cost_center=cost_center, + warehouse=warehouse, update_stock=0, do_not_submit=1, company=company, From a6dd07802ac0569577826357d42977acf32aebc5 Mon Sep 17 00:00:00 2001 From: ljain112 Date: Mon, 9 Mar 2026 19:53:57 +0530 Subject: [PATCH 5/5] fix: enforce permission check for purchase invoice and update test to use service expense account --- erpnext/assets/doctype/asset_repair/asset_repair.py | 4 +++- erpnext/assets/doctype/asset_repair/test_asset_repair.py | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/erpnext/assets/doctype/asset_repair/asset_repair.py b/erpnext/assets/doctype/asset_repair/asset_repair.py index 4d59ba5b96c..7e17dd70e46 100644 --- a/erpnext/assets/doctype/asset_repair/asset_repair.py +++ b/erpnext/assets/doctype/asset_repair/asset_repair.py @@ -490,6 +490,8 @@ def get_repair_cost_for_purchase_invoice(purchase_invoice: str) -> float: if not purchase_invoice: return 0.0 + frappe.has_permission("Purchase Invoice", "read", purchase_invoice, throw=True) + expense_accounts = _get_expense_accounts_for_purchase_invoice(purchase_invoice) if not expense_accounts: @@ -502,7 +504,7 @@ def _get_expense_accounts_for_purchase_invoice(purchase_invoice: str) -> list[st """ Get expense accounts for non-stock items from the purchase invoice. """ - pi_items = frappe.db.get_list( + pi_items = frappe.get_all( "Purchase Invoice Item", filters={"parent": purchase_invoice}, fields=["item_code", "expense_account", "is_fixed_asset"], diff --git a/erpnext/assets/doctype/asset_repair/test_asset_repair.py b/erpnext/assets/doctype/asset_repair/test_asset_repair.py index f59dc49b1a7..3a92f0ec71a 100644 --- a/erpnext/assets/doctype/asset_repair/test_asset_repair.py +++ b/erpnext/assets/doctype/asset_repair/test_asset_repair.py @@ -343,14 +343,14 @@ class TestAssetRepair(unittest.TestCase): company=company, ) - expense_account = frappe.db.get_value("Company", company, "default_expense_account") + service_expense_account = "Miscellaneous Expenses - TCP1" cost_center = frappe.db.get_value("Company", company, "cost_center") pi = make_purchase_invoice( item_code=service_item.name, qty=1, rate=500, - expense_account=expense_account, + expense_account=service_expense_account, cost_center=cost_center, warehouse=warehouse, update_stock=0, @@ -466,6 +466,7 @@ def create_asset_repair(**args): if asset.calculate_depreciation: asset_repair.increase_in_asset_life = 12 pi = make_purchase_invoice( + item=args.item or "_Test Non Stock Item", company=asset.company, expense_account=frappe.db.get_value("Company", asset.company, "default_expense_account"), cost_center=asset_repair.cost_center,